diff --git a/channeldb/channel.go b/channeldb/channel.go index 0a5e47b2..35a0700d 100644 --- a/channeldb/channel.go +++ b/channeldb/channel.go @@ -206,8 +206,7 @@ const ( // AnchorOutputsBit indicates that the channel makes use of anchor // outputs to bump the commitment transaction's effective feerate. This - // channel type also uses a delayed to_remote output script. If bit is - // set, we'll find the size of the anchor outputs in the database. + // channel type also uses a delayed to_remote output script. AnchorOutputsBit ChannelType = 1 << 3 // FrozenBit indicates that the channel is a frozen channel, meaning diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 94e26805..ccdf5530 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -6179,20 +6179,15 @@ func (lc *LightningChannel) CreateCloseProposal(proposedFee btcutil.Amount, return nil, nil, 0, ErrChanClosing } - // Subtract the proposed fee from the appropriate balance, taking care - // not to persist the adjusted balance, as the feeRate may change + // 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. - localCommit := lc.channelState.LocalCommitment - ourBalance := localCommit.LocalBalance.ToSatoshis() - theirBalance := localCommit.RemoteBalance.ToSatoshis() - - // We'll make sure we account for the complete balance by adding the - // current dangling commitment fee to the balance of the initiator. - commitFee := localCommit.CommitFee - if lc.channelState.IsInitiator { - ourBalance = ourBalance - proposedFee + commitFee - } else { - theirBalance = theirBalance - proposedFee + commitFee + ourBalance, theirBalance, err := CoopCloseBalance( + lc.channelState.ChanType, lc.channelState.IsInitiator, + proposedFee, lc.channelState.LocalCommitment, + ) + if err != nil { + return nil, nil, 0, err } closeTx := CreateCooperativeCloseTx( @@ -6248,20 +6243,13 @@ func (lc *LightningChannel) CompleteCooperativeClose( return nil, 0, ErrChanClosing } - // Subtract the proposed fee from the appropriate balance, taking care - // not to persist the adjusted balance, as the feeRate may change - // during the channel closing process. - localCommit := lc.channelState.LocalCommitment - ourBalance := localCommit.LocalBalance.ToSatoshis() - theirBalance := localCommit.RemoteBalance.ToSatoshis() - - // We'll make sure we account for the complete balance by adding the - // current dangling commitment fee to the balance of the initiator. - commitFee := localCommit.CommitFee - if lc.channelState.IsInitiator { - ourBalance = ourBalance - proposedFee + commitFee - } else { - theirBalance = theirBalance - proposedFee + commitFee + // Get the final balances after subtracting the proposed fee. + ourBalance, theirBalance, err := CoopCloseBalance( + lc.channelState.ChanType, lc.channelState.IsInitiator, + proposedFee, lc.channelState.LocalCommitment, + ) + if err != nil { + return nil, 0, err } // Create the transaction used to return the current settled balance diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index f076ae3a..190e31c1 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -683,14 +683,37 @@ func testCommitHTLCSigTieBreak(t *testing.T, restart bool) { } } +// 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. 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, + }) + }) +} + +type coopCloseTestCase struct { + chanType channeldb.ChannelType + anchorAmt btcutil.Amount +} + +func testCoopClose(t *testing.T, testCase *coopCloseTestCase) { t.Parallel() // Create a test channel which will be used for the duration of this // unittest. The channel will be funded evenly with Alice having 5 BTC, // and Bob having 5 BTC. aliceChannel, bobChannel, cleanUp, err := CreateTestChannels( - channeldb.SingleFunderTweaklessBit, + testCase.chanType, ) if err != nil { t.Fatalf("unable to create test channels: %v", err) @@ -707,7 +730,7 @@ func TestCooperativeChannelClosure(t *testing.T) { bobChannel.channelState.LocalCommitment.FeePerKw, ) - // We'll store with both Alice and Bob creating a new close proposal + // 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( @@ -728,7 +751,7 @@ func TestCooperativeChannelClosure(t *testing.T) { // With the proposals created, both sides should be able to properly // process the other party's signature. This indicates that the // transaction is well formed, and the signatures verify. - aliceCloseTx, _, err := bobChannel.CompleteCooperativeClose( + aliceCloseTx, bobTxBalance, err := bobChannel.CompleteCooperativeClose( bobSig, aliceSig, bobDeliveryScript, aliceDeliveryScript, bobFee, ) @@ -737,7 +760,7 @@ func TestCooperativeChannelClosure(t *testing.T) { } bobCloseSha := aliceCloseTx.TxHash() - bobCloseTx, _, err := aliceChannel.CompleteCooperativeClose( + bobCloseTx, aliceTxBalance, err := aliceChannel.CompleteCooperativeClose( aliceSig, bobSig, aliceDeliveryScript, bobDeliveryScript, aliceFee, ) @@ -749,6 +772,32 @@ func TestCooperativeChannelClosure(t *testing.T) { if bobCloseSha != aliceCloseSha { t.Fatalf("alice and bob close transactions don't match: %v", err) } + + // Finally, make sure the final balances are correct from both's + // perspective. + aliceBalance := aliceChannel.channelState.LocalCommitment. + LocalBalance.ToSatoshis() + + // The commit balance have had the initiator's (Alice) commitfee 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 + if aliceTxBalance != expBalanceAlice { + t.Fatalf("expected balance %v got %v", expBalanceAlice, + aliceTxBalance) + } + + // Bob is not the initiator, so his final balance should simply be + // equal to the latest commitment balance. + expBalanceBob := bobChannel.channelState.LocalCommitment. + LocalBalance.ToSatoshis() + if bobTxBalance != expBalanceBob { + t.Fatalf("expected bob's balance to be %v got %v", + expBalanceBob, bobTxBalance) + } } // TestForceClose checks that the resulting ForceCloseSummary is correct when a @@ -2181,11 +2230,11 @@ func TestCooperativeCloseDustAdherence(t *testing.T) { "got %v", 2, len(closeTx.TxOut)) } - // We'll reset the channel states before proceeding to our nest test. + // We'll reset the channel states before proceeding to our next test. resetChannelState() // Next we'll modify the current balances and dust limits such that - // Bob's current balance is above _below_ his dust limit. + // Bob's current balance is _below_ his dust limit. aliceBal := lnwire.NewMSatFromSatoshis(btcutil.SatoshiPerBitcoin) bobBal := lnwire.NewMSatFromSatoshis(250) setBalances(aliceBal, bobBal) @@ -2228,11 +2277,26 @@ func TestCooperativeCloseDustAdherence(t *testing.T) { int64(closeTx.TxOut[0].Value)) } - // Finally, we'll modify the current balances and dust limits such that - // Alice's current balance is _below_ his her limit. + // We'll modify the current balances and dust limits such that + // Alice's current balance is too low to pay the proposed fee. setBalances(bobBal, aliceBal) resetChannelState() + // Attempting to close with this fee now should fail, since Alice + // cannot afford it. + _, _, _, err = aliceChannel.CreateCloseProposal( + aliceFee, aliceDeliveryScript, bobDeliveryScript, + ) + if err == nil { + t.Fatalf("expected error") + } + + // Finally, we'll modify the current balances and dust limits such that + // Alice's balance after paying the coop fee is _below_ her dust limit. + lowBalance := lnwire.NewMSatFromSatoshis(aliceFee) + 1000 + setBalances(lowBalance, aliceBal) + resetChannelState() + // Our final attempt at another cooperative channel closure. It should // succeed without any issues. aliceSig, _, _, err = aliceChannel.CreateCloseProposal( diff --git a/lnwallet/commitment.go b/lnwallet/commitment.go index d70f3ca0..44b3c81c 100644 --- a/lnwallet/commitment.go +++ b/lnwallet/commitment.go @@ -663,6 +663,47 @@ func CreateCommitTx(chanType channeldb.ChannelType, return commitTx, nil } +// CoopCloseBalance returns the final balances that should be used to create +// the cooperative close tx, given the channel type and transaction fee. +func CoopCloseBalance(chanType channeldb.ChannelType, isInitiator bool, + coopCloseFee btcutil.Amount, localCommit channeldb.ChannelCommitment) ( + btcutil.Amount, btcutil.Amount, error) { + + // Get both parties' balances from the latest commitment. + ourBalance := localCommit.LocalBalance.ToSatoshis() + theirBalance := localCommit.RemoteBalance.ToSatoshis() + + // We'll make sure we account for the complete balance by adding the + // current dangling commitment fee to the balance of the initiator. + initiatorDelta := localCommit.CommitFee + + // Since the initiator's balance also is stored after subtracting the + // anchor values, add that back in case this was an anchor commitment. + if chanType.HasAnchors() { + initiatorDelta += 2 * anchorSize + } + + // The initiator will pay the full coop close fee, subtract that value + // from their balance. + initiatorDelta -= coopCloseFee + + if isInitiator { + ourBalance += initiatorDelta + } else { + theirBalance += initiatorDelta + } + + // During fee negotiation it should always be verified that the + // initiator can pay the proposed fee, but we do a sanity check just to + // be sure here. + if ourBalance < 0 || theirBalance < 0 { + return 0, 0, fmt.Errorf("initiator cannot afford proposed " + + "coop close fee") + } + + return ourBalance, theirBalance, nil +} + // genHtlcScript generates the proper P2WSH public key scripts for the HTLC // output modified by two-bits denoting if this is an incoming HTLC, and if the // HTLC is being applied to their commitment transaction or ours. diff --git a/lnwire/features.go b/lnwire/features.go index 6cc89946..5584191b 100644 --- a/lnwire/features.go +++ b/lnwire/features.go @@ -112,12 +112,12 @@ const ( // AnchorsRequired is a required feature bit that signals that the node // requires channels to be made using commitments having anchor // outputs. - AnchorsRequired FeatureBit = 1336 + AnchorsRequired FeatureBit = 20 // AnchorsRequired is an optional feature bit that signals that the // node supports channels to be made using commitments having anchor // outputs. - AnchorsOptional FeatureBit = 1337 + AnchorsOptional FeatureBit = 21 // maxAllowedSize is a maximum allowed size of feature vector. // diff --git a/peer/brontide.go b/peer/brontide.go index cc637207..11e04928 100644 --- a/peer/brontide.go +++ b/peer/brontide.go @@ -2785,11 +2785,11 @@ func (p *Brontide) handleCloseMsg(msg *closeMsg) { func (p *Brontide) HandleLocalCloseChanReqs(req *htlcswitch.ChanClose) { select { case p.localCloseChanReqs <- req: - peerLog.Infof("Local close channel request delivered to peer: %v", - p.PubKey()) + peerLog.Infof("Local close channel request delivered to "+ + "peer: %x", p.PubKey()) case <-p.quit: - peerLog.Infof("Unable to deliver local close channel request to peer "+ - "%x", p.PubKey()) + peerLog.Infof("Unable to deliver local close channel request "+ + "to peer %x", p.PubKey()) } }