From d0a7765c685aa03df31de34a798c1bf1dece5fa6 Mon Sep 17 00:00:00 2001 From: ziggie Date: Wed, 24 Jul 2024 14:51:13 +0200 Subject: [PATCH 1/3] rpcserver: include fee calc. for psbt flow. Include the fee calculaltion for the psbt flow. Moreover include fee rate testing in the itest environment. --- itest/lnd_funding_test.go | 16 ++++++++++++++-- rpcserver.go | 36 +++++++++++++++--------------------- 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/itest/lnd_funding_test.go b/itest/lnd_funding_test.go index 8f43620ee1..6e2f0070cd 100644 --- a/itest/lnd_funding_test.go +++ b/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 diff --git a/rpcserver.go b/rpcserver.go index 465035ea55..8415eb338f 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -2158,30 +2158,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, ) From bf38aed87fcc9b1a3e4e9e6fd411e629fcc1825a Mon Sep 17 00:00:00 2001 From: ziggie Date: Wed, 24 Jul 2024 15:01:47 +0200 Subject: [PATCH 2/3] lnd: unify the default setting behaviour. Setting default values for the channel opening fee rate is already done elsewhere therefore we remove on of those checks and return an error if no fee rate is specified. --- server.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/server.go b/server.go index 7ae6ed21e7..a3b1284067 100644 --- a/server.go +++ b/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 From eb7818a633c6047cf8a5d9abcd2bae6c88156abd Mon Sep 17 00:00:00 2001 From: ziggie Date: Thu, 11 Jul 2024 22:26:15 +0200 Subject: [PATCH 3/3] docs: add release-notes. --- docs/release-notes/release-notes-0.18.3.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/release-notes/release-notes-0.18.3.md b/docs/release-notes/release-notes-0.18.3.md index 8d658624fc..2e7eb643e2 100644 --- a/docs/release-notes/release-notes-0.18.3.md +++ b/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 @@ -130,3 +133,4 @@ * Oliver Gugger * Slyghtning * Yong Yu +* Ziggie