From edc64026882c0c3735b62603bc4023ee7e266f8c Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Sun, 24 Nov 2024 14:08:19 -0800 Subject: [PATCH] lnwallet: update core coop close logic with custom payer In this commit, we update the core coop close logic with the new custom payer param. We also expand the existing unit tests to ensure that the fee is deducted from the proper party. --- lnwallet/channel.go | 38 ++++++++---- lnwallet/channel_test.go | 125 ++++++++++++++++++++++++++++++++------- 2 files changed, 129 insertions(+), 34 deletions(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 10a1071ea8..bffa1f3617 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -8194,6 +8194,8 @@ type chanCloseOpt struct { customSequence fn.Option[uint32] customLockTime fn.Option[uint32] + + customPayer fn.Option[lntypes.ChannelParty] } // ChanCloseOpt is a closure type that cen be used to modify the set of default @@ -8246,6 +8248,15 @@ func WithCustomLockTime(lockTime uint32) ChanCloseOpt { } } +// WithCustomPayer can be used to specify a custom payer for the closing +// transaction. This overrides the default payer, which is the initiator of the +// channel. +func WithCustomPayer(payer lntypes.ChannelParty) ChanCloseOpt { + return func(opts *chanCloseOpt) { + opts.customPayer = fn.Some(payer) + } +} + // CreateCloseProposal is used by both parties in a cooperative channel close // workflow to generate proposed close transactions and signatures. This method // should only be executed once all pending HTLCs (if any) on the channel have @@ -8261,16 +8272,17 @@ func (lc *LightningChannel) CreateCloseProposal(proposedFee btcutil.Amount, lc.Lock() defer lc.Unlock() - // If we're already closing the channel, then ignore this request. - if lc.isClosed { - return nil, nil, 0, ErrChanClosing - } - opts := defaultCloseOpts() for _, optFunc := range closeOpts { optFunc(opts) } + // Unless there's a custom payer (sign of the RBF flow), if we're + // already closing the channel, then ignore this request. + if lc.isClosed && opts.customPayer.IsNone() { + return nil, nil, 0, ErrChanClosing + } + // Get the final balances after subtracting the proposed fee, taking // care not to persist the adjusted balance, as the feeRate may change // during the channel closing process. @@ -8280,7 +8292,7 @@ func (lc *LightningChannel) CreateCloseProposal(proposedFee btcutil.Amount, lc.channelState.LocalCommitment.LocalBalance.ToSatoshis(), lc.channelState.LocalCommitment.RemoteBalance.ToSatoshis(), lc.channelState.LocalCommitment.CommitFee, - fn.None[lntypes.ChannelParty](), + opts.customPayer, ) if err != nil { return nil, nil, 0, err @@ -8375,17 +8387,17 @@ func (lc *LightningChannel) CompleteCooperativeClose( lc.Lock() defer lc.Unlock() - // If the channel is already closing, then ignore this request. - if lc.isClosed { - // TODO(roasbeef): check to ensure no pending payments - return nil, 0, ErrChanClosing - } - opts := defaultCloseOpts() for _, optFunc := range closeOpts { optFunc(opts) } + // Unless there's a custom payer (sign of the RBF flow), if we're + // already closing the channel, then ignore this request. + if lc.isClosed && opts.customPayer.IsNone() { + return nil, 0, ErrChanClosing + } + // Get the final balances after subtracting the proposed fee. ourBalance, theirBalance, err := CoopCloseBalance( lc.channelState.ChanType, lc.channelState.IsInitiator, @@ -8393,7 +8405,7 @@ func (lc *LightningChannel) CompleteCooperativeClose( lc.channelState.LocalCommitment.LocalBalance.ToSatoshis(), lc.channelState.LocalCommitment.RemoteBalance.ToSatoshis(), lc.channelState.LocalCommitment.CommitFee, - fn.None[lntypes.ChannelParty](), + opts.customPayer, ) if err != nil { return nil, 0, err diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index f7ecd32277..3251f58a48 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -767,29 +767,66 @@ func TestCommitHTLCSigCustomRecordSize(t *testing.T) { } // TestCooperativeChannelClosure checks that the coop close process finishes -// with an agreement from both parties, and that the final balances of the -// close tx check out. +// with an agreement from both parties, and that the final balances of the close +// tx check out. func TestCooperativeChannelClosure(t *testing.T) { - t.Run("tweakless", func(t *testing.T) { - testCoopClose(t, &coopCloseTestCase{ - chanType: channeldb.SingleFunderTweaklessBit, - }) - }) - t.Run("anchors", func(t *testing.T) { - testCoopClose(t, &coopCloseTestCase{ - chanType: channeldb.SingleFunderTweaklessBit | - channeldb.AnchorOutputsBit, - anchorAmt: AnchorSize * 2, + testCases := []struct { + name string + closeCase coopCloseTestCase + }{ + { + name: "tweakless", + closeCase: coopCloseTestCase{ + chanType: channeldb.SingleFunderTweaklessBit, + }, + }, + { + name: "anchors", + closeCase: coopCloseTestCase{ + chanType: channeldb.SingleFunderTweaklessBit | + channeldb.AnchorOutputsBit, + anchorAmt: AnchorSize * 2, + }, + }, + { + name: "anchors local pay", + closeCase: coopCloseTestCase{ + chanType: channeldb.SingleFunderTweaklessBit | + channeldb.AnchorOutputsBit, + anchorAmt: AnchorSize * 2, + customPayer: fn.Some(lntypes.Local), + }, + }, + { + name: "anchors remote pay", + closeCase: coopCloseTestCase{ + chanType: channeldb.SingleFunderTweaklessBit | + channeldb.AnchorOutputsBit, + anchorAmt: AnchorSize * 2, + customPayer: fn.Some(lntypes.Remote), + }, + }, + } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + testCoopClose(t, testCase.closeCase) }) - }) + } } type coopCloseTestCase struct { chanType channeldb.ChannelType anchorAmt btcutil.Amount + + customPayer fn.Option[lntypes.ChannelParty] +} + +type closeOpts struct { + aliceOpts []ChanCloseOpt + bobOpts []ChanCloseOpt } -func testCoopClose(t *testing.T, testCase *coopCloseTestCase) { +func testCoopClose(t *testing.T, testCase coopCloseTestCase) { t.Parallel() // Create a test channel which will be used for the duration of this @@ -810,17 +847,38 @@ func testCoopClose(t *testing.T, testCase *coopCloseTestCase) { bobChannel.channelState.LocalCommitment.FeePerKw, ) + customPayer := testCase.customPayer + + closeOpts := fn.MapOptionZ( + customPayer, func(payer lntypes.ChannelParty) closeOpts { + // If the local party is paying then from Alice's PoV, + // then local party is paying. From Bob's PoV, the + // remote party is paying. If the remote party is, then + // the opposite is true. + return closeOpts{ + aliceOpts: []ChanCloseOpt{ + WithCustomPayer(payer), + }, + bobOpts: []ChanCloseOpt{ + WithCustomPayer(payer.CounterParty()), + }, + } + }, + ) + // We'll start with both Alice and Bob creating a new close proposal // with the same fee. aliceFee := aliceChannel.CalcFee(aliceFeeRate) aliceSig, _, _, err := aliceChannel.CreateCloseProposal( aliceFee, aliceDeliveryScript, bobDeliveryScript, + closeOpts.aliceOpts..., ) require.NoError(t, err, "unable to create alice coop close proposal") bobFee := bobChannel.CalcFee(bobFeeRate) bobSig, _, _, err := bobChannel.CreateCloseProposal( bobFee, bobDeliveryScript, aliceDeliveryScript, + closeOpts.bobOpts..., ) require.NoError(t, err, "unable to create bob coop close proposal") @@ -829,14 +887,14 @@ func testCoopClose(t *testing.T, testCase *coopCloseTestCase) { // transaction is well formed, and the signatures verify. aliceCloseTx, bobTxBalance, err := bobChannel.CompleteCooperativeClose( bobSig, aliceSig, bobDeliveryScript, aliceDeliveryScript, - bobFee, + bobFee, closeOpts.bobOpts..., ) require.NoError(t, err, "unable to complete alice cooperative close") bobCloseSha := aliceCloseTx.TxHash() bobCloseTx, aliceTxBalance, err := aliceChannel.CompleteCooperativeClose( aliceSig, bobSig, aliceDeliveryScript, bobDeliveryScript, - aliceFee, + aliceFee, closeOpts.aliceOpts..., ) require.NoError(t, err, "unable to complete bob cooperative close") aliceCloseSha := bobCloseTx.TxHash() @@ -845,18 +903,43 @@ func testCoopClose(t *testing.T, testCase *coopCloseTestCase) { t.Fatalf("alice and bob close transactions don't match: %v", err) } - // Finally, make sure the final balances are correct from both's - // perspective. + type chanFees struct { + alice btcutil.Amount + bob btcutil.Amount + } + + // Compute the closing fees for each party. If not specified, Alice will + // always pay the fees. Otherwise, it depends on who the payer is. + closeFees := fn.MapOption(func(payer lntypes.ChannelParty) chanFees { + var alice, bob btcutil.Amount + + switch payer { + case lntypes.Local: + alice = bobFee + bob = 0 + case lntypes.Remote: + bob = bobFee + alice = 0 + } + + return chanFees{ + alice: alice, + bob: bob, + } + })(testCase.customPayer).UnwrapOr(chanFees{alice: bobFee}) + + // Finally, make sure the final balances are correct from both + // perspectives. aliceBalance := aliceChannel.channelState.LocalCommitment. LocalBalance.ToSatoshis() - // The commit balance have had the initiator's (Alice) commitfee and + // The commit balance have had the initiator's (Alice) commit fee and // any anchors subtracted, so add that back to the final expected // balance. Alice also pays the coop close fee, so that must be // subtracted. commitFee := aliceChannel.channelState.LocalCommitment.CommitFee expBalanceAlice := aliceBalance + commitFee + - testCase.anchorAmt - bobFee + testCase.anchorAmt - closeFees.alice if aliceTxBalance != expBalanceAlice { t.Fatalf("expected balance %v got %v", expBalanceAlice, aliceTxBalance) @@ -865,7 +948,7 @@ func testCoopClose(t *testing.T, testCase *coopCloseTestCase) { // Bob is not the initiator, so his final balance should simply be // equal to the latest commitment balance. expBalanceBob := bobChannel.channelState.LocalCommitment. - LocalBalance.ToSatoshis() + LocalBalance.ToSatoshis() - closeFees.bob if bobTxBalance != expBalanceBob { t.Fatalf("expected bob's balance to be %v got %v", expBalanceBob, bobTxBalance)