multi: integrate new RBF co-op flow into the server+peer

docs/release-notes/release-notes-0.19.0.md

@@ -79,6 +79,23 @@
 
 # New Features
 
+* Add support for [archiving channel backup](https://github.com/lightningnetwork/lnd/pull/9232)
+ in a designated folder which allows for easy referencing in the future. A new 
+ config is added `disable-backup-archive`, with default set to false, to 
+ determine if previous channel backups should be archived or not.
+
+## Protocol Updates
+
+* `lnd` now [supports the new RBF cooperative close
+flow](https://github.com/lightningnetwork/lnd/pull/8453). Unlike the old flow,
+this version now uses RBF to enable either side to increase their fee rate using
+their _own_ channel funds. This removes the old "negotiation" logic that could
+fail, with a version where either side can increase the fee on their coop close
+transaction using their channel balance. 
+
+This new feature can be activated with a new config flag:
+`--protocol.rbf-coop-close`.
+
 * [Support](https://github.com/lightningnetwork/lnd/pull/8390) for 
   [experimental endorsement](https://github.com/lightning/blips/pull/27) 
   signal relay was added. This signal has *no impact* on routing, and
@@ -92,10 +109,7 @@
   initial historical sync may be blocked due to a race condition in handling the
   syncer's internal state.
 
-* Add support for [archiving channel backup](https://github.com/lightningnetwork/lnd/pull/9232)
- in a designated folder which allows for easy referencing in the future. A new 
- config is added `disable-backup-archive`, with default set to false, to 
- determine if previous channel backups should be archived or not.
+
 
 * [The max fee rate](https://github.com/lightningnetwork/lnd/pull/9491) is now
   respected when a coop close is initiated. Before the max fee rate would only
@@ -389,6 +403,7 @@ The underlying functionality between those two options remain the same.
 * Keagan McClelland
 * Nishant Bansal
 * Oliver Gugger
+* Olaoluwa Osuntokun
 * Pins
 * Viktor Tigerström
 * Yong Yu

feature/manager.go

@@ -73,6 +73,10 @@ type Config struct {
 	// forwarding experimental endorsement.
 	NoExperimentalEndorsement bool
 
+	// NoRbfCoopClose unsets any bits that signal support for using RBF for
+	// coop close.
+	NoRbfCoopClose bool
+
 	// CustomFeatures is a set of custom features to advertise in each
 	// set.
 	CustomFeatures map[Set][]lnwire.FeatureBit
@@ -209,11 +213,13 @@ func newManager(cfg Config, desc setDesc) (*Manager, error) {
 			raw.Unset(lnwire.SimpleTaprootOverlayChansOptional)
 			raw.Unset(lnwire.SimpleTaprootOverlayChansRequired)
 		}
-
 		if cfg.NoExperimentalEndorsement {
 			raw.Unset(lnwire.ExperimentalEndorsementOptional)
 			raw.Unset(lnwire.ExperimentalEndorsementRequired)
 		}
+		if cfg.NoRbfCoopClose {
+			raw.Unset(lnwire.RbfCoopCloseOptionalStaging)
+		}
 
 		for _, custom := range cfg.CustomFeatures[set] {
 			if custom > set.Maximum() {

itest/list_on_test.go

@@ -670,6 +670,10 @@ var allTestCases = []*lntest.TestCase{
 		Name:     "fee replacement",
 		TestFunc: testFeeReplacement,
 	},
+	{
+		Name:     "rbf coop close",
+		TestFunc: testCoopCloseRbf,
+	},
 }
 
 // appendPrefixed is used to add a prefix to each test name in the subtests

itest/lnd_coop_close_rbf_test.go

@@ -0,0 +1,135 @@
+package itest
+
+import (
+	"github.com/btcsuite/btcd/btcutil"
+	"github.com/lightningnetwork/lnd/lntest"
+	"github.com/lightningnetwork/lnd/lnwallet/chainfee"
+	"github.com/stretchr/testify/require"
+)
+
+func testCoopCloseRbf(ht *lntest.HarnessTest) {
+	rbfCoopFlags := []string{"--protocol.rbf-coop-close"}
+
+	// Set the fee estimate to 1sat/vbyte. This ensures that our manually
+	// initiated RBF attempts will always be successful.
+	ht.SetFeeEstimate(250)
+	ht.SetFeeEstimateWithConf(250, 6)
+
+	// To kick things off, we'll create two new nodes, then fund them with
+	// fund enough coins to make a 50/50 channel.
+	cfgs := [][]string{rbfCoopFlags, rbfCoopFlags}
+	params := lntest.OpenChannelParams{
+		Amt:     btcutil.Amount(1000000),
+		PushAmt: btcutil.Amount(1000000 / 2),
+	}
+	chanPoints, nodes := ht.CreateSimpleNetwork(cfgs, params)
+	alice, bob := nodes[0], nodes[1]
+	chanPoint := chanPoints[0]
+
+	// Now that both sides are active with a funded channel, we can kick
+	// off the test.
+	//
+	// To start, we'll have Alice try to close the channel, with a fee rate
+	// of 5 sat/byte.
+	aliceFeeRate := chainfee.SatPerVByte(5)
+	aliceCloseStream, aliceCloseUpdate := ht.CloseChannelAssertPending(
+		alice, chanPoint, false,
+		lntest.WithCoopCloseFeeRate(aliceFeeRate),
+		lntest.WithLocalTxNotify(),
+	)
+
+	// Confirm that this new update was at 5 sat/vb.
+	alicePendingUpdate := aliceCloseUpdate.GetClosePending()
+	require.NotNil(ht, aliceCloseUpdate)
+	require.Equal(
+		ht, int64(aliceFeeRate), alicePendingUpdate.FeePerVbyte,
+	)
+	require.True(ht, alicePendingUpdate.LocalCloseTx)
+
+	// Now, we'll have Bob attempt to RBF the close transaction with a
+	// higher fee rate, double that of Alice's.
+	bobFeeRate := aliceFeeRate * 2
+	bobCloseStream, bobCloseUpdate := ht.CloseChannelAssertPending(
+		bob, chanPoint, false, lntest.WithCoopCloseFeeRate(bobFeeRate),
+		lntest.WithLocalTxNotify(),
+	)
+
+	// Confirm that this new update was at 10 sat/vb.
+	bobPendingUpdate := bobCloseUpdate.GetClosePending()
+	require.NotNil(ht, bobCloseUpdate)
+	require.Equal(ht, bobPendingUpdate.FeePerVbyte, int64(bobFeeRate))
+	require.True(ht, bobPendingUpdate.LocalCloseTx)
+
+	var err error
+
+	// Alice should've also received a similar update that Bob has
+	// increased the closing fee rate to 10 sat/vb with his settled funds.
+	aliceCloseUpdate, err = ht.ReceiveCloseChannelUpdate(aliceCloseStream)
+	require.NoError(ht, err)
+	alicePendingUpdate = aliceCloseUpdate.GetClosePending()
+	require.NotNil(ht, aliceCloseUpdate)
+	require.Equal(ht, alicePendingUpdate.FeePerVbyte, int64(bobFeeRate))
+	require.False(ht, alicePendingUpdate.LocalCloseTx)
+
+	// We'll now attempt to make a fee update that increases Alice's fee
+	// rate by 6 sat/vb, which should be rejected as it is too small of an
+	// increase for the RBF rules. The RPC API however will return the new
+	// fee. We'll skip the mempool check here as it won't make it in.
+	aliceRejectedFeeRate := aliceFeeRate + 1
+	_, aliceCloseUpdate = ht.CloseChannelAssertPending(
+		alice, chanPoint, false,
+		lntest.WithCoopCloseFeeRate(aliceRejectedFeeRate),
+		lntest.WithLocalTxNotify(), lntest.WithSkipMempoolCheck(),
+	)
+	alicePendingUpdate = aliceCloseUpdate.GetClosePending()
+	require.NotNil(ht, aliceCloseUpdate)
+	require.Equal(
+		ht, alicePendingUpdate.FeePerVbyte,
+		int64(aliceRejectedFeeRate),
+	)
+	require.True(ht, alicePendingUpdate.LocalCloseTx)
+
+	_, err = ht.ReceiveCloseChannelUpdate(bobCloseStream)
+	require.NoError(ht, err)
+
+	// We'll now attempt a fee update that we can't actually pay for. This
+	// will actually show up as an error to the remote party.
+	aliceRejectedFeeRate = 100_000
+	_, _ = ht.CloseChannelAssertPending(
+		alice, chanPoint, false,
+		lntest.WithCoopCloseFeeRate(aliceRejectedFeeRate),
+		lntest.WithLocalTxNotify(),
+		lntest.WithExpectedErrString("cannot pay for fee"),
+	)
+
+	// At this point, we'll have Alice+Bob reconnect so we can ensure that
+	// we can continue to do RBF bumps even after a reconnection.
+	ht.DisconnectNodes(alice, bob)
+	ht.ConnectNodes(alice, bob)
+
+	// Next, we'll have Alice double that fee rate again to 20 sat/vb.
+	aliceFeeRate = bobFeeRate * 2
+	aliceCloseStream, aliceCloseUpdate = ht.CloseChannelAssertPending(
+		alice, chanPoint, false,
+		lntest.WithCoopCloseFeeRate(aliceFeeRate),
+		lntest.WithLocalTxNotify(),
+	)
+	alicePendingUpdate = aliceCloseUpdate.GetClosePending()
+	require.NotNil(ht, aliceCloseUpdate)
+	require.Equal(
+		ht, alicePendingUpdate.FeePerVbyte, int64(aliceFeeRate),
+	)
+	require.True(ht, alicePendingUpdate.LocalCloseTx)
+
+	// TODO(roasbeef): trigger another bump from Bob
+	//   * assert txids again below
+
+	// To conclude, we'll mine a block which should now confirm Alice's
+	// version of the coop close transaction.
+	block := ht.MineBlocksAndAssertNumTxes(1, 1)[0]
+
+	// Both Alice and Bob should trigger a final close update to signal the
+	// closing transaction has confirmed.
+	aliceClosingTxid := ht.WaitForChannelCloseEvent(aliceCloseStream)
+	ht.AssertTxInBlock(block, aliceClosingTxid)
+}

itest/lnd_coop_close_with_htlcs_test.go

@@ -1,6 +1,7 @@
 package itest
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/btcsuite/btcd/btcutil"
@@ -10,6 +11,7 @@ import (
 	"github.com/lightningnetwork/lnd/lnrpc/routerrpc"
 	"github.com/lightningnetwork/lnd/lnrpc/walletrpc"
 	"github.com/lightningnetwork/lnd/lntest"
+	"github.com/lightningnetwork/lnd/lntest/node"
 	"github.com/lightningnetwork/lnd/lntest/wait"
 	"github.com/lightningnetwork/lnd/lntypes"
 	"github.com/lightningnetwork/lnd/lnwallet/chainfee"
@@ -23,23 +25,47 @@ import (
 // will have the receiver settle the invoice and observe that the channel gets
 // torn down after settlement.
 func testCoopCloseWithHtlcs(ht *lntest.HarnessTest) {
-	ht.Run("no restart", func(t *testing.T) {
-		tt := ht.Subtest(t)
-		coopCloseWithHTLCs(tt)
-	})
+	rbfCoopFlags := []string{"--protocol.rbf-coop-close"}
 
-	ht.Run("with restart", func(t *testing.T) {
-		tt := ht.Subtest(t)
-		coopCloseWithHTLCsWithRestart(tt)
-	})
+	for _, isRbf := range []bool{true, false} {
+		testName := fmt.Sprintf("no restart is_rbf=%v", isRbf)
+		ht.Run(testName, func(t *testing.T) {
+			tt := ht.Subtest(t)
+
+			var flags []string
+			if isRbf {
+				flags = rbfCoopFlags
+			}
+
+			alice := ht.NewNodeWithCoins("Alice", flags)
+			bob := ht.NewNodeWithCoins("bob", flags)
+
+			coopCloseWithHTLCs(tt, alice, bob)
+		})
+	}
+
+	for _, isRbf := range []bool{true, false} {
+		testName := fmt.Sprintf("with restart is_rbf=%v", isRbf)
+		ht.Run(testName, func(t *testing.T) {
+			tt := ht.Subtest(t)
+
+			var flags []string
+			if isRbf {
+				flags = rbfCoopFlags
+			}
+
+			alice := ht.NewNodeWithCoins("Alice", flags)
+			bob := ht.NewNodeWithCoins("bob", flags)
+
+			coopCloseWithHTLCsWithRestart(tt, alice, bob)
+		})
+	}
 }
 
 // coopCloseWithHTLCs tests the basic coop close scenario which occurs when one
 // channel party initiates a channel shutdown while an HTLC is still pending on
 // the channel.
-func coopCloseWithHTLCs(ht *lntest.HarnessTest) {
-	alice := ht.NewNodeWithCoins("Alice", nil)
-	bob := ht.NewNodeWithCoins("bob", nil)
+func coopCloseWithHTLCs(ht *lntest.HarnessTest, alice, bob *node.HarnessNode) {
 	ht.ConnectNodes(alice, bob)
 
 	// Here we set up a channel between Alice and Bob, beginning with a
@@ -131,9 +157,9 @@ func coopCloseWithHTLCs(ht *lntest.HarnessTest) {
 // is still pending on the channel but this time it ensures that the shutdown
 // process continues as expected even if a channel re-establish happens after
 // one party has already initiated the shutdown.
-func coopCloseWithHTLCsWithRestart(ht *lntest.HarnessTest) {
-	alice := ht.NewNodeWithCoins("Alice", nil)
-	bob := ht.NewNodeWithCoins("bob", nil)
+func coopCloseWithHTLCsWithRestart(ht *lntest.HarnessTest, alice,
+	bob *node.HarnessNode) {
+
 	ht.ConnectNodes(alice, bob)
 
 	// Open a channel between Alice and Bob with the balance split equally.
@@ -219,8 +245,8 @@ func coopCloseWithHTLCsWithRestart(ht *lntest.HarnessTest) {
 	}, defaultTimeout)
 	require.NoError(ht, err)
 
-	// Wait for the close tx to be in the Mempool and then mine 6 blocks
-	// to confirm the close.
+	// Wait for the close tx to be in the Mempool and then mine 6 blocks to
+	// confirm the close.
 	closingTx := ht.AssertClosingTxInMempool(
 		chanPoint, lnrpc.CommitmentType_LEGACY,
 	)

itest/lnd_revocation_test.go

@@ -109,7 +109,11 @@ func breachRetributionTestCase(ht *lntest.HarnessTest,
 	// broadcasting his current channel state. This is actually the
 	// commitment transaction of a prior *revoked* state, so he'll soon
 	// feel the wrath of Carol's retribution.
-	_, breachTXID := ht.CloseChannelAssertPending(bob, chanPoint, true)
+	_, breachCloseUpd := ht.CloseChannelAssertPending(bob, chanPoint, true)
+	closeUpd := breachCloseUpd.GetClosePending()
+	require.NotNil(ht, closeUpd)
+	breachTXID, err := chainhash.NewHash(closeUpd.Txid)
+	require.NoError(ht, err)
 
 	// Here, Carol sees Bob's breach transaction in the mempool, but is
 	// waiting for it to confirm before continuing her retribution. We
@@ -122,13 +126,13 @@ func breachRetributionTestCase(ht *lntest.HarnessTest,
 	// update, then ensure that the closing transaction was included in the
 	// block.
 	block := ht.MineBlocksAndAssertNumTxes(1, 1)[0]
-	ht.AssertTxInBlock(block, breachTXID)
+	ht.AssertTxInBlock(block, *breachTXID)
 
 	// Construct to_remote output which pays to Bob. Based on the output
 	// ordering, the first output in this breach tx is the to_remote
 	// output.
 	toRemoteOp := wire.OutPoint{
-		Hash:  breachTXID,
+		Hash:  *breachTXID,
 		Index: 0,
 	}
 
@@ -151,7 +155,7 @@ func breachRetributionTestCase(ht *lntest.HarnessTest,
 	// Assert that all the inputs of this transaction are spending outputs
 	// generated by Bob's breach transaction above.
 	for _, txIn := range justiceTx.TxIn {
-		require.Equal(ht, breachTXID, txIn.PreviousOutPoint.Hash,
+		require.Equal(ht, *breachTXID, txIn.PreviousOutPoint.Hash,
 			"justice tx not spending commitment utxo")
 	}
 
@@ -296,7 +300,7 @@ func revokedCloseRetributionZeroValueRemoteOutputCase(ht *lntest.HarnessTest,
 	// broadcasting her current channel state. This is actually the
 	// commitment transaction of a prior *revoked* state, so she'll soon
 	// feel the wrath of Dave's retribution.
-	stream, closeTxID := ht.CloseChannelAssertPending(
+	stream, closeUpdate := ht.CloseChannelAssertPending(
 		carol, chanPoint, true,
 	)
 
@@ -310,9 +314,11 @@ func revokedCloseRetributionZeroValueRemoteOutputCase(ht *lntest.HarnessTest,
 	// state and continues exacting justice after his node restarts.
 	ht.RestartNode(dave)
 
+	closeTxID := closeUpdate.GetClosePending().Txid
+
 	// The breachTXID should match the above closeTxID.
 	breachTXID := ht.WaitForChannelCloseEvent(stream)
-	require.EqualValues(ht, breachTXID, closeTxID)
+	require.EqualValues(ht, breachTXID[:], closeTxID)
 
 	// Construct to_local output which pays to Dave. Based on the output
 	// ordering, the first output in this breach tx is the to_local
@@ -543,9 +549,13 @@ func revokedCloseRetributionRemoteHodlCase(ht *lntest.HarnessTest,
 	// broadcasting her current channel state. This is actually the
 	// commitment transaction of a prior *revoked* state, so she'll soon
 	// feel the wrath of Dave's retribution.
-	closeUpdates, closeTxID := ht.CloseChannelAssertPending(
+	closeUpdates, closeUpd := ht.CloseChannelAssertPending(
 		carol, chanPoint, true,
 	)
+	pendingCloseUpd := closeUpd.GetClosePending()
+	require.NotNil(ht, pendingCloseUpd)
+	closeTxID, err := chainhash.NewHash(pendingCloseUpd.Txid)
+	require.NoError(ht, err)
 
 	// Generate a single block to mine the breach transaction.
 	block := ht.MineBlocksAndAssertNumTxes(1, 1)[0]
@@ -593,7 +603,7 @@ func revokedCloseRetributionRemoteHodlCase(ht *lntest.HarnessTest,
 		return nil, errNotFound
 	}
 
-	err := wait.NoError(func() error {
+	err = wait.NoError(func() error {
 		txid, err := findJusticeTx()
 		if err != nil {
 			return err