Merge pull request #4527 from cfromknecht/configurable-remote-max-htlcs

fundingmanager: configurable remote max htlcs

config.go

@@ -27,6 +27,7 @@ import (
 	"github.com/lightningnetwork/lnd/discovery"
 	"github.com/lightningnetwork/lnd/htlcswitch"
 	"github.com/lightningnetwork/lnd/htlcswitch/hodl"
+	"github.com/lightningnetwork/lnd/input"
 	"github.com/lightningnetwork/lnd/lncfg"
 	"github.com/lightningnetwork/lnd/lnrpc/routerrpc"
 	"github.com/lightningnetwork/lnd/lnrpc/signrpc"
@@ -101,6 +102,11 @@ const (
 	defaultDiskTimeout  = time.Second * 5
 	defaultDiskBackoff  = time.Minute
 	defaultDiskAttempts = 2
+
+	// defaultRemoteMaxHtlcs specifies the default limit for maximum
+	// concurrent HTLCs the remote party may add to commitment transactions.
+	// This value can be overridden with --default-remote-max-htlcs.
+	defaultRemoteMaxHtlcs = 483
 )
 
 var (
@@ -235,6 +241,8 @@ type Config struct {
 	Color                         string        `long:"color" description:"The color of the node in hex format (i.e. '#3399FF'). Used to customize node appearance in intelligence services"`
 	MinChanSize                   int64         `long:"minchansize" description:"The smallest channel size (in satoshis) that we should accept. Incoming channels smaller than this will be rejected"`
 
+	DefaultRemoteMaxHtlcs uint16 `long:"default-remote-max-htlcs" description:"The default max_htlc applied when opening or accepting channels. This value limits the number of concurrent HTLCs that the remote party can add to the commitment. The maximum possible value is 483."`
+
 	NumGraphSyncPeers      int           `long:"numgraphsyncpeers" description:"The number of peers that we should receive new graph updates from. This option can be tuned to save bandwidth for light clients or routing nodes."`
 	HistoricalSyncInterval time.Duration `long:"historicalsyncinterval" description:"The polling interval between historical graph sync attempts. Each historical graph sync attempt ensures we reconcile with the remote peer's graph from the genesis block."`
 
@@ -379,6 +387,7 @@ func DefaultConfig() Config {
 		Alias:                         defaultAlias,
 		Color:                         defaultColor,
 		MinChanSize:                   int64(minChanFundingSize),
+		DefaultRemoteMaxHtlcs:         defaultRemoteMaxHtlcs,
 		NumGraphSyncPeers:             defaultMinPeers,
 		HistoricalSyncInterval:        discovery.DefaultHistoricalSyncInterval,
 		Tor: &lncfg.Tor{
@@ -1152,6 +1161,15 @@ func ValidateConfig(cfg Config, usageMessage string) (*Config, error) {
 		cfg.DB.Bolt.SyncFreelist = cfg.SyncFreelist
 	}
 
+	// Ensure that the user hasn't chosen a remote-max-htlc value greater
+	// than the protocol maximum.
+	maxRemoteHtlcs := uint16(input.MaxHTLCNumber / 2)
+	if cfg.DefaultRemoteMaxHtlcs > maxRemoteHtlcs {
+		return nil, fmt.Errorf("default-remote-max-htlcs (%v) must be "+
+			"less than %v", cfg.DefaultRemoteMaxHtlcs,
+			maxRemoteHtlcs)
+	}
+
 	// Validate the subconfigs for workers, caches, and the tower client.
 	err = lncfg.Validate(
 		cfg.Workers,

fundingmanager.go

@@ -124,6 +124,7 @@ type reservationWithCtx struct {
 	remoteCsvDelay uint16
 	remoteMinHtlc  lnwire.MilliSatoshi
 	remoteMaxValue lnwire.MilliSatoshi
+	remoteMaxHtlcs uint16
 
 	updateMtx   sync.RWMutex
 	lastUpdated time.Time
@@ -1411,6 +1412,7 @@ func (f *fundingManager) handleFundingOpen(fmsg *fundingOpenMsg) {
 		remoteCsvDelay: remoteCsvDelay,
 		remoteMinHtlc:  minHtlc,
 		remoteMaxValue: remoteMaxValue,
+		remoteMaxHtlcs: maxHtlcs,
 		err:            make(chan error, 1),
 		peer:           fmsg.peer,
 	}
@@ -1560,7 +1562,6 @@ func (f *fundingManager) handleFundingAccept(fmsg *fundingAcceptMsg) {
 	// here so we can properly commit their accepted constraints to the
 	// reservation.
 	chanReserve := f.cfg.RequiredRemoteChanReserve(resCtx.chanAmt, msg.DustLimit)
-	maxHtlcs := f.cfg.RequiredRemoteMaxHTLCs(resCtx.chanAmt)
 
 	// The remote node has responded with their portion of the channel
 	// contribution. At this point, we can process their contribution which
@@ -1574,7 +1575,7 @@ func (f *fundingManager) handleFundingAccept(fmsg *fundingAcceptMsg) {
 				MaxPendingAmount: resCtx.remoteMaxValue,
 				ChanReserve:      chanReserve,
 				MinHTLC:          resCtx.remoteMinHtlc,
-				MaxAcceptedHtlcs: maxHtlcs,
+				MaxAcceptedHtlcs: resCtx.remoteMaxHtlcs,
 				CsvDelay:         resCtx.remoteCsvDelay,
 			},
 			MultiSigKey: keychain.KeyDescriptor{
@@ -3110,6 +3111,7 @@ func (f *fundingManager) handleInitFundingMsg(msg *initFundingMsg) {
 		minHtlcIn      = msg.minHtlcIn
 		remoteCsvDelay = msg.remoteCsvDelay
 		maxValue       = msg.maxValueInFlight
+		maxHtlcs       = msg.maxHtlcs
 	)
 
 	// We'll determine our dust limit depending on which chain is active.
@@ -3248,6 +3250,10 @@ func (f *fundingManager) handleInitFundingMsg(msg *initFundingMsg) {
 		maxValue = f.cfg.RequiredRemoteMaxValue(capacity)
 	}
 
+	if maxHtlcs == 0 {
+		maxHtlcs = f.cfg.RequiredRemoteMaxHTLCs(capacity)
+	}
+
 	// If a pending channel map for this peer isn't already created, then
 	// we create one, ultimately allowing us to track this pending
 	// reservation within the target peer.
@@ -3262,6 +3268,7 @@ func (f *fundingManager) handleInitFundingMsg(msg *initFundingMsg) {
 		remoteCsvDelay: remoteCsvDelay,
 		remoteMinHtlc:  minHtlcIn,
 		remoteMaxValue: maxValue,
+		remoteMaxHtlcs: maxHtlcs,
 		reservation:    reservation,
 		peer:           msg.peer,
 		updates:        msg.updates,
@@ -3281,7 +3288,6 @@ func (f *fundingManager) handleInitFundingMsg(msg *initFundingMsg) {
 	// policy to determine of required commitment constraints for the
 	// remote party.
 	chanReserve := f.cfg.RequiredRemoteChanReserve(capacity, ourDustLimit)
-	maxHtlcs := f.cfg.RequiredRemoteMaxHTLCs(capacity)
 
 	fndgLog.Infof("Starting funding workflow with %v for pending_id(%x), "+
 		"committype=%v", msg.peer.Address(), chanID, commitType)

lnrpc/rpc.proto

@@ -1685,6 +1685,12 @@ message OpenChannelRequest {
     the channel. It only applies to the remote party.
     */
     uint64 remote_max_value_in_flight_msat = 15;
+
+    /*
+    The maximum number of concurrent HTLCs we will allow the remote party to add
+    to the commitment transaction.
+    */
+    uint32 remote_max_htlcs = 16;
 }
 message OpenStatusUpdate {
     oneof update {

lnrpc/rpc.swagger.json

@@ -4494,6 +4494,11 @@
           "type": "string",
           "format": "uint64",
           "description": "The maximum amount of coins in millisatoshi that can be pending within\nthe channel. It only applies to the remote party."
+        },
+        "remote_max_htlcs": {
+          "type": "integer",
+          "format": "int64",
+          "description": "The maximum number of concurrent HTLCs we will allow the remote party to add\nto the commitment transaction."
         }
       }
     },

lntest/harness.go

@@ -855,6 +855,11 @@ type OpenChannelParams struct {
 	// MinHtlc is the htlc_minimum_msat value set when opening the channel.
 	MinHtlc lnwire.MilliSatoshi
 
+	// RemoteMaxHtlcs is the remote_max_htlcs value set when opening the
+	// channel, restricting the number of concurrent HTLCs the remote party
+	// can add to a commitment.
+	RemoteMaxHtlcs uint16
+
 	// FundingShim is an optional funding shim that the caller can specify
 	// in order to modify the channel funding workflow.
 	FundingShim *lnrpc.FundingShim
@@ -874,7 +879,7 @@ func (n *NetworkHarness) OpenChannel(ctx context.Context,
 	// prevents any funding workflows from being kicked off if the chain
 	// isn't yet synced.
 	if err := srcNode.WaitForBlockchainSync(ctx); err != nil {
-		return nil, fmt.Errorf("enable to sync srcNode chain: %v", err)
+		return nil, fmt.Errorf("unable to sync srcNode chain: %v", err)
 	}
 	if err := destNode.WaitForBlockchainSync(ctx); err != nil {
 		return nil, fmt.Errorf("unable to sync destNode chain: %v", err)
@@ -893,6 +898,7 @@ func (n *NetworkHarness) OpenChannel(ctx context.Context,
 		MinConfs:           minConfs,
 		SpendUnconfirmed:   p.SpendUnconfirmed,
 		MinHtlcMsat:        int64(p.MinHtlc),
+		RemoteMaxHtlcs:     uint32(p.RemoteMaxHtlcs),
 		FundingShim:        p.FundingShim,
 	}
 

lntest/itest/lnd_test.go

@@ -4798,6 +4798,8 @@ func testSphinxReplayPersistence(net *lntest.NetworkHarness, t *harnessTest) {
 func assertChannelConstraintsEqual(
 	t *harnessTest, want, got *lnrpc.ChannelConstraints) {
 
+	t.t.Helper()
+
 	if want.CsvDelay != got.CsvDelay {
 		t.Fatalf("CsvDelay mismatched, want: %v, got: %v",
 			want.CsvDelay, got.CsvDelay,
@@ -4842,6 +4844,9 @@ func assertChannelConstraintsEqual(
 func testListChannels(net *lntest.NetworkHarness, t *harnessTest) {
 	ctxb := context.Background()
 
+	const aliceRemoteMaxHtlcs = 50
+	const bobRemoteMaxHtlcs = 100
+
 	// Create two fresh nodes and open a channel between them.
 	alice, err := net.NewNode("Alice", nil)
 	if err != nil {
@@ -4849,7 +4854,9 @@ func testListChannels(net *lntest.NetworkHarness, t *harnessTest) {
 	}
 	defer shutdownAndAssert(net, t, alice)
 
-	bob, err := net.NewNode("Bob", nil)
+	bob, err := net.NewNode("Bob", []string{
+		fmt.Sprintf("--default-remote-max-htlcs=%v", bobRemoteMaxHtlcs),
+	})
 	if err != nil {
 		t.Fatalf("unable to create new node: %v", err)
 	}
@@ -4878,8 +4885,9 @@ func testListChannels(net *lntest.NetworkHarness, t *harnessTest) {
 	chanPoint := openChannelAndAssert(
 		ctxt, t, net, alice, bob,
 		lntest.OpenChannelParams{
-			Amt:     chanAmt,
+			Amt:            chanAmt,
-			MinHtlc: customizedMinHtlc,
+			MinHtlc:        customizedMinHtlc,
+			RemoteMaxHtlcs: aliceRemoteMaxHtlcs,
 		},
 	)
 
@@ -4925,10 +4933,10 @@ func testListChannels(net *lntest.NetworkHarness, t *harnessTest) {
 		DustLimitSat:      uint64(lnwallet.DefaultDustLimit()),
 		MaxPendingAmtMsat: 99000000,
 		MinHtlcMsat:       1,
-		MaxAcceptedHtlcs:  input.MaxHTLCNumber / 2,
+		MaxAcceptedHtlcs:  bobRemoteMaxHtlcs,
 	}
 	assertChannelConstraintsEqual(
-		t, aliceChannel.LocalConstraints, defaultConstraints,
+		t, defaultConstraints, aliceChannel.LocalConstraints,
 	)
 
 	// customizedConstraints is a ChannelConstraints with customized values.
@@ -4941,10 +4949,10 @@ func testListChannels(net *lntest.NetworkHarness, t *harnessTest) {
 		DustLimitSat:      uint64(lnwallet.DefaultDustLimit()),
 		MaxPendingAmtMsat: 99000000,
 		MinHtlcMsat:       customizedMinHtlc,
-		MaxAcceptedHtlcs:  input.MaxHTLCNumber / 2,
+		MaxAcceptedHtlcs:  aliceRemoteMaxHtlcs,
 	}
 	assertChannelConstraintsEqual(
-		t, aliceChannel.RemoteConstraints, customizedConstraints,
+		t, customizedConstraints, aliceChannel.RemoteConstraints,
 	)
 
 	// Get the ListChannel response for Bob.
@@ -14450,7 +14458,10 @@ func TestLightningNetworkDaemon(t *testing.T) {
 	// initialization of the network. args - list of lnd arguments,
 	// example: "--debuglevel=debug"
 	// TODO(roasbeef): create master balanced channel with all the monies?
-	if err = lndHarness.SetUp(nil); err != nil {
+	aliceBobArgs := []string{
+		"--default-remote-max-htlcs=483",
+	}
+	if err = lndHarness.SetUp(aliceBobArgs); err != nil {
 		ht.Fatalf("unable to set up test lightning network: %v", err)
 	}
 

rpcserver.go

@@ -1743,6 +1743,7 @@ func (r *rpcServer) parseOpenChannelReq(in *lnrpc.OpenChannelRequest,
 	minHtlcIn := lnwire.MilliSatoshi(in.MinHtlcMsat)
 	remoteCsvDelay := uint16(in.RemoteCsvDelay)
 	maxValue := lnwire.MilliSatoshi(in.RemoteMaxValueInFlightMsat)
+	maxHtlcs := uint16(in.RemoteMaxHtlcs)
 
 	globalFeatureSet := r.server.featureMgr.Get(feature.SetNodeAnn)
 
@@ -1775,6 +1776,13 @@ func (r *rpcServer) parseOpenChannelReq(in *lnrpc.OpenChannelRequest,
 			"channel size is: %v SAT", int64(minChanFundingSize))
 	}
 
+	// Prevent users from submitting a max-htlc value that would exceed the
+	// protocol maximum.
+	if maxHtlcs > input.MaxHTLCNumber/2 {
+		return nil, fmt.Errorf("remote-max-htlcs (%v) cannot be "+
+			"greater than %v", maxHtlcs, input.MaxHTLCNumber/2)
+	}
+
 	// Then, we'll extract the minimum number of confirmations that each
 	// output we use to fund the channel's funding transaction should
 	// satisfy.
@@ -1863,6 +1871,7 @@ func (r *rpcServer) parseOpenChannelReq(in *lnrpc.OpenChannelRequest,
 		minConfs:         minConfs,
 		shutdownScript:   script,
 		maxValueInFlight: maxValue,
+		maxHtlcs:         maxHtlcs,
 	}, nil
 }
 

server.go

@@ -1156,6 +1156,10 @@ func newServer(cfg *Config, listenAddrs []net.Addr,
 			return lnwire.NewMSatFromSatoshis(chanAmt) - reserve
 		},
 		RequiredRemoteMaxHTLCs: func(chanAmt btcutil.Amount) uint16 {
+			if cfg.DefaultRemoteMaxHtlcs > 0 {
+				return cfg.DefaultRemoteMaxHtlcs
+			}
+
 			// By default, we'll permit them to utilize the full
 			// channel bandwidth.
 			return uint16(input.MaxHTLCNumber / 2)
@@ -3343,6 +3347,8 @@ type openChanReq struct {
 	// be pending within the channel. It only applies to the remote party.
 	maxValueInFlight lnwire.MilliSatoshi
 
+	maxHtlcs uint16
+
 	// TODO(roasbeef): add ability to specify channel constraints as well
 
 	// chanFunder is an optional channel funder that allows the caller to