From 1cd4fdc85920799b95d642a718dc6a7e97b732b4 Mon Sep 17 00:00:00 2001 From: Jonathan Harvey-Buschel Date: Wed, 18 Sep 2024 08:07:21 -0400 Subject: [PATCH 1/9] lnwallet: allow mock aux signer to fail sig jobs --- lnwallet/channel_test.go | 14 ++++++++++---- lnwallet/mock.go | 15 +++++++++------ 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 4d972ece49a..e1d7ac319eb 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -3389,8 +3389,14 @@ func TestChanSyncOweCommitmentAuxSigner(t *testing.T) { aliceChannel, bobChannel, err := CreateTestChannels(t, chanType) require.NoError(t, err, "unable to create test channels") - // We'll now manually attach an aux signer to Alice's channel. - auxSigner := &auxSignerMock{} + // We'll now manually attach an aux signer to Alice's channel. We'll + // set each aux sig job to receive an instant response. + emptyAuxSigJobResponder := func(jobs []AuxSigJob) { + for _, sigJob := range jobs { + sigJob.Resp <- AuxSigJobResp{} + } + } + auxSigner := NewAuxSignerMock(emptyAuxSigJobResponder) aliceChannel.auxSigner = fn.Some[AuxSigner](auxSigner) var fakeOnionBlob [lnwire.OnionPacketSize]byte @@ -3414,8 +3420,8 @@ func TestChanSyncOweCommitmentAuxSigner(t *testing.T) { _, err = aliceChannel.AddHTLC(h, nil) require.NoError(t, err, "unable to recv bob's htlc: %v", err) - // We'll set up the mock to expect calls to PackSigs and also - // SubmitSubmitSecondLevelSigBatch. + // We'll set up the mock aux signer to expect calls to PackSigs and also + // SubmitSecondLevelSigBatch. var sigBlobBuf bytes.Buffer sigBlob := testSigBlob{ BlobInt: tlv.NewPrimitiveRecord[tlv.TlvType65634, uint16](5), diff --git a/lnwallet/mock.go b/lnwallet/mock.go index 89c31ad9857..1e531b92265 100644 --- a/lnwallet/mock.go +++ b/lnwallet/mock.go @@ -391,6 +391,14 @@ func (*mockChainIO) GetBlockHeader( type auxSignerMock struct { mock.Mock + + jobHandlerFunc func([]AuxSigJob) +} + +func NewAuxSignerMock(jobHandler func([]AuxSigJob)) *auxSignerMock { + return &auxSignerMock{ + jobHandlerFunc: jobHandler, + } } func (a *auxSignerMock) SubmitSecondLevelSigBatch( @@ -398,12 +406,7 @@ func (a *auxSignerMock) SubmitSecondLevelSigBatch( commitTx *wire.MsgTx, sigJobs []AuxSigJob) error { args := a.Called(chanState, commitTx, sigJobs) - - // While we return, we'll also send back an instant response for the - // set of jobs. - for _, sigJob := range sigJobs { - sigJob.Resp <- AuxSigJobResp{} - } + a.jobHandlerFunc(sigJobs) return args.Error(0) } From 43ab85b8746f7e7753934f6402d77bc972a7b4b7 Mon Sep 17 00:00:00 2001 From: Jonathan Harvey-Buschel Date: Mon, 30 Sep 2024 15:22:26 -0400 Subject: [PATCH 2/9] lnwallet: refactor test code for HTLC add and recv --- lnwallet/channel_test.go | 402 ++++++++------------------------------- 1 file changed, 75 insertions(+), 327 deletions(-) diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index e1d7ac319eb..1799c4480ba 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -22,6 +22,7 @@ import ( "github.com/davecgh/go-spew/spew" "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/channeldb/models" "github.com/lightningnetwork/lnd/fn" "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/lntypes" @@ -49,6 +50,18 @@ func createHTLC(id int, amount lnwire.MilliSatoshi) (*lnwire.UpdateAddHTLC, [32] }, returnPreimage } +// addAndReceiveHTLC adds an HTLC as local to the first channel, and as remote +// to a second channel. The HTLC ID is not modified. +func addAndReceiveHTLC(t *testing.T, channel1, channel2 *LightningChannel, + htlc *lnwire.UpdateAddHTLC, openKey *models.CircuitKey) { + + _, err := channel1.AddHTLC(htlc, openKey) + require.NoErrorf(t, err, "channel 1 unable to add htlc: %v", err) + + _, err = channel2.ReceiveHTLC(htlc) + require.NoErrorf(t, err, "channel 2 unable to recv htlc: %v", err) +} + func assertOutputExistsByValue(t *testing.T, commitTx *wire.MsgTx, value btcutil.Amount) { @@ -435,10 +448,7 @@ func TestChannelZeroAddLocalHeight(t *testing.T) { htlc, _ := createHTLC(0, lnwire.MilliSatoshi(500000)) // -----add-----> - _, err = aliceChannel.AddHTLC(htlc, nil) - require.NoError(t, err) - _, err = bobChannel.ReceiveHTLC(htlc) - require.NoError(t, err) + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) // Force a state transition to lock in this add on both commitments. // -----sig-----> @@ -484,10 +494,7 @@ func TestChannelZeroAddLocalHeight(t *testing.T) { htlc2, _ := createHTLC(0, lnwire.MilliSatoshi(500000)) // <----add----- - _, err = bobChannel.AddHTLC(htlc2, nil) - require.NoError(t, err) - _, err = newAliceChannel.ReceiveHTLC(htlc2) - require.NoError(t, err) + addAndReceiveHTLC(t, bobChannel, newAliceChannel, htlc2, nil) // Bob should now send a commitment signature to Alice. // <----sig----- @@ -544,12 +551,7 @@ func TestCheckCommitTxSize(t *testing.T) { for i := 0; i <= 10; i++ { htlc, _ := createHTLC(i, lnwire.MilliSatoshi(1e7)) - if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { - t.Fatalf("alice unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { - t.Fatalf("bob unable to receive htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) if err := ForceStateTransition(aliceChannel, bobChannel); err != nil { t.Fatalf("unable to complete state update: %v", err) @@ -624,12 +626,7 @@ func testCommitHTLCSigTieBreak(t *testing.T, restart bool) { Expiry: uint32(numHtlcs - i), } - if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { - t.Fatalf("alice unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { - t.Fatalf("bob unable to receive htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) } // Have Alice initiate the first half of the commitment dance. The @@ -873,23 +870,13 @@ func testForceClose(t *testing.T, testCase *forceCloseTestCase) { // We'll ensure that the HTLC amount is above Alice's dust limit. htlcAmount := lnwire.NewMSatFromSatoshis(20000) htlcAlice, _ := createHTLC(0, htlcAmount) - if _, err := aliceChannel.AddHTLC(htlcAlice, nil); err != nil { - t.Fatalf("alice unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlcAlice); err != nil { - t.Fatalf("bob unable to recv add htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlcAlice, nil) // We'll also a distinct HTLC from Bob -> Alice. This way, Alice will // have both an incoming and outgoing HTLC on her commitment // transaction. htlcBob, preimageBob := createHTLC(0, htlcAmount) - if _, err := bobChannel.AddHTLC(htlcBob, nil); err != nil { - t.Fatalf("alice unable to add htlc: %v", err) - } - if _, err := aliceChannel.ReceiveHTLC(htlcBob); err != nil { - t.Fatalf("bob unable to recv add htlc: %v", err) - } + addAndReceiveHTLC(t, bobChannel, aliceChannel, htlcBob, nil) // Next, we'll perform two state transitions to ensure that both HTLC's // get fully locked-in. @@ -1343,12 +1330,7 @@ func TestDustHTLCFees(t *testing.T) { // This HTLC amount should be lower than the dust limits of both nodes. htlcAmount := lnwire.NewMSatFromSatoshis(100) htlc, _ := createHTLC(0, htlcAmount) - if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { - t.Fatalf("alice unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { - t.Fatalf("bob unable to receive htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) if err := ForceStateTransition(aliceChannel, bobChannel); err != nil { t.Fatalf("Can't update the channel state: %v", err) } @@ -1501,14 +1483,9 @@ func TestHTLCSigNumber(t *testing.T) { for i, htlcSat := range htlcValues { htlcMsat := lnwire.NewMSatFromSatoshis(htlcSat) htlc, _ := createHTLC(i, htlcMsat) - _, err := aliceChannel.AddHTLC(htlc, nil) - if err != nil { - t.Fatalf("alice unable to add htlc: %v", err) - } - _, err = bobChannel.ReceiveHTLC(htlc) - if err != nil { - t.Fatalf("bob unable to receive htlc: %v", err) - } + addAndReceiveHTLC( + t, aliceChannel, bobChannel, htlc, nil, + ) } return aliceChannel, bobChannel @@ -1742,12 +1719,7 @@ func TestStateUpdatePersistence(t *testing.T) { OnionBlob: fakeOnionBlob, } - if _, err := aliceChannel.AddHTLC(h, nil); err != nil { - t.Fatalf("unable to add alice's htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(h); err != nil { - t.Fatalf("unable to recv alice's htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, h, nil) } rHash := sha256.Sum256(bobPreimage[:]) bobh := &lnwire.UpdateAddHTLC{ @@ -1756,12 +1728,7 @@ func TestStateUpdatePersistence(t *testing.T) { Expiry: uint32(10), OnionBlob: fakeOnionBlob, } - if _, err := bobChannel.AddHTLC(bobh, nil); err != nil { - t.Fatalf("unable to add bob's htlc: %v", err) - } - if _, err := aliceChannel.ReceiveHTLC(bobh); err != nil { - t.Fatalf("unable to recv bob's htlc: %v", err) - } + addAndReceiveHTLC(t, bobChannel, aliceChannel, bobh, nil) // Also add a fee update to the update logs. fee := chainfee.SatPerKWeight(333) @@ -2410,12 +2377,7 @@ func TestUpdateFeeConcurrentSig(t *testing.T) { // First Alice adds the outgoing HTLC to her local channel's state // update log. Then Alice sends this wire message over to Bob who // adds this htlc to his remote state update log. - if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { - t.Fatalf("unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { - t.Fatalf("unable to recv htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) // Simulate Alice sending update fee message to bob. fee := chainfee.SatPerKWeight(333) @@ -2485,12 +2447,7 @@ func TestUpdateFeeSenderCommits(t *testing.T) { // First Alice adds the outgoing HTLC to her local channel's state // update log. Then Alice sends this wire message over to Bob who // adds this htlc to his remote state update log. - if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { - t.Fatalf("unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { - t.Fatalf("unable to recv htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) // Simulate Alice sending update fee message to bob. fee := chainfee.SatPerKWeight(333) @@ -2594,12 +2551,7 @@ func TestUpdateFeeReceiverCommits(t *testing.T) { // First Alice adds the outgoing HTLC to her local channel's state // update log. Then Alice sends this wire message over to Bob who // adds this htlc to his remote state update log. - if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { - t.Fatalf("unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { - t.Fatalf("unable to recv htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) // Simulate Alice sending update fee message to bob fee := chainfee.SatPerKWeight(333) @@ -3792,12 +3744,7 @@ func testChanSyncOweRevocation(t *testing.T, chanType channeldb.ChannelType) { Amount: htlcAmt, Expiry: uint32(10), } - if _, err := aliceChannel.AddHTLC(aliceHtlc, nil); err != nil { - t.Fatalf("unable to add alice's htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(aliceHtlc); err != nil { - t.Fatalf("unable to recv alice's htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, aliceHtlc, nil) if err := ForceStateTransition(aliceChannel, bobChannel); err != nil { t.Fatalf("unable to complete alice's state transition: %v", err) } @@ -4086,10 +4033,7 @@ func testChanSyncOweRevocationAndCommitForceTransition(t *testing.T, Expiry: uint32(10), ID: 1, } - _, err = bobChannel.AddHTLC(bobHtlc[1], nil) - require.NoError(t, err, "unable to add bob's htlc") - _, err = aliceChannel.ReceiveHTLC(bobHtlc[1]) - require.NoError(t, err, "unable to recv bob's htlc") + addAndReceiveHTLC(t, bobChannel, aliceChannel, bobHtlc[1], nil) // Bob signs the new state update, and sends the signature to Alice. bobNewCommit, err := bobChannel.SignNextCommitment() @@ -4322,14 +4266,7 @@ func TestChanSyncFailure(t *testing.T) { } index++ - _, err := bobChannel.AddHTLC(bobHtlc, nil) - if err != nil { - t.Fatalf("unable to add bob's htlc: %v", err) - } - _, err = aliceChannel.ReceiveHTLC(bobHtlc) - if err != nil { - t.Fatalf("unable to recv bob's htlc: %v", err) - } + addAndReceiveHTLC(t, bobChannel, aliceChannel, bobHtlc, nil) err = ForceStateTransition(bobChannel, aliceChannel) if err != nil { t.Fatalf("unable to complete bob's state "+ @@ -4355,14 +4292,7 @@ func TestChanSyncFailure(t *testing.T) { } index++ - _, err := bobChannel.AddHTLC(bobHtlc, nil) - if err != nil { - t.Fatalf("unable to add bob's htlc: %v", err) - } - _, err = aliceChannel.ReceiveHTLC(bobHtlc) - if err != nil { - t.Fatalf("unable to recv bob's htlc: %v", err) - } + addAndReceiveHTLC(t, bobChannel, aliceChannel, bobHtlc, nil) aliceNewCommit, err := aliceChannel.SignNextCommitment() if err != nil { @@ -4705,12 +4635,7 @@ func TestChannelRetransmissionFeeUpdate(t *testing.T) { Amount: lnwire.NewMSatFromSatoshis(20000), Expiry: uint32(10), } - if _, err := bobChannel.AddHTLC(bobHtlc, nil); err != nil { - t.Fatalf("unable to add bob's htlc: %v", err) - } - if _, err := aliceChannel.ReceiveHTLC(bobHtlc); err != nil { - t.Fatalf("unable to recv bob's htlc: %v", err) - } + addAndReceiveHTLC(t, bobChannel, aliceChannel, bobHtlc, nil) if err := ForceStateTransition(bobChannel, aliceChannel); err != nil { t.Fatalf("unable to complete bob's state transition: %v", err) } @@ -4782,12 +4707,7 @@ func TestFeeUpdateOldDiskFormat(t *testing.T) { var htlcs []*lnwire.UpdateAddHTLC for i := 0; i < numHTLCs; i++ { htlc, _ := createHTLC(i, htlcAmt) - if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { - t.Fatalf("unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { - t.Fatalf("unable to recv htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) htlcs = append(htlcs, htlc) if i%5 != 0 { @@ -4896,12 +4816,7 @@ func TestFeeUpdateOldDiskFormat(t *testing.T) { // Finally, to trigger a compactLogs execution, we'll add a new HTLC, // then force a state transition. htlc, _ := createHTLC(numHTLCs, htlcAmt) - if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { - t.Fatalf("unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { - t.Fatalf("unable to recv htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) if err := ForceStateTransition(aliceChannel, bobChannel); err != nil { t.Fatalf("unable to complete bob's state transition: %v", err) } @@ -4995,12 +4910,7 @@ func TestChanSyncInvalidLastSecret(t *testing.T) { Amount: htlcAmt, Expiry: uint32(5), } - if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { - t.Fatalf("unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { - t.Fatalf("unable to recv htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) // Then we'll initiate a state transition to lock in this new HTLC. if err := ForceStateTransition(aliceChannel, bobChannel); err != nil { @@ -5122,12 +5032,7 @@ func TestChanAvailableBandwidth(t *testing.T) { alicePreimages := make([][32]byte, numHtlcs) for i := 0; i < numHtlcs; i++ { htlc, preImage := createHTLC(i, dustAmt) - if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { - t.Fatalf("unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { - t.Fatalf("unable to recv htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) alicePreimages[i] = preImage } @@ -5140,12 +5045,7 @@ func TestChanAvailableBandwidth(t *testing.T) { htlcAmt := lnwire.NewMSatFromSatoshis(30000) for i := 0; i < numHtlcs; i++ { htlc, preImage := createHTLC(numHtlcs+i, htlcAmt) - if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { - t.Fatalf("unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { - t.Fatalf("unable to recv htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) alicePreimages = append(alicePreimages, preImage) } @@ -5414,13 +5314,7 @@ func TestChanCommitWeightDustHtlcs(t *testing.T) { t.Helper() htlc, preImage := createHTLC(int(htlcIndex), htlcAmt) - if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { - t.Fatalf("unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { - t.Fatalf("unable to recv htlc: %v", err) - } - + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) if err := ForceStateTransition(aliceChannel, bobChannel); err != nil { t.Fatalf("unable to complete alice's state "+ "transition: %v", err) @@ -5553,19 +5447,10 @@ func TestLockedInHtlcForwardingSkipAfterRestart(t *testing.T) { // a state transition. var htlcAmt lnwire.MilliSatoshi = 100000 htlc, _ := createHTLC(0, htlcAmt) - if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { - t.Fatalf("unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { - t.Fatalf("unable to recv htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) + htlc2, _ := createHTLC(1, htlcAmt) - if _, err := aliceChannel.AddHTLC(htlc2, nil); err != nil { - t.Fatalf("unable to add htlc2: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlc2); err != nil { - t.Fatalf("unable to recv htlc2: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc2, nil) // We'll now manually initiate a state transition between Alice and // bob. @@ -5853,12 +5738,7 @@ func TestInvalidCommitSigError(t *testing.T) { // Alice to Bob. var htlcAmt lnwire.MilliSatoshi = 100000 htlc, _ := createHTLC(0, htlcAmt) - if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { - t.Fatalf("unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { - t.Fatalf("unable to recv htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) // Alice will now attempt to initiate a state transition. aliceNewCommit, err := aliceChannel.SignNextCommitment() @@ -5903,19 +5783,9 @@ func TestChannelUnilateralCloseHtlcResolution(t *testing.T) { // initiating enough state transitions to lock both of them in. htlcAmount := lnwire.NewMSatFromSatoshis(20000) htlcAlice, _ := createHTLC(0, htlcAmount) - if _, err := aliceChannel.AddHTLC(htlcAlice, nil); err != nil { - t.Fatalf("alice unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlcAlice); err != nil { - t.Fatalf("bob unable to recv add htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlcAlice, nil) htlcBob, preimageBob := createHTLC(0, htlcAmount) - if _, err := bobChannel.AddHTLC(htlcBob, nil); err != nil { - t.Fatalf("bob unable to add htlc: %v", err) - } - if _, err := aliceChannel.ReceiveHTLC(htlcBob); err != nil { - t.Fatalf("alice unable to recv add htlc: %v", err) - } + addAndReceiveHTLC(t, bobChannel, aliceChannel, htlcBob, nil) if err := ForceStateTransition(aliceChannel, bobChannel); err != nil { t.Fatalf("Can't update the channel state: %v", err) } @@ -6059,12 +5929,7 @@ func TestChannelUnilateralClosePendingCommit(t *testing.T) { // create a new state transition. htlcAmount := lnwire.NewMSatFromSatoshis(20000) htlcAlice, _ := createHTLC(0, htlcAmount) - if _, err := aliceChannel.AddHTLC(htlcAlice, nil); err != nil { - t.Fatalf("alice unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlcAlice); err != nil { - t.Fatalf("bob unable to recv add htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlcAlice, nil) // With the HTLC added, we'll now manually initiate a state transition // from Alice to Bob. @@ -6258,12 +6123,7 @@ func TestMaxAcceptedHTLCs(t *testing.T) { // Send the maximum allowed number of HTLCs. for i := 0; i < numHTLCs; i++ { htlc, _ := createHTLC(i, htlcAmt) - if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { - t.Fatalf("unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { - t.Fatalf("unable to recv htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) // Just assign htlcID to the last received HTLC. htlcID = htlc.ID @@ -6302,12 +6162,7 @@ func TestMaxAcceptedHTLCs(t *testing.T) { // failed. We use numHTLCs here since the previous AddHTLC with this index // failed. htlc, _ = createHTLC(numHTLCs, htlcAmt) - if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { - t.Fatalf("unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { - t.Fatalf("unable to recv htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) // Add a commitment to Bob's commitment chain. aliceNewCommit, err := aliceChannel.SignNextCommitment() @@ -6383,12 +6238,7 @@ func TestMaxAsynchronousHtlcs(t *testing.T) { // Send the maximum allowed number of HTLCs minus one. for i := 0; i < numHTLCs-1; i++ { htlc, _ := createHTLC(i, htlcAmt) - if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { - t.Fatalf("unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { - t.Fatalf("unable to recv htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) // Just assign htlcID to the last received HTLC. htlcID = htlc.ID @@ -6400,12 +6250,7 @@ func TestMaxAsynchronousHtlcs(t *testing.T) { // Send an HTLC to Bob so that Bob's commitment transaction is full. htlc, _ := createHTLC(numHTLCs-1, htlcAmt) - if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { - t.Fatalf("unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { - t.Fatalf("unable to recv htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) // Fail back an HTLC and sign a commitment as in steps 1 & 2. err = bobChannel.FailHTLC(htlcID, []byte{}, nil, nil, nil) @@ -6444,12 +6289,7 @@ func TestMaxAsynchronousHtlcs(t *testing.T) { // Send the final Add which should succeed as in step 6. htlc, _ = createHTLC(numHTLCs, htlcAmt) - if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { - t.Fatalf("unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { - t.Fatalf("unable to recv htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) // Receiving the commitment should succeed as in step 7 since space was // made. @@ -6490,12 +6330,7 @@ func TestMaxPendingAmount(t *testing.T) { htlcAmt := lnwire.NewMSatFromSatoshis(1.5 * btcutil.SatoshiPerBitcoin) for i := 0; i < numHTLCs; i++ { htlc, _ := createHTLC(i, htlcAmt) - if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { - t.Fatalf("unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { - t.Fatalf("unable to recv htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) } // We finally add one more HTLC of 0.1 BTC to Alice's commitment. This @@ -6597,12 +6432,7 @@ func TestChanReserve(t *testing.T) { htlcAmt := lnwire.NewMSatFromSatoshis(0.5 * btcutil.SatoshiPerBitcoin) htlc, _ := createHTLC(aliceIndex, htlcAmt) aliceIndex++ - if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { - t.Fatalf("unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { - t.Fatalf("unable to recv htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) // Force a state transition, making sure this HTLC is considered valid // even though the channel reserves are not met. @@ -6649,12 +6479,7 @@ func TestChanReserve(t *testing.T) { // The first HTLC should successfully be sent. htlc, _ = createHTLC(aliceIndex, htlcAmt) aliceIndex++ - if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { - t.Fatalf("unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { - t.Fatalf("unable to recv htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) // Add a second HTLC of 1 BTC. This should fail because it will take // Alice's balance all the way down to her channel reserve, but since @@ -6720,12 +6545,7 @@ func TestChanReserve(t *testing.T) { htlcAmt = lnwire.NewMSatFromSatoshis(1 * btcutil.SatoshiPerBitcoin) htlc, _ = createHTLC(bobIndex, htlcAmt) bobIndex++ - if _, err := bobChannel.AddHTLC(htlc, nil); err != nil { - t.Fatalf("unable to add htlc: %v", err) - } - if _, err := aliceChannel.ReceiveHTLC(htlc); err != nil { - t.Fatalf("unable to recv htlc: %v", err) - } + addAndReceiveHTLC(t, bobChannel, aliceChannel, htlc, nil) // Do a last state transition, which should succeed. if err := ForceStateTransition(bobChannel, aliceChannel); err != nil { @@ -6853,12 +6673,7 @@ func TestMinHTLC(t *testing.T) { // ErrBelowMinHTLC. htlcAmt := lnwire.NewMSatFromSatoshis(0.5 * btcutil.SatoshiPerBitcoin) htlc, _ := createHTLC(0, htlcAmt) - if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil { - t.Fatalf("unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { - t.Fatalf("unable to recv htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) // We add an HTLC below the min value, this should result in // an ErrBelowMinHTLC error. @@ -7099,12 +6914,7 @@ func TestChannelRestoreUpdateLogs(t *testing.T) { // Bob's commit, but not on Alice's. htlcAmount := lnwire.NewMSatFromSatoshis(20000) htlcAlice, _ := createHTLC(0, htlcAmount) - if _, err := aliceChannel.AddHTLC(htlcAlice, nil); err != nil { - t.Fatalf("alice unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlcAlice); err != nil { - t.Fatalf("bob unable to recv add htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlcAlice, nil) // Let Alice sign a new state, which will include the HTLC just sent. aliceNewCommit, err := aliceChannel.SignNextCommitment() @@ -7345,11 +7155,7 @@ func TestDuplicateFailRejection(t *testing.T) { // parties. htlcAmount := lnwire.NewMSatFromSatoshis(20000) htlcAlice, _ := createHTLC(0, htlcAmount) - if _, err := aliceChannel.AddHTLC(htlcAlice, nil); err != nil { - t.Fatalf("alice unable to add htlc: %v", err) - } - _, err = bobChannel.ReceiveHTLC(htlcAlice) - require.NoError(t, err, "unable to recv htlc") + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlcAlice, nil) if err := ForceStateTransition(aliceChannel, bobChannel); err != nil { t.Fatalf("unable to complete state update: %v", err) @@ -7412,11 +7218,7 @@ func TestDuplicateSettleRejection(t *testing.T) { // parties. htlcAmount := lnwire.NewMSatFromSatoshis(20000) htlcAlice, alicePreimage := createHTLC(0, htlcAmount) - if _, err := aliceChannel.AddHTLC(htlcAlice, nil); err != nil { - t.Fatalf("alice unable to add htlc: %v", err) - } - _, err = bobChannel.ReceiveHTLC(htlcAlice) - require.NoError(t, err, "unable to recv htlc") + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlcAlice, nil) if err := ForceStateTransition(aliceChannel, bobChannel); err != nil { t.Fatalf("unable to complete state update: %v", err) @@ -7518,12 +7320,7 @@ func TestChannelRestoreCommitHeight(t *testing.T) { // We'll send an HtLC from Alice to Bob. htlcAmount := lnwire.NewMSatFromSatoshis(100000000) htlcAlice, _ := createHTLC(0, htlcAmount) - if _, err := aliceChannel.AddHTLC(htlcAlice, nil); err != nil { - t.Fatalf("alice unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlcAlice); err != nil { - t.Fatalf("bob unable to recv add htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlcAlice, nil) // Let Alice sign a new state, which will include the HTLC just sent. aliceNewCommit, err := aliceChannel.SignNextCommitment() @@ -7587,12 +7384,7 @@ func TestChannelRestoreCommitHeight(t *testing.T) { // existing HTLCs (the HTLC with index 0) keep getting the add heights // restored properly. htlcAlice, _ = createHTLC(1, htlcAmount) - if _, err := aliceChannel.AddHTLC(htlcAlice, nil); err != nil { - t.Fatalf("alice unable to add htlc: %v", err) - } - if _, err := bobChannel.ReceiveHTLC(htlcAlice); err != nil { - t.Fatalf("bob unable to recv add htlc: %v", err) - } + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlcAlice, nil) // Send a new signature from Alice to Bob, making Alice have a pending // remote commitment. @@ -9522,10 +9314,7 @@ func TestChannelUnsignedAckedFailure(t *testing.T) { htlc, _ := createHTLC(0, lnwire.MilliSatoshi(500000)) // -----add-----> - _, err = aliceChannel.AddHTLC(htlc, nil) - require.NoError(t, err) - _, err = bobChannel.ReceiveHTLC(htlc) - require.NoError(t, err) + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) // Force a state transition to lock in this add on both commitments. // -----sig-----> @@ -9585,10 +9374,7 @@ func TestChannelUnsignedAckedFailure(t *testing.T) { htlc2, _ := createHTLC(0, lnwire.MilliSatoshi(500000)) // <----add------ - _, err = bobChannel.AddHTLC(htlc2, nil) - require.NoError(t, err) - _, err = newAliceChannel.ReceiveHTLC(htlc2) - require.NoError(t, err) + addAndReceiveHTLC(t, bobChannel, newAliceChannel, htlc2, nil) // Bob sends the final signature to Alice and Alice should not // reject it, given that we properly restore the unsigned acked @@ -9632,10 +9418,7 @@ func TestChannelLocalUnsignedUpdatesFailure(t *testing.T) { htlc, _ := createHTLC(0, lnwire.MilliSatoshi(500000)) // <----add----- - _, err = bobChannel.AddHTLC(htlc, nil) - require.NoError(t, err) - _, err = aliceChannel.ReceiveHTLC(htlc) - require.NoError(t, err) + addAndReceiveHTLC(t, bobChannel, aliceChannel, htlc, nil) // Force a state transition to lock in this add on both commitments. // <----sig----- @@ -9719,10 +9502,7 @@ func TestChannelSignedAckRegression(t *testing.T) { htlc, preimage := createHTLC(0, lnwire.MilliSatoshi(5000000)) // <----add------ - _, err = bobChannel.AddHTLC(htlc, nil) - require.NoError(t, err) - _, err = aliceChannel.ReceiveHTLC(htlc) - require.NoError(t, err) + addAndReceiveHTLC(t, bobChannel, aliceChannel, htlc, nil) // Force a state transition to lock in the HTLC. // <----sig------ @@ -9761,10 +9541,7 @@ func TestChannelSignedAckRegression(t *testing.T) { htlc2, _ := createHTLC(0, lnwire.MilliSatoshi(5000000)) // -----add----> - _, err = aliceChannel.AddHTLC(htlc2, nil) - require.NoError(t, err) - _, err = bobChannel.ReceiveHTLC(htlc2) - require.NoError(t, err) + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc2, nil) // -----sig----> aliceNewCommit, err = aliceChannel.SignNextCommitment() @@ -9861,10 +9638,7 @@ func TestIsChannelClean(t *testing.T) { // sends an htlc. // ---add---> htlc, preimage := createHTLC(0, lnwire.MilliSatoshi(5000000)) - _, err = aliceChannel.AddHTLC(htlc, nil) - require.NoError(t, err) - _, err = bobChannel.ReceiveHTLC(htlc) - require.NoError(t, err) + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) assertCleanOrDirty(false, aliceChannel, bobChannel, t) // Assert that the channel remains dirty until the HTLC is completely @@ -10038,10 +9812,7 @@ func testGetDustSum(t *testing.T, chantype channeldb.ChannelType) { htlc1Amt := lnwire.MilliSatoshi(700_000) htlc1, preimage1 := createHTLC(0, htlc1Amt) - _, err = bobChannel.AddHTLC(htlc1, nil) - require.NoError(t, err) - _, err = aliceChannel.ReceiveHTLC(htlc1) - require.NoError(t, err) + addAndReceiveHTLC(t, bobChannel, aliceChannel, htlc1, nil) // Assert that GetDustSum from Alice's perspective does not consider // the HTLC dust on her commitment, but does on Bob's commitment. @@ -10081,10 +9852,7 @@ func testGetDustSum(t *testing.T, chantype channeldb.ChannelType) { htlc2Amt := lnwire.MilliSatoshi(100_000) htlc2, _ := createHTLC(0, htlc2Amt) - _, err = aliceChannel.AddHTLC(htlc2, nil) - require.NoError(t, err) - _, err = bobChannel.ReceiveHTLC(htlc2) - require.NoError(t, err) + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc2, nil) // Assert that GetDustSum from Alice's perspective includes the new // HTLC as dust on both commitments. @@ -10132,10 +9900,7 @@ func testGetDustSum(t *testing.T, chantype channeldb.ChannelType) { htlc3Amt := lnwire.MilliSatoshi(400_000) htlc3, _ := createHTLC(1, htlc3Amt) - _, err = aliceChannel.AddHTLC(htlc3, nil) - require.NoError(t, err) - _, err = bobChannel.ReceiveHTLC(htlc3) - require.NoError(t, err) + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc3, nil) // Assert that this new HTLC is not counted on Alice's local commitment // in the dust sum. Bob's commitment should count it. @@ -11118,11 +10883,7 @@ func TestAsynchronousSendingWithFeeBuffer(t *testing.T) { aliceChannel.channelState.LocalChanCfg.DustLimit + htlcFee, ) htlc3, _ := createHTLC(1, htlcAmt3) - _, err = bobChannel.AddHTLC(htlc3, nil) - require.NoError(t, err) - - _, err = aliceChannel.ReceiveHTLC(htlc3) - require.NoError(t, err) + addAndReceiveHTLC(t, bobChannel, aliceChannel, htlc3, nil) err = ForceStateTransition(bobChannel, aliceChannel) require.NoError(t, err) @@ -11228,10 +10989,7 @@ func TestEnforceFeeBuffer(t *testing.T) { // --------------- |-----rev------> htlc1, _ := createHTLC(0, lnwire.NewMSatFromSatoshis(htlcAmt1)) - _, err = aliceChannel.AddHTLC(htlc1, nil) - require.NoError(t, err) - _, err = bobChannel.ReceiveHTLC(htlc1) - require.NoError(t, err) + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc1, nil) err = ForceStateTransition(aliceChannel, bobChannel) require.NoError(t, err) @@ -11245,10 +11003,7 @@ func TestEnforceFeeBuffer(t *testing.T) { htlcAmt2 := lnwire.NewMSatFromSatoshis(btcutil.SatoshiPerBitcent) htlc2, _ := createHTLC(0, htlcAmt2) - _, err = bobChannel.AddHTLC(htlc2, nil) - require.NoError(t, err) - _, err = aliceChannel.ReceiveHTLC(htlc2) - require.NoError(t, err) + addAndReceiveHTLC(t, bobChannel, aliceChannel, htlc2, nil) err = ForceStateTransition(bobChannel, aliceChannel) require.NoError(t, err) @@ -11268,10 +11023,7 @@ func TestEnforceFeeBuffer(t *testing.T) { // <----rev------- |--------------- htlc4, _ := createHTLC(1, htlcAmt2) - _, err = bobChannel.AddHTLC(htlc4, nil) - require.NoError(t, err) - _, err = aliceChannel.ReceiveHTLC(htlc4) - require.NoError(t, err) + addAndReceiveHTLC(t, bobChannel, aliceChannel, htlc4, nil) err = ForceStateTransition(bobChannel, aliceChannel) require.NoError(t, err) @@ -11311,11 +11063,7 @@ func TestBlindingPointPersistence(t *testing.T) { tlv.NewPrimitiveRecord[lnwire.BlindingPointTlvType](blinding), ) - _, err = aliceChannel.AddHTLC(htlc, nil) - - require.NoError(t, err) - _, err = bobChannel.ReceiveHTLC(htlc) - require.NoError(t, err) + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) // Now, Alice will send a new commitment to Bob, which will persist our // pending HTLC to disk. From aa435034bfa3aa8d14ada27aa8c3e7c14e2111bd Mon Sep 17 00:00:00 2001 From: Jonathan Harvey-Buschel Date: Wed, 18 Sep 2024 08:13:00 -0400 Subject: [PATCH 3/9] lnwallet: test aux signer shutdown handling --- lnwallet/channel_test.go | 80 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 1799c4480ba..0abc386d542 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -4,6 +4,7 @@ import ( "bytes" "container/list" "crypto/sha256" + "errors" "fmt" "math/rand" "reflect" @@ -3421,6 +3422,85 @@ func TestChanSyncOweCommitmentAuxSigner(t *testing.T) { require.NotEmpty(t, sigMsg.CustomRecords) } +// TestAuxSignerShutdown tests that the channel state machine gracefully handles +// a failure of the aux signer when signing a new commitment. +func TestAuxSignerShutdown(t *testing.T) { + t.Parallel() + + // We'll kick off the test by creating our channels which both are + // loaded with 5 BTC each. + aliceChannel, bobChannel, err := CreateTestChannels( + t, channeldb.SingleFunderTweaklessBit, + ) + require.NoError(t, err, "unable to create test channels") + + auxSignerShutdownErr := errors.New("aux signer shutdown") + + // We know that aux sig jobs will be checked in SignNextCommitment() in + // ascending output index order. So we'll fail on the first job that is + // out of order, i.e. with an output index greater than its position in + // the submitted jobs slice. If the jobs are ordered, we'll fail on the + // job that is at the middle of the submitted job slice. + failAuxSigJob := func(jobs []AuxSigJob) { + for idx, sigJob := range jobs { + // Simulate a clean shutdown of the aux signer and send + // an error. Skip all remaining jobs. + isMiddleJob := idx == len(jobs)/2 + if int(sigJob.OutputIndex) > idx || isMiddleJob { + sigJob.Resp <- AuxSigJobResp{ + Err: auxSignerShutdownErr, + } + + return + } + + // If the job is 'in order', send a response with no + // error. + sigJob.Resp <- AuxSigJobResp{} + } + } + + auxSigner := NewAuxSignerMock(failAuxSigJob) + aliceChannel.auxSigner = fn.Some[AuxSigner](auxSigner) + + // Each HTLC amount is 0.01 BTC. + htlcAmt := lnwire.NewMSatFromSatoshis(0.01 * btcutil.SatoshiPerBitcoin) + + // Create enough HTLCs to create multiple sig jobs (one job per HTLC). + const numHTLCs = 24 + + // Send the specified number of HTLCs. + for i := 0; i < numHTLCs; i++ { + htlc, _ := createHTLC(i, htlcAmt) + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) + } + + // We'll set up the mock aux signer to expect calls to PackSigs and also + // SubmitSecondLevelSigBatch. The direct return values for this mock aux + // signer are nil. The expected error comes from the sig jobs being + // passed to failAuxSigJob above, which mimics a faulty aux signer. + var sigBlobBuf bytes.Buffer + sigBlob := testSigBlob{ + BlobInt: tlv.NewPrimitiveRecord[tlv.TlvType65634, uint16](5), + } + tlvStream, err := tlv.NewStream(sigBlob.BlobInt.Record()) + require.NoError(t, err, "unable to create tlv stream") + require.NoError(t, tlvStream.Encode(&sigBlobBuf)) + + auxSigner.On( + "SubmitSecondLevelSigBatch", mock.Anything, mock.Anything, + mock.Anything, + ).Return(nil).Twice() + auxSigner.On( + "PackSigs", mock.Anything, + ).Return( + fn.Some(sigBlobBuf.Bytes()), nil, + ) + + _, err = aliceChannel.SignNextCommitment() + require.ErrorIs(t, err, auxSignerShutdownErr) +} + func testChanSyncOweCommitmentPendingRemote(t *testing.T, chanType channeldb.ChannelType) { From 36faab35532961de8ead0d3eace11b7da754eb31 Mon Sep 17 00:00:00 2001 From: Jonathan Harvey-Buschel Date: Sun, 8 Sep 2024 17:00:13 -0400 Subject: [PATCH 4/9] lnwallet: sort sig jobs before submission --- lnwallet/channel.go | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 4a0716055e5..4c65ed54c19 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -2,12 +2,13 @@ package lnwallet import ( "bytes" + "cmp" "container/list" "crypto/sha256" "errors" "fmt" "math" - "sort" + "slices" "sync" "github.com/btcsuite/btcd/blockchain" @@ -4626,6 +4627,17 @@ func (lc *LightningChannel) SignNextCommitment() (*NewCommitState, error) { if err != nil { return nil, err } + + // We'll need to send over the signatures to the remote party in the + // order as they appear on the commitment transaction after BIP 69 + // sorting. + slices.SortFunc(sigBatch, func(i, j SignJob) int { + return cmp.Compare(i.OutputIndex, j.OutputIndex) + }) + slices.SortFunc(auxSigBatch, func(i, j AuxSigJob) int { + return cmp.Compare(i.OutputIndex, j.OutputIndex) + }) + lc.sigPool.SubmitSignBatch(sigBatch) err = fn.MapOptionZ(lc.auxSigner, func(a AuxSigner) error { @@ -4675,18 +4687,8 @@ func (lc *LightningChannel) SignNextCommitment() (*NewCommitState, error) { } } - // We'll need to send over the signatures to the remote party in the - // order as they appear on the commitment transaction after BIP 69 - // sorting. - sort.Slice(sigBatch, func(i, j int) bool { - return sigBatch[i].OutputIndex < sigBatch[j].OutputIndex - }) - sort.Slice(auxSigBatch, func(i, j int) bool { - return auxSigBatch[i].OutputIndex < auxSigBatch[j].OutputIndex - }) - - // With the jobs sorted, we'll now iterate through all the responses to - // gather each of the signatures in order. + // Iterate through all the responses to gather each of the signatures + // in the order they were submitted. htlcSigs = make([]lnwire.Sig, 0, len(sigBatch)) auxSigs := make([]fn.Option[tlv.Blob], 0, len(auxSigBatch)) for i := range sigBatch { From fe463533927bbc7995fe0a4aac8313d4f272d842 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Wed, 11 Sep 2024 15:28:46 +0200 Subject: [PATCH 5/9] lnwallet: clarify usage of cancel and response channels --- lnwallet/aux_signer.go | 11 ++++++----- lnwallet/sigpool.go | 21 +++++++++++---------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/lnwallet/aux_signer.go b/lnwallet/aux_signer.go index a2a4aa2e7cf..74b2d309fa7 100644 --- a/lnwallet/aux_signer.go +++ b/lnwallet/aux_signer.go @@ -47,18 +47,19 @@ type AuxSigJob struct { BaseAuxJob // Resp is a channel that will be used to send the result of the sign - // job. + // job. This channel MUST be buffered. Resp chan AuxSigJobResp - // Cancel is a channel that should be closed if the caller wishes to - // abandon all pending sign jobs part of a single batch. - Cancel chan struct{} + // Cancel is a channel that is closed by the caller if they wish to + // abandon all pending sign jobs part of a single batch. This should + // never be closed by the validator. + Cancel <-chan struct{} } // NewAuxSigJob creates a new AuxSigJob. func NewAuxSigJob(sigJob SignJob, keyRing CommitmentKeyRing, incoming bool, htlc PaymentDescriptor, commitBlob fn.Option[tlv.Blob], - htlcLeaf input.AuxTapLeaf, cancelChan chan struct{}) AuxSigJob { + htlcLeaf input.AuxTapLeaf, cancelChan <-chan struct{}) AuxSigJob { return AuxSigJob{ SignDesc: sigJob.SignDesc, diff --git a/lnwallet/sigpool.go b/lnwallet/sigpool.go index 78e66d94867..3ee6c399ae8 100644 --- a/lnwallet/sigpool.go +++ b/lnwallet/sigpool.go @@ -47,17 +47,17 @@ type VerifyJob struct { // TODO(roasbeef): remove -- never actually used? HtlcIndex uint64 - // Cancel is a channel that should be closed if the caller wishes to + // Cancel is a channel that is closed by the caller if they wish to // cancel all pending verification jobs part of a single batch. This - // channel is to be closed in the case that a single signature in a - // batch has been returned as invalid, as there is no need to verify - // the remainder of the signatures. - Cancel chan struct{} + // channel is closed in the case that a single signature in a batch has + // been returned as invalid, as there is no need to verify the remainder + // of the signatures. + Cancel <-chan struct{} // ErrResp is the channel that the result of the signature verification // is to be sent over. In the see that the signature is valid, a nil // error will be passed. Otherwise, a concrete error detailing the - // issue will be passed. + // issue will be passed. This channel MUST be buffered. ErrResp chan *HtlcIndexErr } @@ -88,12 +88,13 @@ type SignJob struct { // transaction being signed. OutputIndex int32 - // Cancel is a channel that should be closed if the caller wishes to - // abandon all pending sign jobs part of a single batch. - Cancel chan struct{} + // Cancel is a channel that is closed by the caller if they wish to + // abandon all pending sign jobs part of a single batch. This should + // never be closed by the validator. + Cancel <-chan struct{} // Resp is the channel that the response to this particular SignJob - // will be sent over. + // will be sent over. This channel MUST be buffered. // // TODO(roasbeef): actually need to allow caller to set, need to retain // order mark commit sig as special From 836a9657f3ebf60af024a832d1a5689044d37ad5 Mon Sep 17 00:00:00 2001 From: Jonathan Harvey-Buschel Date: Mon, 14 Oct 2024 16:14:17 -0400 Subject: [PATCH 6/9] htlcswitch: pass quit chans as unidirectional This is a requirement for replacing the quit channel with a Context. The Done() channel of a Context is always recv-only, so all users of that channel must not expect a bidirectional channel. --- htlcswitch/interceptable_switch.go | 6 ++--- htlcswitch/link.go | 2 +- htlcswitch/link_test.go | 40 ++++++++++++++++++------------ htlcswitch/mailbox.go | 4 +-- htlcswitch/mailbox_test.go | 4 +-- htlcswitch/switch.go | 4 +-- htlcswitch/test_utils.go | 18 ++++++++------ 7 files changed, 45 insertions(+), 33 deletions(-) diff --git a/htlcswitch/interceptable_switch.go b/htlcswitch/interceptable_switch.go index 67832a98580..55f5980b62f 100644 --- a/htlcswitch/interceptable_switch.go +++ b/htlcswitch/interceptable_switch.go @@ -92,7 +92,7 @@ type InterceptableSwitch struct { type interceptedPackets struct { packets []*htlcPacket - linkQuit chan struct{} + linkQuit <-chan struct{} isReplay bool } @@ -442,8 +442,8 @@ func (s *InterceptableSwitch) Resolve(res *FwdResolution) error { // interceptor. If the interceptor signals the resume action, the htlcs are // forwarded to the switch. The link's quit signal should be provided to allow // cancellation of forwarding during link shutdown. -func (s *InterceptableSwitch) ForwardPackets(linkQuit chan struct{}, isReplay bool, - packets ...*htlcPacket) error { +func (s *InterceptableSwitch) ForwardPackets(linkQuit <-chan struct{}, + isReplay bool, packets ...*htlcPacket) error { // Synchronize with the main event loop. This should be light in the // case where there is no interceptor. diff --git a/htlcswitch/link.go b/htlcswitch/link.go index a59cedf06f3..079bf58d424 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -101,7 +101,7 @@ type ChannelLinkConfig struct { // switch. The function returns and error in case it fails to send one or // more packets. The link's quit signal should be provided to allow // cancellation of forwarding during link shutdown. - ForwardPackets func(chan struct{}, bool, ...*htlcPacket) error + ForwardPackets func(<-chan struct{}, bool, ...*htlcPacket) error // DecodeHopIterators facilitates batched decoding of HTLC Sphinx onion // blobs, which are then used to inform how to forward an HTLC. diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index 93c76bd4062..54e479b7ab2 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -2189,17 +2189,21 @@ func newSingleLinkTestHarness(t *testing.T, chanAmt, return nil } + forwardPackets := func(linkQuit <-chan struct{}, _ bool, + packets ...*htlcPacket) error { + + return aliceSwitch.ForwardPackets(linkQuit, packets...) + } + // Instantiate with a long interval, so that we can precisely control // the firing via force feeding. bticker := ticker.NewForce(time.Hour) aliceCfg := ChannelLinkConfig{ - FwrdingPolicy: globalPolicy, - Peer: alicePeer, - BestHeight: aliceSwitch.BestHeight, - Circuits: aliceSwitch.CircuitModifier(), - ForwardPackets: func(linkQuit chan struct{}, _ bool, packets ...*htlcPacket) error { - return aliceSwitch.ForwardPackets(linkQuit, packets...) - }, + FwrdingPolicy: globalPolicy, + Peer: alicePeer, + BestHeight: aliceSwitch.BestHeight, + Circuits: aliceSwitch.CircuitModifier(), + ForwardPackets: forwardPackets, DecodeHopIterators: decoder.DecodeHopIterators, ExtractErrorEncrypter: func(*btcec.PublicKey) ( hop.ErrorEncrypter, lnwire.FailCode) { @@ -4849,17 +4853,21 @@ func (h *persistentLinkHarness) restartLink( return nil } + forwardPackets := func(linkQuit <-chan struct{}, _ bool, + packets ...*htlcPacket) error { + + return h.hSwitch.ForwardPackets(linkQuit, packets...) + } + // Instantiate with a long interval, so that we can precisely control // the firing via force feeding. bticker := ticker.NewForce(time.Hour) aliceCfg := ChannelLinkConfig{ - FwrdingPolicy: globalPolicy, - Peer: alicePeer, - BestHeight: h.hSwitch.BestHeight, - Circuits: h.hSwitch.CircuitModifier(), - ForwardPackets: func(linkQuit chan struct{}, _ bool, packets ...*htlcPacket) error { - return h.hSwitch.ForwardPackets(linkQuit, packets...) - }, + FwrdingPolicy: globalPolicy, + Peer: alicePeer, + BestHeight: h.hSwitch.BestHeight, + Circuits: h.hSwitch.CircuitModifier(), + ForwardPackets: forwardPackets, DecodeHopIterators: decoder.DecodeHopIterators, ExtractErrorEncrypter: func(*btcec.PublicKey) ( hop.ErrorEncrypter, lnwire.FailCode) { @@ -7018,7 +7026,7 @@ func TestPipelineSettle(t *testing.T) { // erroneously forwarded. If the forwardChan is closed before the last // step, then the test will fail. forwardChan := make(chan struct{}) - fwdPkts := func(c chan struct{}, _ bool, hp ...*htlcPacket) error { + fwdPkts := func(c <-chan struct{}, _ bool, hp ...*htlcPacket) error { close(forwardChan) return nil } @@ -7204,7 +7212,7 @@ func TestChannelLinkShortFailureRelay(t *testing.T) { aliceMsgs := mockPeer.sentMsgs switchChan := make(chan *htlcPacket) - coreLink.cfg.ForwardPackets = func(linkQuit chan struct{}, _ bool, + coreLink.cfg.ForwardPackets = func(linkQuit <-chan struct{}, _ bool, packets ...*htlcPacket) error { for _, p := range packets { diff --git a/htlcswitch/mailbox.go b/htlcswitch/mailbox.go index a729e3ba505..7f5bf5b37d4 100644 --- a/htlcswitch/mailbox.go +++ b/htlcswitch/mailbox.go @@ -94,7 +94,7 @@ type mailBoxConfig struct { // forwardPackets send a varidic number of htlcPackets to the switch to // be routed. A quit channel should be provided so that the call can // properly exit during shutdown. - forwardPackets func(chan struct{}, ...*htlcPacket) error + forwardPackets func(<-chan struct{}, ...*htlcPacket) error // clock is a time source for the mailbox. clock clock.Clock @@ -801,7 +801,7 @@ type mailOrchConfig struct { // forwardPackets send a varidic number of htlcPackets to the switch to // be routed. A quit channel should be provided so that the call can // properly exit during shutdown. - forwardPackets func(chan struct{}, ...*htlcPacket) error + forwardPackets func(<-chan struct{}, ...*htlcPacket) error // clock is a time source for the generated mailboxes. clock clock.Clock diff --git a/htlcswitch/mailbox_test.go b/htlcswitch/mailbox_test.go index e48aacfcd6d..aa2f1b3ee6e 100644 --- a/htlcswitch/mailbox_test.go +++ b/htlcswitch/mailbox_test.go @@ -250,7 +250,7 @@ func newMailboxContext(t *testing.T, startTime time.Time, return ctx } -func (c *mailboxContext) forward(_ chan struct{}, +func (c *mailboxContext) forward(_ <-chan struct{}, pkts ...*htlcPacket) error { for _, pkt := range pkts { @@ -706,7 +706,7 @@ func TestMailOrchestrator(t *testing.T) { // First, we'll create a new instance of our orchestrator. mo := newMailOrchestrator(&mailOrchConfig{ failMailboxUpdate: failMailboxUpdate, - forwardPackets: func(_ chan struct{}, + forwardPackets: func(_ <-chan struct{}, pkts ...*htlcPacket) error { return nil diff --git a/htlcswitch/switch.go b/htlcswitch/switch.go index 2c0262581c7..cad7d6e6bd7 100644 --- a/htlcswitch/switch.go +++ b/htlcswitch/switch.go @@ -654,7 +654,7 @@ func (s *Switch) IsForwardedHTLC(chanID lnwire.ShortChannelID, // given to forward them through the router. The sending link's quit channel is // used to prevent deadlocks when the switch stops a link in the midst of // forwarding. -func (s *Switch) ForwardPackets(linkQuit chan struct{}, +func (s *Switch) ForwardPackets(linkQuit <-chan struct{}, packets ...*htlcPacket) error { var ( @@ -832,7 +832,7 @@ func (s *Switch) logFwdErrs(num *int, wg *sync.WaitGroup, fwdChan chan error) { // receive a shutdown requuest. This method does not wait for a response from // the htlcForwarder before returning. func (s *Switch) routeAsync(packet *htlcPacket, errChan chan error, - linkQuit chan struct{}) error { + linkQuit <-chan struct{}) error { command := &plexPacket{ pkt: packet, diff --git a/htlcswitch/test_utils.go b/htlcswitch/test_utils.go index 9a72197eca2..141d9b26eae 100644 --- a/htlcswitch/test_utils.go +++ b/htlcswitch/test_utils.go @@ -1127,15 +1127,19 @@ func (h *hopNetwork) createChannelLink(server, peer *mockServer, return nil } + forwardPackets := func(linkQuit <-chan struct{}, _ bool, + packets ...*htlcPacket) error { + + return server.htlcSwitch.ForwardPackets(linkQuit, packets...) + } + link := NewChannelLink( ChannelLinkConfig{ - BestHeight: server.htlcSwitch.BestHeight, - FwrdingPolicy: h.globalPolicy, - Peer: peer, - Circuits: server.htlcSwitch.CircuitModifier(), - ForwardPackets: func(linkQuit chan struct{}, _ bool, packets ...*htlcPacket) error { - return server.htlcSwitch.ForwardPackets(linkQuit, packets...) - }, + BestHeight: server.htlcSwitch.BestHeight, + FwrdingPolicy: h.globalPolicy, + Peer: peer, + Circuits: server.htlcSwitch.CircuitModifier(), + ForwardPackets: forwardPackets, DecodeHopIterators: decoder.DecodeHopIterators, ExtractErrorEncrypter: func(*btcec.PublicKey) ( hop.ErrorEncrypter, lnwire.FailCode) { From 62ffe6ca56315fe36b8075705051fb36b6160e94 Mon Sep 17 00:00:00 2001 From: Jonathan Harvey-Buschel Date: Wed, 18 Sep 2024 09:57:28 -0400 Subject: [PATCH 7/9] multi: link quit can interrupt commitment signing In this commit, we make sig job handling when singing a next commitment non-blocking by allowing the shutdown of a channel link to prevent further waiting on sig jobs by the channel state machine. This addresses possible cases where the aux signer may be shut down via a separate quit signal, so the state machine could block indefinitely on receiving an update on a sig job. --- contractcourt/chain_watcher_test.go | 5 +- htlcswitch/link.go | 39 +++-- htlcswitch/link_isolated_test.go | 5 +- htlcswitch/link_test.go | 42 +++-- htlcswitch/test_utils.go | 14 +- lnwallet/channel.go | 29 +++- lnwallet/channel_test.go | 259 +++++++++++++++------------- lnwallet/test_utils.go | 10 +- lnwallet/transactions_test.go | 4 +- 9 files changed, 241 insertions(+), 166 deletions(-) diff --git a/contractcourt/chain_watcher_test.go b/contractcourt/chain_watcher_test.go index 489a6051854..26ffe206848 100644 --- a/contractcourt/chain_watcher_test.go +++ b/contractcourt/chain_watcher_test.go @@ -2,6 +2,7 @@ package contractcourt import ( "bytes" + "context" "crypto/sha256" "fmt" "testing" @@ -145,7 +146,9 @@ func TestChainWatcherRemoteUnilateralClosePendingCommit(t *testing.T) { // With the HTLC added, we'll now manually initiate a state transition // from Alice to Bob. - _, err = aliceChannel.SignNextCommitment() + testQuit, testQuitFunc := context.WithCancel(context.Background()) + _ = testQuitFunc + _, err = aliceChannel.SignNextCommitment(testQuit) if err != nil { t.Fatal(err) } diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 079bf58d424..01abc6a3eb9 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -2,6 +2,7 @@ package htlcswitch import ( "bytes" + "context" crand "crypto/rand" "crypto/sha256" "fmt" @@ -382,8 +383,9 @@ type channelLink struct { // our next CommitSig. incomingCommitHooks hookMap - wg sync.WaitGroup - quit chan struct{} + wg sync.WaitGroup + quit context.Context //nolint:containedctx + quitFunc context.CancelFunc } // hookMap is a data structure that is used to track the hooks that need to be @@ -448,6 +450,10 @@ func NewChannelLink(cfg ChannelLinkConfig, channel *lnwallet.LightningChannel) ChannelLink { logPrefix := fmt.Sprintf("ChannelLink(%v):", channel.ChannelPoint()) + quitCtx, quitFunc := context.WithCancel(context.Background()) + + // Initialize the Done channel for our quit context. + _ = quitCtx.Done() return &channelLink{ cfg: cfg, @@ -458,7 +464,8 @@ func NewChannelLink(cfg ChannelLinkConfig, flushHooks: newHookMap(), outgoingCommitHooks: newHookMap(), incomingCommitHooks: newHookMap(), - quit: make(chan struct{}), + quit: quitCtx, + quitFunc: quitFunc, } } @@ -573,7 +580,7 @@ func (l *channelLink) Stop() { l.hodlQueue.Stop() - close(l.quit) + l.quitFunc() l.wg.Wait() // Now that the htlcManager has completely exited, reset the packet @@ -660,7 +667,7 @@ func (l *channelLink) IsFlushing(linkDirection LinkDirection) bool { func (l *channelLink) OnFlushedOnce(hook func()) { select { case l.flushHooks.newTransients <- hook: - case <-l.quit: + case <-l.quit.Done(): } } @@ -679,7 +686,7 @@ func (l *channelLink) OnCommitOnce(direction LinkDirection, hook func()) { select { case queue <- hook: - case <-l.quit: + case <-l.quit.Done(): } } @@ -889,7 +896,7 @@ func (l *channelLink) syncChanStates() error { // party, so we'll process the message in order to determine // if we need to re-transmit any messages to the remote party. msgsToReSend, openedCircuits, closedCircuits, err = - l.channel.ProcessChanSyncMsg(remoteChanSyncMsg) + l.channel.ProcessChanSyncMsg(l.quit, remoteChanSyncMsg) if err != nil { return err } @@ -918,7 +925,7 @@ func (l *channelLink) syncChanStates() error { l.cfg.Peer.SendMessage(false, msg) } - case <-l.quit: + case <-l.quit.Done(): return ErrLinkShuttingDown } @@ -1041,7 +1048,7 @@ func (l *channelLink) fwdPkgGarbager() { err) continue } - case <-l.quit: + case <-l.quit.Done(): return } } @@ -1442,7 +1449,7 @@ func (l *channelLink) htlcManager() { ) } - case <-l.quit: + case <-l.quit.Done(): return } } @@ -2272,7 +2279,7 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) { } select { - case <-l.quit: + case <-l.quit.Done(): return default: } @@ -2334,7 +2341,7 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) { } select { - case <-l.quit: + case <-l.quit.Done(): return default: } @@ -2541,7 +2548,7 @@ func (l *channelLink) updateCommitTx() error { return nil } - newCommit, err := l.channel.SignNextCommitment() + newCommit, err := l.channel.SignNextCommitment(l.quit) if err == lnwallet.ErrNoWindow { l.cfg.PendingCommitTicker.Resume() l.log.Trace("PendingCommitTicker resumed") @@ -2582,7 +2589,7 @@ func (l *channelLink) updateCommitTx() error { } select { - case <-l.quit: + case <-l.quit.Done(): return ErrLinkShuttingDown default: } @@ -3057,7 +3064,7 @@ func (l *channelLink) handleSwitchPacket(pkt *htlcPacket) error { // NOTE: Part of the ChannelLink interface. func (l *channelLink) HandleChannelUpdate(message lnwire.Message) { select { - case <-l.quit: + case <-l.quit.Done(): // Return early if the link is already in the process of // quitting. It doesn't make sense to hand the message to the // mailbox here. @@ -3744,7 +3751,7 @@ func (l *channelLink) forwardBatch(replay bool, packets ...*htlcPacket) { filteredPkts = append(filteredPkts, pkt) } - err := l.cfg.ForwardPackets(l.quit, replay, filteredPkts...) + err := l.cfg.ForwardPackets(l.quit.Done(), replay, filteredPkts...) if err != nil { log.Errorf("Unhandled error while reforwarding htlc "+ "settle/fail over htlcswitch: %v", err) diff --git a/htlcswitch/link_isolated_test.go b/htlcswitch/link_isolated_test.go index 89d0ef31937..0640e211dfe 100644 --- a/htlcswitch/link_isolated_test.go +++ b/htlcswitch/link_isolated_test.go @@ -1,6 +1,7 @@ package htlcswitch import ( + "context" "crypto/sha256" "testing" "time" @@ -94,7 +95,9 @@ func (l *linkTestContext) receiveHtlcAliceToBob() { func (l *linkTestContext) sendCommitSigBobToAlice(expHtlcs int) { l.t.Helper() - sigs, err := l.bobChannel.SignNextCommitment() + testQuit, testQuitFunc := context.WithCancel(context.Background()) + _ = testQuitFunc + sigs, err := l.bobChannel.SignNextCommitment(testQuit) if err != nil { l.t.Fatalf("error signing commitment: %v", err) } diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index 54e479b7ab2..7833efb480c 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -2244,12 +2244,14 @@ func newSingleLinkTestHarness(t *testing.T, chanAmt, return aliceSwitch.AddLink(aliceLink) } go func() { - for { - select { - case <-notifyUpdateChan: - case <-aliceLink.(*channelLink).quit: - close(doneChan) - return + if chanLink, ok := aliceLink.(*channelLink); ok { + for { + select { + case <-notifyUpdateChan: + case <-chanLink.quit.Done(): + close(doneChan) + return + } } } }() @@ -2316,7 +2318,7 @@ func handleStateUpdate(link *channelLink, } link.HandleChannelUpdate(remoteRev) - remoteSigs, err := remoteChannel.SignNextCommitment() + remoteSigs, err := remoteChannel.SignNextCommitment(link.quit) if err != nil { return err } @@ -2359,7 +2361,7 @@ func updateState(batchTick chan time.Time, link *channelLink, // Trigger update by ticking the batchTicker. select { case batchTick <- time.Now(): - case <-link.quit: + case <-link.quit.Done(): return fmt.Errorf("link shutting down") } return handleStateUpdate(link, remoteChannel) @@ -2367,7 +2369,7 @@ func updateState(batchTick chan time.Time, link *channelLink, // The remote is triggering the state update, emulate this by // signing and sending CommitSig to the link. - remoteSigs, err := remoteChannel.SignNextCommitment() + remoteSigs, err := remoteChannel.SignNextCommitment(link.quit) if err != nil { return err } @@ -4912,12 +4914,14 @@ func (h *persistentLinkHarness) restartLink( return nil, nil, err } go func() { - for { - select { - case <-notifyUpdateChan: - case <-aliceLink.(*channelLink).quit: - close(doneChan) - return + if chanLink, ok := aliceLink.(*channelLink); ok { + for { + select { + case <-notifyUpdateChan: + case <-chanLink.quit.Done(): + close(doneChan) + return + } } } }() @@ -5900,7 +5904,9 @@ func TestChannelLinkFail(t *testing.T) { // Sign a commitment that will include // signature for the HTLC just sent. - sigs, err := remoteChannel.SignNextCommitment() + sigs, err := remoteChannel.SignNextCommitment( + c.quit, + ) if err != nil { t.Fatalf("error signing commitment: %v", err) @@ -5942,7 +5948,9 @@ func TestChannelLinkFail(t *testing.T) { // Sign a commitment that will include // signature for the HTLC just sent. - sigs, err := remoteChannel.SignNextCommitment() + sigs, err := remoteChannel.SignNextCommitment( + c.quit, + ) if err != nil { t.Fatalf("error signing commitment: %v", err) diff --git a/htlcswitch/test_utils.go b/htlcswitch/test_utils.go index 141d9b26eae..76e7db753af 100644 --- a/htlcswitch/test_utils.go +++ b/htlcswitch/test_utils.go @@ -1180,12 +1180,14 @@ func (h *hopNetwork) createChannelLink(server, peer *mockServer, } go func() { - for { - select { - case <-notifyUpdateChan: - case <-link.(*channelLink).quit: - close(doneChan) - return + if chanLink, ok := link.(*channelLink); ok { + for { + select { + case <-notifyUpdateChan: + case <-chanLink.quit.Done(): + close(doneChan) + return + } } } }() diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 4c65ed54c19..faa9a284e89 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -4,6 +4,7 @@ import ( "bytes" "cmp" "container/list" + "context" "crypto/sha256" "errors" "fmt" @@ -138,6 +139,10 @@ var ( // errNoPartialSig is returned when a partial signature is required, // but none is found. errNoPartialSig = errors.New("no partial signature found") + + // errQuit is returned when a quit signal was received, interrupting the + // current operation. + errQuit = errors.New("received quit signal") ) // ErrCommitSyncLocalDataLoss is returned in the case that we receive a valid @@ -4526,7 +4531,9 @@ type NewCommitState struct { // for the remote party's commitment are also returned. // //nolint:funlen -func (lc *LightningChannel) SignNextCommitment() (*NewCommitState, error) { +func (lc *LightningChannel) SignNextCommitment( + ctx context.Context) (*NewCommitState, error) { + lc.Lock() defer lc.Unlock() @@ -4693,7 +4700,13 @@ func (lc *LightningChannel) SignNextCommitment() (*NewCommitState, error) { auxSigs := make([]fn.Option[tlv.Blob], 0, len(auxSigBatch)) for i := range sigBatch { htlcSigJob := sigBatch[i] - jobResp := <-htlcSigJob.Resp + var jobResp SignJobResp + + select { + case jobResp = <-htlcSigJob.Resp: + case <-ctx.Done(): + return nil, errQuit + } // If an error occurred, then we'll cancel any other active // jobs. @@ -4709,7 +4722,13 @@ func (lc *LightningChannel) SignNextCommitment() (*NewCommitState, error) { } auxHtlcSigJob := auxSigBatch[i] - auxJobResp := <-auxHtlcSigJob.Resp + var auxJobResp AuxSigJobResp + + select { + case auxJobResp = <-auxHtlcSigJob.Resp: + case <-ctx.Done(): + return nil, errQuit + } // If an error occurred, then we'll cancel any other active // jobs. @@ -4802,7 +4821,7 @@ func (lc *LightningChannel) resignMusigCommit(commitTx *wire.MsgTx, // previous commitment txn. This allows the link to clear its mailbox of those // circuits in case they are still in memory, and ensure the switch's circuit // map has been updated by deleting the closed circuits. -func (lc *LightningChannel) ProcessChanSyncMsg( +func (lc *LightningChannel) ProcessChanSyncMsg(ctx context.Context, msg *lnwire.ChannelReestablish) ([]lnwire.Message, []models.CircuitKey, []models.CircuitKey, error) { @@ -4966,7 +4985,7 @@ func (lc *LightningChannel) ProcessChanSyncMsg( // revocation, but also initiate a state transition to re-sync // them. if lc.OweCommitment() { - newCommit, err := lc.SignNextCommitment() + newCommit, err := lc.SignNextCommitment(ctx) switch { // If we signed this state, then we'll accumulate diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 0abc386d542..e0470f03680 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -124,7 +124,7 @@ func testAddSettleWorkflow(t *testing.T, tweakless bool, // we expect the messages to be ordered, Bob will receive the HTLC we // just sent before he receives this signature, so the signature will // cover the HTLC. - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "alice unable to sign commitment") // Bob receives this signature message, and checks that this covers the @@ -142,7 +142,7 @@ func testAddSettleWorkflow(t *testing.T, tweakless bool, // This signature will cover the HTLC, since Bob will first send the // revocation just created. The revocation also acks every received // HTLC up to the point where Alice sent here signature. - bobNewCommit, err := bobChannel.SignNextCommitment() + bobNewCommit, err := bobChannel.SignNextCommitment(testQuit) require.NoError(t, err, "bob unable to sign alice's commitment") // Alice then processes this revocation, sending her own revocation for @@ -255,14 +255,14 @@ func testAddSettleWorkflow(t *testing.T, tweakless bool, t.Fatalf("alice unable to accept settle of outbound htlc: %v", err) } - bobNewCommit, err = bobChannel.SignNextCommitment() + bobNewCommit, err = bobChannel.SignNextCommitment(testQuit) require.NoError(t, err, "bob unable to sign settle commitment") err = aliceChannel.ReceiveNewCommitment(bobNewCommit.CommitSigs) require.NoError(t, err, "alice unable to process bob's new commitment") aliceRevocation2, _, _, err := aliceChannel.RevokeCurrentCommitment() require.NoError(t, err, "alice unable to generate revocation") - aliceNewCommit, err = aliceChannel.SignNextCommitment() + aliceNewCommit, err = aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "alice unable to sign new commitment") fwdPkg, _, _, _, err = bobChannel.ReceiveRevocation(aliceRevocation2) @@ -468,7 +468,7 @@ func TestChannelZeroAddLocalHeight(t *testing.T) { // Bob should send a commitment signature to Alice. // <----sig------ - bobNewCommit, err := bobChannel.SignNextCommitment() + bobNewCommit, err := bobChannel.SignNextCommitment(testQuit) require.NoError(t, err) err = aliceChannel.ReceiveNewCommitment(bobNewCommit.CommitSigs) @@ -499,7 +499,7 @@ func TestChannelZeroAddLocalHeight(t *testing.T) { // Bob should now send a commitment signature to Alice. // <----sig----- - bobNewCommit, err = bobChannel.SignNextCommitment() + bobNewCommit, err = bobChannel.SignNextCommitment(testQuit) require.NoError(t, err) // Alice should accept the commitment. Previously she would @@ -634,7 +634,7 @@ func testCommitHTLCSigTieBreak(t *testing.T, restart bool) { // tie-breaking for commitment sorting won't affect the commitment // signed by Alice because received HTLC scripts commit to the CLTV // directly, so the outputs will have different scriptPubkeys. - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign alice's commitment") err = bobChannel.ReceiveNewCommitment(aliceNewCommit.CommitSigs) @@ -649,7 +649,7 @@ func testCommitHTLCSigTieBreak(t *testing.T, restart bool) { // the offered HTLC scripts he adds for Alice will need to have the // tie-breaking applied because the CLTV is not committed, but instead // implicit via the construction of the second-level transactions. - bobNewCommit, err := bobChannel.SignNextCommitment() + bobNewCommit, err := bobChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign bob's commitment") if len(bobNewCommit.PendingHTLCs) != numHtlcs { @@ -1510,7 +1510,7 @@ func TestHTLCSigNumber(t *testing.T) { // =================================================================== aliceChannel, bobChannel := createChanWithHTLC(aboveDust, aboveDust) - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "Error signing next commitment") if len(aliceNewCommit.HtlcSigs) != 2 { @@ -1533,7 +1533,7 @@ func TestHTLCSigNumber(t *testing.T) { // =================================================================== aliceChannel, bobChannel = createChanWithHTLC(aboveDust) - aliceNewCommit, err = aliceChannel.SignNextCommitment() + aliceNewCommit, err = aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "Error signing next commitment") if len(aliceNewCommit.HtlcSigs) != 1 { @@ -1555,7 +1555,7 @@ func TestHTLCSigNumber(t *testing.T) { // ============================================================== aliceChannel, bobChannel = createChanWithHTLC(belowDust) - aliceNewCommit, err = aliceChannel.SignNextCommitment() + aliceNewCommit, err = aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "Error signing next commitment") // Since the HTLC is below Bob's dust limit, Alice won't need to send @@ -1573,7 +1573,7 @@ func TestHTLCSigNumber(t *testing.T) { // ================================================================ aliceChannel, bobChannel = createChanWithHTLC(aboveDust) - aliceNewCommit, err = aliceChannel.SignNextCommitment() + aliceNewCommit, err = aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "Error signing next commitment") // Since the HTLC is above Bob's dust limit, Alice should send a @@ -1594,7 +1594,7 @@ func TestHTLCSigNumber(t *testing.T) { // Alice should produce only one signature, since one HTLC is below // dust. - aliceNewCommit, err = aliceChannel.SignNextCommitment() + aliceNewCommit, err = aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "Error signing next commitment") if len(aliceNewCommit.HtlcSigs) != 1 { @@ -2344,7 +2344,7 @@ func TestUpdateFeeFail(t *testing.T) { // Alice sends signature for commitment that does not cover any fee // update. - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "alice unable to sign commitment") // Bob verifies this commit, meaning that he checks that it is @@ -2387,11 +2387,11 @@ func TestUpdateFeeConcurrentSig(t *testing.T) { } // Alice signs a commitment, and sends this to bob. - aliceNewCommits, err := aliceChannel.SignNextCommitment() + aliceNewCommits, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "alice unable to sign commitment") // At the same time, Bob signs a commitment. - bobNewCommits, err := bobChannel.SignNextCommitment() + bobNewCommits, err := bobChannel.SignNextCommitment(testQuit) require.NoError(t, err, "bob unable to sign alice's commitment") // ...that Alice receives. @@ -2458,7 +2458,7 @@ func TestUpdateFeeSenderCommits(t *testing.T) { // Alice signs a commitment, which will cover everything sent to Bob // (the HTLC and the fee update), and everything acked by Bob (nothing // so far). - aliceNewCommits, err := aliceChannel.SignNextCommitment() + aliceNewCommits, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "alice unable to sign commitment") // Bob receives this signature message, and verifies that it is @@ -2488,7 +2488,7 @@ func TestUpdateFeeSenderCommits(t *testing.T) { // Bob commits to all updates he has received from Alice. This includes // the HTLC he received, and the fee update. - bobNewCommit, err := bobChannel.SignNextCommitment() + bobNewCommit, err := bobChannel.SignNextCommitment(testQuit) require.NoError(t, err, "bob unable to sign alice's commitment") // Alice receives the revocation of the old one, and can now assume @@ -2562,7 +2562,7 @@ func TestUpdateFeeReceiverCommits(t *testing.T) { // Bob commits to every change he has sent since last time (none). He // does not commit to the received HTLC and fee update, since Alice // cannot know if he has received them. - bobNewCommit, err := bobChannel.SignNextCommitment() + bobNewCommit, err := bobChannel.SignNextCommitment(testQuit) require.NoError(t, err, "alice unable to sign commitment") // Alice receives this signature message, and verifies that it is @@ -2583,7 +2583,7 @@ func TestUpdateFeeReceiverCommits(t *testing.T) { // Alice will sign next commitment. Since she sent the revocation, she // also ack'ed everything received, but in this case this is nothing. // Since she sent the two updates, this signature will cover those two. - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "bob unable to sign alice's commitment") // Bob gets the signature for the new commitment from Alice. He assumes @@ -2613,7 +2613,7 @@ func TestUpdateFeeReceiverCommits(t *testing.T) { // Bob will send a new signature, which will cover what he just acked: // the HTLC and fee update. - bobNewCommit, err = bobChannel.SignNextCommitment() + bobNewCommit, err = bobChannel.SignNextCommitment(testQuit) require.NoError(t, err, "alice unable to sign commitment") // Alice receives revocation from Bob, and can now be sure that Bob @@ -2703,7 +2703,7 @@ func TestUpdateFeeMultipleUpdates(t *testing.T) { // Alice signs a commitment, which will cover everything sent to Bob // (the HTLC and the fee update), and everything acked by Bob (nothing // so far). - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "alice unable to sign commitment") bobChannel.ReceiveUpdateFee(fee1) @@ -2749,7 +2749,7 @@ func TestUpdateFeeMultipleUpdates(t *testing.T) { // Bob commits to all updates he has received from Alice. This includes // the HTLC he received, and the fee update. - bobNewCommit, err := bobChannel.SignNextCommitment() + bobNewCommit, err := bobChannel.SignNextCommitment(testQuit) require.NoError(t, err, "bob unable to sign alice's commitment") // Alice receives the revocation of the old one, and can now assume that @@ -2855,7 +2855,9 @@ func assertNoChanSyncNeeded(t *testing.T, aliceChannel *LightningChannel, } } - bobMsgsToSend, _, _, err := bobChannel.ProcessChanSyncMsg(aliceChanSyncMsg) + bobMsgsToSend, _, _, err := bobChannel.ProcessChanSyncMsg( + testQuit, aliceChanSyncMsg, + ) if err != nil { t.Fatalf("line #%v: unable to process ChannelReestablish "+ "msg: %v", line, err) @@ -2865,7 +2867,9 @@ func assertNoChanSyncNeeded(t *testing.T, aliceChannel *LightningChannel, "instead wants to send: %v", line, spew.Sdump(bobMsgsToSend)) } - aliceMsgsToSend, _, _, err := aliceChannel.ProcessChanSyncMsg(bobChanSyncMsg) + aliceMsgsToSend, _, _, err := aliceChannel.ProcessChanSyncMsg( + testQuit, bobChanSyncMsg, + ) if err != nil { t.Fatalf("line #%v: unable to process ChannelReestablish "+ "msg: %v", line, err) @@ -3055,7 +3059,7 @@ func testChanSyncOweCommitment(t *testing.T, chanType channeldb.ChannelType) { // Now we'll begin the core of the test itself. Alice will extend a new // commitment to Bob, but the connection drops before Bob can process // it. - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign commitment") // If this is a taproot channel, then we'll generate fresh verification @@ -3080,7 +3084,7 @@ func testChanSyncOweCommitment(t *testing.T, chanType channeldb.ChannelType) { // above. assertAliceCommitRetransmit := func() *lnwire.CommitSig { aliceMsgsToSend, _, _, err := aliceChannel.ProcessChanSyncMsg( - bobSyncMsg, + testQuit, bobSyncMsg, ) if err != nil { t.Fatalf("unable to process chan sync msg: %v", err) @@ -3170,7 +3174,9 @@ func testChanSyncOweCommitment(t *testing.T, chanType channeldb.ChannelType) { // From Bob's Pov he has nothing else to send, so he should conclude he // has no further action remaining. - bobMsgsToSend, _, _, err := bobChannel.ProcessChanSyncMsg(aliceSyncMsg) + bobMsgsToSend, _, _, err := bobChannel.ProcessChanSyncMsg( + testQuit, aliceSyncMsg, + ) require.NoError(t, err, "unable to process chan sync msg") if len(bobMsgsToSend) != 0 { t.Fatalf("expected bob to send %v messages instead will "+ @@ -3198,7 +3204,7 @@ func testChanSyncOweCommitment(t *testing.T, chanType channeldb.ChannelType) { require.NoError(t, err, "bob unable to process alice's commitment") bobRevocation, _, _, err := bobChannel.RevokeCurrentCommitment() require.NoError(t, err, "unable to revoke bob commitment") - bobNewCommit, err := bobChannel.SignNextCommitment() + bobNewCommit, err := bobChannel.SignNextCommitment(testQuit) require.NoError(t, err, "bob unable to sign commitment") _, _, _, _, err = aliceChannel.ReceiveRevocation(bobRevocation) require.NoError(t, err, "alice unable to recv revocation") @@ -3393,7 +3399,7 @@ func TestChanSyncOweCommitmentAuxSigner(t *testing.T) { fn.Some(sigBlobBuf.Bytes()), nil, ) - _, err = aliceChannel.SignNextCommitment() + _, err = aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign commitment") _, err = aliceChannel.GenMusigNonces() @@ -3405,7 +3411,7 @@ func TestChanSyncOweCommitmentAuxSigner(t *testing.T) { require.NoError(t, err, "unable to produce chan sync msg") aliceMsgsToSend, _, _, err := aliceChannel.ProcessChanSyncMsg( - bobSyncMsg, + testQuit, bobSyncMsg, ) require.NoError(t, err) require.Len(t, aliceMsgsToSend, 2) @@ -3497,7 +3503,7 @@ func TestAuxSignerShutdown(t *testing.T) { fn.Some(sigBlobBuf.Bytes()), nil, ) - _, err = aliceChannel.SignNextCommitment() + _, err = aliceChannel.SignNextCommitment(testQuit) require.ErrorIs(t, err, auxSignerShutdownErr) } @@ -3510,7 +3516,9 @@ func testChanSyncOweCommitmentPendingRemote(t *testing.T, require.NoError(t, err, "unable to create test channels") var fakeOnionBlob [lnwire.OnionPacketSize]byte - copy(fakeOnionBlob[:], bytes.Repeat([]byte{0x05}, lnwire.OnionPacketSize)) + copy(fakeOnionBlob[:], bytes.Repeat( + []byte{0x05}, lnwire.OnionPacketSize, + )) // We'll start off the scenario where Bob send two htlcs to Alice in a // single state update. @@ -3549,7 +3557,10 @@ func testChanSyncOweCommitmentPendingRemote(t *testing.T, // Next, Alice settles the HTLCs from Bob in distinct state updates. for i := 0; i < numHtlcs; i++ { - err = aliceChannel.SettleHTLC(preimages[i], uint64(i), nil, nil, nil) + err = aliceChannel.SettleHTLC( + preimages[i], uint64(i), nil, nil, nil, + ) + if err != nil { t.Fatalf("unable to settle htlc: %v", err) } @@ -3558,7 +3569,9 @@ func testChanSyncOweCommitmentPendingRemote(t *testing.T, t.Fatalf("unable to settle htlc: %v", err) } - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment( + testQuit, + ) if err != nil { t.Fatalf("unable to sign commitment: %v", err) } @@ -3596,7 +3609,7 @@ func testChanSyncOweCommitmentPendingRemote(t *testing.T, } // Bob signs the commitment he owes. - bobNewCommit, err := bobChannel.SignNextCommitment() + bobNewCommit, err := bobChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign commitment") // This commitment is expected to contain no htlcs anymore. @@ -3705,14 +3718,14 @@ func testChanSyncOweRevocation(t *testing.T, chanType channeldb.ChannelType) { // // Alice signs the next state, then Bob receives and sends his // revocation message. - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign commitment") err = bobChannel.ReceiveNewCommitment(aliceNewCommit.CommitSigs) require.NoError(t, err, "bob unable to process alice's commitment") bobRevocation, _, _, err := bobChannel.RevokeCurrentCommitment() require.NoError(t, err, "unable to revoke bob commitment") - bobNewCommit, err := bobChannel.SignNextCommitment() + bobNewCommit, err := bobChannel.SignNextCommitment(testQuit) require.NoError(t, err, "bob unable to sign commitment") _, _, _, _, err = aliceChannel.ReceiveRevocation(bobRevocation) @@ -3748,7 +3761,7 @@ func testChanSyncOweRevocation(t *testing.T, chanType channeldb.ChannelType) { t.Helper() aliceMsgsToSend, _, _, err := aliceChannel.ProcessChanSyncMsg( - bobSyncMsg, + testQuit, bobSyncMsg, ) if err != nil { t.Fatalf("unable to process chan sync msg: %v", err) @@ -3779,7 +3792,9 @@ func testChanSyncOweRevocation(t *testing.T, chanType channeldb.ChannelType) { } // From Bob's PoV he shouldn't think that he owes Alice any messages. - bobMsgsToSend, _, _, err := bobChannel.ProcessChanSyncMsg(aliceSyncMsg) + bobMsgsToSend, _, _, err := bobChannel.ProcessChanSyncMsg( + testQuit, aliceSyncMsg, + ) require.NoError(t, err, "unable to process chan sync msg") if len(bobMsgsToSend) != 0 { t.Fatalf("expected bob to not retransmit, instead has: %v", @@ -3908,7 +3923,7 @@ func testChanSyncOweRevocationAndCommit(t *testing.T, // Progressing the exchange: Alice will send her signature, Bob will // receive, send a revocation and also a signature for Alice's state. - aliceNewCommits, err := aliceChannel.SignNextCommitment() + aliceNewCommits, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign commitment") err = bobChannel.ReceiveNewCommitment(aliceNewCommits.CommitSigs) require.NoError(t, err, "bob unable to process alice's commitment") @@ -3917,7 +3932,7 @@ func testChanSyncOweRevocationAndCommit(t *testing.T, // reach Alice before the connection dies. bobRevocation, _, _, err := bobChannel.RevokeCurrentCommitment() require.NoError(t, err, "unable to revoke bob commitment") - bobNewCommit, err := bobChannel.SignNextCommitment() + bobNewCommit, err := bobChannel.SignNextCommitment(testQuit) require.NoError(t, err, "bob unable to sign commitment") // If we now attempt to resync, then Alice should conclude that she @@ -3939,7 +3954,9 @@ func testChanSyncOweRevocationAndCommit(t *testing.T, } } - aliceMsgsToSend, _, _, err := aliceChannel.ProcessChanSyncMsg(bobSyncMsg) + aliceMsgsToSend, _, _, err := aliceChannel.ProcessChanSyncMsg( + testQuit, bobSyncMsg, + ) require.NoError(t, err, "unable to process chan sync msg") if len(aliceMsgsToSend) != 0 { t.Fatalf("expected alice to not retransmit, instead she's "+ @@ -3950,7 +3967,7 @@ func testChanSyncOweRevocationAndCommit(t *testing.T, t.Helper() bobMsgsToSend, _, _, err := bobChannel.ProcessChanSyncMsg( - aliceSyncMsg, + testQuit, aliceSyncMsg, ) if err != nil { t.Fatalf("unable to process chan sync msg: %v", err) @@ -4116,7 +4133,7 @@ func testChanSyncOweRevocationAndCommitForceTransition(t *testing.T, addAndReceiveHTLC(t, bobChannel, aliceChannel, bobHtlc[1], nil) // Bob signs the new state update, and sends the signature to Alice. - bobNewCommit, err := bobChannel.SignNextCommitment() + bobNewCommit, err := bobChannel.SignNextCommitment(testQuit) require.NoError(t, err, "bob unable to sign commitment") err = aliceChannel.ReceiveNewCommitment(bobNewCommit.CommitSigs) @@ -4140,7 +4157,7 @@ func testChanSyncOweRevocationAndCommitForceTransition(t *testing.T, // Progressing the exchange: Alice will send her signature, with Bob // processing the new state locally. - aliceNewCommits, err := aliceChannel.SignNextCommitment() + aliceNewCommits, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign commitment") err = bobChannel.ReceiveNewCommitment(aliceNewCommits.CommitSigs) require.NoError(t, err, "bob unable to process alice's commitment") @@ -4170,7 +4187,9 @@ func testChanSyncOweRevocationAndCommitForceTransition(t *testing.T, } } - aliceMsgsToSend, _, _, err := aliceChannel.ProcessChanSyncMsg(bobSyncMsg) + aliceMsgsToSend, _, _, err := aliceChannel.ProcessChanSyncMsg( + testQuit, bobSyncMsg, + ) require.NoError(t, err, "unable to process chan sync msg") if len(aliceMsgsToSend) != 0 { t.Fatalf("expected alice to not retransmit, instead she's "+ @@ -4181,7 +4200,9 @@ func testChanSyncOweRevocationAndCommitForceTransition(t *testing.T, // send his RevokeAndAck message again. Additionally, the CommitSig // message that he sends should be sufficient to finalize the state // transition. - bobMsgsToSend, _, _, err := bobChannel.ProcessChanSyncMsg(aliceSyncMsg) + bobMsgsToSend, _, _, err := bobChannel.ProcessChanSyncMsg( + testQuit, aliceSyncMsg, + ) require.NoError(t, err, "unable to process chan sync msg") if len(bobMsgsToSend) != 2 { t.Fatalf("expected bob to send %v messages, instead "+ @@ -4374,7 +4395,9 @@ func TestChanSyncFailure(t *testing.T) { addAndReceiveHTLC(t, bobChannel, aliceChannel, bobHtlc, nil) - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment( + testQuit, + ) if err != nil { t.Fatalf("unable to sign next commit: %v", err) } @@ -4399,7 +4422,7 @@ func TestChanSyncFailure(t *testing.T) { } // Alice should detect from Bob's message that she lost state. - _, _, _, err = aliceOld.ProcessChanSyncMsg(bobSyncMsg) + _, _, _, err = aliceOld.ProcessChanSyncMsg(testQuit, bobSyncMsg) if _, ok := err.(*ErrCommitSyncLocalDataLoss); !ok { t.Fatalf("wrong error, expected "+ "ErrCommitSyncLocalDataLoss instead got: %v", @@ -4407,7 +4430,9 @@ func TestChanSyncFailure(t *testing.T) { } // Bob should detect that Alice probably lost state. - _, _, _, err = bobChannel.ProcessChanSyncMsg(aliceSyncMsg) + _, _, _, err = bobChannel.ProcessChanSyncMsg( + testQuit, aliceSyncMsg, + ) if err != ErrCommitSyncRemoteDataLoss { t.Fatalf("wrong error, expected "+ "ErrCommitSyncRemoteDataLoss instead got: %v", @@ -4468,7 +4493,7 @@ func TestChanSyncFailure(t *testing.T) { bobSyncMsg, err := bobChannel.channelState.ChanSyncMsg() require.NoError(t, err, "unable to produce chan sync msg") bobSyncMsg.LocalUnrevokedCommitPoint = nil - _, _, _, err = aliceOld.ProcessChanSyncMsg(bobSyncMsg) + _, _, _, err = aliceOld.ProcessChanSyncMsg(testQuit, bobSyncMsg) if err != ErrCannotSyncCommitChains { t.Fatalf("wrong error, expected ErrCannotSyncCommitChains "+ "instead got: %v", err) @@ -4480,7 +4505,7 @@ func TestChanSyncFailure(t *testing.T) { bobSyncMsg, err = bobChannel.channelState.ChanSyncMsg() require.NoError(t, err, "unable to produce chan sync msg") bobSyncMsg.NextLocalCommitHeight++ - _, _, _, err = aliceChannel.ProcessChanSyncMsg(bobSyncMsg) + _, _, _, err = aliceChannel.ProcessChanSyncMsg(testQuit, bobSyncMsg) if err != ErrCannotSyncCommitChains { t.Fatalf("wrong error, expected ErrCannotSyncCommitChains "+ "instead got: %v", err) @@ -4491,7 +4516,7 @@ func TestChanSyncFailure(t *testing.T) { bobSyncMsg, err = bobChannel.channelState.ChanSyncMsg() require.NoError(t, err, "unable to produce chan sync msg") bobSyncMsg.NextLocalCommitHeight-- - _, _, _, err = aliceChannel.ProcessChanSyncMsg(bobSyncMsg) + _, _, _, err = aliceChannel.ProcessChanSyncMsg(testQuit, bobSyncMsg) if err != ErrCommitSyncRemoteDataLoss { t.Fatalf("wrong error, expected ErrCommitSyncRemoteDataLoss "+ "instead got: %v", err) @@ -4507,7 +4532,7 @@ func TestChanSyncFailure(t *testing.T) { require.NoError(t, err, "unable to parse pubkey") bobSyncMsg.LocalUnrevokedCommitPoint = modCommitPoint - _, _, _, err = aliceChannel.ProcessChanSyncMsg(bobSyncMsg) + _, _, _, err = aliceChannel.ProcessChanSyncMsg(testQuit, bobSyncMsg) if err != ErrInvalidLocalUnrevokedCommitPoint { t.Fatalf("wrong error, expected "+ "ErrInvalidLocalUnrevokedCommitPoint instead got: %v", @@ -4527,7 +4552,7 @@ func TestChanSyncFailure(t *testing.T) { bobSyncMsg, err = bobChannel.channelState.ChanSyncMsg() require.NoError(t, err, "unable to produce chan sync msg") bobSyncMsg.LocalUnrevokedCommitPoint = modCommitPoint - _, _, _, err = aliceChannel.ProcessChanSyncMsg(bobSyncMsg) + _, _, _, err = aliceChannel.ProcessChanSyncMsg(testQuit, bobSyncMsg) if err != ErrInvalidLocalUnrevokedCommitPoint { t.Fatalf("wrong error, expected "+ "ErrInvalidLocalUnrevokedCommitPoint instead got: %v", @@ -4594,7 +4619,7 @@ func TestChannelRetransmissionFeeUpdate(t *testing.T) { // Now, Alice will send a new commitment to Bob, but we'll simulate a // connection failure, so Bob doesn't get her signature. - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign commitment") // Restart both channels to simulate a connection restart. @@ -4612,7 +4637,9 @@ func TestChannelRetransmissionFeeUpdate(t *testing.T) { require.NoError(t, err, "unable to produce chan sync msg") // Bob should detect that he doesn't need to send anything to Alice. - bobMsgsToSend, _, _, err := bobChannel.ProcessChanSyncMsg(aliceSyncMsg) + bobMsgsToSend, _, _, err := bobChannel.ProcessChanSyncMsg( + testQuit, aliceSyncMsg, + ) require.NoError(t, err, "unable to process chan sync msg") if len(bobMsgsToSend) != 0 { t.Fatalf("expected bob to send %v messages instead "+ @@ -4624,7 +4651,7 @@ func TestChannelRetransmissionFeeUpdate(t *testing.T) { // that she needs to first send a new UpdateFee message, and also a // CommitSig. aliceMsgsToSend, _, _, err := aliceChannel.ProcessChanSyncMsg( - bobSyncMsg, + testQuit, bobSyncMsg, ) require.NoError(t, err, "unable to process chan sync msg") if len(aliceMsgsToSend) != 2 { @@ -4680,7 +4707,7 @@ func TestChannelRetransmissionFeeUpdate(t *testing.T) { require.NoError(t, err, "bob unable to process alice's commitment") bobRevocation, _, _, err := bobChannel.RevokeCurrentCommitment() require.NoError(t, err, "unable to revoke bob commitment") - bobNewCommit, err := bobChannel.SignNextCommitment() + bobNewCommit, err := bobChannel.SignNextCommitment(testQuit) require.NoError(t, err, "bob unable to sign commitment") _, _, _, _, err = aliceChannel.ReceiveRevocation(bobRevocation) require.NoError(t, err, "alice unable to recv revocation") @@ -4811,7 +4838,7 @@ func TestFeeUpdateOldDiskFormat(t *testing.T) { // Now, Alice will send a new commitment to Bob, but we'll simulate a // connection failure, so Bob doesn't get the signature. - aliceNewCommitSig, err := aliceChannel.SignNextCommitment() + aliceNewCommitSig, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign commitment") // Before restarting Alice, to mimic the old format, we fetch the @@ -4868,7 +4895,7 @@ func TestFeeUpdateOldDiskFormat(t *testing.T) { require.NoError(t, err, "bob unable to process alice's commitment") bobRevocation, _, _, err := bobChannel.RevokeCurrentCommitment() require.NoError(t, err, "unable to revoke bob commitment") - bobNewCommitSigs, err := bobChannel.SignNextCommitment() + bobNewCommitSigs, err := bobChannel.SignNextCommitment(testQuit) require.NoError(t, err, "bob unable to sign commitment") _, _, _, _, err = aliceChannel.ReceiveRevocation(bobRevocation) require.NoError(t, err, "alice unable to recv revocation") @@ -4944,11 +4971,11 @@ func TestChanSyncUnableToSync(t *testing.T) { NextLocalCommitHeight: 1000, RemoteCommitTailHeight: 9000, } - _, _, _, err = bobChannel.ProcessChanSyncMsg(badChanSync) + _, _, _, err = bobChannel.ProcessChanSyncMsg(testQuit, badChanSync) if err != ErrCannotSyncCommitChains { t.Fatalf("expected error instead have: %v", err) } - _, _, _, err = aliceChannel.ProcessChanSyncMsg(badChanSync) + _, _, _, err = aliceChannel.ProcessChanSyncMsg(testQuit, badChanSync) if err != ErrCannotSyncCommitChains { t.Fatalf("expected error instead have: %v", err) } @@ -5016,7 +5043,7 @@ func TestChanSyncInvalidLastSecret(t *testing.T) { // Alice's former self should conclude that she possibly lost data as // Bob is sending a valid commit secret for the latest state. - _, _, _, err = aliceOld.ProcessChanSyncMsg(bobChanSync) + _, _, _, err = aliceOld.ProcessChanSyncMsg(testQuit, bobChanSync) if _, ok := err.(*ErrCommitSyncLocalDataLoss); !ok { t.Fatalf("wrong error, expected ErrCommitSyncLocalDataLoss "+ "instead got: %v", err) @@ -5024,7 +5051,7 @@ func TestChanSyncInvalidLastSecret(t *testing.T) { // Bob should conclude that he should force close the channel, as Alice // cannot continue operation. - _, _, _, err = bobChannel.ProcessChanSyncMsg(aliceChanSync) + _, _, _, err = bobChannel.ProcessChanSyncMsg(testQuit, aliceChanSync) if err != ErrInvalidLastCommitSecret { t.Fatalf("wrong error, expected ErrInvalidLastCommitSecret, "+ "instead got: %v", err) @@ -5505,7 +5532,7 @@ func TestSignCommitmentFailNotLockedIn(t *testing.T) { // If we now try to initiate a state update, then it should fail as // Alice is unable to actually create a new state. - _, err = aliceChannel.SignNextCommitment() + _, err = aliceChannel.SignNextCommitment(testQuit) if err != ErrNoWindow { t.Fatalf("expected ErrNoWindow, instead have: %v", err) } @@ -5534,7 +5561,7 @@ func TestLockedInHtlcForwardingSkipAfterRestart(t *testing.T) { // We'll now manually initiate a state transition between Alice and // bob. - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment(testQuit) if err != nil { t.Fatal(err) } @@ -5563,7 +5590,7 @@ func TestLockedInHtlcForwardingSkipAfterRestart(t *testing.T) { // Now, have Bob initiate a transition to lock in the Adds sent by // Alice. - bobNewCommit, err := bobChannel.SignNextCommitment() + bobNewCommit, err := bobChannel.SignNextCommitment(testQuit) if err != nil { t.Fatal(err) } @@ -5608,7 +5635,7 @@ func TestLockedInHtlcForwardingSkipAfterRestart(t *testing.T) { // We'll now initiate another state transition, but this time Bob will // lead. - bobNewCommit, err = bobChannel.SignNextCommitment() + bobNewCommit, err = bobChannel.SignNextCommitment(testQuit) if err != nil { t.Fatal(err) } @@ -5643,7 +5670,7 @@ func TestLockedInHtlcForwardingSkipAfterRestart(t *testing.T) { // Now, begin another state transition led by Alice, and fail the second // HTLC part-way through the dance. - aliceNewCommit, err = aliceChannel.SignNextCommitment() + aliceNewCommit, err = aliceChannel.SignNextCommitment(testQuit) if err != nil { t.Fatal(err) } @@ -5707,7 +5734,7 @@ func TestLockedInHtlcForwardingSkipAfterRestart(t *testing.T) { // Have Alice initiate a state transition, which does not include the // HTLCs just re-added to the channel state. - aliceNewCommit, err = aliceChannel.SignNextCommitment() + aliceNewCommit, err = aliceChannel.SignNextCommitment(testQuit) if err != nil { t.Fatal(err) } @@ -5736,7 +5763,7 @@ func TestLockedInHtlcForwardingSkipAfterRestart(t *testing.T) { } // Now initiate a final update from Bob to lock in the final Fail. - bobNewCommit, err = bobChannel.SignNextCommitment() + bobNewCommit, err = bobChannel.SignNextCommitment(testQuit) if err != nil { t.Fatal(err) } @@ -5768,7 +5795,7 @@ func TestLockedInHtlcForwardingSkipAfterRestart(t *testing.T) { // Finally, have Bob initiate a state transition that locks in the Fail // added after the restart. - aliceNewCommit, err = aliceChannel.SignNextCommitment() + aliceNewCommit, err = aliceChannel.SignNextCommitment(testQuit) if err != nil { t.Fatal(err) } @@ -5821,7 +5848,7 @@ func TestInvalidCommitSigError(t *testing.T) { addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) // Alice will now attempt to initiate a state transition. - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign new commit") // Before the signature gets to Bob, we'll mutate it, such that the @@ -6013,7 +6040,7 @@ func TestChannelUnilateralClosePendingCommit(t *testing.T) { // With the HTLC added, we'll now manually initiate a state transition // from Alice to Bob. - _, err = aliceChannel.SignNextCommitment() + _, err = aliceChannel.SignNextCommitment(testQuit) if err != nil { t.Fatal(err) } @@ -6245,7 +6272,7 @@ func TestMaxAcceptedHTLCs(t *testing.T) { addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) // Add a commitment to Bob's commitment chain. - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign next commitment") err = bobChannel.ReceiveNewCommitment(aliceNewCommit.CommitSigs) require.NoError(t, err, "unable to recv new commitment") @@ -6340,7 +6367,7 @@ func TestMaxAsynchronousHtlcs(t *testing.T) { t.Fatalf("unable to receive fail htlc: %v", err) } - bobNewCommit, err := bobChannel.SignNextCommitment() + bobNewCommit, err := bobChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign next commitment") err = aliceChannel.ReceiveNewCommitment(bobNewCommit.CommitSigs) @@ -6348,7 +6375,7 @@ func TestMaxAsynchronousHtlcs(t *testing.T) { // Cover the HTLC referenced with id equal to numHTLCs-1 with a new // signature (step 3). - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign next commitment") err = bobChannel.ReceiveNewCommitment(aliceNewCommit.CommitSigs) @@ -6373,7 +6400,7 @@ func TestMaxAsynchronousHtlcs(t *testing.T) { // Receiving the commitment should succeed as in step 7 since space was // made. - aliceNewCommit, err = aliceChannel.SignNextCommitment() + aliceNewCommit, err = aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign next commitment") err = bobChannel.ReceiveNewCommitment(aliceNewCommit.CommitSigs) @@ -6997,7 +7024,7 @@ func TestChannelRestoreUpdateLogs(t *testing.T) { addAndReceiveHTLC(t, aliceChannel, bobChannel, htlcAlice, nil) // Let Alice sign a new state, which will include the HTLC just sent. - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign commitment") // Bob receives this commitment signature, and revokes his old state. @@ -7027,7 +7054,7 @@ func TestChannelRestoreUpdateLogs(t *testing.T) { // and remote commit chains are updated in an async fashion. Since the // remote chain was updated with the latest state (since Bob sent the // revocation earlier) we can keep advancing the remote commit chain. - aliceNewCommit, err = aliceChannel.SignNextCommitment() + aliceNewCommit, err = aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign commitment") // After Alice has signed this commitment, her local commitment will @@ -7171,7 +7198,7 @@ func TestChannelRestoreUpdateLogsFailedHTLC(t *testing.T) { restoreAndAssert(t, aliceChannel, 1, 0, 0, 0) // Bob sends a signature. - bobNewCommit, err := bobChannel.SignNextCommitment() + bobNewCommit, err := bobChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign commitment") err = aliceChannel.ReceiveNewCommitment(bobNewCommit.CommitSigs) require.NoError(t, err, "unable to receive commitment") @@ -7198,7 +7225,7 @@ func TestChannelRestoreUpdateLogsFailedHTLC(t *testing.T) { // Now send a signature from Alice. This will give Bob a new commitment // where the HTLC is removed. - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign commitment") err = bobChannel.ReceiveNewCommitment(aliceNewCommit.CommitSigs) require.NoError(t, err, "unable to receive commitment") @@ -7261,7 +7288,7 @@ func TestDuplicateFailRejection(t *testing.T) { // We'll now have Bob sign a new commitment to lock in the HTLC fail // for Alice. - _, err = bobChannel.SignNextCommitment() + _, err = bobChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign commit") // We'll now force a restart for Bob and Alice, so we can test the @@ -7324,7 +7351,7 @@ func TestDuplicateSettleRejection(t *testing.T) { // We'll now have Bob sign a new commitment to lock in the HTLC fail // for Alice. - _, err = bobChannel.SignNextCommitment() + _, err = bobChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign commit") // We'll now force a restart for Bob and Alice, so we can test the @@ -7403,7 +7430,7 @@ func TestChannelRestoreCommitHeight(t *testing.T) { addAndReceiveHTLC(t, aliceChannel, bobChannel, htlcAlice, nil) // Let Alice sign a new state, which will include the HTLC just sent. - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign commitment") // The HTLC should only be on the pending remote commitment, so the @@ -7435,7 +7462,7 @@ func TestChannelRestoreCommitHeight(t *testing.T) { // Now let Bob send the commitment signature making the HTLC lock in on // Alice's commitment. - bobNewCommit, err := bobChannel.SignNextCommitment() + bobNewCommit, err := bobChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign commitment") // At this stage Bob has a pending remote commitment. Make sure @@ -7468,7 +7495,7 @@ func TestChannelRestoreCommitHeight(t *testing.T) { // Send a new signature from Alice to Bob, making Alice have a pending // remote commitment. - aliceNewCommit, err = aliceChannel.SignNextCommitment() + aliceNewCommit, err = aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign commitment") // A restoration should keep the add heights iof the first HTLC, and @@ -7507,7 +7534,7 @@ func TestChannelRestoreCommitHeight(t *testing.T) { // Sign a new state for Alice, making Bob have a pending remote // commitment. - bobNewCommit, err = bobChannel.SignNextCommitment() + bobNewCommit, err = bobChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign commitment") // The signing of a new commitment for Alice should have given the new @@ -7544,7 +7571,7 @@ func TestChannelRestoreCommitHeight(t *testing.T) { require.NoError(t, err, "unable to recv htlc cancel") // Now Bob signs for the fail update. - bobNewCommit, err = bobChannel.SignNextCommitment() + bobNewCommit, err = bobChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign commitment") // Bob has a pending commitment for Alice, it shouldn't affect the add @@ -7602,13 +7629,13 @@ func TestForceCloseBorkedState(t *testing.T) { // Do the commitment dance until Bob sends a revocation so Alice is // able to receive the revocation, and then also make a new state // herself. - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign commit") err = bobChannel.ReceiveNewCommitment(aliceNewCommit.CommitSigs) require.NoError(t, err, "unable to receive commitment") revokeMsg, _, _, err := bobChannel.RevokeCurrentCommitment() require.NoError(t, err, "unable to revoke bob commitment") - bobNewCommit, err := bobChannel.SignNextCommitment() + bobNewCommit, err := bobChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign commit") err = aliceChannel.ReceiveNewCommitment(bobNewCommit.CommitSigs) require.NoError(t, err, "unable to receive commitment") @@ -7642,7 +7669,7 @@ func TestForceCloseBorkedState(t *testing.T) { // We manually advance the commitment tail here since the above // ReceiveRevocation call will fail before it's actually advanced. aliceChannel.remoteCommitChain.advanceTail() - _, err = aliceChannel.SignNextCommitment() + _, err = aliceChannel.SignNextCommitment(testQuit) if err != channeldb.ErrChanBorked { t.Fatalf("sign commitment should have failed: %v", err) } @@ -8109,7 +8136,7 @@ func TestChannelFeeRateFloor(t *testing.T) { } // Check that alice can still sign commitments. - aliceNewCommit, err := alice.SignNextCommitment() + aliceNewCommit, err := alice.SignNextCommitment(testQuit) require.NoError(t, err, "alice unable to sign commitment") // Check that bob can still receive commitments. @@ -9413,7 +9440,7 @@ func TestChannelUnsignedAckedFailure(t *testing.T) { // Bob should send a commitment signature to Alice. // <----sig------ - bobNewCommit, err := bobChannel.SignNextCommitment() + bobNewCommit, err := bobChannel.SignNextCommitment(testQuit) require.NoError(t, err) err = aliceChannel.ReceiveNewCommitment(bobNewCommit.CommitSigs) require.NoError(t, err) @@ -9428,7 +9455,7 @@ func TestChannelUnsignedAckedFailure(t *testing.T) { // Alice should sign the next commitment and go down before // sending it. // -----sig-----X - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err) newAliceChannel, err := NewLightningChannel( @@ -9459,7 +9486,7 @@ func TestChannelUnsignedAckedFailure(t *testing.T) { // Bob sends the final signature to Alice and Alice should not // reject it, given that we properly restore the unsigned acked // updates and therefore our update log is structured correctly. - bobNewCommit, err = bobChannel.SignNextCommitment() + bobNewCommit, err = bobChannel.SignNextCommitment(testQuit) require.NoError(t, err) err = newAliceChannel.ReceiveNewCommitment(bobNewCommit.CommitSigs) require.NoError(t, err) @@ -9517,7 +9544,7 @@ func TestChannelLocalUnsignedUpdatesFailure(t *testing.T) { // Alice should send a commitment signature to Bob. // -----sig----> - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err) err = bobChannel.ReceiveNewCommitment(aliceNewCommit.CommitSigs) require.NoError(t, err) @@ -9541,7 +9568,7 @@ func TestChannelLocalUnsignedUpdatesFailure(t *testing.T) { // Bob sends the final signature and Alice should not reject it. // <----sig----- - bobNewCommit, err := bobChannel.SignNextCommitment() + bobNewCommit, err := bobChannel.SignNextCommitment(testQuit) require.NoError(t, err) err = newAliceChannel.ReceiveNewCommitment(bobNewCommit.CommitSigs) require.NoError(t, err) @@ -9600,7 +9627,7 @@ func TestChannelSignedAckRegression(t *testing.T) { require.NoError(t, err) // -----sig----> - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err) err = bobChannel.ReceiveNewCommitment(aliceNewCommit.CommitSigs) require.NoError(t, err) @@ -9612,7 +9639,7 @@ func TestChannelSignedAckRegression(t *testing.T) { require.NoError(t, err) // <----sig----- - bobNewCommit, err := bobChannel.SignNextCommitment() + bobNewCommit, err := bobChannel.SignNextCommitment(testQuit) require.NoError(t, err) err = aliceChannel.ReceiveNewCommitment(bobNewCommit.CommitSigs) require.NoError(t, err) @@ -9624,7 +9651,7 @@ func TestChannelSignedAckRegression(t *testing.T) { addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc2, nil) // -----sig----> - aliceNewCommit, err = aliceChannel.SignNextCommitment() + aliceNewCommit, err = aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err) err = bobChannel.ReceiveNewCommitment(aliceNewCommit.CommitSigs) require.NoError(t, err) @@ -9654,7 +9681,7 @@ func TestChannelSignedAckRegression(t *testing.T) { // Bob should no longer fail to sign this commitment due to faulty // update logs. // <----sig----- - bobNewCommit, err = newBobChannel.SignNextCommitment() + bobNewCommit, err = newBobChannel.SignNextCommitment(testQuit) require.NoError(t, err) // Alice should receive the new commitment without hiccups. @@ -9725,7 +9752,7 @@ func TestIsChannelClean(t *testing.T) { // removed from both commitments. // ---sig---> - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err) err = bobChannel.ReceiveNewCommitment(aliceNewCommit.CommitSigs) require.NoError(t, err) @@ -9739,7 +9766,7 @@ func TestIsChannelClean(t *testing.T) { assertCleanOrDirty(false, aliceChannel, bobChannel, t) // <---sig--- - bobNewCommit, err := bobChannel.SignNextCommitment() + bobNewCommit, err := bobChannel.SignNextCommitment(testQuit) require.NoError(t, err) err = aliceChannel.ReceiveNewCommitment(bobNewCommit.CommitSigs) require.NoError(t, err) @@ -9760,7 +9787,7 @@ func TestIsChannelClean(t *testing.T) { assertCleanOrDirty(false, aliceChannel, bobChannel, t) // <---sig--- - bobNewCommit, err = bobChannel.SignNextCommitment() + bobNewCommit, err = bobChannel.SignNextCommitment(testQuit) require.NoError(t, err) err = aliceChannel.ReceiveNewCommitment(bobNewCommit.CommitSigs) require.NoError(t, err) @@ -9774,7 +9801,7 @@ func TestIsChannelClean(t *testing.T) { assertCleanOrDirty(false, aliceChannel, bobChannel, t) // ---sig---> - aliceNewCommit, err = aliceChannel.SignNextCommitment() + aliceNewCommit, err = aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err) err = bobChannel.ReceiveNewCommitment(aliceNewCommit.CommitSigs) require.NoError(t, err) @@ -9798,7 +9825,7 @@ func TestIsChannelClean(t *testing.T) { assertCleanOrDirty(false, aliceChannel, bobChannel, t) // ---sig---> - aliceNewCommit, err = aliceChannel.SignNextCommitment() + aliceNewCommit, err = aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err) err = bobChannel.ReceiveNewCommitment(aliceNewCommit.CommitSigs) require.NoError(t, err) @@ -9812,7 +9839,7 @@ func TestIsChannelClean(t *testing.T) { assertCleanOrDirty(false, aliceChannel, bobChannel, t) // <---sig--- - bobNewCommit, err = bobChannel.SignNextCommitment() + bobNewCommit, err = bobChannel.SignNextCommitment(testQuit) require.NoError(t, err) err = aliceChannel.ReceiveNewCommitment(bobNewCommit.CommitSigs) require.NoError(t, err) @@ -9943,7 +9970,7 @@ func testGetDustSum(t *testing.T, chantype channeldb.ChannelType) { checkDust(bobChannel, htlc2Amt, htlc2Amt) // Alice signs for this HTLC and neither perspective should change. - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err) err = bobChannel.ReceiveNewCommitment(aliceNewCommit.CommitSigs) require.NoError(t, err) @@ -9962,7 +9989,7 @@ func testGetDustSum(t *testing.T, chantype channeldb.ChannelType) { // The rest of the dance is completed and neither perspective should // change. - bobNewCommit, err := bobChannel.SignNextCommitment() + bobNewCommit, err := bobChannel.SignNextCommitment(testQuit) require.NoError(t, err) err = aliceChannel.ReceiveNewCommitment(bobNewCommit.CommitSigs) require.NoError(t, err) @@ -10764,7 +10791,7 @@ func TestAsynchronousSendingContraint(t *testing.T) { // Bob signs the new state for alice, which ONLY has his htlc on it // because he only includes acked updates of alice. // <----sig-------|--------------- - bobNewCommit, err := bobChannel.SignNextCommitment() + bobNewCommit, err := bobChannel.SignNextCommitment(testQuit) require.NoError(t, err) err = aliceChannel.ReceiveNewCommitment(bobNewCommit.CommitSigs) @@ -10780,7 +10807,7 @@ func TestAsynchronousSendingContraint(t *testing.T) { // incoming htlc in her commitment sig to bob, but this will dip her // local balance below her reserve because she already used everything // up when adding her htlc. - _, err = aliceChannel.SignNextCommitment() + _, err = aliceChannel.SignNextCommitment(testQuit) require.ErrorIs(t, err, ErrBelowChanReserve) } @@ -10905,7 +10932,7 @@ func TestAsynchronousSendingWithFeeBuffer(t *testing.T) { // Force a state transition, this will lock-in the htlc of bob. // ------sig-----> (includes bob's htlc) // <----rev------ (locks in bob's htlc for alice) - aliceNewCommit, err := aliceChannel.SignNextCommitment() + aliceNewCommit, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err) err = bobChannel.ReceiveNewCommitment(aliceNewCommit.CommitSigs) require.NoError(t, err) @@ -11147,7 +11174,7 @@ func TestBlindingPointPersistence(t *testing.T) { // Now, Alice will send a new commitment to Bob, which will persist our // pending HTLC to disk. - aliceCommit, err := aliceChannel.SignNextCommitment() + aliceCommit, err := aliceChannel.SignNextCommitment(testQuit) require.NoError(t, err, "unable to sign commitment") // Restart alice to force fetching state from disk. diff --git a/lnwallet/test_utils.go b/lnwallet/test_utils.go index 8ff8860e969..63f96e8c3bd 100644 --- a/lnwallet/test_utils.go +++ b/lnwallet/test_utils.go @@ -1,6 +1,7 @@ package lnwallet import ( + "context" "crypto/rand" "encoding/binary" "encoding/hex" @@ -100,6 +101,11 @@ var ( bobDustLimit = btcutil.Amount(1300) testChannelCapacity float64 = 10 + + // testQuit is a context that will never be cancelled, that is used in + // place of a real quit context. + testQuit, testQuitFunc = context.WithCancel(context.Background()) + _ = testQuitFunc ) // CreateTestChannels creates to fully populated channels to be used within @@ -541,7 +547,7 @@ func calcStaticFee(chanType channeldb.ChannelType, numHTLCs int) btcutil.Amount // pending updates. This method is useful when testing interactions between two // live state machines. func ForceStateTransition(chanA, chanB *LightningChannel) error { - aliceNewCommit, err := chanA.SignNextCommitment() + aliceNewCommit, err := chanA.SignNextCommitment(testQuit) if err != nil { return err } @@ -554,7 +560,7 @@ func ForceStateTransition(chanA, chanB *LightningChannel) error { if err != nil { return err } - bobNewCommit, err := chanB.SignNextCommitment() + bobNewCommit, err := chanB.SignNextCommitment(testQuit) if err != nil { return err } diff --git a/lnwallet/transactions_test.go b/lnwallet/transactions_test.go index 464597e12c7..2bdca78d718 100644 --- a/lnwallet/transactions_test.go +++ b/lnwallet/transactions_test.go @@ -357,7 +357,7 @@ func testVectors(t *testing.T, chanType channeldb.ChannelType, test testCase) { // Execute commit dance to arrive at the point where the local node has // received the test commitment and the remote signature. - localNewCommit, err := localChannel.SignNextCommitment() + localNewCommit, err := localChannel.SignNextCommitment(testQuit) require.NoError(t, err, "local unable to sign commitment") err = remoteChannel.ReceiveNewCommitment(localNewCommit.CommitSigs) @@ -369,7 +369,7 @@ func testVectors(t *testing.T, chanType channeldb.ChannelType, test testCase) { _, _, _, _, err = localChannel.ReceiveRevocation(revMsg) require.NoError(t, err) - remoteNewCommit, err := remoteChannel.SignNextCommitment() + remoteNewCommit, err := remoteChannel.SignNextCommitment(testQuit) require.NoError(t, err) require.Equal( From a680224c0249154bb1212ec56ca39238380488af Mon Sep 17 00:00:00 2001 From: Jonathan Harvey-Buschel Date: Mon, 30 Sep 2024 16:46:44 -0400 Subject: [PATCH 8/9] lnwallet: test link quit signal handling --- lnwallet/channel_test.go | 71 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index e0470f03680..6d30916c1a7 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -3,6 +3,7 @@ package lnwallet import ( "bytes" "container/list" + "context" "crypto/sha256" "errors" "fmt" @@ -11,6 +12,7 @@ import ( "runtime" "testing" "testing/quick" + "time" "github.com/btcsuite/btcd/blockchain" "github.com/btcsuite/btcd/btcec/v2" @@ -3507,6 +3509,75 @@ func TestAuxSignerShutdown(t *testing.T) { require.ErrorIs(t, err, auxSignerShutdownErr) } +// TestQuitDuringSignNextCommitment tests that the channel state machine can +// successfully exit on receiving a quit signal when signing a new commitment. +func TestQuitDuringSignNextCommitment(t *testing.T) { + t.Parallel() + + // We'll kick off the test by creating our channels which both are + // loaded with 5 BTC each. + aliceChannel, bobChannel, err := CreateTestChannels( + t, channeldb.SingleFunderTweaklessBit, + ) + require.NoError(t, err, "unable to create test channels") + + // We'll simulate an aux signer that was started successfully, but is + // now frozen / inactive. This could happen if the aux signer shut down + // without sending an error on any aux sig job error channel. + noopAuxSigJob := func(jobs []AuxSigJob) {} + + auxSigner := NewAuxSignerMock(noopAuxSigJob) + aliceChannel.auxSigner = fn.Some[AuxSigner](auxSigner) + + // Each HTLC amount is 0.01 BTC. + htlcAmt := lnwire.NewMSatFromSatoshis(0.01 * btcutil.SatoshiPerBitcoin) + + // Create enough HTLCs to create multiple sig jobs (one job per HTLC). + const numHTLCs = 24 + + // Send the specified number of HTLCs. + for i := 0; i < numHTLCs; i++ { + htlc, _ := createHTLC(i, htlcAmt) + addAndReceiveHTLC(t, aliceChannel, bobChannel, htlc, nil) + } + + // We'll set up the mock aux signer to expect calls to PackSigs and also + // SubmitSecondLevelSigBatch. The direct return values for this mock aux + // signer are nil. The expected error comes from the behavior of + // noopAuxSigJob above, which mimics a faulty aux signer. + var sigBlobBuf bytes.Buffer + sigBlob := testSigBlob{ + BlobInt: tlv.NewPrimitiveRecord[tlv.TlvType65634, uint16](5), + } + tlvStream, err := tlv.NewStream(sigBlob.BlobInt.Record()) + require.NoError(t, err, "unable to create tlv stream") + require.NoError(t, tlvStream.Encode(&sigBlobBuf)) + + auxSigner.On( + "SubmitSecondLevelSigBatch", mock.Anything, mock.Anything, + mock.Anything, + ).Return(nil).Twice() + auxSigner.On( + "PackSigs", mock.Anything, + ).Return( + fn.Some(sigBlobBuf.Bytes()), nil, + ) + + quitDelay := time.Millisecond * 20 + quit, quitFunc := context.WithCancel(context.Background()) + + // Alice's channel will be stuck waiting for aux sig job responses until + // we send the quit signal. We add an explicit sleep here so that we can + // cause a failure if we run the test with a very short timeout. + go func() { + time.Sleep(quitDelay) + quitFunc() + }() + + _, err = aliceChannel.SignNextCommitment(quit) + require.ErrorIs(t, err, errQuit) +} + func testChanSyncOweCommitmentPendingRemote(t *testing.T, chanType channeldb.ChannelType) { From 41e491c32b7bdd4f25fdfc134f7ff6395f21dcd1 Mon Sep 17 00:00:00 2001 From: Jonathan Harvey-Buschel Date: Mon, 30 Sep 2024 15:26:28 -0400 Subject: [PATCH 9/9] gitignore: ignore vscode workspace files --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index a44a7b8f50a..6be439ed7e9 100644 --- a/.gitignore +++ b/.gitignore @@ -66,6 +66,7 @@ profile.tmp .DS_Store .vscode +*.code-workspace # Coverage test coverage.txt