diff --git a/cmd/commands/assets.go b/cmd/commands/assets.go index 84552a06b0..7dadc06905 100644 --- a/cmd/commands/assets.go +++ b/cmd/commands/assets.go @@ -301,6 +301,19 @@ func parseFeeRate(ctx *cli.Context) (uint32, error) { return uint32(0), nil } +func parseSatPerVByte(ctx *cli.Context) (uint32, error) { + if !ctx.IsSet(feeRateName) { + return 0, nil + } + + satPerVByte := ctx.Uint64(feeRateName) + if satPerVByte > math.MaxUint32 { + return 0, fmt.Errorf("fee rate exceeds 2^32") + } + + return uint32(satPerVByte), nil +} + func mintAsset(ctx *cli.Context) error { switch { case ctx.String(assetTagName) == "": @@ -1106,7 +1119,7 @@ func sendAssets(ctx *cli.Context) error { client, cleanUp := getClient(ctx) defer cleanUp() - feeRate, err := parseFeeRate(ctx) + satPerVByte, err := parseSatPerVByte(ctx) if err != nil { return err } @@ -1177,7 +1190,7 @@ func sendAssets(ctx *cli.Context) error { } resp, err := client.SendAsset(ctxc, &taprpc.SendAssetRequest{ - FeeRate: feeRate, + SatPerVbyte: uint64(satPerVByte), SkipProofCourierPingCheck: ctx.Bool( skipProofCourierPingCheckName, ), diff --git a/docs/release-notes/release-notes-0.8.0.md b/docs/release-notes/release-notes-0.8.0.md index 954472d313..70ed068c1a 100644 --- a/docs/release-notes/release-notes-0.8.0.md +++ b/docs/release-notes/release-notes-0.8.0.md @@ -403,6 +403,13 @@ `BurnAssetResponse` adds a repeated `burn_proofs` field; the singular `burn_proof` field is deprecated. +- [PR#2054](https://github.com/lightninglabs/taproot-assets/pull/2054) + ([#1661](https://github.com/lightninglabs/taproot-assets/issues/1661)) + Replaces the `fee_rate` send option with `sat_per_vbyte` in tapcli and adds + `sat_per_vbyte` to `SendAssetRequest`. The legacy `fee_rate` RPC field is + now deprecated and remains accepted as a sat/kw fallback when + `sat_per_vbyte` is not set. + ## Performance Improvements * [PR#2104](https://github.com/lightninglabs/taproot-assets/pull/2104) diff --git a/itest/addrs_test.go b/itest/addrs_test.go index 424467434c..88557d17b3 100644 --- a/itest/addrs_test.go +++ b/itest/addrs_test.go @@ -1207,10 +1207,10 @@ func withSkipProofCourierPingCheck() sendOption { } } -// withFeeRate is an option to specify the fee rate for the send. -func withFeeRate(feeRate uint32) sendOption { +// withSatPerVByte is an option to specify the fee rate for the send. +func withSatPerVByte(satPerVByte uint64) sendOption { return func(options *sendOptions) { - options.sendAssetRequest.FeeRate = feeRate + options.sendAssetRequest.SatPerVbyte = satPerVByte } } diff --git a/itest/custom_channels/helpers.go b/itest/custom_channels/helpers.go index 5245d60a94..08b8920687 100644 --- a/itest/custom_channels/helpers.go +++ b/itest/custom_channels/helpers.go @@ -920,7 +920,8 @@ func assertPendingChannelAssetData(t *testing.T, node *itest.IntegratedNode, ) if err != nil { return fmt.Errorf("error unmarshalling custom channel "+ - "data: %v", err) + "data: %v", err, + ) } if len(closeData.FundingAssets) == 0 { diff --git a/itest/fee_estimation_test.go b/itest/fee_estimation_test.go index e9592d648f..2900351e25 100644 --- a/itest/fee_estimation_test.go +++ b/itest/fee_estimation_test.go @@ -168,8 +168,8 @@ func testFeeEstimation(t *harnessTest) { // fee rate fails the sanity check against the fee estimator's fee floor // of 253 sat/kw, or 1.012 sat/vB. _, err = t.tapd.SendAsset(ctx, &taprpc.SendAssetRequest{ - TapAddrs: []string{addr3.Encoded}, - FeeRate: uint32(chainfee.FeePerKwFloor) - 1, + TapAddrs: []string{addr3.Encoded}, + SatPerVbyte: 1, }) require.ErrorContains(t.t, err, "manual fee rate below floor") diff --git a/itest/send_test.go b/itest/send_test.go index 6db23d530a..7e33ec6bbb 100644 --- a/itest/send_test.go +++ b/itest/send_test.go @@ -156,9 +156,11 @@ func testMinRelayFeeBump(t *harnessTest) { // Set the variables for the fee rates we'll use in this test. belowFloorFeeRate := chainfee.SatPerVByte(1).FeePerKWeight() belowMinRelayFeeRate := chainfee.SatPerKVByte(1500).FeePerKWeight() - realWorldMinRelayFeeRate := chainfee.SatPerKVByte(1952) + realWorldMinRelayFeeRate := chainfee.SatPerKVByte(3000) harnessMinRelayFeeRate := chainfee.SatPerKVByte(1000) defaultFeeRate := chainfee.SatPerKWeight(3125) + sendBelowFloorFeeRate := uint32(1) + sendBelowMinRelayFeeRate := uint32(2) t.lndHarness.SetFeeEstimateWithConf(belowFloorFeeRate, 6) t.lndHarness.SetMinRelayFeerate(realWorldMinRelayFeeRate) @@ -230,13 +232,13 @@ func testMinRelayFeeBump(t *harnessTest) { sendAsset( t, t.tapd, withReceiverAddresses(bobAddr), - withFeeRate(uint32(belowFloorFeeRate)), + withSatPerVByte(uint64(sendBelowFloorFeeRate)), withError("manual fee rate below floor"), ) sendAsset( t, t.tapd, withReceiverAddresses(bobAddr), - withFeeRate(uint32(belowMinRelayFeeRate)), + withSatPerVByte(uint64(sendBelowMinRelayFeeRate)), withError("feerate does not meet minrelayfee"), ) diff --git a/rpcserver/rpcserver.go b/rpcserver/rpcserver.go index 6b195ab3a5..7c9c8c945d 100644 --- a/rpcserver/rpcserver.go +++ b/rpcserver/rpcserver.go @@ -116,6 +116,10 @@ const ( // encoded as hop hints in a bolt11 invoice. maxRfqHopHints = 20 + // maxSendSatPerVByte is the highest sat/vB value that can be converted + // to sat/kw without overflowing the internal signed fee representation. + maxSendSatPerVByte = uint64(math.MaxInt64 / 1000) + // multiRfqNegotiationTimeout is the timeout within which we expect our // peer and our node to reach an agreement over a quote. This timeout // is a bit shorter in the context of multi-RFQ in order to not block @@ -832,6 +836,24 @@ func checkFeeRateSanity(ctx context.Context, rpcFeeRate chainfee.SatPerKWeight, } } +// parseSendFeeRate parses the legacy sat/kw fee field or the preferred +// sat_per_vbyte field from a SendAssetRequest. +func parseSendFeeRate(req *taprpc.SendAssetRequest) (chainfee.SatPerKWeight, + error) { + + if req.SatPerVbyte == 0 { + return chainfee.SatPerKWeight(req.FeeRate), nil + } + + if req.SatPerVbyte > maxSendSatPerVByte { + return 0, fmt.Errorf("manual fee rate exceeds maximum: "+ + "(sat_per_vbyte=%d, max=%d)", req.SatPerVbyte, + maxSendSatPerVByte) + } + + return chainfee.SatPerVByte(req.SatPerVbyte).FeePerKWeight(), nil +} + // FundBatch attempts to fund the current pending batch. func (r *RPCServer) FundBatch(ctx context.Context, req *mintrpc.FundBatchRequest) (*mintrpc.FundBatchResponse, error) { @@ -3798,8 +3820,12 @@ func (r *RPCServer) SendAsset(ctx context.Context, return nil, fmt.Errorf("error parsing addresses: %w", err) } + manualFeeRate, err := parseSendFeeRate(req) + if err != nil { + return nil, err + } feeRate, err := checkFeeRateSanity( - ctx, chainfee.SatPerKWeight(req.FeeRate), r.cfg.Lnd.WalletKit, + ctx, manualFeeRate, r.cfg.Lnd.WalletKit, ) if err != nil { return nil, err diff --git a/rpcserver/send_fee_rate_test.go b/rpcserver/send_fee_rate_test.go new file mode 100644 index 0000000000..62f86d3e43 --- /dev/null +++ b/rpcserver/send_fee_rate_test.go @@ -0,0 +1,65 @@ +package rpcserver + +import ( + "math" + "testing" + + "github.com/lightninglabs/taproot-assets/taprpc" + "github.com/lightningnetwork/lnd/lnwallet/chainfee" + "github.com/stretchr/testify/require" +) + +func TestParseSendFeeRate(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + req *taprpc.SendAssetRequest + expectedFee chainfee.SatPerKWeight + expectedErrStr string + }{ + { + name: "legacy fee rate", + req: &taprpc.SendAssetRequest{ + FeeRate: 1234, + }, + expectedFee: 1234, + }, + { + name: "sat per vbyte preferred", + req: &taprpc.SendAssetRequest{ + FeeRate: 1234, + SatPerVbyte: 5, + }, + expectedFee: chainfee.SatPerVByte(5).FeePerKWeight(), + }, + { + name: "sat per vbyte too large", + req: &taprpc.SendAssetRequest{ + SatPerVbyte: uint64(math.MaxInt64/1000) + 1, + }, + expectedErrStr: "manual fee rate exceeds maximum", + }, + } + + for _, testCase := range testCases { + testCase := testCase + + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + feeRate, err := parseSendFeeRate(testCase.req) + if testCase.expectedErrStr != "" { + require.ErrorContains( + t, + err, + testCase.expectedErrStr, + ) + return + } + + require.NoError(t, err) + require.Equal(t, testCase.expectedFee, feeRate) + }) + } +} diff --git a/taprpc/taprootassets.pb.go b/taprpc/taprootassets.pb.go index dea9a04962..1be75cd951 100644 --- a/taprpc/taprootassets.pb.go +++ b/taprpc/taprootassets.pb.go @@ -5708,7 +5708,10 @@ type SendAssetRequest struct { // exclusive, meaning that if addresses_with_amounts is set, then tap_addrs // must be empty, and vice versa. TapAddrs []string `protobuf:"bytes,1,rep,name=tap_addrs,json=tapAddrs,proto3" json:"tap_addrs,omitempty"` - // The optional fee rate to use for the minting transaction, in sat/kw. + // Deprecated: Use sat_per_vbyte instead. + // The optional fee rate to use for the anchor transaction, in sat/kw. + // + // Deprecated: Marked as deprecated in taprootassets.proto. FeeRate uint32 `protobuf:"varint,2,opt,name=fee_rate,json=feeRate,proto3" json:"fee_rate,omitempty"` // An optional short label for the send transfer. This label can be used to // track the progress of the transfer via the logs or an event subscription. @@ -5725,8 +5728,10 @@ type SendAssetRequest struct { // meaning that if addresses_with_amounts is set, then tap_addrs must be // empty, and vice versa. AddressesWithAmounts []*AddressWithAmount `protobuf:"bytes,5,rep,name=addresses_with_amounts,json=addressesWithAmounts,proto3" json:"addresses_with_amounts,omitempty"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + // The optional fee rate to use for the anchor transaction, in sat/vB. + SatPerVbyte uint64 `protobuf:"varint,8,opt,name=sat_per_vbyte,json=satPerVbyte,proto3" json:"sat_per_vbyte,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *SendAssetRequest) Reset() { @@ -5766,6 +5771,7 @@ func (x *SendAssetRequest) GetTapAddrs() []string { return nil } +// Deprecated: Marked as deprecated in taprootassets.proto. func (x *SendAssetRequest) GetFeeRate() uint32 { if x != nil { return x.FeeRate @@ -5794,6 +5800,13 @@ func (x *SendAssetRequest) GetAddressesWithAmounts() []*AddressWithAmount { return nil } +func (x *SendAssetRequest) GetSatPerVbyte() uint64 { + if x != nil { + return x.SatPerVbyte + } + return 0 +} + type AddressWithAmount struct { state protoimpl.MessageState `protogen:"open.v1"` // The TAP address to send assets to. @@ -7877,13 +7890,14 @@ const file_taprootassets_proto_rawDesc = "" + "\x05limit\x18\x06 \x01(\x05R\x05limit\x123\n" + "\tdirection\x18\a \x01(\x0e2\x15.taprpc.SortDirectionR\tdirection\"A\n" + "\x14AddrReceivesResponse\x12)\n" + - "\x06events\x18\x01 \x03(\v2\x11.taprpc.AddrEventR\x06events\"\xf3\x01\n" + + "\x06events\x18\x01 \x03(\v2\x11.taprpc.AddrEventR\x06events\"\x9b\x02\n" + "\x10SendAssetRequest\x12\x1b\n" + - "\ttap_addrs\x18\x01 \x03(\tR\btapAddrs\x12\x19\n" + - "\bfee_rate\x18\x02 \x01(\rR\afeeRate\x12\x14\n" + + "\ttap_addrs\x18\x01 \x03(\tR\btapAddrs\x12\x1d\n" + + "\bfee_rate\x18\x02 \x01(\rB\x02\x18\x01R\afeeRate\x12\x14\n" + "\x05label\x18\x03 \x01(\tR\x05label\x12@\n" + "\x1dskip_proof_courier_ping_check\x18\x04 \x01(\bR\x19skipProofCourierPingCheck\x12O\n" + - "\x16addresses_with_amounts\x18\x05 \x03(\v2\x19.taprpc.AddressWithAmountR\x14addressesWithAmounts\"F\n" + + "\x16addresses_with_amounts\x18\x05 \x03(\v2\x19.taprpc.AddressWithAmountR\x14addressesWithAmounts\x12\"\n" + + "\rsat_per_vbyte\x18\b \x01(\x04R\vsatPerVbyte\"F\n" + "\x11AddressWithAmount\x12\x19\n" + "\btap_addr\x18\x01 \x01(\tR\atapAddr\x12\x16\n" + "\x06amount\x18\x02 \x01(\x04R\x06amount\"\x85\x01\n" + diff --git a/taprpc/taprootassets.proto b/taprpc/taprootassets.proto index 9a50826da9..3b453cc47f 100644 --- a/taprpc/taprootassets.proto +++ b/taprpc/taprootassets.proto @@ -1655,8 +1655,9 @@ message SendAssetRequest { // must be empty, and vice versa. repeated string tap_addrs = 1; - // The optional fee rate to use for the minting transaction, in sat/kw. - uint32 fee_rate = 2; + // Deprecated: Use sat_per_vbyte instead. + // The optional fee rate to use for the anchor transaction, in sat/kw. + uint32 fee_rate = 2 [deprecated = true]; // An optional short label for the send transfer. This label can be used to // track the progress of the transfer via the logs or an event subscription. @@ -1675,6 +1676,9 @@ message SendAssetRequest { // meaning that if addresses_with_amounts is set, then tap_addrs must be // empty, and vice versa. repeated AddressWithAmount addresses_with_amounts = 5; + + // The optional fee rate to use for the anchor transaction, in sat/vB. + uint64 sat_per_vbyte = 6; } message AddressWithAmount { diff --git a/taprpc/taprootassets.swagger.json b/taprpc/taprootassets.swagger.json index 0ce2b859ca..19ee42424e 100644 --- a/taprpc/taprootassets.swagger.json +++ b/taprpc/taprootassets.swagger.json @@ -2978,7 +2978,7 @@ "fee_rate": { "type": "integer", "format": "int64", - "description": "The optional fee rate to use for the minting transaction, in sat/kw." + "description": "Deprecated: Use sat_per_vbyte instead.\nThe optional fee rate to use for the anchor transaction, in sat/kw." }, "label": { "type": "string", @@ -2995,6 +2995,11 @@ "$ref": "#/definitions/taprpcAddressWithAmount" }, "description": "A list of addresses and the amounts of asset units to send to them. This\nmust be used for V2 TAP addresses that don't specify an amount in the\naddress itself and allow the sender to choose the amount to send. The\ntap_addrs and addresses_with_amounts lists are mutually exclusive,\nmeaning that if addresses_with_amounts is set, then tap_addrs must be\nempty, and vice versa." + }, + "sat_per_vbyte": { + "type": "string", + "format": "uint64", + "description": "The optional fee rate to use for the anchor transaction, in sat/vB." } } },