Merge pull request #4558 from halseth/anchors-spec

[anchor commitment] Make the anchor commitment type spec compliant

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

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

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(

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.

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.
 	//

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())
 	}
 }