From a48c369250831d8ca5ed3312091237c35acc56bc Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 24 Aug 2020 15:44:13 +0200 Subject: [PATCH] lnwallet: check coop close fee negative balance Also modify the test to check for this condition. --- lnwallet/channel.go | 10 ++++++++-- lnwallet/channel_test.go | 23 +++++++++++++++++++---- lnwallet/commitment.go | 12 ++++++++++-- 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 30deb5a2..ccdf5530 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -6182,10 +6182,13 @@ func (lc *LightningChannel) CreateCloseProposal(proposedFee btcutil.Amount, // 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. - ourBalance, theirBalance := CoopCloseBalance( + ourBalance, theirBalance, err := CoopCloseBalance( lc.channelState.ChanType, lc.channelState.IsInitiator, proposedFee, lc.channelState.LocalCommitment, ) + if err != nil { + return nil, nil, 0, err + } closeTx := CreateCooperativeCloseTx( fundingTxIn(lc.channelState), lc.channelState.LocalChanCfg.DustLimit, @@ -6241,10 +6244,13 @@ func (lc *LightningChannel) CompleteCooperativeClose( } // Get the final balances after subtracting the proposed fee. - ourBalance, theirBalance := CoopCloseBalance( + 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 // on this active channel back to both parties. In this current model, diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 5347e132..190e31c1 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -2230,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) @@ -2277,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 1039bbfc..44b3c81c 100644 --- a/lnwallet/commitment.go +++ b/lnwallet/commitment.go @@ -667,7 +667,7 @@ func CreateCommitTx(chanType channeldb.ChannelType, // 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) { + btcutil.Amount, btcutil.Amount, error) { // Get both parties' balances from the latest commitment. ourBalance := localCommit.LocalBalance.ToSatoshis() @@ -693,7 +693,15 @@ func CoopCloseBalance(chanType channeldb.ChannelType, isInitiator bool, theirBalance += initiatorDelta } - return ourBalance, theirBalance + // 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