multi: add control over maximum acceptable local CSV delay

cmd/lncli/cmd_open_channel.go

@@ -135,6 +135,13 @@ var openChannelCommand = cli.Command{
 				"not set, we will scale the value according to the " +
 				"channel size",
 		},
+		cli.Uint64Flag{
+			Name: "max_local_csv",
+			Usage: "(optional) the maximum number of blocks that " +
+				"we will allow the remote peer to require we " +
+				"wait before accessing our funds in the case " +
+				"of a unilateral close.",
+		},
 		cli.Uint64Flag{
 			Name: "min_confs",
 			Usage: "(optional) the minimum number of confirmations " +
@@ -207,6 +214,7 @@ func openChannel(ctx *cli.Context) error {
 		SpendUnconfirmed:           minConfs == 0,
 		CloseAddress:               ctx.String("close_address"),
 		RemoteMaxValueInFlightMsat: ctx.Uint64("remote_max_value_in_flight_msat"),
+		MaxLocalCsv:                uint32(ctx.Uint64("max_local_csv")),
 	}
 
 	switch {

config.go

@@ -112,6 +112,11 @@ const (
 	// concurrent HTLCs the remote party may add to commitment transactions.
 	// This value can be overridden with --default-remote-max-htlcs.
 	defaultRemoteMaxHtlcs = 483
+
+	// defaultMaxLocalCSVDelay is the maximum delay we accept on our
+	// commitment output.
+	// TODO(halseth): find a more scientific choice of value.
+	defaultMaxLocalCSVDelay = 10000
 )
 
 var (
@@ -344,6 +349,7 @@ func DefaultConfig() Config {
 			BaseFee:       chainreg.DefaultBitcoinBaseFeeMSat,
 			FeeRate:       chainreg.DefaultBitcoinFeeRate,
 			TimeLockDelta: chainreg.DefaultBitcoinTimeLockDelta,
+			MaxLocalDelay: defaultMaxLocalCSVDelay,
 			Node:          "btcd",
 		},
 		BtcdMode: &lncfg.Btcd{
@@ -362,6 +368,7 @@ func DefaultConfig() Config {
 			BaseFee:       chainreg.DefaultLitecoinBaseFeeMSat,
 			FeeRate:       chainreg.DefaultLitecoinFeeRate,
 			TimeLockDelta: chainreg.DefaultLitecoinTimeLockDelta,
+			MaxLocalDelay: defaultMaxLocalCSVDelay,
 			Node:          "ltcd",
 		},
 		LtcdMode: &lncfg.Btcd{
@@ -822,10 +829,11 @@ func ValidateConfig(cfg Config, usageMessage string) (*Config, error) {
 			"litecoin.active must be set to 1 (true)", funcName)
 
 	case cfg.Litecoin.Active:
-		if cfg.Litecoin.TimeLockDelta < minTimeLockDelta {
-			return nil, fmt.Errorf("timelockdelta must be at least %v",
-				minTimeLockDelta)
+		err := cfg.Litecoin.Validate(minTimeLockDelta, minLtcRemoteDelay)
+		if err != nil {
+			return nil, err
 		}
+
 		// Multiple networks can't be selected simultaneously.  Count
 		// number of network flags passed; assign active network params
 		// while we're at it.
@@ -946,9 +954,9 @@ func ValidateConfig(cfg Config, usageMessage string) (*Config, error) {
 			return nil, err
 		}
 
-		if cfg.Bitcoin.TimeLockDelta < minTimeLockDelta {
-			return nil, fmt.Errorf("timelockdelta must be at least %v",
-				minTimeLockDelta)
+		err := cfg.Bitcoin.Validate(minTimeLockDelta, minBtcRemoteDelay)
+		if err != nil {
+			return nil, err
 		}
 
 		switch cfg.Bitcoin.Node {

fundingmanager.go

@@ -132,6 +132,9 @@ type reservationWithCtx struct {
 	remoteMaxValue lnwire.MilliSatoshi
 	remoteMaxHtlcs uint16
 
+	// maxLocalCsv is the maximum csv we will accept from the remote.
+	maxLocalCsv uint16
+
 	updateMtx   sync.RWMutex
 	lastUpdated time.Time
 
@@ -339,6 +342,10 @@ type fundingConfig struct {
 	// incoming channels having a non-zero push amount.
 	RejectPush bool
 
+	// MaxLocalCSVDelay is the maximum csv delay we will allow for our
+	// commit output. Channels that exceed this value will be failed.
+	MaxLocalCSVDelay uint16
+
 	// NotifyOpenChannelEvent informs the ChannelNotifier when channels
 	// transition from pending open to open.
 	NotifyOpenChannelEvent func(wire.OutPoint)
@@ -1342,7 +1349,9 @@ func (f *fundingManager) handleFundingOpen(peer lnpeer.Peer,
 		MaxAcceptedHtlcs: msg.MaxAcceptedHTLCs,
 		CsvDelay:         msg.CsvDelay,
 	}
-	err = reservation.CommitConstraints(channelConstraints)
+	err = reservation.CommitConstraints(
+		channelConstraints, f.cfg.MaxLocalCSVDelay,
+	)
 	if err != nil {
 		fndgLog.Errorf("Unacceptable channel constraints: %v", err)
 		f.failFundingFlow(peer, msg.PendingChannelID, err)
@@ -1398,6 +1407,7 @@ func (f *fundingManager) handleFundingOpen(peer lnpeer.Peer,
 		remoteMinHtlc:  minHtlc,
 		remoteMaxValue: remoteMaxValue,
 		remoteMaxHtlcs: maxHtlcs,
+		maxLocalCsv:    f.cfg.MaxLocalCSVDelay,
 		err:            make(chan error, 1),
 		peer:           peer,
 	}
@@ -1525,7 +1535,9 @@ func (f *fundingManager) handleFundingAccept(peer lnpeer.Peer,
 		MaxAcceptedHtlcs: msg.MaxAcceptedHTLCs,
 		CsvDelay:         msg.CsvDelay,
 	}
-	err = resCtx.reservation.CommitConstraints(channelConstraints)
+	err = resCtx.reservation.CommitConstraints(
+		channelConstraints, resCtx.maxLocalCsv,
+	)
 	if err != nil {
 		fndgLog.Warnf("Unacceptable channel constraints: %v", err)
 		f.failFundingFlow(peer, msg.PendingChannelID, err)
@@ -3056,8 +3068,15 @@ func (f *fundingManager) handleInitFundingMsg(msg *initFundingMsg) {
 		remoteCsvDelay = msg.remoteCsvDelay
 		maxValue       = msg.maxValueInFlight
 		maxHtlcs       = msg.maxHtlcs
+		maxCSV         = msg.maxLocalCsv
 	)
 
+	// If no maximum CSV delay was set for this channel, we use our default
+	// value.
+	if maxCSV == 0 {
+		maxCSV = f.cfg.MaxLocalCSVDelay
+	}
+
 	// We'll determine our dust limit depending on which chain is active.
 	var ourDustLimit btcutil.Amount
 	switch f.cfg.RegisteredChains.PrimaryChain() {
@@ -3213,6 +3232,7 @@ func (f *fundingManager) handleInitFundingMsg(msg *initFundingMsg) {
 		remoteMinHtlc:  minHtlcIn,
 		remoteMaxValue: maxValue,
 		remoteMaxHtlcs: maxHtlcs,
+		maxLocalCsv:    maxCSV,
 		reservation:    reservation,
 		peer:           msg.peer,
 		updates:        msg.updates,

fundingmanager_test.go

@@ -37,6 +37,7 @@ import (
 	"github.com/lightningnetwork/lnd/lnwallet"
 	"github.com/lightningnetwork/lnd/lnwallet/chainfee"
 	"github.com/lightningnetwork/lnd/lnwire"
+	"github.com/stretchr/testify/require"
 )
 
 const (
@@ -433,6 +434,7 @@ func createTestFundingManager(t *testing.T, privKey *btcec.PrivateKey,
 		ZombieSweeperInterval:         1 * time.Hour,
 		ReservationTimeout:            1 * time.Nanosecond,
 		MaxChanSize:                   MaxFundingAmount,
+		MaxLocalCSVDelay:              defaultMaxLocalCSVDelay,
 		MaxPendingChannels:            lncfg.DefaultMaxPendingChannels,
 		NotifyOpenChannelEvent:        evt.NotifyOpenChannelEvent,
 		OpenChannelPredicate:          chainedAcceptor,
@@ -1273,6 +1275,159 @@ func TestFundingManagerNormalWorkflow(t *testing.T) {
 	assertNoChannelState(t, alice, bob, fundingOutPoint)
 }
 
+// TestFundingManagerRejectCSV tests checking of local CSV values against our
+// local CSV limit for incoming and outgoing channels.
+func TestFundingManagerRejectCSV(t *testing.T) {
+	t.Run("csv too high", func(t *testing.T) {
+		testLocalCSVLimit(t, 400, 500)
+	})
+	t.Run("csv within limit", func(t *testing.T) {
+		testLocalCSVLimit(t, 600, 500)
+	})
+}
+
+// testLocalCSVLimit creates two funding managers, alice and bob, where alice
+// has a limit on her maximum local CSV and bob sets his required CSV for alice.
+// We test an incoming and outgoing channel, ensuring that alice accepts csvs
+// below her maximum, and rejects those above it.
+func testLocalCSVLimit(t *testing.T, aliceMaxCSV, bobRequiredCSV uint16) {
+	t.Parallel()
+
+	alice, bob := setupFundingManagers(t)
+	defer tearDownFundingManagers(t, alice, bob)
+
+	// Set a maximum local delay in alice's config to aliceMaxCSV and overwrite
+	// bob's required remote delay function to return bobRequiredCSV.
+	alice.fundingMgr.cfg.MaxLocalCSVDelay = aliceMaxCSV
+	bob.fundingMgr.cfg.RequiredRemoteDelay = func(_ btcutil.Amount) uint16 {
+		return bobRequiredCSV
+	}
+
+	// For convenience, we bump our max pending channels to 2 so that we
+	// can test incoming and outgoing channels without needing to step
+	// through the full funding process.
+	alice.fundingMgr.cfg.MaxPendingChannels = 2
+	bob.fundingMgr.cfg.MaxPendingChannels = 2
+
+	// If our maximum is less than the value bob sets, we expect this test
+	// to fail.
+	expectFail := aliceMaxCSV < bobRequiredCSV
+
+	// First, we will initiate an outgoing channel from Alice -> Bob.
+	errChan := make(chan error, 1)
+	updateChan := make(chan *lnrpc.OpenStatusUpdate)
+	initReq := &openChanReq{
+		targetPubkey:    bob.privKey.PubKey(),
+		chainHash:       *fundingNetParams.GenesisHash,
+		localFundingAmt: 200000,
+		fundingFeePerKw: 1000,
+		updates:         updateChan,
+		err:             errChan,
+	}
+
+	// Alice should have sent the OpenChannel message to Bob.
+	alice.fundingMgr.initFundingWorkflow(bob, initReq)
+	var aliceMsg lnwire.Message
+	select {
+	case aliceMsg = <-alice.msgChan:
+
+	case err := <-initReq.err:
+		t.Fatalf("error init funding workflow: %v", err)
+
+	case <-time.After(time.Second * 5):
+		t.Fatalf("alice did not send OpenChannel message")
+	}
+
+	openChannelReq, ok := aliceMsg.(*lnwire.OpenChannel)
+	require.True(t, ok)
+
+	// Let Bob handle the init message.
+	bob.fundingMgr.ProcessFundingMsg(openChannelReq, alice)
+
+	// Bob should answer with an AcceptChannel message.
+	acceptChannelResponse := assertFundingMsgSent(
+		t, bob.msgChan, "AcceptChannel",
+	).(*lnwire.AcceptChannel)
+
+	// They now should both have pending reservations for this channel
+	// active.
+	assertNumPendingReservations(t, alice, bobPubKey, 1)
+	assertNumPendingReservations(t, bob, alicePubKey, 1)
+
+	// Forward the response to Alice.
+	alice.fundingMgr.ProcessFundingMsg(acceptChannelResponse, bob)
+
+	// At this point, Alice has received an AcceptChannel message from
+	// bob with the CSV value that he has set for her, and has to evaluate
+	// whether she wants to accept this channel. If we get an error, we
+	// assert that we expected the channel to fail, otherwise we assert that
+	// she proceeded with the channel open as usual.
+	select {
+	case err := <-errChan:
+		require.Error(t, err)
+		require.True(t, expectFail)
+
+	case msg := <-alice.msgChan:
+		_, ok := msg.(*lnwire.FundingCreated)
+		require.True(t, ok)
+		require.False(t, expectFail)
+
+	case <-time.After(time.Second):
+		t.Fatal("funding flow was not failed")
+	}
+
+	// We do not need to complete the rest of the funding flow (it is
+	// covered in other tests). So now we test that Alice will appropriately
+	// handle incoming channels, opening a channel from Bob->Alice.
+	errChan = make(chan error, 1)
+	updateChan = make(chan *lnrpc.OpenStatusUpdate)
+	initReq = &openChanReq{
+		targetPubkey:    alice.privKey.PubKey(),
+		chainHash:       *fundingNetParams.GenesisHash,
+		localFundingAmt: 200000,
+		fundingFeePerKw: 1000,
+		updates:         updateChan,
+		err:             errChan,
+	}
+
+	bob.fundingMgr.initFundingWorkflow(alice, initReq)
+
+	// Bob should have sent the OpenChannel message to Alice.
+	var bobMsg lnwire.Message
+	select {
+	case bobMsg = <-bob.msgChan:
+
+	case err := <-initReq.err:
+		t.Fatalf("bob OpenChannel message failed: %v", err)
+
+	case <-time.After(time.Second * 5):
+		t.Fatalf("bob did not send OpenChannel message")
+	}
+
+	openChannelReq, ok = bobMsg.(*lnwire.OpenChannel)
+	require.True(t, ok)
+
+	// Let Alice handle the init message.
+	alice.fundingMgr.ProcessFundingMsg(openChannelReq, bob)
+
+	// We expect a error message from Alice if we're expecting the channel
+	// to fail, otherwise we expect her to proceed with the channel as
+	// usual.
+	select {
+	case msg := <-alice.msgChan:
+		var ok bool
+		if expectFail {
+			_, ok = msg.(*lnwire.Error)
+		} else {
+			_, ok = msg.(*lnwire.AcceptChannel)
+		}
+		require.True(t, ok)
+
+	case <-time.After(time.Second * 5):
+		t.Fatal("funding flow was not failed")
+	}
+}
+
 func TestFundingManagerRestartBehavior(t *testing.T) {
 	t.Parallel()
 

lncfg/chain.go

@@ -1,6 +1,10 @@
 package lncfg
 
-import "github.com/lightningnetwork/lnd/lnwire"
+import (
+	"fmt"
+
+	"github.com/lightningnetwork/lnd/lnwire"
+)
 
 // Chain holds the configuration options for the daemon's chain settings.
 type Chain struct {
@@ -16,9 +20,28 @@ type Chain struct {
 
 	DefaultNumChanConfs int                 `long:"defaultchanconfs" description:"The default number of confirmations a channel must have before it's considered open. If this is not set, we will scale the value according to the channel size."`
 	DefaultRemoteDelay  int                 `long:"defaultremotedelay" description:"The default number of blocks we will require our channel counterparty to wait before accessing its funds in case of unilateral close. If this is not set, we will scale the value according to the channel size."`
+	MaxLocalDelay       uint16              `long:"maxlocaldelay" description:"The maximum blocks we will allow our funds to be timelocked before accessing its funds in case of unilateral close. If a peer proposes a value greater than this, we will reject the channel."`
 	MinHTLCIn           lnwire.MilliSatoshi `long:"minhtlc" description:"The smallest HTLC we are willing to accept on our channels, in millisatoshi"`
 	MinHTLCOut          lnwire.MilliSatoshi `long:"minhtlcout" description:"The smallest HTLC we are willing to send out on our channels, in millisatoshi"`
 	BaseFee             lnwire.MilliSatoshi `long:"basefee" description:"The base fee in millisatoshi we will charge for forwarding payments on our channels"`
 	FeeRate             lnwire.MilliSatoshi `long:"feerate" description:"The fee rate used when forwarding payments on our channels. The total fee charged is basefee + (amount * feerate / 1000000), where amount is the forwarded amount."`
 	TimeLockDelta       uint32              `long:"timelockdelta" description:"The CLTV delta we will subtract from a forwarded HTLC's timelock value"`
 }
+
+// Validate performs validation on our chain config.
+func (c *Chain) Validate(minTimeLockDelta uint32, minDelay uint16) error {
+	if c.TimeLockDelta < minTimeLockDelta {
+		return fmt.Errorf("timelockdelta must be at least %v",
+			minTimeLockDelta)
+	}
+
+	// Check that our max local delay isn't set below some reasonable
+	// minimum value. We do this to prevent setting an unreasonably low
+	// delay, which would mean that the node would accept no channels.
+	if c.MaxLocalDelay < minDelay {
+		return fmt.Errorf("MaxLocalDelay must be at least: %v",
+			minDelay)
+	}
+
+	return nil
+}