Merge pull request #9610 from lightningnetwork/rbf-staging

multi: integrate rbf changes from staging branch

Author: Roasbeef

channeldb/channel.go

@@ -4104,6 +4104,34 @@ func (c *OpenChannel) AbsoluteThawHeight() (uint32, error) {
 	return c.ThawHeight, nil
 }
 
+// DeriveHeightHint derives the block height for the channel opening.
+func (c *OpenChannel) DeriveHeightHint() uint32 {
+	// As a height hint, we'll try to use the opening height, but if the
+	// channel isn't yet open, then we'll use the height it was broadcast
+	// at. This may be an unconfirmed zero-conf channel.
+	heightHint := c.ShortChanID().BlockHeight
+	if heightHint == 0 {
+		heightHint = c.BroadcastHeight()
+	}
+
+	// Since no zero-conf state is stored in a channel backup, the below
+	// logic will not be triggered for restored, zero-conf channels. Set
+	// the height hint for zero-conf channels.
+	if c.IsZeroConf() {
+		if c.ZeroConfConfirmed() {
+			// If the zero-conf channel is confirmed, we'll use the
+			// confirmed SCID's block height.
+			heightHint = c.ZeroConfRealScid().BlockHeight
+		} else {
+			// The zero-conf channel is unconfirmed. We'll need to
+			// use the FundingBroadcastHeight.
+			heightHint = c.BroadcastHeight()
+		}
+	}
+
+	return heightHint
+}
+
 func putChannelCloseSummary(tx kvdb.RwTx, chanID []byte,
 	summary *ChannelCloseSummary, lastChanState *OpenChannel) error {
 

contractcourt/chain_watcher.go

@@ -281,7 +281,7 @@ func newChainWatcher(cfg chainWatcherConfig) (*chainWatcher, error) {
 	}
 
 	// Get the channel opening block height.
-	heightHint := deriveHeightHint(chanState)
+	heightHint := chanState.DeriveHeightHint()
 
 	// We'll register for a notification to be dispatched if the funding
 	// output is spent.
@@ -1328,34 +1328,6 @@ func deriveFundingPkScript(chanState *channeldb.OpenChannel) ([]byte, error) {
 	return fundingPkScript, nil
 }
 
-// deriveHeightHint derives the block height for the channel opening.
-func deriveHeightHint(chanState *channeldb.OpenChannel) uint32 {
-	// As a height hint, we'll try to use the opening height, but if the
-	// channel isn't yet open, then we'll use the height it was broadcast
-	// at. This may be an unconfirmed zero-conf channel.
-	heightHint := chanState.ShortChanID().BlockHeight
-	if heightHint == 0 {
-		heightHint = chanState.BroadcastHeight()
-	}
-
-	// Since no zero-conf state is stored in a channel backup, the below
-	// logic will not be triggered for restored, zero-conf channels. Set
-	// the height hint for zero-conf channels.
-	if chanState.IsZeroConf() {
-		if chanState.ZeroConfConfirmed() {
-			// If the zero-conf channel is confirmed, we'll use the
-			// confirmed SCID's block height.
-			heightHint = chanState.ZeroConfRealScid().BlockHeight
-		} else {
-			// The zero-conf channel is unconfirmed. We'll need to
-			// use the FundingBroadcastHeight.
-			heightHint = chanState.BroadcastHeight()
-		}
-	}
-
-	return heightHint
-}
-
 // handleCommitSpend takes a spending tx of the funding output and handles the
 // channel close based on the closure type.
 func (c *chainWatcher) handleCommitSpend(

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

@@ -93,6 +93,27 @@
 
 # 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/9610). 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`.
+
+With this new co-op close type, users can issue multiple `lncli closechannnel`
+commands with increasing fee rates to use RBF to bump an existing signed co-op
+close transaction.
+
 * [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
@@ -106,10 +127,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
@@ -432,6 +450,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/default_sets.go

@@ -103,4 +103,8 @@ var defaultSetDesc = setDesc{
 	lnwire.ExperimentalEndorsementOptional: {
 		SetNodeAnn: {}, // N
 	},
+	lnwire.RbfCoopCloseOptionalStaging: {
+		SetInit:    {}, // I
+		SetNodeAnn: {}, // N
+	},
 }

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

htlcswitch/interfaces.go

@@ -196,7 +196,7 @@ type FlushHookID uint64
 
 // LinkDirection is used to query and change any link state on a per-direction
 // basis.
-type LinkDirection bool
+type LinkDirection = bool
 
 const (
 	// Incoming is the direction from the remote peer to our node.

htlcswitch/switch.go

@@ -2,6 +2,7 @@ package htlcswitch
 
 import (
 	"bytes"
+	"context"
 	"errors"
 	"fmt"
 	"math/rand"
@@ -125,6 +126,9 @@ type ChanClose struct {
 
 	// Err is used by request creator to receive request execution error.
 	Err chan error
+
+	// Ctx is a context linked to the lifetime of the caller.
+	Ctx context.Context //nolint:containedctx
 }
 
 // Config defines the configuration for the service. ALL elements within the
@@ -1413,7 +1417,7 @@ func (s *Switch) teardownCircuit(pkt *htlcPacket) error {
 // targetFeePerKw parameter should be the ideal fee-per-kw that will be used as
 // a starting point for close negotiation. The deliveryScript parameter is an
 // optional parameter which sets a user specified script to close out to.
-func (s *Switch) CloseLink(chanPoint *wire.OutPoint,
+func (s *Switch) CloseLink(ctx context.Context, chanPoint *wire.OutPoint,
 	closeType contractcourt.ChannelCloseType,
 	targetFeePerKw, maxFee chainfee.SatPerKWeight,
 	deliveryScript lnwire.DeliveryAddress) (chan interface{}, chan error) {
@@ -1427,9 +1431,10 @@ func (s *Switch) CloseLink(chanPoint *wire.OutPoint,
 		ChanPoint:      chanPoint,
 		Updates:        updateChan,
 		TargetFeePerKw: targetFeePerKw,
-		MaxFee:         maxFee,
 		DeliveryScript: deliveryScript,
 		Err:            errChan,
+		MaxFee:         maxFee,
+		Ctx:            ctx,
 	}
 
 	select {

input/fuzz_script_is_op_return_test.go

@@ -0,0 +1,51 @@
+package input
+
+import (
+	"testing"
+
+	"github.com/btcsuite/btcd/chaincfg"
+	"github.com/btcsuite/btcd/txscript"
+	"github.com/stretchr/testify/require"
+)
+
+// FuzzScriptIsOpReturn fuzzes the ScriptIsOpReturn function.
+func FuzzScriptIsOpReturn(f *testing.F) {
+	// Seed the corpus with some representative inputs.
+	f.Add([]byte{})
+	f.Add([]byte{txscript.OP_RETURN})
+
+	// Use our canonical ScriptBuilder to produce a valid OP_RETURN script.
+	builder := txscript.NewScriptBuilder()
+	builder.AddOp(txscript.OP_RETURN)
+	builder.AddData([]byte("valid data"))
+	script, err := builder.Script()
+	require.NoError(f, err)
+	f.Add(script)
+
+	// An example of a script that does not start with OP_RETURN.
+	f.Add([]byte{txscript.OP_DUP, txscript.OP_RETURN})
+
+	f.Fuzz(func(t *testing.T, pkScript []byte) {
+		result := ScriptIsOpReturn(pkScript)
+
+		var expected bool
+		if len(script) == 0 || script[0] != txscript.OP_RETURN {
+			expected = false
+		} else {
+			scriptClass, _, _, err := txscript.ExtractPkScriptAddrs(
+				pkScript, &chaincfg.MainNetParams,
+			)
+			if err != nil {
+				t.Fatalf("unable to extract pk script "+
+					"addresses: %v", err)
+			}
+
+			expected = scriptClass == txscript.NullDataTy
+		}
+
+		if result != expected {
+			t.Fatalf("for script %x, expected ScriptIsOpReturn=%v;"+
+				" got %v", script, expected, result)
+		}
+	})
+}

input/script_utils.go

@@ -3239,3 +3239,36 @@ func ComputeCommitmentPoint(commitSecret []byte) *btcec.PublicKey {
 	_, pubKey := btcec.PrivKeyFromBytes(commitSecret)
 	return pubKey
 }
+
+// ScriptIsOpReturn returns true if the passed script is an OP_RETURN script.
+//
+// Lifted from the txscript package:
+// https://github.com/btcsuite/btcd/blob/cc26860b40265e1332cca8748c5dbaf3c81cc094/txscript/standard.go#L493-L526.
+//
+//nolint:ll
+func ScriptIsOpReturn(script []byte) bool {
+	// A null script is of the form:
+	//  OP_RETURN <optional data>
+	//
+	// Thus, it can either be a single OP_RETURN or an OP_RETURN followed by
+	// a data push up to MaxDataCarrierSize bytes.
+
+	// The script can't possibly be a null data script if it doesn't start
+	// with OP_RETURN.  Fail fast to avoid more work below.
+	if len(script) < 1 || script[0] != txscript.OP_RETURN {
+		return false
+	}
+
+	// Single OP_RETURN.
+	if len(script) == 1 {
+		return true
+	}
+
+	// OP_RETURN followed by data push up to MaxDataCarrierSize bytes.
+	tokenizer := txscript.MakeScriptTokenizer(0, script[1:])
+
+	return tokenizer.Next() && tokenizer.Done() &&
+		(txscript.IsSmallInt(tokenizer.Opcode()) ||
+			tokenizer.Opcode() <= txscript.OP_PUSHDATA4) &&
+		len(tokenizer.Data()) <= txscript.MaxDataCarrierSize
+}

itest/list_on_test.go

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