Merge pull request #10488 from yashbhutwala/rpcserver-fix-fundmax-maxchansize

rpcserver: use protocol max for fundMax, not maxChanSize

docs/release-notes/release-notes-0.21.0.md

@@ -21,6 +21,12 @@
 
 # Bug Fixes
 
+* [Fixed `OpenChannel` with
+  `fund_max`](https://github.com/lightningnetwork/lnd/pull/10488) to use the
+  protocol-level maximum channel size instead of the user-configured
+  `maxchansize`. The `maxchansize` config option is intended only for limiting
+  incoming channel requests from peers, not outgoing ones.
+
 - Chain notifier RPCs now [return the gRPC `Unavailable`
   status](https://github.com/lightningnetwork/lnd/pull/10352) while the
   sub-server is still starting. This allows clients to reliably detect the

itest/list_on_test.go

@@ -567,6 +567,10 @@ var allTestCases = []*lntest.TestCase{
 		Name:     "channel fundmax anchor reserve",
 		TestFunc: testChannelFundMaxAnchorReserve,
 	},
+	{
+		Name:     "channel fundmax maxchansize",
+		TestFunc: testChannelFundMaxMaxChanSize,
+	},
 	{
 		Name:     "htlc timeout resolver extract preimage remote",
 		TestFunc: testHtlcTimeoutResolverExtractPreimageRemote,

itest/lnd_channel_funding_fund_max_test.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/btcsuite/btcd/btcutil"
 	"github.com/lightningnetwork/lnd"
+	"github.com/lightningnetwork/lnd/funding"
 	"github.com/lightningnetwork/lnd/input"
 	"github.com/lightningnetwork/lnd/lnrpc"
 	"github.com/lightningnetwork/lnd/lnrpc/walletrpc"
@@ -408,3 +409,79 @@ func sweepNodeWalletAndAssert(ht *lntest.HarnessTest, node *node.HarnessNode) {
 	// Ensure that the node's balance is 0
 	checkChannelBalance(ht, node, 0, 0)
 }
+
+// testChannelFundMaxMaxChanSize verifies that fundMax uses the protocol-level
+// maximum channel size, not the user-configured maxChanSize. The maxChanSize
+// config option is intended only for limiting incoming channel requests, not
+// outgoing ones.
+func testChannelFundMaxMaxChanSize(ht *lntest.HarnessTest) {
+	testCases := []struct {
+		name        string
+		wumbo       bool
+		expectedMax btcutil.Amount
+	}{
+		{
+			name:        "non-wumbo",
+			wumbo:       false,
+			expectedMax: funding.MaxBtcFundingAmount,
+		},
+		{
+			name:        "wumbo",
+			wumbo:       true,
+			expectedMax: funding.MaxBtcFundingAmountWumbo,
+		},
+	}
+
+	for _, tc := range testCases {
+		success := ht.Run(tc.name, func(t *testing.T) {
+			st := ht.Subtest(t)
+
+			// Configure Alice with a restrictive maxChanSize (5M
+			// sats), which is below both protocol maximums.
+			aliceArgs := []string{
+				"--maxchansize=5000000",
+			}
+			if tc.wumbo {
+				aliceArgs = append(
+					aliceArgs, "--protocol.wumbo-channels",
+				)
+			}
+
+			alice := st.NewNode("Alice", aliceArgs)
+
+			// Bob needs wumbo enabled to accept large channels.
+			var bobArgs []string
+			if tc.wumbo {
+				bobArgs = []string{"--protocol.wumbo-channels"}
+			}
+			bob := st.NewNode("Bob", bobArgs)
+
+			st.EnsureConnected(alice, bob)
+
+			// Fund Alice with more than the protocol maximum.
+			fundAmt := tc.expectedMax + btcutil.SatoshiPerBitcoin
+			st.FundCoins(fundAmt, alice)
+
+			// Open channel with fundMax. This should use the
+			// protocol maximum, not the configured maxChanSize.
+			chanPoint := st.OpenChannel(
+				alice, bob, lntest.OpenChannelParams{
+					FundMax: true,
+				},
+			)
+
+			cType := st.GetChannelCommitType(alice, chanPoint)
+
+			// The expected balance is the protocol maximum minus
+			// the commitment fee.
+			expectedBalance := tc.expectedMax -
+				lntest.CalcStaticFee(cType, 0)
+
+			checkChannelBalance(st, alice, expectedBalance, 0)
+			checkChannelBalance(st, bob, 0, expectedBalance)
+		})
+		if !success {
+			break
+		}
+	}
+}

rpcserver.go

@@ -2152,15 +2152,26 @@ func (r *rpcServer) parseOpenChannelReq(in *lnrpc.OpenChannelRequest,
 			"the channel opening")
 	}
 
+	// Fetch our own feature set and determine wumbo support early, as it's
+	// needed for both FundMax and explicit amount validation.
+	globalFeatureSet := r.server.featureMgr.Get(feature.SetNodeAnn)
+	wumboEnabled := globalFeatureSet.HasFeature(
+		lnwire.WumboChannelsOptional,
+	)
+
 	// If the FundMax flag is set, ensure that the acceptable minimum local
 	// amount adheres to the amount to be pushed to the remote, and to
-	// current rules, while also respecting the settings for the maximum
+	// current rules, while also respecting the protocol-level maximum
 	// channel size.
 	var minFundAmt, fundUpToMaxAmt btcutil.Amount
 	if in.FundMax {
-		// We assume the configured maximum channel size to be the upper
-		// bound of our "maxed" out funding attempt.
-		fundUpToMaxAmt = btcutil.Amount(r.cfg.MaxChanSize)
+		// Use the protocol-level maximum as the upper bound for our
+		// funding attempt.
+		if wumboEnabled {
+			fundUpToMaxAmt = funding.MaxBtcFundingAmountWumbo
+		} else {
+			fundUpToMaxAmt = MaxFundingAmount
+		}
 
 		// Since the standard non-fundmax flow requires the minimum
 		// funding amount to be at least in the amount of the initial
@@ -2186,8 +2197,6 @@ func (r *rpcServer) parseOpenChannelReq(in *lnrpc.OpenChannelRequest,
 	maxHtlcs := uint16(in.RemoteMaxHtlcs)
 	remoteChanReserve := btcutil.Amount(in.RemoteChanReserveSat)
 
-	globalFeatureSet := r.server.featureMgr.Get(feature.SetNodeAnn)
-
 	// Determine if the user provided channel fees
 	// and if so pass them on to the funding workflow.
 	var channelBaseFee, channelFeeRate *uint64
@@ -2212,9 +2221,6 @@ func (r *rpcServer) parseOpenChannelReq(in *lnrpc.OpenChannelRequest,
 	// in the wallet hence we do not check it here against the maximum
 	// funding amount. Only if the localFundingAmt is specified we can check
 	// if it exceeds the maximum funding amount.
-	wumboEnabled := globalFeatureSet.HasFeature(
-		lnwire.WumboChannelsOptional,
-	)
 	if !in.FundMax && !wumboEnabled && localFundingAmt > MaxFundingAmount {
 		return nil, fmt.Errorf("funding amount is too large, the max "+
 			"channel size is: %v", MaxFundingAmount)