diff --git a/cmd/commands/cmd_open_channel.go b/cmd/commands/cmd_open_channel.go index 921f8478756..b5beb038540 100644 --- a/cmd/commands/cmd_open_channel.go +++ b/cmd/commands/cmd_open_channel.go @@ -179,11 +179,6 @@ var openChannelCommand = cli.Command{ "transaction *should* confirm in, will be " + "used for fee estimation", }, - cli.Int64Flag{ - Name: "sat_per_byte", - Usage: "Deprecated, use sat_per_vbyte instead.", - Hidden: true, - }, cli.Int64Flag{ Name: "sat_per_vbyte", Usage: "(optional) a manual fee expressed in " + @@ -319,19 +314,10 @@ func openChannel(ctx *cli.Context) error { return nil } - // Check that only the field sat_per_vbyte or the deprecated field - // sat_per_byte is used. - feeRateFlag, err := checkNotBothSet( - ctx, "sat_per_vbyte", "sat_per_byte", - ) - if err != nil { - return err - } - minConfs := int32(ctx.Uint64("min_confs")) req := &lnrpc.OpenChannelRequest{ TargetConf: int32(ctx.Int64("conf_target")), - SatPerVbyte: ctx.Uint64(feeRateFlag), + SatPerVbyte: ctx.Uint64("sat_per_vbyte"), MinHtlcMsat: ctx.Int64("min_htlc_msat"), RemoteCsvDelay: uint32(ctx.Uint64("remote_csv_delay")), MinConfs: minConfs, @@ -1080,7 +1066,7 @@ func checkPsbtFlags(req *lnrpc.OpenChannelRequest) error { return fmt.Errorf("specifying minimum confirmations for PSBT " + "funding is not supported") } - if req.TargetConf != 0 || req.SatPerByte != 0 || req.SatPerVbyte != 0 { // nolint:staticcheck + if req.TargetConf != 0 || req.SatPerVbyte != 0 { return fmt.Errorf("setting fee estimation parameters not " + "supported for PSBT funding") } diff --git a/cmd/commands/commands.go b/cmd/commands/commands.go index 0c389a610b0..95db39d65cd 100644 --- a/cmd/commands/commands.go +++ b/cmd/commands/commands.go @@ -497,11 +497,6 @@ var sendCoinsCommand = cli.Command{ "transaction *should* confirm in, will be " + "used for fee estimation", }, - cli.Int64Flag{ - Name: "sat_per_byte", - Usage: "Deprecated, use sat_per_vbyte instead.", - Hidden: true, - }, cli.Int64Flag{ Name: "sat_per_vbyte", Usage: "(optional) a manual fee expressed in " + @@ -555,18 +550,9 @@ func sendCoins(ctx *cli.Context) error { return nil } - // Check that only the field sat_per_vbyte or the deprecated field - // sat_per_byte is used. - feeRateFlag, err := checkNotBothSet( - ctx, "sat_per_vbyte", "sat_per_byte", - ) - if err != nil { - return err - } - - // Only fee rate flag or conf_target should be set, not both. + // Check that only one is set sat_per_vbyte or conf_target, not both. if _, err := checkNotBothSet( - ctx, feeRateFlag, "conf_target", + ctx, "sat_per_vbyte", "conf_target", ); err != nil { return err } @@ -675,7 +661,7 @@ func sendCoins(ctx *cli.Context) error { Addr: addr, Amount: amt, TargetConf: int32(ctx.Int64("conf_target")), - SatPerVbyte: ctx.Uint64(feeRateFlag), + SatPerVbyte: ctx.Uint64("sat_per_vbyte"), SendAll: ctx.Bool("sweepall"), Label: ctx.String(txLabelFlag.Name), MinConfs: minConfs, @@ -836,11 +822,6 @@ var sendManyCommand = cli.Command{ Usage: "(optional) the number of blocks that the transaction *should* " + "confirm in, will be used for fee estimation", }, - cli.Int64Flag{ - Name: "sat_per_byte", - Usage: "Deprecated, use sat_per_vbyte instead.", - Hidden: true, - }, cli.Int64Flag{ Name: "sat_per_vbyte", Usage: "(optional) a manual fee expressed in " + @@ -869,18 +850,9 @@ func sendMany(ctx *cli.Context) error { return err } - // Check that only the field sat_per_vbyte or the deprecated field - // sat_per_byte is used. - feeRateFlag, err := checkNotBothSet( - ctx, "sat_per_vbyte", "sat_per_byte", - ) - if err != nil { - return err - } - - // Only fee rate flag or conf_target should be set, not both. + // Check that only one is set sat_per_vbyte or conf_target, not both. if _, err := checkNotBothSet( - ctx, feeRateFlag, "conf_target", + ctx, "sat_per_vbyte", "conf_target", ); err != nil { return err } @@ -897,7 +869,7 @@ func sendMany(ctx *cli.Context) error { txid, err := client.SendMany(ctxc, &lnrpc.SendManyRequest{ AddrToAmount: amountToAddr, TargetConf: int32(ctx.Int64("conf_target")), - SatPerVbyte: ctx.Uint64(feeRateFlag), + SatPerVbyte: ctx.Uint64("sat_per_vbyte"), Label: ctx.String(txLabelFlag.Name), MinConfs: minConfs, SpendUnconfirmed: minConfs == 0, @@ -1087,11 +1059,6 @@ var closeChannelCommand = cli.Command{ "then the conf-target value set in the main " + "lnd config will be used.", }, - cli.Int64Flag{ - Name: "sat_per_byte", - Usage: "Deprecated, use sat_per_vbyte instead.", - Hidden: true, - }, cli.Int64Flag{ Name: "sat_per_vbyte", Usage: "(optional) a manual fee expressed in " + @@ -1129,15 +1096,6 @@ func closeChannel(ctx *cli.Context) error { return nil } - // Check that only the field sat_per_vbyte or the deprecated field - // sat_per_byte is used. - feeRateFlag, err := checkNotBothSet( - ctx, "sat_per_vbyte", "sat_per_byte", - ) - if err != nil { - return err - } - channelPoint, err := parseChannelPoint(ctx) if err != nil { return err @@ -1148,7 +1106,7 @@ func closeChannel(ctx *cli.Context) error { ChannelPoint: channelPoint, Force: ctx.Bool("force"), TargetConf: int32(ctx.Int64("conf_target")), - SatPerVbyte: ctx.Uint64(feeRateFlag), + SatPerVbyte: ctx.Uint64("sat_per_vbyte"), DeliveryAddress: ctx.String("delivery_addr"), MaxFeePerVbyte: ctx.Uint64("max_fee_rate"), // This makes sure that a coop close will also be executed if @@ -1291,11 +1249,6 @@ var closeAllChannelsCommand = cli.Command{ "closing transactions *should* confirm in, will be " + "used for fee estimation", }, - cli.Int64Flag{ - Name: "sat_per_byte", - Usage: "Deprecated, use sat_per_vbyte instead.", - Hidden: true, - }, cli.Int64Flag{ Name: "sat_per_vbyte", Usage: "(optional) a manual fee expressed in " + @@ -1316,15 +1269,6 @@ func closeAllChannels(ctx *cli.Context) error { client, cleanUp := getClient(ctx) defer cleanUp() - // Check that only the field sat_per_vbyte or the deprecated field - // sat_per_byte is used. - feeRateFlag, err := checkNotBothSet( - ctx, "sat_per_vbyte", "sat_per_byte", - ) - if err != nil { - return err - } - prompt := "Do you really want to close ALL channels? (yes/no): " if !ctx.Bool("skip_confirmation") && !promptForConfirmation(prompt) { return errors.New("action aborted by user") @@ -1458,7 +1402,7 @@ func closeAllChannels(ctx *cli.Context) error { }, Force: !channel.GetActive(), TargetConf: int32(ctx.Int64("conf_target")), - SatPerVbyte: ctx.Uint64(feeRateFlag), + SatPerVbyte: ctx.Uint64("sat_per_vbyte"), } txidChan := make(chan string, 1) diff --git a/cmd/commands/walletrpc_active.go b/cmd/commands/walletrpc_active.go index dcb91a71ff5..17fbec1d060 100644 --- a/cmd/commands/walletrpc_active.go +++ b/cmd/commands/walletrpc_active.go @@ -275,11 +275,6 @@ var bumpFeeCommand = cli.Command{ can be specified and LND will query its fee estimator for the current fee rate for the given target.`, }, - cli.Uint64Flag{ - Name: "sat_per_byte", - Usage: "Deprecated, use sat_per_vbyte instead.", - Hidden: true, - }, cli.BoolFlag{ Name: "force", Usage: "Deprecated, use immediate instead.", @@ -394,11 +389,6 @@ var bumpCloseFeeCommand = cli.Command{ can be specified and LND will query its fee estimator for the current fee rate for the given target.`, }, - cli.Uint64Flag{ - Name: "sat_per_byte", - Usage: "Deprecated, use sat_per_vbyte instead.", - Hidden: true, - }, cli.BoolFlag{ Name: "force", Usage: "Deprecated, use immediate instead.", @@ -460,11 +450,6 @@ var bumpForceCloseFeeCommand = cli.Command{ be spent within to bump the closing transaction. When the deadline is reached, ALL the budget will be spent as fees.`, }, - cli.Uint64Flag{ - Name: "sat_per_byte", - Usage: "Deprecated, use sat_per_vbyte instead.", - Hidden: true, - }, cli.Uint64Flag{ Name: "sat_per_vbyte", Usage: ` @@ -516,11 +501,6 @@ func bumpForceCloseFee(ctx *cli.Context) error { return err } - // `sat_per_byte` was deprecated we only use sats/vbyte now. - if ctx.IsSet("sat_per_byte") { - return fmt.Errorf("deprecated, use sat_per_vbyte instead") - } - // Retrieve pending sweeps. walletClient, cleanUp := getWalletClient(ctx) defer cleanUp() diff --git a/docs/release-notes/release-notes-0.22.0.md b/docs/release-notes/release-notes-0.22.0.md index 45e76051922..b4ca5823b2f 100644 --- a/docs/release-notes/release-notes-0.22.0.md +++ b/docs/release-notes/release-notes-0.22.0.md @@ -62,6 +62,20 @@ ## Breaking Changes +* The deprecated fee rate option `--sat_per_byte` was removed +[PR#10889](https://github.com/lightningnetwork/lnd/pull/10889) + + The following RPCs were impacted: + + | RPC Method | Messages | Removed Option | + |----------------------|----------------|-------------| + | [`lnrpc.CloseChannel`](https://lightning.engineering/api-docs/api/lnd/lightning/close-channel/) | [`lnrpc.CloseChannelRequest`](https://lightning.engineering/api-docs/api/lnd/lightning/close-channel/#lnrpcclosechannelrequest) | sat_per_byte + | [`lnrpc.OpenChannelSync`](https://lightning.engineering/api-docs/api/lnd/lightning/open-channel-sync/) | [`lnrpc.OpenChannelRequest`](https://lightning.engineering/api-docs/api/lnd/lightning/open-channel-sync/#lnrpcopenchannelrequest) | sat_per_byte + | [`lnrpc.OpenChannel`](https://lightning.engineering/api-docs/api/lnd/lightning/open-channel/) | [`lnrpc.OpenChannelRequest`](https://lightning.engineering/api-docs/api/lnd/lightning/open-channel/#lnrpcopenchannelrequest) | sat_per_byte + | [`lnrpc.SendCoins`](https://lightning.engineering/api-docs/api/lnd/lightning/send-coins/) | [`lnrpc.SendCoinsRequest`](https://lightning.engineering/api-docs/api/lnd/lightning/send-coins/#lnrpcsendcoinsrequest) | sat_per_byte + | [`lnrpc.SendMany`](https://lightning.engineering/api-docs/api/lnd/lightning/send-many/) | [`lnrpc.SendManyRequest`](https://lightning.engineering/api-docs/api/lnd/lightning/send-many/#lnrpcsendmanyrequest) | sat_per_byte + | [`walletrpc.BumpFee`](https://lightning.engineering/api-docs/api/lnd/wallet-kit/bump-fee/) | [`walletrpc.BumpFeeRequest`](https://lightning.engineering/api-docs/api/lnd/wallet-kit/bump-fee/#walletrpcbumpfeerequest) | sat_per_byte + ## Performance Improvements ## Deprecations @@ -89,3 +103,4 @@ * Boris Nagaev * Erick Cestari +* Pins diff --git a/itest/lnd_wallet_import_test.go b/itest/lnd_wallet_import_test.go index 4a08fb2e6cf..8fdcee24932 100644 --- a/itest/lnd_wallet_import_test.go +++ b/itest/lnd_wallet_import_test.go @@ -566,9 +566,9 @@ func runWalletImportAccountScenario(ht *lntest.HarnessTest, // balance updates accordingly. alice := ht.NewNodeWithCoins("Alice", nil) req := &lnrpc.SendCoinsRequest{ - Addr: externalAddr, - Amount: utxoAmt, - SatPerByte: 1, + Addr: externalAddr, + Amount: utxoAmt, + SatPerVbyte: 1, } alice.RPC.SendCoins(req) @@ -599,9 +599,9 @@ func runWalletImportAccountScenario(ht *lntest.HarnessTest, // Send coins to Carol's address and confirm them, making sure the // balance updates accordingly. req = &lnrpc.SendCoinsRequest{ - Addr: externalAddr, - Amount: utxoAmt, - SatPerByte: 1, + Addr: externalAddr, + Amount: utxoAmt, + SatPerVbyte: 1, } alice.RPC.SendCoins(req) @@ -739,9 +739,9 @@ func testWalletImportPubKeyScenario(ht *lntest.HarnessTest, // Send coins to Carol's address and confirm them, making sure // the balance updates accordingly. req := &lnrpc.SendCoinsRequest{ - Addr: carolAddrResp.Address, - Amount: utxoAmt, - SatPerByte: 1, + Addr: carolAddrResp.Address, + Amount: utxoAmt, + SatPerVbyte: 1, } alice.RPC.SendCoins(req) diff --git a/lnrpc/rpc_utils.go b/lnrpc/rpc_utils.go index 15e3f2f7622..9e7e4f4bcbb 100644 --- a/lnrpc/rpc_utils.go +++ b/lnrpc/rpc_utils.go @@ -3,7 +3,6 @@ package lnrpc import ( "encoding/hex" "errors" - "fmt" "sort" "github.com/btcsuite/btcd/chaincfg/chainhash" @@ -55,6 +54,12 @@ var ( RESTJsonUnmarshalOpts = &protojson.UnmarshalOptions{ AllowPartial: false, } + + // ErrSatPerByteRemoved is returned when a request still sets the + // deprecated sat_per_byte field. The field was deprecated in v0.13.0 + // in favor of sat_per_vbyte and is no longer honored. + ErrSatPerByteRemoved = errors.New("sat_per_byte is no longer " + + "supported, use sat_per_vbyte instead") ) // RPCTransaction returns a rpc transaction. @@ -227,28 +232,13 @@ func GetChannelOutPoint(chanPoint *ChannelPoint) (*OutPoint, error) { }, nil } -// CalculateFeeRate uses either satPerByte or satPerVByte, but not both, from a -// request to calculate the fee rate. It provides compatibility for the -// deprecated field, satPerByte. Once the field is safe to be removed, the -// check can then be deleted. -func CalculateFeeRate(satPerByte, satPerVByte uint64, targetConf uint32, +// CalculateFeeRate uses satPerVByte from a request to calculate the fee rate. +func CalculateFeeRate(satPerVByte uint64, targetConf uint32, estimator chainfee.Estimator) (chainfee.SatPerKWeight, error) { var feeRate chainfee.SatPerKWeight - // We only allow using either the deprecated field or the new field. - if satPerByte != 0 && satPerVByte != 0 { - return feeRate, fmt.Errorf("either SatPerByte or " + - "SatPerVByte should be set, but not both") - } - - // Default to satPerVByte, and overwrite it if satPerByte is set. satPerKw := chainfee.SatPerKVByte(satPerVByte * 1000).FeePerKWeight() - if satPerByte != 0 { - satPerKw = chainfee.SatPerKVByte( - satPerByte * 1000, - ).FeePerKWeight() - } // Based on the passed fee related parameters, we'll determine an // appropriate fee rate for this transaction. diff --git a/lnrpc/walletrpc/walletkit_server.go b/lnrpc/walletrpc/walletkit_server.go index bece6001d6a..13b19dd7a14 100644 --- a/lnrpc/walletrpc/walletkit_server.go +++ b/lnrpc/walletrpc/walletkit_server.go @@ -968,19 +968,12 @@ func validateBumpFeeRequest(in *BumpFeeRequest, estimator chainfee.Estimator) ( // Get the specified fee rate if set. satPerKwOpt := fn.None[chainfee.SatPerKWeight]() - // We only allow using either the deprecated field or the new field. - switch { - case in.SatPerByte != 0 && in.SatPerVbyte != 0: - return satPerKwOpt, false, fmt.Errorf("either SatPerByte or " + - "SatPerVbyte should be set, but not both") - - case in.SatPerByte != 0: - satPerKw := chainfee.SatPerVByte( - in.SatPerByte, - ).FeePerKWeight() - satPerKwOpt = fn.Some(satPerKw) + // The deprecated sat_per_byte field is no longer honored. + if in.SatPerByte != 0 { + return satPerKwOpt, false, lnrpc.ErrSatPerByteRemoved + } - case in.SatPerVbyte != 0: + if in.SatPerVbyte != 0 { satPerKw := chainfee.SatPerVByte( in.SatPerVbyte, ).FeePerKWeight() diff --git a/lnrpc/walletrpc/walletkit_server_test.go b/lnrpc/walletrpc/walletkit_server_test.go index 40fb62ef2ad..df0addb7ca1 100644 --- a/lnrpc/walletrpc/walletkit_server_test.go +++ b/lnrpc/walletrpc/walletkit_server_test.go @@ -18,6 +18,7 @@ import ( "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcwallet/wallet" "github.com/lightningnetwork/lnd/input" + "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lntest/mock" "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/lnwallet/chainfee" @@ -723,3 +724,28 @@ func TestFundPsbtCoinSelect(t *testing.T) { }) } } + +// TestValidateBumpFeeRequestSatPerByte tests that validateBumpFeeRequest +// rejects the deprecated sat_per_byte field while still accepting the same fee +// rate expressed via sat_per_vbyte. +func TestValidateBumpFeeRequestSatPerByte(t *testing.T) { + t.Parallel() + + // A request that sets the deprecated sat_per_byte field must be + // rejected with ErrSatPerByteRemoved. The check happens before the fee + // estimator is used, so a nil estimator is fine here. + deprecated := &BumpFeeRequest{ + SatPerByte: 1, + } + _, _, err := validateBumpFeeRequest(deprecated, nil) + require.ErrorIs(t, err, lnrpc.ErrSatPerByteRemoved) + + // The same fee rate expressed via sat_per_vbyte is accepted and yields + // a starting fee rate. + valid := &BumpFeeRequest{ + SatPerVbyte: 1, + } + feeRate, _, err := validateBumpFeeRequest(valid, nil) + require.NoError(t, err) + require.False(t, feeRate.IsNone()) +} diff --git a/rpcserver.go b/rpcserver.go index 8b40192db84..79f6868ffe4 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -1330,13 +1330,11 @@ func (r *rpcServer) EstimateFee(ctx context.Context, // maybeUseDefaultConf makes sure that when the user doesn't set either the fee // rate or conf target, the default conf target is used. -func maybeUseDefaultConf(satPerByte int64, satPerVByte uint64, - targetConf uint32) uint32 { - +func maybeUseDefaultConf(satPerVByte uint64, targetConf uint32) uint32 { // If the fee rate is set, there's no need to use the default conf // target. In this case, we just return the targetConf from the // request. - if satPerByte != 0 || satPerVByte != 0 { + if satPerVByte != 0 { return targetConf } @@ -1358,16 +1356,18 @@ func maybeUseDefaultConf(satPerByte int64, satPerVByte uint64, func (r *rpcServer) SendCoins(ctx context.Context, in *lnrpc.SendCoinsRequest) (*lnrpc.SendCoinsResponse, error) { + // The deprecated sat_per_byte field is no longer honored. + if in.SatPerByte != 0 { + return nil, lnrpc.ErrSatPerByteRemoved + } + // 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), - ) + targetConf := maybeUseDefaultConf(in.SatPerVbyte, uint32(in.TargetConf)) // Calculate an appropriate fee rate for this transaction. feePerKw, err := lnrpc.CalculateFeeRate( - uint64(in.SatPerByte), in.SatPerVbyte, // nolint:staticcheck - targetConf, r.server.cc.FeeEstimator, + in.SatPerVbyte, targetConf, r.server.cc.FeeEstimator, ) if err != nil { return nil, err @@ -1602,16 +1602,18 @@ func (r *rpcServer) SendCoins(ctx context.Context, func (r *rpcServer) SendMany(ctx context.Context, in *lnrpc.SendManyRequest) (*lnrpc.SendManyResponse, error) { + // The deprecated sat_per_byte field is no longer honored. + if in.SatPerByte != 0 { + return nil, lnrpc.ErrSatPerByteRemoved + } + // 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), - ) + targetConf := maybeUseDefaultConf(in.SatPerVbyte, uint32(in.TargetConf)) // Calculate an appropriate fee rate for this transaction. feePerKw, err := lnrpc.CalculateFeeRate( - uint64(in.SatPerByte), in.SatPerVbyte, // nolint:staticcheck - targetConf, r.server.cc.FeeEstimator, + in.SatPerVbyte, targetConf, r.server.cc.FeeEstimator, ) if err != nil { return nil, err @@ -2052,7 +2054,7 @@ func newPsbtAssembler(req *lnrpc.OpenChannelRequest, if len(psbtShim.PendingChanId) != 32 { return nil, fmt.Errorf("pending chan ID not set") } - if req.SatPerByte != 0 || req.SatPerVbyte != 0 || req.TargetConf != 0 { // nolint:staticcheck + if req.SatPerVbyte != 0 || req.TargetConf != 0 { return nil, fmt.Errorf("specifying fee estimation parameters " + "is not supported for PSBT funding") } @@ -2109,6 +2111,11 @@ func (r *rpcServer) parseOpenChannelReq(in *lnrpc.OpenChannelRequest, "allocation(us=%v, them=%v)", in.NodePubkey, in.LocalFundingAmount, in.PushSat) + // The deprecated sat_per_byte field is no longer honored. + if in.SatPerByte != 0 { + return nil, lnrpc.ErrSatPerByteRemoved + } + localFundingAmt := btcutil.Amount(in.LocalFundingAmount) remoteInitialBalance := btcutil.Amount(in.PushSat) @@ -2281,14 +2288,11 @@ func (r *rpcServer) parseOpenChannelReq(in *lnrpc.OpenChannelRequest, // 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), - ) + targetConf := maybeUseDefaultConf(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, + in.SatPerVbyte, targetConf, r.server.cc.FeeEstimator, ) if err != nil { return nil, err @@ -2746,11 +2750,14 @@ func (r *rpcServer) CloseChannel(in *lnrpc.CloseChannelRequest, return fmt.Errorf("must specify channel point in close channel") } + // The deprecated sat_per_byte field is no longer honored. + if in.SatPerByte != 0 { + return lnrpc.ErrSatPerByteRemoved + } + // If force closing a channel, the fee set in the commitment transaction // is used. - if in.Force && (in.SatPerByte != 0 || in.SatPerVbyte != 0 || // nolint:staticcheck - in.TargetConf != 0) { - + if in.Force && (in.SatPerVbyte != 0 || in.TargetConf != 0) { return fmt.Errorf("force closing a channel uses a pre-defined fee") } @@ -2925,15 +2932,14 @@ func (r *rpcServer) CloseChannel(in *lnrpc.CloseChannelRequest, // 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), + in.SatPerVbyte, uint32(in.TargetConf), ) // Based on the passed fee related parameters, we'll determine // an appropriate fee rate for the cooperative closure // transaction. feeRate, err := lnrpc.CalculateFeeRate( - uint64(in.SatPerByte), in.SatPerVbyte, // nolint:staticcheck - targetConf, r.server.cc.FeeEstimator, + in.SatPerVbyte, targetConf, r.server.cc.FeeEstimator, ) if err != nil { return err diff --git a/rpcserver_test.go b/rpcserver_test.go index b686c9020a8..692b1729644 100644 --- a/rpcserver_test.go +++ b/rpcserver_test.go @@ -123,3 +123,35 @@ func TestRpcCommitmentType(t *testing.T) { }) } } + +// TestRejectDeprecatedSatPerByte ensures the RPCs that previously honored the +// deprecated sat_per_byte field now reject requests that set it. The rejection +// happens before any server state is accessed, so a zero-value rpcServer is +// sufficient here. +func TestRejectDeprecatedSatPerByte(t *testing.T) { + t.Parallel() + + r := &rpcServer{} + ctx := t.Context() + + _, err := r.SendCoins(ctx, &lnrpc.SendCoinsRequest{SatPerByte: 1}) + require.ErrorIs(t, err, lnrpc.ErrSatPerByteRemoved) + + _, err = r.SendMany(ctx, &lnrpc.SendManyRequest{SatPerByte: 1}) + require.ErrorIs(t, err, lnrpc.ErrSatPerByteRemoved) + + _, err = r.parseOpenChannelReq( + &lnrpc.OpenChannelRequest{SatPerByte: 1}, false, + ) + require.ErrorIs(t, err, lnrpc.ErrSatPerByteRemoved) + + // CloseChannel checks that the server is started before validating the + // request, so it needs a minimally-started server. The deprecated field + // is still rejected before any close logic runs. + started := &rpcServer{server: &server{active: 1}} + err = started.CloseChannel(&lnrpc.CloseChannelRequest{ + ChannelPoint: &lnrpc.ChannelPoint{}, + SatPerByte: 1, + }, nil) + require.ErrorIs(t, err, lnrpc.ErrSatPerByteRemoved) +}