From 38db17b8cc3c1931314ad22dce0f06174eba393d Mon Sep 17 00:00:00 2001 From: Keagan McClelland Date: Thu, 22 Feb 2024 16:17:09 -0800 Subject: [PATCH 1/5] htlcswitch: remove redundant field from channelLink --- htlcswitch/link.go | 8 ++------ htlcswitch/switch_test.go | 1 - 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/htlcswitch/link.go b/htlcswitch/link.go index f8f87ca8f..b045b5ab1 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -320,9 +320,6 @@ type channelLink struct { // updates. channel *lnwallet.LightningChannel - // shortChanID is the most up to date short channel ID for the link. - shortChanID lnwire.ShortChannelID - // cfg is a structure which carries all dependable fields/handlers // which may affect behaviour of the service. cfg ChannelLinkConfig @@ -455,7 +452,6 @@ func NewChannelLink(cfg ChannelLinkConfig, return &channelLink{ cfg: cfg, channel: channel, - shortChanID: channel.ShortChanID(), hodlMap: make(map[models.CircuitKey]hodlHtlc), hodlQueue: queue.NewConcurrentQueue(10), log: build.NewPrefixLog(logPrefix, log), @@ -2202,7 +2198,7 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) { for id, settled := range finalHTLCs { l.cfg.HtlcNotifier.NotifyFinalHtlcEvent( models.CircuitKey{ - ChanID: l.shortChanID, + ChanID: l.ShortChanID(), HtlcID: id, }, channeldb.FinalHtlcInfo{ @@ -2582,7 +2578,7 @@ func (l *channelLink) ShortChanID() lnwire.ShortChannelID { l.RLock() defer l.RUnlock() - return l.shortChanID + return l.channel.ShortChanID() } // UpdateShortChanID updates the short channel ID for a link. This may be diff --git a/htlcswitch/switch_test.go b/htlcswitch/switch_test.go index 2d507f01b..b443be355 100644 --- a/htlcswitch/switch_test.go +++ b/htlcswitch/switch_test.go @@ -5313,7 +5313,6 @@ func testSwitchHandlePacketForward(t *testing.T, zeroConf, private, if zeroConf { // Store the alias in the shortChanID field and mark the real // scid in the database. - aliceChannelLink.shortChanID = aliceAlias err = aliceChannelState.MarkRealScid(aliceScid) require.NoError(t, err) From be69b022d9d6b4f687221ba7bfcf140810379c74 Mon Sep 17 00:00:00 2001 From: Keagan McClelland Date: Thu, 22 Feb 2024 17:38:40 -0800 Subject: [PATCH 2/5] htlcswitch: remove Switch reference from channelLink --- htlcswitch/link.go | 7 - htlcswitch/link_isolated_test.go | 9 +- htlcswitch/link_test.go | 677 ++++++++++++++++++------------- htlcswitch/switch_test.go | 4 +- htlcswitch/test_utils.go | 1 - 5 files changed, 395 insertions(+), 303 deletions(-) diff --git a/htlcswitch/link.go b/htlcswitch/link.go index b045b5ab1..944ac0591 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -92,13 +92,6 @@ type ChannelLinkConfig struct { // allowing the link to open and close circuits. Circuits CircuitModifier - // Switch provides a reference to the HTLC switch, we only use this in - // testing to access circuit operations not typically exposed by the - // CircuitModifier. - // - // TODO(conner): remove after refactoring htlcswitch testing framework. - Switch *Switch - // BestHeight returns the best known height. BestHeight func() uint32 diff --git a/htlcswitch/link_isolated_test.go b/htlcswitch/link_isolated_test.go index 6b616d2d8..89d0ef319 100644 --- a/htlcswitch/link_isolated_test.go +++ b/htlcswitch/link_isolated_test.go @@ -13,9 +13,10 @@ import ( type linkTestContext struct { t *testing.T - aliceLink ChannelLink - bobChannel *lnwallet.LightningChannel - aliceMsgs <-chan lnwire.Message + aliceSwitch *Switch + aliceLink ChannelLink + bobChannel *lnwallet.LightningChannel + aliceMsgs <-chan lnwire.Message } // sendHtlcBobToAlice sends an HTLC from Bob to Alice, that pays to a preimage @@ -39,7 +40,7 @@ func (l *linkTestContext) sendHtlcAliceToBob(htlcID int, l.t.Helper() - circuitMap := l.aliceLink.(*channelLink).cfg.Switch.circuits + circuitMap := l.aliceSwitch.circuits fwdActions, err := circuitMap.CommitCircuits( &PaymentCircuit{ Incoming: CircuitKey{ diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index d9e583876..fdd2a0d6a 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -187,28 +187,30 @@ func TestChannelLinkRevThenSig(t *testing.T) { const chanAmt = btcutil.SatoshiPerBitcoin * 5 const chanReserve = btcutil.SatoshiPerBitcoin * 1 - aliceLink, bobChannel, batchTicker, start, restore, err := - newSingleLinkTestHarness(t, chanAmt, chanReserve) + harness, err := newSingleLinkTestHarness(t, chanAmt, chanReserve) require.NoError(t, err) - err = start() + err = harness.start() require.NoError(t, err) - defer aliceLink.Stop() + defer harness.aliceLink.Stop() - alice := newPersistentLinkHarness( - t, aliceLink, batchTicker, restore, + alice := newPersistentLinkHarness(t, harness.aliceSwitch, + harness.aliceLink, harness.aliceBatchTicker, + harness.aliceRestore, ) var ( - coreLink = aliceLink.(*channelLink) + //nolint: forcetypeassert + coreLink = harness.aliceLink.(*channelLink) aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs ) ctx := linkTestContext{ - t: t, - aliceLink: aliceLink, - aliceMsgs: aliceMsgs, - bobChannel: bobChannel, + t: t, + aliceSwitch: harness.aliceSwitch, + aliceLink: harness.aliceLink, + aliceMsgs: aliceMsgs, + bobChannel: harness.bobChannel, } bobHtlc1 := generateHtlc(t, coreLink, 0) @@ -240,7 +242,7 @@ func TestChannelLinkRevThenSig(t *testing.T) { // ------sig----x // Trigger a commitsig from Alice->Bob. select { - case batchTicker <- time.Now(): + case harness.aliceBatchTicker <- time.Now(): case <-time.After(5 * time.Second): t.Fatalf("could not force commit sig") } @@ -265,10 +267,10 @@ func TestChannelLinkRevThenSig(t *testing.T) { ctx.aliceMsgs = alice.msgs // Restart Bob as well by calling NewLightningChannel. - bobSigner := bobChannel.Signer + bobSigner := harness.bobChannel.Signer bobPool := lnwallet.NewSigPool(runtime.NumCPU(), bobSigner) - bobChannel, err = lnwallet.NewLightningChannel( - bobSigner, bobChannel.State(), bobPool, + bobChannel, err := lnwallet.NewLightningChannel( + bobSigner, harness.bobChannel.State(), bobPool, ) require.NoError(t, err) err = bobPool.Start() @@ -319,28 +321,31 @@ func TestChannelLinkSigThenRev(t *testing.T) { const chanAmt = btcutil.SatoshiPerBitcoin * 5 const chanReserve = btcutil.SatoshiPerBitcoin * 1 - aliceLink, bobChannel, batchTicker, start, restore, err := + harness, err := newSingleLinkTestHarness(t, chanAmt, chanReserve) require.NoError(t, err) - err = start() + err = harness.start() require.NoError(t, err) - defer aliceLink.Stop() + defer harness.aliceLink.Stop() alice := newPersistentLinkHarness( - t, aliceLink, batchTicker, restore, + t, harness.aliceSwitch, harness.aliceLink, + harness.aliceBatchTicker, harness.aliceRestore, ) var ( - coreLink = aliceLink.(*channelLink) + //nolint: forcetypeassert + coreLink = harness.aliceLink.(*channelLink) aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs ) ctx := linkTestContext{ - t: t, - aliceLink: aliceLink, - aliceMsgs: aliceMsgs, - bobChannel: bobChannel, + t: t, + aliceSwitch: harness.aliceSwitch, + aliceLink: harness.aliceLink, + aliceMsgs: aliceMsgs, + bobChannel: harness.bobChannel, } bobHtlc1 := generateHtlc(t, coreLink, 0) @@ -358,7 +363,7 @@ func TestChannelLinkSigThenRev(t *testing.T) { // ------sig----x // Trigger a commitsig from Alice->Bob. select { - case batchTicker <- time.Now(): + case harness.aliceBatchTicker <- time.Now(): case <-time.After(5 * time.Second): t.Fatalf("could not force commit sig") } @@ -397,10 +402,10 @@ func TestChannelLinkSigThenRev(t *testing.T) { ctx.aliceMsgs = alice.msgs // Restart Bob as well by calling NewLightningChannel. - bobSigner := bobChannel.Signer + bobSigner := harness.bobChannel.Signer bobPool := lnwallet.NewSigPool(runtime.NumCPU(), bobSigner) - bobChannel, err = lnwallet.NewLightningChannel( - bobSigner, bobChannel.State(), bobPool, + bobChannel, err := lnwallet.NewLightningChannel( + bobSigner, harness.bobChannel.State(), bobPool, ) require.NoError(t, err) err = bobPool.Start() @@ -1915,13 +1920,21 @@ func (m *mockPeer) RemovePendingChannel(_ lnwire.ChannelID) error { return nil } -func newSingleLinkTestHarness(t *testing.T, chanAmt, chanReserve btcutil.Amount) ( - ChannelLink, *lnwallet.LightningChannel, chan time.Time, func() error, - func() (*lnwallet.LightningChannel, error), error) { +type singleLinkTestHarness struct { + aliceSwitch *Switch + aliceLink ChannelLink + bobChannel *lnwallet.LightningChannel + aliceBatchTicker chan time.Time + start func() error + aliceRestore func() (*lnwallet.LightningChannel, error) +} + +func newSingleLinkTestHarness(t *testing.T, chanAmt, + chanReserve btcutil.Amount) (singleLinkTestHarness, error) { var chanIDBytes [8]byte if _, err := io.ReadFull(rand.Reader, chanIDBytes[:]); err != nil { - return nil, nil, nil, nil, nil, err + return singleLinkTestHarness{}, err } chanID := lnwire.NewShortChanIDFromInt( @@ -1932,7 +1945,7 @@ func newSingleLinkTestHarness(t *testing.T, chanAmt, chanReserve btcutil.Amount) chanReserve, chanReserve, chanID, ) if err != nil { - return nil, nil, nil, nil, nil, err + return singleLinkTestHarness{}, err } var ( @@ -1956,7 +1969,7 @@ func newSingleLinkTestHarness(t *testing.T, chanAmt, chanReserve btcutil.Amount) aliceDb := aliceLc.channel.State().Db.GetParentDB() aliceSwitch, err := initSwitchWithDB(testStartingHeight, aliceDb) if err != nil { - return nil, nil, nil, nil, nil, err + return singleLinkTestHarness{}, err } notifyUpdateChan := make(chan *contractcourt.ContractUpdate) @@ -1982,7 +1995,6 @@ func newSingleLinkTestHarness(t *testing.T, chanAmt, chanReserve btcutil.Amount) aliceCfg := ChannelLinkConfig{ FwrdingPolicy: globalPolicy, Peer: alicePeer, - Switch: aliceSwitch, BestHeight: aliceSwitch.BestHeight, Circuits: aliceSwitch.CircuitModifier(), ForwardPackets: func(linkQuit chan struct{}, _ bool, packets ...*htlcPacket) error { @@ -2043,8 +2055,16 @@ func newSingleLinkTestHarness(t *testing.T, chanAmt, chanReserve btcutil.Amount) invoiceRegistry.cleanup() }) - return aliceLink, bobLc.channel, bticker.Force, start, - aliceLc.restore, nil + harness := singleLinkTestHarness{ + aliceSwitch: aliceSwitch, + aliceLink: aliceLink, + bobChannel: bobLc.channel, + aliceBatchTicker: bticker.Force, + start: start, + aliceRestore: aliceLc.restore, + } + + return harness, nil } func assertLinkBandwidth(t *testing.T, link ChannelLink, @@ -2223,11 +2243,11 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { // We'll start the test by creating a single instance of const chanAmt = btcutil.SatoshiPerBitcoin * 5 - aliceLink, bobChannel, tmr, start, _, err := + harness, err := newSingleLinkTestHarness(t, chanAmt, 0) require.NoError(t, err, "unable to create link") - if err := start(); err != nil { + if err := harness.start(); err != nil { t.Fatalf("unable to start test harness: %v", err) } @@ -2241,7 +2261,7 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { commitWeight = lnwallet.CommitWeight(cType) ) - link, ok := aliceLink.(*channelLink) + link, ok := harness.aliceLink.(*channelLink) require.True(t, ok) coreLink = link mockedPeer := coreLink.cfg.Peer @@ -2268,7 +2288,7 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { // The starting bandwidth of the channel should be exactly the amount // that we created the channel between her and Bob, minus the feebuffer. expectedBandwidth := chanAmtMSat - feeBuffer - assertLinkBandwidth(t, aliceLink, expectedBandwidth) + assertLinkBandwidth(t, harness.aliceLink, expectedBandwidth) // Next, we'll create an HTLC worth 1 BTC, and send it into the link as // a switch initiated payment. The resulting bandwidth should @@ -2286,11 +2306,11 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { } circuit := makePaymentCircuit(&htlc.PaymentHash, &addPkt) - _, err = coreLink.cfg.Switch.commitCircuits(&circuit) + _, err = harness.aliceSwitch.commitCircuits(&circuit) require.NoError(t, err, "unable to commit circuit") addPkt.circuit = &circuit - if err := aliceLink.handleSwitchPacket(&addPkt); err != nil { + if err := harness.aliceLink.handleSwitchPacket(&addPkt); err != nil { t.Fatalf("unable to handle switch packet: %v", err) } time.Sleep(time.Millisecond * 500) @@ -2304,7 +2324,7 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { // The resulting bandwidth should reflect that Alice is paying the // htlc amount in addition to the fee buffer. - assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer-htlcAmt) + assertLinkBandwidth(t, harness.aliceLink, chanAmtMSat-feeBuffer-htlcAmt) // Alice should send the HTLC to Bob. var msg lnwire.Message @@ -2319,34 +2339,40 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { t.Fatalf("expected UpdateAddHTLC, got %T", msg) } - bobIndex, err := bobChannel.ReceiveHTLC(addHtlc) + bobIndex, err := harness.bobChannel.ReceiveHTLC(addHtlc) require.NoError(t, err, "bob failed receiving htlc") // Lock in the HTLC. - if err := updateState(tmr, coreLink, bobChannel, true); err != nil { + if err := updateState( + harness.aliceBatchTicker, coreLink, harness.bobChannel, true, + ); err != nil { t.Fatalf("unable to update state: %v", err) } // Locking in the HTLC should not change Alice's bandwidth. - assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer-htlcAmt) + assertLinkBandwidth(t, harness.aliceLink, chanAmtMSat-feeBuffer-htlcAmt) // If we now send in a valid HTLC settle for the prior HTLC we added, // then the bandwidth should remain unchanged as the remote party will // gain additional channel balance. - err = bobChannel.SettleHTLC(*invoice.Terms.PaymentPreimage, bobIndex, nil, nil, nil) + err = harness.bobChannel.SettleHTLC( + *invoice.Terms.PaymentPreimage, bobIndex, nil, nil, nil, + ) require.NoError(t, err, "unable to settle htlc") htlcSettle := &lnwire.UpdateFulfillHTLC{ ID: 0, PaymentPreimage: *invoice.Terms.PaymentPreimage, } - aliceLink.HandleChannelUpdate(htlcSettle) + harness.aliceLink.HandleChannelUpdate(htlcSettle) time.Sleep(time.Millisecond * 500) // Since the settle is not locked in yet, Alice's bandwidth should still // reflect that she has to account for the fee buffer. - assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer-htlcAmt) + assertLinkBandwidth(t, harness.aliceLink, chanAmtMSat-feeBuffer-htlcAmt) // Lock in the settle. - if err := updateState(tmr, coreLink, bobChannel, false); err != nil { + if err := updateState( + harness.aliceBatchTicker, coreLink, harness.bobChannel, false, + ); err != nil { t.Fatalf("unable to update state: %v", err) } @@ -2358,7 +2384,7 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { ) // Now that it is settled, Alice fee buffer should have been decreased. - assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer-htlcAmt) + assertLinkBandwidth(t, harness.aliceLink, chanAmtMSat-feeBuffer-htlcAmt) // Next, we'll add another HTLC initiated by the switch (of the same // amount as the prior one). @@ -2372,11 +2398,11 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { } circuit = makePaymentCircuit(&htlc.PaymentHash, &addPkt) - _, err = coreLink.cfg.Switch.commitCircuits(&circuit) + _, err = harness.aliceSwitch.commitCircuits(&circuit) require.NoError(t, err, "unable to commit circuit") addPkt.circuit = &circuit - if err := aliceLink.handleSwitchPacket(&addPkt); err != nil { + if err := harness.aliceLink.handleSwitchPacket(&addPkt); err != nil { t.Fatalf("unable to handle switch packet: %v", err) } time.Sleep(time.Millisecond * 500) @@ -2389,7 +2415,9 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { ) // Again, Alice's bandwidth decreases by htlcAmt and fee buffer. - assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer-2*htlcAmt) + assertLinkBandwidth( + t, harness.aliceLink, chanAmtMSat-feeBuffer-2*htlcAmt, + ) // Alice will send the HTLC to Bob. select { @@ -2403,15 +2431,19 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { t.Fatalf("expected UpdateAddHTLC, got %T", msg) } - bobIndex, err = bobChannel.ReceiveHTLC(addHtlc) + bobIndex, err = harness.bobChannel.ReceiveHTLC(addHtlc) require.NoError(t, err, "bob failed receiving htlc") // Lock in the HTLC, which should not affect the bandwidth. - if err := updateState(tmr, coreLink, bobChannel, true); err != nil { + if err := updateState( + harness.aliceBatchTicker, coreLink, harness.bobChannel, true, + ); err != nil { t.Fatalf("unable to update state: %v", err) } - assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer-htlcAmt*2) + assertLinkBandwidth( + t, harness.aliceLink, chanAmtMSat-feeBuffer-htlcAmt*2, + ) // With that processed, we'll now generate an HTLC fail (sent by the // remote peer) to cancel the HTLC we just added. This should return us @@ -2419,21 +2451,25 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { reason := make([]byte, 292) copy(reason, []byte("nop")) - err = bobChannel.FailHTLC(bobIndex, reason, nil, nil, nil) + err = harness.bobChannel.FailHTLC(bobIndex, reason, nil, nil, nil) require.NoError(t, err, "unable to fail htlc") failMsg := &lnwire.UpdateFailHTLC{ ID: 1, Reason: lnwire.OpaqueReason(reason), } - aliceLink.HandleChannelUpdate(failMsg) + harness.aliceLink.HandleChannelUpdate(failMsg) time.Sleep(time.Millisecond * 500) // Before the Fail gets locked in, the bandwidth should remain unchanged. - assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer-htlcAmt*2) + assertLinkBandwidth( + t, harness.aliceLink, chanAmtMSat-feeBuffer-htlcAmt*2, + ) // Lock in the Fail. - if err := updateState(tmr, coreLink, bobChannel, false); err != nil { + if err := updateState( + harness.aliceBatchTicker, coreLink, harness.bobChannel, false, + ); err != nil { t.Fatalf("unable to update state: %v", err) } @@ -2445,7 +2481,7 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { ) // Now the bandwidth should reflect the failed HTLC. - assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer-htlcAmt) + assertLinkBandwidth(t, harness.aliceLink, chanAmtMSat-feeBuffer-htlcAmt) // Moving along, we'll now receive a new HTLC from the remote peer, // with an ID of 0 as this is their first HTLC. The bandwidth should @@ -2469,15 +2505,17 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { require.NoError(t, err, "unable to add invoice to registry") htlc.ID = 0 - _, err = bobChannel.AddHTLC(htlc, nil) + _, err = harness.bobChannel.AddHTLC(htlc, nil) require.NoError(t, err, "unable to add htlc") - aliceLink.HandleChannelUpdate(htlc) + harness.aliceLink.HandleChannelUpdate(htlc) // Alice's balance remains unchanged until this HTLC is locked in. - assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer-htlcAmt) + assertLinkBandwidth(t, harness.aliceLink, chanAmtMSat-feeBuffer-htlcAmt) // Lock in the HTLC. - if err := updateState(tmr, coreLink, bobChannel, false); err != nil { + if err := updateState( + harness.aliceBatchTicker, coreLink, harness.bobChannel, false, + ); err != nil { t.Fatalf("unable to update state: %v", err) } @@ -2490,18 +2528,18 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { // Since Bob is adding this HTLC, Alice will only have an increased fee // buffer. - assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer-htlcAmt) + assertLinkBandwidth(t, harness.aliceLink, chanAmtMSat-feeBuffer-htlcAmt) time.Sleep(time.Millisecond * 500) addPkt = htlcPacket{ htlc: htlc, - incomingChanID: aliceLink.ShortChanID(), + incomingChanID: harness.aliceLink.ShortChanID(), incomingHTLCID: 0, obfuscator: NewMockObfuscator(), } circuit = makePaymentCircuit(&htlc.PaymentHash, &addPkt) - _, err = coreLink.cfg.Switch.commitCircuits(&circuit) + _, err = harness.aliceSwitch.commitCircuits(&circuit) require.NoError(t, err, "unable to commit circuit") addPkt.outgoingChanID = carolChanID @@ -2514,7 +2552,7 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { // we eventually learn (simulating a multi-hop payment). The bandwidth // of the channel should now be re-balanced to the starting point. settlePkt := htlcPacket{ - incomingChanID: aliceLink.ShortChanID(), + incomingChanID: harness.aliceLink.ShortChanID(), incomingHTLCID: 0, circuit: &circuit, outgoingChanID: addPkt.outgoingChanID, @@ -2526,7 +2564,7 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { obfuscator: NewMockObfuscator(), } - if err := aliceLink.handleSwitchPacket(&settlePkt); err != nil { + if err := harness.aliceLink.handleSwitchPacket(&settlePkt); err != nil { t.Fatalf("unable to handle switch packet: %v", err) } time.Sleep(time.Millisecond * 500) @@ -2539,7 +2577,7 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { ) // Settling this HTLC gives Alice all her original bandwidth back. - assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer) + assertLinkBandwidth(t, harness.aliceLink, chanAmtMSat-feeBuffer) select { case msg = <-aliceMsgs: @@ -2551,12 +2589,14 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { if !ok { t.Fatalf("expected UpdateFulfillHTLC, got %T", msg) } - err = bobChannel.ReceiveHTLCSettle(settleMsg.PaymentPreimage, settleMsg.ID) + err = harness.bobChannel.ReceiveHTLCSettle( + settleMsg.PaymentPreimage, settleMsg.ID, + ) require.NoError(t, err, "failed receiving fail htlc") // After failing an HTLC, the link will automatically trigger // a state update. - if err := handleStateUpdate(coreLink, bobChannel); err != nil { + if err := handleStateUpdate(coreLink, harness.bobChannel); err != nil { t.Fatalf("unable to update state: %v", err) } @@ -2580,14 +2620,16 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { // HTLC we add, hence it should have an ID of 1 (Alice's channel // link will set this automatically for her side). htlc.ID = 1 - _, err = bobChannel.AddHTLC(htlc, nil) + _, err = harness.bobChannel.AddHTLC(htlc, nil) require.NoError(t, err, "unable to add htlc") - aliceLink.HandleChannelUpdate(htlc) + harness.aliceLink.HandleChannelUpdate(htlc) time.Sleep(time.Millisecond * 500) // No changes before the HTLC is locked in. - assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer) - if err := updateState(tmr, coreLink, bobChannel, false); err != nil { + assertLinkBandwidth(t, harness.aliceLink, chanAmtMSat-feeBuffer) + if err := updateState( + harness.aliceBatchTicker, coreLink, harness.bobChannel, false, + ); err != nil { t.Fatalf("unable to update state: %v", err) } @@ -2599,17 +2641,17 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { ) // After lock-in, Alice will have to account for a fee buffer. - assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer) + assertLinkBandwidth(t, harness.aliceLink, chanAmtMSat-feeBuffer) addPkt = htlcPacket{ htlc: htlc, - incomingChanID: aliceLink.ShortChanID(), + incomingChanID: harness.aliceLink.ShortChanID(), incomingHTLCID: 1, obfuscator: NewMockObfuscator(), } circuit = makePaymentCircuit(&htlc.PaymentHash, &addPkt) - _, err = coreLink.cfg.Switch.commitCircuits(&circuit) + _, err = harness.aliceSwitch.commitCircuits(&circuit) require.NoError(t, err, "unable to commit circuit") addPkt.outgoingChanID = carolChanID @@ -2619,7 +2661,7 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { require.NoError(t, err, "unable to set keystone") failPkt := htlcPacket{ - incomingChanID: aliceLink.ShortChanID(), + incomingChanID: harness.aliceLink.ShortChanID(), incomingHTLCID: 1, circuit: &circuit, outgoingChanID: addPkt.outgoingChanID, @@ -2631,7 +2673,7 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { obfuscator: NewMockObfuscator(), } - if err := aliceLink.handleSwitchPacket(&failPkt); err != nil { + if err := harness.aliceLink.handleSwitchPacket(&failPkt); err != nil { t.Fatalf("unable to handle switch packet: %v", err) } time.Sleep(time.Millisecond * 500) @@ -2644,7 +2686,7 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { ) // Alice should get all her bandwidth back. - assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer) + assertLinkBandwidth(t, harness.aliceLink, chanAmtMSat-feeBuffer) // Message should be sent to Bob. select { @@ -2656,15 +2698,15 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { if !ok { t.Fatalf("expected UpdateFailHTLC, got %T", msg) } - err = bobChannel.ReceiveFailHTLC(failMsg.ID, []byte("fail")) + err = harness.bobChannel.ReceiveFailHTLC(failMsg.ID, []byte("fail")) require.NoError(t, err, "failed receiving fail htlc") // After failing an HTLC, the link will automatically trigger // a state update. - if err := handleStateUpdate(coreLink, bobChannel); err != nil { + if err := handleStateUpdate(coreLink, harness.bobChannel); err != nil { t.Fatalf("unable to update state: %v", err) } - assertLinkBandwidth(t, aliceLink, chanAmtMSat-feeBuffer) + assertLinkBandwidth(t, harness.aliceLink, chanAmtMSat-feeBuffer) } // genAddsAndCircuits creates `numHtlcs` sequential ADD packets and there @@ -2713,16 +2755,17 @@ func TestChannelLinkTrimCircuitsPending(t *testing.T) { // We'll start by creating a new link with our chanAmt (5 BTC). We will // only be testing Alice's behavior, so the reference to Bob's channel // state is unnecessary. - aliceLink, _, batchTicker, start, restore, err := + harness, err := newSingleLinkTestHarness(t, chanAmt, 0) require.NoError(t, err, "unable to create link") - if err := start(); err != nil { + if err := harness.start(); err != nil { t.Fatalf("unable to start test harness: %v", err) } alice := newPersistentLinkHarness( - t, aliceLink, batchTicker, restore, + t, harness.aliceSwitch, harness.aliceLink, + harness.aliceBatchTicker, harness.aliceRestore, ) // Compute the static fees that will be used to determine the @@ -2995,16 +3038,17 @@ func TestChannelLinkTrimCircuitsNoCommit(t *testing.T) { // We'll start by creating a new link with our chanAmt (5 BTC). We will // only be testing Alice's behavior, so the reference to Bob's channel // state is unnecessary. - aliceLink, _, batchTicker, start, restore, err := + harness, err := newSingleLinkTestHarness(t, chanAmt, 0) require.NoError(t, err, "unable to create link") - if err := start(); err != nil { + if err := harness.start(); err != nil { t.Fatalf("unable to start test harness: %v", err) } alice := newPersistentLinkHarness( - t, aliceLink, batchTicker, restore, + t, harness.aliceSwitch, harness.aliceLink, + harness.aliceBatchTicker, harness.aliceRestore, ) // We'll put Alice into hodl.Commit mode, such that the circuits for any @@ -3275,16 +3319,17 @@ func TestChannelLinkTrimCircuitsRemoteCommit(t *testing.T) { ) // We'll start by creating a new link with our chanAmt (5 BTC). - aliceLink, bobChan, batchTicker, start, restore, err := + harness, err := newSingleLinkTestHarness(t, chanAmt, 0) require.NoError(t, err, "unable to create link") - if err := start(); err != nil { + if err := harness.start(); err != nil { t.Fatalf("unable to start test harness: %v", err) } alice := newPersistentLinkHarness( - t, aliceLink, batchTicker, restore, + t, harness.aliceSwitch, harness.aliceLink, + harness.aliceBatchTicker, harness.aliceRestore, ) // Compute the static fees that will be used to determine the @@ -3349,7 +3394,7 @@ func TestChannelLinkTrimCircuitsRemoteCommit(t *testing.T) { pkt.ID = uint64(i) - _, err := bobChan.ReceiveHTLC(pkt) + _, err := harness.bobChannel.ReceiveHTLC(pkt) if err != nil { t.Fatalf("unable to receive htlc: %v", err) } @@ -3379,10 +3424,12 @@ func TestChannelLinkTrimCircuitsRemoteCommit(t *testing.T) { t.Fatalf("alice did not send commitment signature") } - err := bobChan.ReceiveNewCommitment(&lnwallet.CommitSigs{ - CommitSig: sig.CommitSig, - HtlcSigs: sig.HtlcSigs, - }) + err := harness.bobChannel.ReceiveNewCommitment( + &lnwallet.CommitSigs{ + CommitSig: sig.CommitSig, + HtlcSigs: sig.HtlcSigs, + }, + ) if err != nil { t.Fatalf("unable to receive new commitment: %v", err) } @@ -3391,7 +3438,7 @@ func TestChannelLinkTrimCircuitsRemoteCommit(t *testing.T) { // Next, revoke Bob's current commitment and send it to Alice so that we // can test that Alice's circuits aren't trimmed. - rev, _, _, err := bobChan.RevokeCurrentCommitment() + rev, _, _, err := harness.bobChannel.RevokeCurrentCommitment() require.NoError(t, err, "unable to revoke current commitment") _, _, _, _, err = alice.channel.ReceiveRevocation(rev) @@ -3420,11 +3467,11 @@ func TestChannelLinkBandwidthChanReserve(t *testing.T) { // channel reserve. const chanAmt = btcutil.SatoshiPerBitcoin * 5 const chanReserve = btcutil.SatoshiPerBitcoin * 1 - aliceLink, bobChannel, batchTimer, start, _, err := + harness, err := newSingleLinkTestHarness(t, chanAmt, chanReserve) require.NoError(t, err, "unable to create link") - if err := start(); err != nil { + if err := harness.start(); err != nil { t.Fatalf("unable to start test harness: %v", err) } @@ -3438,7 +3485,7 @@ func TestChannelLinkBandwidthChanReserve(t *testing.T) { commitWeight = lnwallet.CommitWeight(cType) ) - link, ok := aliceLink.(*channelLink) + link, ok := harness.aliceLink.(*channelLink) require.True(t, ok) coreLink = link mockedPeer := coreLink.cfg.Peer @@ -3460,7 +3507,7 @@ func TestChannelLinkBandwidthChanReserve(t *testing.T) { // that we created the channel between her and Bob, minus the channel // reserve and the fee buffer. expectedBandwidth := chanAmtMSat - feeBuffer - chanReserveMSat - assertLinkBandwidth(t, aliceLink, expectedBandwidth) + assertLinkBandwidth(t, harness.aliceLink, expectedBandwidth) // Next, we'll create an HTLC worth 3 BTC, and send it into the link as // a switch initiated payment. The resulting bandwidth should @@ -3474,10 +3521,10 @@ func TestChannelLinkBandwidthChanReserve(t *testing.T) { obfuscator: NewMockObfuscator(), } circuit := makePaymentCircuit(&htlc.PaymentHash, addPkt) - _, err = coreLink.cfg.Switch.commitCircuits(&circuit) + _, err = harness.aliceSwitch.commitCircuits(&circuit) require.NoError(t, err, "unable to commit circuit") - _ = aliceLink.handleSwitchPacket(addPkt) + _ = harness.aliceLink.handleSwitchPacket(addPkt) time.Sleep(time.Millisecond * 100) htlcWeight = int64(2) * input.HTLCWeight @@ -3485,7 +3532,8 @@ func TestChannelLinkBandwidthChanReserve(t *testing.T) { feePerKw, commitWeight+htlcWeight, ) assertLinkBandwidth( - t, aliceLink, chanAmtMSat-htlcAmt-feeBuffer-chanReserveMSat, + t, harness.aliceLink, + chanAmtMSat-htlcAmt-feeBuffer-chanReserveMSat, ) // Alice should send the HTLC to Bob. @@ -3501,38 +3549,46 @@ func TestChannelLinkBandwidthChanReserve(t *testing.T) { t.Fatalf("expected UpdateAddHTLC, got %T", msg) } - bobIndex, err := bobChannel.ReceiveHTLC(addHtlc) + bobIndex, err := harness.bobChannel.ReceiveHTLC(addHtlc) require.NoError(t, err, "bob failed receiving htlc") // Lock in the HTLC. - if err := updateState(batchTimer, coreLink, bobChannel, true); err != nil { + if err := updateState( + harness.aliceBatchTicker, coreLink, harness.bobChannel, true, + ); err != nil { t.Fatalf("unable to update state: %v", err) } assertLinkBandwidth( - t, aliceLink, chanAmtMSat-htlcAmt-feeBuffer-chanReserveMSat, + t, harness.aliceLink, + chanAmtMSat-htlcAmt-feeBuffer-chanReserveMSat, ) // If we now send in a valid HTLC settle for the prior HTLC we added, // then the bandwidth should remain unchanged as the remote party will // gain additional channel balance. - err = bobChannel.SettleHTLC(*invoice.Terms.PaymentPreimage, bobIndex, nil, nil, nil) + err = harness.bobChannel.SettleHTLC( + *invoice.Terms.PaymentPreimage, bobIndex, nil, nil, nil, + ) require.NoError(t, err, "unable to settle htlc") htlcSettle := &lnwire.UpdateFulfillHTLC{ ID: bobIndex, PaymentPreimage: *invoice.Terms.PaymentPreimage, } - aliceLink.HandleChannelUpdate(htlcSettle) + harness.aliceLink.HandleChannelUpdate(htlcSettle) time.Sleep(time.Millisecond * 500) // Since the settle is not locked in yet, Alice's bandwidth should still // reflect that she has to pay the fee. assertLinkBandwidth( - t, aliceLink, chanAmtMSat-htlcAmt-feeBuffer-chanReserveMSat, + t, harness.aliceLink, + chanAmtMSat-htlcAmt-feeBuffer-chanReserveMSat, ) // Lock in the settle. - if err := updateState(batchTimer, coreLink, bobChannel, false); err != nil { + if err := updateState( + harness.aliceBatchTicker, coreLink, harness.bobChannel, false, + ); err != nil { t.Fatalf("unable to update state: %v", err) } @@ -3542,7 +3598,8 @@ func TestChannelLinkBandwidthChanReserve(t *testing.T) { time.Sleep(time.Millisecond * 100) assertLinkBandwidth( - t, aliceLink, chanAmtMSat-htlcAmt-feeBuffer-chanReserveMSat, + t, harness.aliceLink, + chanAmtMSat-htlcAmt-feeBuffer-chanReserveMSat, ) // Now we create a channel that has a channel reserve that is @@ -3551,16 +3608,15 @@ func TestChannelLinkBandwidthChanReserve(t *testing.T) { // should therefore be 0. const bobChanAmt = btcutil.SatoshiPerBitcoin * 1 const bobChanReserve = btcutil.SatoshiPerBitcoin * 1.5 - bobLink, _, _, start, _, err := - newSingleLinkTestHarness(t, bobChanAmt, bobChanReserve) + harness, err = newSingleLinkTestHarness(t, bobChanAmt, bobChanReserve) require.NoError(t, err, "unable to create link") - if err := start(); err != nil { + if err := harness.start(); err != nil { t.Fatalf("unable to start test harness: %v", err) } // Make sure bandwidth is reported as 0. - assertLinkBandwidth(t, bobLink, 0) + assertLinkBandwidth(t, harness.aliceLink, 0) } // TestChannelRetransmission tests the ability of the channel links to @@ -3988,7 +4044,7 @@ func TestChannelLinkShutdownDuringForward(t *testing.T) { // Define a helper method that strobes the switch's log ticker, and // unblocks after nothing has been pulled for two seconds. waitForBobsSwitchToBlock := func() { - bobSwitch := n.firstBobChannelLink.cfg.Switch + bobSwitch := n.bobServer.htlcSwitch ticker := bobSwitch.cfg.LogEventTicker.(*ticker.Force) timeout := time.After(15 * time.Second) for { @@ -4038,7 +4094,7 @@ func TestChannelLinkShutdownDuringForward(t *testing.T) { // first acquiring the index mutex and forcing a log event so that the // htlcForwarder is blocked inside the logTicker case, which also needs // the indexMtx. - n.firstBobChannelLink.cfg.Switch.indexMtx.Lock() + n.bobServer.htlcSwitch.indexMtx.Lock() // Strobe the log ticker, and wait for switch to stop accepting any more // log ticks. @@ -4048,8 +4104,8 @@ func TestChannelLinkShutdownDuringForward(t *testing.T) { // channel, and unlock the indexMtx to allow return to the // htlcForwarder's main select. After this, any attempt to forward // through the switch will block. - n.firstBobChannelLink.cfg.Switch.htlcPlex = nil - n.firstBobChannelLink.cfg.Switch.indexMtx.Unlock() + n.bobServer.htlcSwitch.htlcPlex = nil + n.bobServer.htlcSwitch.indexMtx.Unlock() // Now, make a payment from Alice to Carol, which should cause Bob's // incoming link to block when it tries to submit the packet to the nil @@ -4400,6 +4456,7 @@ func TestChannelLinkAcceptOverpay(t *testing.T) { type persistentLinkHarness struct { t *testing.T + hSwitch *Switch link ChannelLink coreLink *channelLink channel *lnwallet.LightningChannel @@ -4412,7 +4469,7 @@ type persistentLinkHarness struct { // newPersistentLinkHarness initializes a new persistentLinkHarness and derives // the supporting references from the active link. -func newPersistentLinkHarness(t *testing.T, link ChannelLink, +func newPersistentLinkHarness(t *testing.T, hSwitch *Switch, link ChannelLink, batchTicker chan time.Time, restore func() (*lnwallet.LightningChannel, error)) *persistentLinkHarness { @@ -4421,6 +4478,7 @@ func newPersistentLinkHarness(t *testing.T, link ChannelLink, return &persistentLinkHarness{ t: t, + hSwitch: hSwitch, link: link, coreLink: coreLink, channel: coreLink.channel, @@ -4444,12 +4502,15 @@ func (h *persistentLinkHarness) restart(restartSwitch, syncStates bool, hodlFlags ...hodl.Flag) { // First, remove the link from the switch. - h.coreLink.cfg.Switch.RemoveLink(h.link.ChanID()) + h.hSwitch.RemoveLink(h.link.ChanID()) if restartSwitch { // If a switch restart is requested, we will stop it. It will be // reinstantiated in restartLink. - h.coreLink.cfg.Switch.Stop() + err := h.hSwitch.Stop() + if err != nil { + h.t.Fatalf("unable to stop switch: %v", err) + } } // Since our in-memory state may have diverged from our persistent @@ -4498,7 +4559,7 @@ func (h *persistentLinkHarness) checkSent(pkts []*htlcPacket) { // switch's circuit map. The forwarding actions are returned if there was no // failure. func (h *persistentLinkHarness) commitCircuits(circuits []*PaymentCircuit) *CircuitFwdActions { - fwdActions, err := h.coreLink.cfg.Switch.commitCircuits(circuits...) + fwdActions, err := h.hSwitch.commitCircuits(circuits...) if err != nil { h.t.Fatalf("unable to commit circuit: %v", err) } @@ -4511,12 +4572,12 @@ func (h *persistentLinkHarness) assertNumPendingNumOpenCircuits( _, _, line, _ := runtime.Caller(1) - numPending := h.coreLink.cfg.Switch.circuits.NumPending() + numPending := h.hSwitch.circuits.NumPending() if numPending != wantPending { h.t.Fatalf("line: %d: wrong number of pending circuits: "+ "want %d, got %d", line, wantPending, numPending) } - numOpen := h.coreLink.cfg.Switch.circuits.NumOpen() + numOpen := h.hSwitch.circuits.NumOpen() if numOpen != wantOpen { h.t.Fatalf("line: %d: wrong number of open circuits: "+ "want %d, got %d", line, wantOpen, numOpen) @@ -4562,10 +4623,9 @@ func (h *persistentLinkHarness) restartLink( ) aliceDb := aliceChannel.State().Db.GetParentDB() - aliceSwitch := h.coreLink.cfg.Switch if restartSwitch { var err error - aliceSwitch, err = initSwitchWithDB(testStartingHeight, aliceDb) + h.hSwitch, err = initSwitchWithDB(testStartingHeight, aliceDb) if err != nil { return nil, nil, err } @@ -4594,11 +4654,10 @@ func (h *persistentLinkHarness) restartLink( aliceCfg := ChannelLinkConfig{ FwrdingPolicy: globalPolicy, Peer: alicePeer, - Switch: aliceSwitch, - BestHeight: aliceSwitch.BestHeight, - Circuits: aliceSwitch.CircuitModifier(), + BestHeight: h.hSwitch.BestHeight, + Circuits: h.hSwitch.CircuitModifier(), ForwardPackets: func(linkQuit chan struct{}, _ bool, packets ...*htlcPacket) error { - return aliceSwitch.ForwardPackets(linkQuit, packets...) + return h.hSwitch.ForwardPackets(linkQuit, packets...) }, DecodeHopIterators: decoder.DecodeHopIterators, ExtractErrorEncrypter: func(*btcec.PublicKey) ( @@ -4634,13 +4693,13 @@ func (h *persistentLinkHarness) restartLink( NotifyActiveChannel: func(wire.OutPoint) {}, NotifyInactiveChannel: func(wire.OutPoint) {}, NotifyInactiveLinkEvent: func(wire.OutPoint) {}, - HtlcNotifier: aliceSwitch.cfg.HtlcNotifier, + HtlcNotifier: h.hSwitch.cfg.HtlcNotifier, SyncStates: syncStates, GetAliases: getAliases, } aliceLink := NewChannelLink(aliceCfg, aliceChannel) - if err := aliceSwitch.AddLink(aliceLink); err != nil { + if err := h.hSwitch.AddLink(aliceLink); err != nil { return nil, nil, err } go func() { @@ -4717,16 +4776,17 @@ func TestChannelLinkNoMoreUpdates(t *testing.T) { const chanAmt = btcutil.SatoshiPerBitcoin * 5 const chanReserve = btcutil.SatoshiPerBitcoin * 1 - aliceLink, bobChannel, _, start, _, err := + harness, err := newSingleLinkTestHarness(t, chanAmt, chanReserve) require.NoError(t, err, "unable to create link") - if err := start(); err != nil { + if err := harness.start(); err != nil { t.Fatalf("unable to start test harness: %v", err) } var ( - coreLink = aliceLink.(*channelLink) + //nolint:forcetypeassert + coreLink = harness.aliceLink.(*channelLink) aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs ) @@ -4735,10 +4795,11 @@ func TestChannelLinkNoMoreUpdates(t *testing.T) { htlc2 := generateHtlc(t, coreLink, 1) ctx := linkTestContext{ - t: t, - aliceLink: aliceLink, - aliceMsgs: aliceMsgs, - bobChannel: bobChannel, + t: t, + aliceSwitch: harness.aliceSwitch, + aliceLink: harness.aliceLink, + aliceMsgs: aliceMsgs, + bobChannel: harness.bobChannel, } // We now play out the following scanario: @@ -4840,16 +4901,17 @@ func TestChannelLinkWaitForRevocation(t *testing.T) { const chanAmt = btcutil.SatoshiPerBitcoin * 5 const chanReserve = btcutil.SatoshiPerBitcoin * 1 - aliceLink, bobChannel, _, start, _, err := + harness, err := newSingleLinkTestHarness(t, chanAmt, chanReserve) require.NoError(t, err, "unable to create link") - if err := start(); err != nil { + if err := harness.start(); err != nil { t.Fatalf("unable to start test harness: %v", err) } var ( - coreLink = aliceLink.(*channelLink) + //nolint:forcetypeassert + coreLink = harness.aliceLink.(*channelLink) aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs ) @@ -4862,10 +4924,11 @@ func TestChannelLinkWaitForRevocation(t *testing.T) { } ctx := linkTestContext{ - t: t, - aliceLink: aliceLink, - aliceMsgs: aliceMsgs, - bobChannel: bobChannel, + t: t, + aliceSwitch: harness.aliceSwitch, + aliceLink: harness.aliceLink, + aliceMsgs: aliceMsgs, + bobChannel: harness.bobChannel, } assertNoMsgFromAlice := func() { @@ -4958,25 +5021,27 @@ func TestChannelLinkNoEmptySig(t *testing.T) { const chanAmt = btcutil.SatoshiPerBitcoin * 5 const chanReserve = btcutil.SatoshiPerBitcoin * 1 - aliceLink, bobChannel, batchTicker, start, _, err := + harness, err := newSingleLinkTestHarness(t, chanAmt, chanReserve) require.NoError(t, err, "unable to create link") - if err := start(); err != nil { + if err := harness.start(); err != nil { t.Fatalf("unable to start test harness: %v", err) } - t.Cleanup(aliceLink.Stop) + t.Cleanup(harness.aliceLink.Stop) var ( - coreLink = aliceLink.(*channelLink) + //nolint:forcetypeassert + coreLink = harness.aliceLink.(*channelLink) aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs ) ctx := linkTestContext{ - t: t, - aliceLink: aliceLink, - aliceMsgs: aliceMsgs, - bobChannel: bobChannel, + t: t, + aliceSwitch: harness.aliceSwitch, + aliceLink: harness.aliceLink, + aliceMsgs: aliceMsgs, + bobChannel: harness.bobChannel, } // Send htlc 1 from Alice to Bob. @@ -4986,7 +5051,7 @@ func TestChannelLinkNoEmptySig(t *testing.T) { // Tick the batch ticker to trigger a commitsig from Alice->Bob. select { - case batchTicker <- time.Now(): + case harness.aliceBatchTicker <- time.Now(): case <-time.After(5 * time.Second): t.Fatalf("could not force commit sig") } @@ -5004,7 +5069,7 @@ func TestChannelLinkNoEmptySig(t *testing.T) { // Tick the batch ticker to trigger a commitsig from Alice->Bob. select { - case batchTicker <- time.Now(): + case harness.aliceBatchTicker <- time.Now(): case <-time.After(5 * time.Second): t.Fatalf("could not force commit sig") } @@ -5016,7 +5081,7 @@ func TestChannelLinkNoEmptySig(t *testing.T) { ctx.sendCommitSigBobToAlice(1) // Now send Bob the signature from Alice covering both htlcs. - err = bobChannel.ReceiveNewCommitment(&lnwallet.CommitSigs{ + err = harness.bobChannel.ReceiveNewCommitment(&lnwallet.CommitSigs{ CommitSig: commitSigAlice.CommitSig, HtlcSigs: commitSigAlice.HtlcSigs, }) @@ -5063,16 +5128,17 @@ func TestChannelLinkBatchPreimageWrite(t *testing.T) { func testChannelLinkBatchPreimageWrite(t *testing.T, disconnect bool) { const chanAmt = btcutil.SatoshiPerBitcoin * 5 const chanReserve = btcutil.SatoshiPerBitcoin * 1 - aliceLink, bobChannel, batchTicker, startUp, _, err := + harness, err := newSingleLinkTestHarness(t, chanAmt, chanReserve) require.NoError(t, err, "unable to create link") - if err := startUp(); err != nil { + if err := harness.start(); err != nil { t.Fatalf("unable to start test harness: %v", err) } var ( - coreLink = aliceLink.(*channelLink) + //nolint:forcetypeassert + coreLink = harness.aliceLink.(*channelLink) aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs ) @@ -5087,10 +5153,11 @@ func testChannelLinkBatchPreimageWrite(t *testing.T, disconnect bool) { } ctx := linkTestContext{ - t: t, - aliceLink: aliceLink, - aliceMsgs: aliceMsgs, - bobChannel: bobChannel, + t: t, + aliceSwitch: harness.aliceSwitch, + aliceLink: harness.aliceLink, + aliceMsgs: aliceMsgs, + bobChannel: harness.bobChannel, } // First, send a batch of Adds from Alice to Bob. @@ -5105,7 +5172,7 @@ func testChannelLinkBatchPreimageWrite(t *testing.T, disconnect bool) { // Force alice's link to sign a commitment covering the htlcs sent thus // far. select { - case batchTicker <- time.Now(): + case harness.aliceBatchTicker <- time.Now(): case <-time.After(15 * time.Second): t.Fatalf("could not force commit sig") } @@ -5137,7 +5204,7 @@ func testChannelLinkBatchPreimageWrite(t *testing.T, disconnect bool) { // CommitSig, and simply stop Alice's link. As she exits, we should // detect that she has uncommitted preimages and write them to disk. if disconnect { - aliceLink.Stop() + harness.aliceLink.Stop() checkHasPreimages(t, coreLink, htlcs, true) return } @@ -5171,16 +5238,17 @@ func TestChannelLinkCleanupSpuriousResponses(t *testing.T) { const chanAmt = btcutil.SatoshiPerBitcoin * 5 const chanReserve = btcutil.SatoshiPerBitcoin * 1 - aliceLink, bobChannel, _, start, _, err := + harness, err := newSingleLinkTestHarness(t, chanAmt, chanReserve) require.NoError(t, err, "unable to create link") - if err := start(); err != nil { + if err := harness.start(); err != nil { t.Fatalf("unable to start test harness: %v", err) } var ( - coreLink = aliceLink.(*channelLink) + //nolint:forcetypeassert + coreLink = harness.aliceLink.(*channelLink) aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs ) @@ -5194,10 +5262,11 @@ func TestChannelLinkCleanupSpuriousResponses(t *testing.T) { htlc2 := generateHtlc(t, coreLink, 1) ctx := linkTestContext{ - t: t, - aliceLink: aliceLink, - aliceMsgs: aliceMsgs, - bobChannel: bobChannel, + t: t, + aliceSwitch: harness.aliceSwitch, + aliceLink: harness.aliceLink, + aliceMsgs: aliceMsgs, + bobChannel: harness.bobChannel, } // We start with he following scenario: Bob sends Alice two HTLCs, and a @@ -5255,12 +5324,12 @@ func TestChannelLinkCleanupSpuriousResponses(t *testing.T) { // packet will NOT include a sourceRef, meaning the AddRef on disk will // not be acked after committing this response. fail0 := &htlcPacket{ - incomingChanID: bobChannel.ShortChanID(), + incomingChanID: harness.bobChannel.ShortChanID(), incomingHTLCID: 0, obfuscator: NewMockObfuscator(), htlc: &lnwire.UpdateFailHTLC{}, } - _ = aliceLink.handleSwitchPacket(fail0) + _ = harness.aliceLink.handleSwitchPacket(fail0) // Bob Alice // |<----- fal-1 ------| @@ -5313,12 +5382,12 @@ func TestChannelLinkCleanupSpuriousResponses(t *testing.T) { Height: addHeight, Index: 1, }, - incomingChanID: bobChannel.ShortChanID(), + incomingChanID: harness.bobChannel.ShortChanID(), incomingHTLCID: 1, obfuscator: NewMockObfuscator(), htlc: &lnwire.UpdateFailHTLC{}, } - _ = aliceLink.handleSwitchPacket(fail1) + _ = harness.aliceLink.handleSwitchPacket(fail1) // Bob Alice // |<----- fal-1 ------| @@ -5373,7 +5442,7 @@ func TestChannelLinkCleanupSpuriousResponses(t *testing.T) { // this should trigger an attempt to cleanup the spurious response. // However, we expect it to result in a NOP since it is still missing // its sourceRef. - _ = aliceLink.handleSwitchPacket(fail0) + _ = harness.aliceLink.handleSwitchPacket(fail0) // Allow the link enough time to process and reject the duplicate // packet, we'll also check that this doesn't trigger Alice to send the @@ -5421,12 +5490,12 @@ func TestChannelLinkCleanupSpuriousResponses(t *testing.T) { Height: addHeight, Index: 0, }, - incomingChanID: bobChannel.ShortChanID(), + incomingChanID: harness.bobChannel.ShortChanID(), incomingHTLCID: 0, obfuscator: NewMockObfuscator(), htlc: &lnwire.UpdateFailHTLC{}, } - _ = aliceLink.handleSwitchPacket(fail0) + _ = harness.aliceLink.handleSwitchPacket(fail0) // Allow the link enough time to process and reject the duplicate // packet, we'll also check that this doesn't trigger Alice to send the @@ -5510,7 +5579,7 @@ func TestChannelLinkFail(t *testing.T) { // link test is used to execute the given test on the channel // link after it is started. - linkTest func(*testing.T, *channelLink, + linkTest func(*testing.T, *Switch, *channelLink, *lnwallet.LightningChannel) // shouldFail indicates whether or not the link should fail @@ -5532,7 +5601,7 @@ func TestChannelLinkFail(t *testing.T) { "message", func(c *channelLink) { }, - func(_ *testing.T, c *channelLink, + func(_ *testing.T, _ *Switch, c *channelLink, _ *lnwallet.LightningChannel) { warningMsg := &lnwire.Warning{ @@ -5553,7 +5622,7 @@ func TestChannelLinkFail(t *testing.T) { // the SendMessage call fail. c.cfg.Peer.(*mockPeer).disconnected = true }, - func(*testing.T, *channelLink, + func(*testing.T, *Switch, *channelLink, *lnwallet.LightningChannel) { //nolint:whitespace,lll // Should fail at startup. @@ -5573,7 +5642,7 @@ func TestChannelLinkFail(t *testing.T) { } c.channel.State().Packager = pkg }, - func(*testing.T, *channelLink, + func(*testing.T, *Switch, *channelLink, *lnwallet.LightningChannel) { //nolint:whitespace,lll // Should fail at startup. @@ -5587,7 +5656,7 @@ func TestChannelLinkFail(t *testing.T) { "invalid Settle message", func(c *channelLink) { }, - func(_ *testing.T, c *channelLink, + func(_ *testing.T, s *Switch, c *channelLink, _ *lnwallet.LightningChannel) { // Recevive an htlc settle for an htlc that was @@ -5607,15 +5676,16 @@ func TestChannelLinkFail(t *testing.T) { "CommitSig, not containing enough HTLC sigs", func(c *channelLink) { }, - func(_ *testing.T, c *channelLink, + func(_ *testing.T, s *Switch, c *channelLink, remoteChannel *lnwallet.LightningChannel) { // Generate an HTLC and send to the link. htlc1 := generateHtlc(t, c, 0) ctx := linkTestContext{ - t: t, - aliceLink: c, - bobChannel: remoteChannel, + t: t, + aliceSwitch: s, + aliceLink: c, + bobChannel: remoteChannel, } ctx.sendHtlcBobToAlice(htlc1) @@ -5645,7 +5715,7 @@ func TestChannelLinkFail(t *testing.T) { "CommitSig, where the sig itself is corrupted", func(c *channelLink) { }, - func(t *testing.T, c *channelLink, + func(t *testing.T, s *Switch, c *channelLink, remoteChannel *lnwallet.LightningChannel) { t.Helper() @@ -5653,9 +5723,10 @@ func TestChannelLinkFail(t *testing.T) { // Generate an HTLC and send to the link. htlc1 := generateHtlc(t, c, 0) ctx := linkTestContext{ - t: t, - aliceLink: c, - bobChannel: remoteChannel, + t: t, + aliceSwitch: s, + aliceLink: c, + bobChannel: remoteChannel, } ctx.sendHtlcBobToAlice(htlc1) @@ -5693,7 +5764,7 @@ func TestChannelLinkFail(t *testing.T) { "error from the remote", func(c *channelLink) { }, - func(_ *testing.T, c *channelLink, + func(_ *testing.T, _ *Switch, c *channelLink, remoteChannel *lnwallet.LightningChannel) { err := &lnwire.Error{} @@ -5711,11 +5782,12 @@ func TestChannelLinkFail(t *testing.T) { // Execute each test case. for _, test := range testCases { - link, remoteChannel, _, start, _, err := + harness, err := newSingleLinkTestHarness(t, chanAmt, 0) require.NoError(t, err, test.name) - coreLink := link.(*channelLink) + //nolint:forcetypeassert + coreLink := harness.aliceLink.(*channelLink) // Set up a channel used to check whether the link error // force closed the channel. @@ -5728,11 +5800,13 @@ func TestChannelLinkFail(t *testing.T) { // Set up the link before starting it. test.options(coreLink) - err = start() + err = harness.start() require.NoError(t, err, test.name) // Execute the test case. - test.linkTest(t, coreLink, remoteChannel) + test.linkTest( + t, harness.aliceSwitch, coreLink, harness.bobChannel, + ) // Currently we expect all test cases to lead to link error. var linkErr LinkFailureError @@ -6207,15 +6281,16 @@ func TestChannelLinkHoldInvoiceRestart(t *testing.T) { // We'll start by creating a new link with our chanAmt (5 BTC). We will // only be testing Alice's behavior, so the reference to Bob's channel // state is unnecessary. - aliceLink, bobChannel, _, start, restore, err := + harness, err := newSingleLinkTestHarness(t, chanAmt, 0) require.NoError(t, err, "unable to create link") alice := newPersistentLinkHarness( - t, aliceLink, nil, restore, + t, harness.aliceSwitch, harness.aliceLink, nil, + harness.aliceRestore, ) - if err := start(); err != nil { + if err := harness.start(); err != nil { t.Fatalf("unable to start test harness: %v", err) } @@ -6241,10 +6316,11 @@ func TestChannelLinkHoldInvoiceRestart(t *testing.T) { require.NoError(t, err, "unable to add invoice to registry") ctx := linkTestContext{ - t: t, - aliceLink: alice.link, - aliceMsgs: alice.msgs, - bobChannel: bobChannel, + t: t, + aliceSwitch: harness.aliceSwitch, + aliceLink: alice.link, + aliceMsgs: alice.msgs, + bobChannel: harness.bobChannel, } // Lock in htlc paying the hodl invoice. @@ -6260,7 +6336,7 @@ func TestChannelLinkHoldInvoiceRestart(t *testing.T) { // Increase block height. This height will be retrieved by the link // after restart. - coreLink.cfg.Switch.bestHeight++ + harness.aliceSwitch.bestHeight++ // Restart link. alice.restart(false, false) @@ -6301,26 +6377,28 @@ func TestChannelLinkRevocationWindowRegular(t *testing.T) { // We'll start by creating a new link with our chanAmt (5 BTC). We will // only be testing Alice's behavior, so the reference to Bob's channel // state is unnecessary. - aliceLink, bobChannel, _, start, _, err := + harness, err := newSingleLinkTestHarness(t, chanAmt, 0) require.NoError(t, err, "unable to create link") - if err := start(); err != nil { + if err := harness.start(); err != nil { t.Fatalf("unable to start test harness: %v", err) } - t.Cleanup(aliceLink.Stop) + t.Cleanup(harness.aliceLink.Stop) var ( - coreLink = aliceLink.(*channelLink) + //nolint:forcetypeassert + coreLink = harness.aliceLink.(*channelLink) registry = coreLink.cfg.Registry.(*mockInvoiceRegistry) aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs ) ctx := linkTestContext{ - t: t, - aliceLink: aliceLink, - aliceMsgs: aliceMsgs, - bobChannel: bobChannel, + t: t, + aliceSwitch: harness.aliceSwitch, + aliceLink: harness.aliceLink, + aliceMsgs: aliceMsgs, + bobChannel: harness.bobChannel, } registry.settleChan = make(chan lntypes.Hash) @@ -6384,16 +6462,17 @@ func TestChannelLinkRevocationWindowHodl(t *testing.T) { // We'll start by creating a new link with our chanAmt (5 BTC). We will // only be testing Alice's behavior, so the reference to Bob's channel // state is unnecessary. - aliceLink, bobChannel, batchTicker, start, _, err := + harness, err := newSingleLinkTestHarness(t, chanAmt, 0) require.NoError(t, err, "unable to create link") - if err := start(); err != nil { + if err := harness.start(); err != nil { t.Fatalf("unable to start test harness: %v", err) } var ( - coreLink = aliceLink.(*channelLink) + //nolint:forcetypeassert + coreLink = harness.aliceLink.(*channelLink) registry = coreLink.cfg.Registry.(*mockInvoiceRegistry) aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs ) @@ -6422,10 +6501,11 @@ func TestChannelLinkRevocationWindowHodl(t *testing.T) { require.NoError(t, err, "unable to add invoice to registry") ctx := linkTestContext{ - t: t, - aliceLink: aliceLink, - aliceMsgs: aliceMsgs, - bobChannel: bobChannel, + t: t, + aliceSwitch: harness.aliceSwitch, + aliceLink: harness.aliceLink, + aliceMsgs: aliceMsgs, + bobChannel: harness.bobChannel, } // Lock in htlc 1 on both sides. @@ -6493,7 +6573,7 @@ func TestChannelLinkRevocationWindowHodl(t *testing.T) { // Trigger the batch timer as this may trigger Alice to send a commit // sig. - batchTicker <- time.Time{} + harness.aliceBatchTicker <- time.Time{} // After the revocation, it is again possible for Alice to send a commit // sig no more htlcs. Bob acks the update. @@ -6505,7 +6585,7 @@ func TestChannelLinkRevocationWindowHodl(t *testing.T) { ctx.receiveRevAndAckAliceToBob() // Stop the link - aliceLink.Stop() + harness.aliceLink.Stop() // Check that no unexpected messages were sent. select { @@ -6523,24 +6603,26 @@ func TestChannelLinkReceiveEmptySig(t *testing.T) { const chanAmt = btcutil.SatoshiPerBitcoin * 5 const chanReserve = btcutil.SatoshiPerBitcoin * 1 - aliceLink, bobChannel, batchTicker, start, _, err := + harness, err := newSingleLinkTestHarness(t, chanAmt, chanReserve) require.NoError(t, err, "unable to create link") - if err := start(); err != nil { + if err := harness.start(); err != nil { t.Fatalf("unable to start test harness: %v", err) } var ( - coreLink = aliceLink.(*channelLink) + //nolint:forcetypeassert + coreLink = harness.aliceLink.(*channelLink) aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs ) ctx := linkTestContext{ - t: t, - aliceLink: aliceLink, - aliceMsgs: aliceMsgs, - bobChannel: bobChannel, + t: t, + aliceSwitch: harness.aliceSwitch, + aliceLink: harness.aliceLink, + aliceMsgs: aliceMsgs, + bobChannel: harness.bobChannel, } htlc, _ := generateHtlcAndInvoice(t, 0) @@ -6551,7 +6633,7 @@ func TestChannelLinkReceiveEmptySig(t *testing.T) { // Tick the batch ticker to trigger a commitsig from Alice->Bob. select { - case batchTicker <- time.Now(): + case harness.aliceBatchTicker <- time.Now(): case <-time.After(5 * time.Second): t.Fatalf("could not force commit sig") } @@ -6580,7 +6662,7 @@ func TestChannelLinkReceiveEmptySig(t *testing.T) { ctx.assertNoMsgFromAlice(time.Second) // Stop the link - aliceLink.Stop() + harness.aliceLink.Stop() } // TestPendingCommitTicker tests that a link will fail itself after a timeout if @@ -6590,12 +6672,13 @@ func TestPendingCommitTicker(t *testing.T) { const chanAmt = btcutil.SatoshiPerBitcoin * 5 const chanReserve = btcutil.SatoshiPerBitcoin * 1 - aliceLink, bobChannel, batchTicker, start, _, err := + harness, err := newSingleLinkTestHarness(t, chanAmt, chanReserve) require.NoError(t, err, "unable to create link") var ( - coreLink = aliceLink.(*channelLink) + //nolint:forcetypeassert + coreLink = harness.aliceLink.(*channelLink) aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs ) @@ -6608,15 +6691,16 @@ func TestPendingCommitTicker(t *testing.T) { linkErrs <- linkErr } - if err := start(); err != nil { + if err := harness.start(); err != nil { t.Fatalf("unable to start test harness: %v", err) } ctx := linkTestContext{ - t: t, - aliceLink: aliceLink, - bobChannel: bobChannel, - aliceMsgs: aliceMsgs, + t: t, + aliceSwitch: harness.aliceSwitch, + aliceLink: harness.aliceLink, + bobChannel: harness.bobChannel, + aliceMsgs: aliceMsgs, } // Send an HTLC from Alice to Bob, and signal the batch ticker to signa @@ -6624,7 +6708,7 @@ func TestPendingCommitTicker(t *testing.T) { htlc, _ := generateHtlcAndInvoice(t, 0) ctx.sendHtlcAliceToBob(0, htlc) ctx.receiveHtlcAliceToBob() - batchTicker <- time.Now() + harness.aliceBatchTicker <- time.Now() select { case msg := <-aliceMsgs: @@ -6649,7 +6733,7 @@ func TestPendingCommitTicker(t *testing.T) { htlc, _ = generateHtlcAndInvoice(t, 1) ctx.sendHtlcAliceToBob(1, htlc) ctx.receiveHtlcAliceToBob() - batchTicker <- time.Now() + harness.aliceBatchTicker <- time.Now() // Assert that we get the expected link failure from Alice. select { @@ -6673,12 +6757,13 @@ func TestPipelineSettle(t *testing.T) { const chanAmt = btcutil.SatoshiPerBitcoin * 5 const chanReserve = btcutil.SatoshiPerBitcoin * 1 - aliceLink, bobChannel, _, start, restore, err := + harness, err := newSingleLinkTestHarness(t, chanAmt, chanReserve) require.NoError(t, err) alice := newPersistentLinkHarness( - t, aliceLink, nil, restore, + t, harness.aliceSwitch, harness.aliceLink, nil, + harness.aliceRestore, ) linkErrors := make(chan LinkFailureError, 1) @@ -6706,14 +6791,15 @@ func TestPipelineSettle(t *testing.T) { // settle. alice.coreLink.cfg.HodlMask = hodl.Mask(hodl.ExitSettle) - err = start() + err = harness.start() require.NoError(t, err) ctx := linkTestContext{ - t: t, - aliceLink: alice.link, - bobChannel: bobChannel, - aliceMsgs: alice.msgs, + t: t, + aliceSwitch: harness.aliceSwitch, + aliceLink: alice.link, + bobChannel: harness.bobChannel, + aliceMsgs: alice.msgs, } // First lock in an HTLC from Bob to Alice. @@ -6864,13 +6950,13 @@ func TestChannelLinkShortFailureRelay(t *testing.T) { const chanAmt = btcutil.SatoshiPerBitcoin * 5 - aliceLink, bobChannel, batchTicker, start, _, err := + harness, err := newSingleLinkTestHarness(t, chanAmt, 0) require.NoError(t, err, "unable to create link") - require.NoError(t, start()) + require.NoError(t, harness.start()) - coreLink, ok := aliceLink.(*channelLink) + coreLink, ok := harness.aliceLink.(*channelLink) require.True(t, ok) mockPeer, ok := coreLink.cfg.Peer.(*mockPeer) @@ -6890,10 +6976,11 @@ func TestChannelLinkShortFailureRelay(t *testing.T) { } ctx := linkTestContext{ - t: t, - aliceLink: aliceLink, - aliceMsgs: aliceMsgs, - bobChannel: bobChannel, + t: t, + aliceSwitch: harness.aliceSwitch, + aliceLink: harness.aliceLink, + aliceMsgs: aliceMsgs, + bobChannel: harness.bobChannel, } // Send and lock in htlc from Alice to Bob. @@ -6903,7 +6990,7 @@ func TestChannelLinkShortFailureRelay(t *testing.T) { ctx.sendHtlcAliceToBob(htlcID, htlc) ctx.receiveHtlcAliceToBob() - batchTicker <- time.Now() + harness.aliceBatchTicker <- time.Now() ctx.receiveCommitSigAliceToBob(1) ctx.sendRevAndAckBobToAlice() @@ -6914,10 +7001,10 @@ func TestChannelLinkShortFailureRelay(t *testing.T) { // Return a short htlc failure from Bob to Alice and lock in. shortReason := make([]byte, 260) - err = bobChannel.FailHTLC(0, shortReason, nil, nil, nil) + err = harness.bobChannel.FailHTLC(0, shortReason, nil, nil, nil) require.NoError(t, err) - aliceLink.HandleChannelUpdate(&lnwire.UpdateFailHTLC{ + harness.aliceLink.HandleChannelUpdate(&lnwire.UpdateFailHTLC{ ID: htlcID, Reason: shortReason, }) @@ -6941,7 +7028,7 @@ func TestChannelLinkShortFailureRelay(t *testing.T) { require.Len(t, htlcFailMsg.Reason, 292) // Stop the link - aliceLink.Stop() + harness.aliceLink.Stop() // Check that no unexpected messages were sent. select { @@ -6961,11 +7048,12 @@ func TestChannelLinkShortFailureRelay(t *testing.T) { // unaffected by calls to EnableAdds/DisableAdds on the Incoming direction and // vice-versa. func TestLinkFlushApiDirectionIsolation(t *testing.T) { - aliceLink, _, _, _, _, _ := + harness, _ := newSingleLinkTestHarness( t, 5*btcutil.SatoshiPerBitcoin, 1*btcutil.SatoshiPerBitcoin, ) + aliceLink := harness.aliceLink for i := 0; i < 10; i++ { if prand.Uint64()%2 == 0 { @@ -6998,11 +7086,12 @@ func TestLinkFlushApiDirectionIsolation(t *testing.T) { // error (to inform the caller that the call was unnecessary in case it implies // a bug in their logic). func TestLinkFlushApiGateStateIdempotence(t *testing.T) { - aliceLink, _, _, _, _, _ := + harness, _ := newSingleLinkTestHarness( t, 5*btcutil.SatoshiPerBitcoin, 1*btcutil.SatoshiPerBitcoin, ) + aliceLink := harness.aliceLink for _, dir := range []LinkDirection{Incoming, Outgoing} { require.True(t, aliceLink.DisableAdds(dir)) @@ -7020,34 +7109,39 @@ func TestLinkFlushApiGateStateIdempotence(t *testing.T) { } func TestLinkOutgoingCommitHooksCalled(t *testing.T) { - aliceLink, bobChannel, batchTicker, start, _, err := + harness, err := newSingleLinkTestHarness( t, 5*btcutil.SatoshiPerBitcoin, btcutil.SatoshiPerBitcoin, ) require.NoError(t, err) - require.NoError(t, start(), "could not start link") + require.NoError(t, harness.start(), "could not start link") hookCalled := make(chan struct{}) - //nolint:forcetypeassert - aliceMsgs := aliceLink.(*channelLink).cfg.Peer.(*mockPeer).sentMsgs + var ( + //nolint:forcetypeassert + coreLink = harness.aliceLink.(*channelLink) + //nolint:forcetypeassert + aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs + ) ctx := linkTestContext{ - t: t, - aliceLink: aliceLink, - bobChannel: bobChannel, - aliceMsgs: aliceMsgs, + t: t, + aliceSwitch: harness.aliceSwitch, + aliceLink: harness.aliceLink, + bobChannel: harness.bobChannel, + aliceMsgs: aliceMsgs, } // Set up a pending HTLC on the link. //nolint:forcetypeassert - htlc := generateHtlc(t, aliceLink.(*channelLink), 0) + htlc := generateHtlc(t, harness.aliceLink.(*channelLink), 0) ctx.sendHtlcAliceToBob(0, htlc) ctx.receiveHtlcAliceToBob() - aliceLink.OnCommitOnce(Outgoing, func() { + harness.aliceLink.OnCommitOnce(Outgoing, func() { close(hookCalled) }) @@ -7057,11 +7151,11 @@ func TestLinkOutgoingCommitHooksCalled(t *testing.T) { case <-time.NewTimer(time.Second).C: } - batchTicker <- time.Now() + harness.aliceBatchTicker <- time.Now() // Send a second tick just to ensure the hook isn't called more than // once. - batchTicker <- time.Now() + harness.aliceBatchTicker <- time.Now() select { case <-hookCalled: @@ -7071,23 +7165,26 @@ func TestLinkOutgoingCommitHooksCalled(t *testing.T) { } func TestLinkFlushHooksCalled(t *testing.T) { - aliceLink, bobChannel, _, start, _, err := + harness, err := newSingleLinkTestHarness( t, 5*btcutil.SatoshiPerBitcoin, btcutil.SatoshiPerBitcoin, ) require.NoError(t, err) - require.NoError(t, start(), "could not start link") + require.NoError(t, harness.start(), "could not start link") //nolint:forcetypeassert - aliceMsgs := aliceLink.(*channelLink).cfg.Peer.(*mockPeer).sentMsgs + coreLink := harness.aliceLink.(*channelLink) + //nolint:forcetypeassert + aliceMsgs := coreLink.cfg.Peer.(*mockPeer).sentMsgs ctx := linkTestContext{ - t: t, - aliceLink: aliceLink, - bobChannel: bobChannel, - aliceMsgs: aliceMsgs, + t: t, + aliceSwitch: harness.aliceSwitch, + aliceLink: harness.aliceLink, + bobChannel: harness.bobChannel, + aliceMsgs: aliceMsgs, } hookCalled := make(chan struct{}) @@ -7104,7 +7201,7 @@ func TestLinkFlushHooksCalled(t *testing.T) { } //nolint:forcetypeassert - htlc := generateHtlc(t, aliceLink.(*channelLink), 0) + htlc := generateHtlc(t, harness.aliceLink.(*channelLink), 0) // A <- add -- B ctx.sendHtlcBobToAlice(htlc) @@ -7116,7 +7213,7 @@ func TestLinkFlushHooksCalled(t *testing.T) { ctx.receiveRevAndAckAliceToBob() // Register flush hook - aliceLink.OnFlushedOnce(func() { + harness.aliceLink.OnFlushedOnce(func() { close(hookCalled) }) diff --git a/htlcswitch/switch_test.go b/htlcswitch/switch_test.go index b443be355..c4982b296 100644 --- a/htlcswitch/switch_test.go +++ b/htlcswitch/switch_test.go @@ -5263,10 +5263,12 @@ func testSwitchHandlePacketForward(t *testing.T, zeroConf, private, t.Parallel() // Create a link for Alice that we'll add to the switch. - aliceLink, _, _, _, _, err := + harness, err := newSingleLinkTestHarness(t, btcutil.SatoshiPerBitcoin, 0) require.NoError(t, err) + aliceLink := harness.aliceLink + s, err := initSwitchWithTempDB(t, testStartingHeight) if err != nil { t.Fatalf("unable to init switch: %v", err) diff --git a/htlcswitch/test_utils.go b/htlcswitch/test_utils.go index e33c5fe20..c566118d6 100644 --- a/htlcswitch/test_utils.go +++ b/htlcswitch/test_utils.go @@ -1129,7 +1129,6 @@ func (h *hopNetwork) createChannelLink(server, peer *mockServer, link := NewChannelLink( ChannelLinkConfig{ - Switch: server.htlcSwitch, BestHeight: server.htlcSwitch.BestHeight, FwrdingPolicy: h.globalPolicy, Peer: peer, From 37619126805656c2e2eab06a892e9ea0b2703912 Mon Sep 17 00:00:00 2001 From: Keagan McClelland Date: Mon, 26 Feb 2024 13:10:22 -0800 Subject: [PATCH 3/5] htlcswitch: avoid leaking peer interface from link Here we notice that the only use of the Peer call on the link is to find out what the peer's pubkey is. To avoid leaking handles to IO actions outside the interface we reduce the surface area to just return the peer's public key. --- htlcswitch/interfaces.go | 7 +++---- htlcswitch/link.go | 4 ++-- htlcswitch/mock.go | 20 ++++++++++++++++---- htlcswitch/switch.go | 10 +++++----- 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/htlcswitch/interfaces.go b/htlcswitch/interfaces.go index 75866beba..ac3e10bab 100644 --- a/htlcswitch/interfaces.go +++ b/htlcswitch/interfaces.go @@ -7,7 +7,6 @@ import ( "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/channeldb/models" "github.com/lightningnetwork/lnd/invoices" - "github.com/lightningnetwork/lnd/lnpeer" "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwallet/chainfee" "github.com/lightningnetwork/lnd/lnwire" @@ -262,9 +261,9 @@ type ChannelLink interface { // total sent/received milli-satoshis. Stats() (uint64, lnwire.MilliSatoshi, lnwire.MilliSatoshi) - // Peer returns the representation of remote peer with which we have - // the channel link opened. - Peer() lnpeer.Peer + // Peer returns the serialized public key of remote peer with which we + // have the channel link opened. + PeerPubKey() [33]byte // AttachMailBox delivers an active MailBox to the link. The MailBox may // have buffered messages. diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 944ac0591..0b1a3b160 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -2552,8 +2552,8 @@ func (l *channelLink) updateCommitTx() error { // channel link opened. // // NOTE: Part of the ChannelLink interface. -func (l *channelLink) Peer() lnpeer.Peer { - return l.cfg.Peer +func (l *channelLink) PeerPubKey() [33]byte { + return l.cfg.Peer.PubKey() } // ChannelPoint returns the channel outpoint for the channel link. diff --git a/htlcswitch/mock.go b/htlcswitch/mock.go index ab6fbe76a..e995b3ed7 100644 --- a/htlcswitch/mock.go +++ b/htlcswitch/mock.go @@ -891,10 +891,22 @@ func (f *mockChannelLink) Start() error { return nil } -func (f *mockChannelLink) ChanID() lnwire.ChannelID { return f.chanID } -func (f *mockChannelLink) ShortChanID() lnwire.ShortChannelID { return f.shortChanID } -func (f *mockChannelLink) Bandwidth() lnwire.MilliSatoshi { return 99999999 } -func (f *mockChannelLink) Peer() lnpeer.Peer { return f.peer } +func (f *mockChannelLink) ChanID() lnwire.ChannelID { + return f.chanID +} + +func (f *mockChannelLink) ShortChanID() lnwire.ShortChannelID { + return f.shortChanID +} + +func (f *mockChannelLink) Bandwidth() lnwire.MilliSatoshi { + return 99999999 +} + +func (f *mockChannelLink) PeerPubKey() [33]byte { + return f.peer.PubKey() +} + func (f *mockChannelLink) ChannelPoint() *wire.OutPoint { return &wire.OutPoint{} } func (f *mockChannelLink) Stop() {} func (f *mockChannelLink) EligibleToForward() bool { return f.eligible } diff --git a/htlcswitch/switch.go b/htlcswitch/switch.go index fa98cca11..0e2101027 100644 --- a/htlcswitch/switch.go +++ b/htlcswitch/switch.go @@ -1147,7 +1147,7 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error { return s.failAddPacket(packet, linkError) } - targetPeerKey := targetLink.Peer().PubKey() + targetPeerKey := targetLink.PeerPubKey() interfaceLinks, _ := s.getLinks(targetPeerKey) s.indexMtx.RUnlock() @@ -1810,9 +1810,9 @@ out: } s.indexMtx.RUnlock() - peerPub := link.Peer().PubKey() + peerPub := link.PeerPubKey() log.Debugf("Requesting local channel close: peer=%v, "+ - "chan_id=%x", link.Peer(), chanID[:]) + "chan_id=%x", link.PeerPubKey(), chanID[:]) go s.cfg.LocalChannelClose(peerPub[:], req) @@ -2335,7 +2335,7 @@ func (s *Switch) addLiveLink(link ChannelLink) { // Next we'll add the link to the interface index so we can // quickly look up all the channels for a particular node. - peerPub := link.Peer().PubKey() + peerPub := link.PeerPubKey() if _, ok := s.interfaceIndex[peerPub]; !ok { s.interfaceIndex[peerPub] = make(map[lnwire.ChannelID]ChannelLink) } @@ -2610,7 +2610,7 @@ func (s *Switch) removeLink(chanID lnwire.ChannelID) ChannelLink { // If the link has been added to the peer index, then we'll move to // delete the entry within the index. - peerPub := link.Peer().PubKey() + peerPub := link.PeerPubKey() if peerIndex, ok := s.interfaceIndex[peerPub]; ok { delete(peerIndex, link.ChanID()) From f5f0286e3400049e81b2adb86966e4610e54d225 Mon Sep 17 00:00:00 2001 From: Keagan McClelland Date: Sun, 3 Mar 2024 11:53:39 -0800 Subject: [PATCH 4/5] peer+lnwire: move LinkUpdater to lnwire The purpose of this interface is to distinguish messages that are bound to a particular channel_id from those that are not. There is no reason this ought to be a property of the peer package as it is entirely determined by the contents of the message itself. Therefore we move it to the lnwire package so it can be used without having import cycles elsewhere in the codebase. --- lnwire/message.go | 8 ++++++++ peer/brontide.go | 2 +- peer/interfaces.go | 8 -------- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lnwire/message.go b/lnwire/message.go index 634f78ab3..0f28a2465 100644 --- a/lnwire/message.go +++ b/lnwire/message.go @@ -190,6 +190,14 @@ type Message interface { MsgType() MessageType } +// LinkUpdater is an interface implemented by most messages in BOLT 2 that are +// allowed to update the channel state. +type LinkUpdater interface { + // TargetChanID returns the channel id of the link for which this + // message is intended. + TargetChanID() ChannelID +} + // makeEmptyMessage creates a new empty message of the proper concrete type // based on the passed message type. func makeEmptyMessage(msgType MessageType) (Message, error) { diff --git a/peer/brontide.go b/peer/brontide.go index 696ad8610..189fb1660 100644 --- a/peer/brontide.go +++ b/peer/brontide.go @@ -1750,7 +1750,7 @@ out: // will consider them as link updates and send them to // chanStream. These messages will be queued inside chanStream // if the channel is not active yet. - case LinkUpdater: + case lnwire.LinkUpdater: targetChan = msg.TargetChanID() isLinkUpdate = p.hasChannel(targetChan) diff --git a/peer/interfaces.go b/peer/interfaces.go index 562d89723..4f4619fef 100644 --- a/peer/interfaces.go +++ b/peer/interfaces.go @@ -34,14 +34,6 @@ type messageSwitch interface { error) } -// LinkUpdater is an interface implemented by most messages in BOLT 2 that are -// allowed to update the channel state. -type LinkUpdater interface { - // TargetChanID returns the channel id of the link for which this message - // is intended. - TargetChanID() lnwire.ChannelID -} - // MessageConn is an interface implemented by anything that delivers // an lnwire.Message using a net.Conn interface. type MessageConn interface { From 59700892b713f03105099f76d50c0df84026ae6b Mon Sep 17 00:00:00 2001 From: Keagan McClelland Date: Sun, 3 Mar 2024 11:59:57 -0800 Subject: [PATCH 5/5] lnwire: make LinkUpdater a subclass of Message Conceptually, all messages that are capable of identifying the channel_id on which they operate are themselves messages. Here we codify this. --- lnwire/message.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lnwire/message.go b/lnwire/message.go index 0f28a2465..bcee9f86d 100644 --- a/lnwire/message.go +++ b/lnwire/message.go @@ -193,6 +193,11 @@ type Message interface { // LinkUpdater is an interface implemented by most messages in BOLT 2 that are // allowed to update the channel state. type LinkUpdater interface { + // All LinkUpdater messages are messages and so we embed the interface + // so that we can treat it as a message if all we know about it is that + // it is a LinkUpdater message. + Message + // TargetChanID returns the channel id of the link for which this // message is intended. TargetChanID() ChannelID