Merge pull request #8896 from ziggie1984/batchopen-feerate-fix

Fix batchopen fee calculation

Author: guggero

docs/release-notes/release-notes-0.18.3.md

@@ -32,6 +32,9 @@
 * [Avoids duplicate wallet addresses being
   created](https://github.com/lightningnetwork/lnd/pull/8921) when multiple RPC
   calls are made concurrently.
+  
+* [Fixed a bug](https://github.com/lightningnetwork/lnd/pull/8896) that caused
+  LND to use a default fee rate for the batch channel opening flow.
 
 # New Features
 ## Functional Enhancements
@@ -148,3 +151,4 @@
 * Oliver Gugger
 * Slyghtning
 * Yong Yu
+* Ziggie

itest/lnd_funding_test.go

@@ -19,6 +19,7 @@ import (
 	"github.com/lightningnetwork/lnd/lnrpc/signrpc"
 	"github.com/lightningnetwork/lnd/lntest"
 	"github.com/lightningnetwork/lnd/lntest/node"
+	"github.com/lightningnetwork/lnd/lnwallet/chainfee"
 	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/stretchr/testify/require"
 )
@@ -968,11 +969,16 @@ func testBatchChanFunding(ht *lntest.HarnessTest) {
 	ht.EnsureConnected(alice, dave)
 	ht.EnsureConnected(alice, eve)
 
+	expectedFeeRate := chainfee.SatPerKWeight(2500)
+
+	// We verify that the channel opening uses the correct fee rate.
+	ht.SetFeeEstimateWithConf(expectedFeeRate, 3)
+
 	// Let's create our batch TX request. This first one should fail as we
 	// open a channel to Carol that is too small for her min chan size.
 	batchReq := &lnrpc.BatchOpenChannelRequest{
-		SatPerVbyte: 12,
-		MinConfs:    1,
+		TargetConf: 3,
+		MinConfs:   1,
 		Channels: []*lnrpc.BatchOpenChannel{{
 			NodePubkey:         bob.PubKey[:],
 			LocalFundingAmount: 100_000,
@@ -1069,6 +1075,12 @@ func testBatchChanFunding(ht *lntest.HarnessTest) {
 	rawTx := ht.GetRawTransaction(txHash)
 	require.Len(ht, rawTx.MsgTx().TxOut, 5)
 
+	// Check the fee rate of the batch-opening transaction. We expect slight
+	// inaccuracies because of the DER signature fee estimation.
+	openingFeeRate := ht.CalculateTxFeeRate(rawTx.MsgTx())
+	require.InEpsilonf(ht, uint64(expectedFeeRate), uint64(openingFeeRate),
+		0.01, "want %v, got %v", expectedFeeRate, openingFeeRate)
+
 	// For calculating the change output index we use the formula for the
 	// sum of consecutive of integers (n(n+1)/2). All the channel point
 	// indexes are known, so we just calculate the difference to get the

rpcserver.go

@@ -2159,30 +2159,24 @@ func (r *rpcServer) parseOpenChannelReq(in *lnrpc.OpenChannelRequest,
 		return nil, fmt.Errorf("cannot open channel to self")
 	}
 
-	var feeRate chainfee.SatPerKWeight
-
-	// Skip estimating fee rate for PSBT funding.
-	if in.FundingShim == nil || in.FundingShim.GetPsbtShim() == nil {
-		// Keep the old behavior prior to 0.18.0 - when the user
-		// doesn't set fee rate or conf target, the default conf target
-		// of 6 is used.
-		targetConf := maybeUseDefaultConf(
-			in.SatPerByte, in.SatPerVbyte, uint32(in.TargetConf),
-		)
-
-		// Calculate an appropriate fee rate for this transaction.
-		feeRate, err = lnrpc.CalculateFeeRate(
-			uint64(in.SatPerByte), in.SatPerVbyte,
-			targetConf, r.server.cc.FeeEstimator,
-		)
-		if err != nil {
-			return nil, err
-		}
+	// NOTE: We also need to do the fee rate calculation for the psbt
+	// funding flow because the `batchfund` depends on it.
+	targetConf := maybeUseDefaultConf(
+		in.SatPerByte, in.SatPerVbyte, uint32(in.TargetConf),
+	)
 
-		rpcsLog.Debugf("[openchannel]: using fee of %v sat/kw for "+
-			"funding tx", int64(feeRate))
+	// Calculate an appropriate fee rate for this transaction.
+	feeRate, err := lnrpc.CalculateFeeRate(
+		uint64(in.SatPerByte), in.SatPerVbyte,
+		targetConf, r.server.cc.FeeEstimator,
+	)
+	if err != nil {
+		return nil, err
 	}
 
+	rpcsLog.Debugf("[openchannel]: using fee of %v sat/kw for "+
+		"funding tx", int64(feeRate))
+
 	script, err := chancloser.ParseUpfrontShutdownAddress(
 		in.CloseAddress, r.cfg.ActiveNetParams.Params,
 	)

server.go

@@ -4578,16 +4578,15 @@ func (s *server) OpenChannel(
 		return req.Updates, req.Err
 	}
 
-	// If the fee rate wasn't specified, then we'll use a default
-	// confirmation target.
+	// If the fee rate wasn't specified at this point we fail the funding
+	// because of the missing fee rate information. The caller of the
+	// `OpenChannel` method needs to make sure that default values for the
+	// fee rate are set beforehand.
 	if req.FundingFeePerKw == 0 {
-		estimator := s.cc.FeeEstimator
-		feeRate, err := estimator.EstimateFeePerKW(6)
-		if err != nil {
-			req.Err <- err
-			return req.Updates, req.Err
-		}
-		req.FundingFeePerKw = feeRate
+		req.Err <- fmt.Errorf("no FundingFeePerKw specified for " +
+			"the channel opening transaction")
+
+		return req.Updates, req.Err
 	}
 
 	// Spawn a goroutine to send the funding workflow request to the funding