Merge pull request #1191 from lightninglabs/min-relay-fee-bump

Fee bumping when fee estimation doesn't meet min relay fee

itest/addrs_test.go

@@ -1078,24 +1078,70 @@ func sendProofUniRPC(t *harnessTest, src, dst *tapdHarness, scriptKey []byte,
 	return importResp
 }
 
-// sendAssetsToAddr spends the given input asset and sends the amount specified
+// sendOptions is a struct that holds a SendAssetRequest and an
+// optional error string that should be tested against.
+type sendOptions struct {
+	sendAssetRequest taprpc.SendAssetRequest
+	errText          string
+}
+
+// sendOption is a functional option for configuring the sendAssets call.
+type sendOption func(*sendOptions)
+
+// withReceiverAddresses is an option to specify the receiver addresses for the
+// send.
+func withReceiverAddresses(addrs ...*taprpc.Addr) sendOption {
+	return func(options *sendOptions) {
+		encodedAddrs := make([]string, len(addrs))
+		for i, addr := range addrs {
+			encodedAddrs[i] = addr.Encoded
+		}
+		options.sendAssetRequest.TapAddrs = encodedAddrs
+	}
+}
+
+// withFeeRate is an option to specify the fee rate for the send.
+func withFeeRate(feeRate uint32) sendOption {
+	return func(options *sendOptions) {
+		options.sendAssetRequest.FeeRate = feeRate
+	}
+}
+
+// withError is an option to specify the string that is expected in the error
+// returned by the SendAsset call.
+func withError(errorText string) sendOption {
+	return func(options *sendOptions) {
+		options.errText = errorText
+	}
+}
+
+// sendAsset spends the given input asset and sends the amount specified
 // in the address to the Taproot output derived from the address.
-func sendAssetsToAddr(t *harnessTest, sender *tapdHarness,
-	receiverAddrs ...*taprpc.Addr) (*taprpc.SendAssetResponse,
+func sendAsset(t *harnessTest, sender *tapdHarness,
+	opts ...sendOption) (*taprpc.SendAssetResponse,
 	*EventSubscription[*taprpc.SendEvent]) {
 
 	ctxb := context.Background()
 	ctxt, cancel := context.WithTimeout(ctxb, defaultWaitTimeout)
 	defer cancel()
 
-	require.NotEmpty(t.t, receiverAddrs)
-	scriptKey := receiverAddrs[0].ScriptKey
+	// Create base request that will be modified by options.
+	options := &sendOptions{}
 
-	encodedAddrs := make([]string, len(receiverAddrs))
-	for i, addr := range receiverAddrs {
-		encodedAddrs[i] = addr.Encoded
+	// Apply all the functional options.
+	for _, opt := range opts {
+		opt(options)
 	}
 
+	require.NotEmpty(t.t, options.sendAssetRequest.TapAddrs)
+
+	// We need the first address's scriptkey to subscribe to events.
+	firstAddr, err := address.DecodeAddress(
+		options.sendAssetRequest.TapAddrs[0], &address.RegressionNetTap,
+	)
+	require.NoError(t.t, err)
+	scriptKey := firstAddr.ScriptKey.SerializeCompressed()
+
 	ctxc, streamCancel := context.WithCancel(ctxb)
 	stream, err := sender.SubscribeSendEvents(
 		ctxc, &taprpc.SubscribeSendEventsRequest{
@@ -1108,9 +1154,12 @@ func sendAssetsToAddr(t *harnessTest, sender *tapdHarness,
 		Cancel:            streamCancel,
 	}
 
-	resp, err := sender.SendAsset(ctxt, &taprpc.SendAssetRequest{
-		TapAddrs: encodedAddrs,
-	})
+	resp, err := sender.SendAsset(ctxt, &options.sendAssetRequest)
+	if options.errText != "" {
+		require.ErrorContains(t.t, err, options.errText)
+		return nil, nil
+	}
+
 	require.NoError(t.t, err)
 
 	// We'll get events up to the point where we broadcast the transaction.
@@ -1123,6 +1172,15 @@ func sendAssetsToAddr(t *harnessTest, sender *tapdHarness,
 	return resp, sub
 }
 
+// sendAssetsToAddr is a variadic wrapper around sendAsset that enables passsing
+// a multitude of addresses.
+func sendAssetsToAddr(t *harnessTest, sender *tapdHarness,
+	receiverAddrs ...*taprpc.Addr) (*taprpc.SendAssetResponse,
+	*EventSubscription[*taprpc.SendEvent]) {
+
+	return sendAsset(t, sender, withReceiverAddresses(receiverAddrs...))
+}
+
 // fundAddressSendPacket asks the wallet to fund a new virtual packet with the
 // given address as the single receiver.
 func fundAddressSendPacket(t *harnessTest, tapd *tapdHarness,

itest/send_test.go

@@ -9,6 +9,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/btcsuite/btcd/btcutil"
+	"github.com/btcsuite/btcd/wire"
 	"github.com/lightninglabs/taproot-assets/internal/test"
 	"github.com/lightninglabs/taproot-assets/proof"
 	"github.com/lightninglabs/taproot-assets/tapfreighter"
@@ -18,7 +20,9 @@ import (
 	"github.com/lightninglabs/taproot-assets/taprpc/tapdevrpc"
 	unirpc "github.com/lightninglabs/taproot-assets/taprpc/universerpc"
 	"github.com/lightninglabs/taproot-assets/tapsend"
+	"github.com/lightningnetwork/lnd/lnrpc"
 	"github.com/lightningnetwork/lnd/lntest/wait"
+	"github.com/lightningnetwork/lnd/lnwallet/chainfee"
 	"github.com/stretchr/testify/require"
 )
 
@@ -140,6 +144,144 @@ func testBasicSendUnidirectional(t *harnessTest) {
 	wg.Wait()
 }
 
+// testMinRelayFeeBump tests that if the fee estimation is below the min relay
+// fee the feerate is bumped to the min relay fee for both the minting
+// transaction and a basic asset send.
+func testMinRelayFeeBump(t *harnessTest) {
+	var ctxb = context.Background()
+
+	const numUnits = 10
+
+	// Subscribe to receive assent send events from primary tapd node.
+	events := SubscribeSendEvents(t.t, t.tapd)
+
+	// We will mint assets using the first output and then use the second
+	// output for the transfer. This ensures a valid fee calculation.
+	initialUTXOs := []*UTXORequest{
+		{
+			Type:   lnrpc.AddressType_NESTED_PUBKEY_HASH,
+			Amount: 1_000_000,
+		},
+		{
+			Type:   lnrpc.AddressType_NESTED_PUBKEY_HASH,
+			Amount: 999_990,
+		},
+	}
+
+	// Set the initial state of the wallet of the first node. The wallet
+	// state will reset at the end of this test.
+	SetNodeUTXOs(t, t.lndHarness.Alice, btcutil.Amount(1), initialUTXOs)
+	defer ResetNodeWallet(t, t.lndHarness.Alice)
+
+	// Set the min relay fee to a higher value than the fee rate that will
+	// be returned by the fee estimation.
+	lowFeeRate := chainfee.SatPerVByte(1).FeePerKWeight()
+	highMinRelayFeeRate := chainfee.SatPerVByte(2).FeePerKVByte()
+	defaultMinRelayFeeRate := chainfee.SatPerVByte(1).FeePerKVByte()
+	defaultFeeRate := chainfee.SatPerKWeight(3125)
+	t.lndHarness.SetFeeEstimateWithConf(lowFeeRate, 6)
+	t.lndHarness.SetMinRelayFeerate(highMinRelayFeeRate)
+
+	// Reset all fee rates to their default value at the end of this test.
+	defer t.lndHarness.SetMinRelayFeerate(defaultMinRelayFeeRate)
+	defer t.lndHarness.SetFeeEstimateWithConf(defaultFeeRate, 6)
+
+	// First, we'll make a normal assets with enough units to allow us to
+	// send it around a few times.
+	MintAssetsConfirmBatch(
+		t.t, t.lndHarness.Miner().Client, t.tapd,
+		[]*mintrpc.MintAssetRequest{issuableAssets[0]},
+		WithFeeRate(uint32(lowFeeRate)),
+		WithError("manual fee rate below floor"),
+	)
+
+	MintAssetsConfirmBatch(
+		t.t, t.lndHarness.Miner().Client, t.tapd,
+		[]*mintrpc.MintAssetRequest{issuableAssets[0]},
+		WithFeeRate(uint32(lowFeeRate)+10),
+		WithError("feerate does not meet minrelayfee"),
+	)
+
+	rpcAssets := MintAssetsConfirmBatch(
+		t.t, t.lndHarness.Miner().Client, t.tapd,
+		[]*mintrpc.MintAssetRequest{issuableAssets[0]},
+	)
+
+	genInfo := rpcAssets[0].AssetGenesis
+
+	// Check the final fee rate of the mint TX.
+	rpcMintOutpoint := rpcAssets[0].ChainAnchor.AnchorOutpoint
+	mintOutpoint, err := wire.NewOutPointFromString(rpcMintOutpoint)
+	require.NoError(t.t, err)
+
+	// We check whether the minting TX is bumped to the min relay fee.
+	AssertFeeRate(
+		t.t, t.lndHarness.Miner().Client, initialUTXOs[0].Amount,
+		&mintOutpoint.Hash, highMinRelayFeeRate.FeePerKWeight(),
+	)
+
+	// Now that we have the asset created, we'll make a new node that'll
+	// serve as the node which'll receive the assets. The existing tapd
+	// node will be used to synchronize universe state.
+	secondTapd := setupTapdHarness(
+		t.t, t, t.lndHarness.Bob, t.universeServer,
+	)
+	defer func() {
+		require.NoError(t.t, secondTapd.stop(!*noDelete))
+	}()
+
+	// Next, we'll attempt to complete two transfers with distinct
+	// addresses from our main node to Bob.
+	currentUnits := issuableAssets[0].Asset.Amount
+
+	// Issue a single address which will be reused for each send.
+	bobAddr, err := secondTapd.NewAddr(ctxb, &taprpc.NewAddrRequest{
+		AssetId:      genInfo.AssetId,
+		Amt:          numUnits,
+		AssetVersion: rpcAssets[0].Version,
+	})
+	require.NoError(t.t, err)
+
+	// Deduct what we sent from the expected current number of
+	// units.
+	currentUnits -= numUnits
+
+	AssertAddrCreated(t.t, secondTapd, rpcAssets[0], bobAddr)
+
+	sendAsset(
+		t, t.tapd, withReceiverAddresses(bobAddr),
+		withFeeRate(uint32(lowFeeRate)),
+		withError("manual fee rate below floor"),
+	)
+
+	sendAsset(
+		t, t.tapd, withReceiverAddresses(bobAddr),
+		withFeeRate(uint32(lowFeeRate)+10),
+		withError("feerate does not meet minrelayfee"),
+	)
+
+	sendResp, sendEvents := sendAssetsToAddr(t, t.tapd, bobAddr)
+
+	ConfirmAndAssertOutboundTransfer(
+		t.t, t.lndHarness.Miner().Client, t.tapd, sendResp,
+		genInfo.AssetId,
+		[]uint64{currentUnits, numUnits}, 0, 1,
+	)
+
+	sendInputAmt := initialUTXOs[1].Amount + 1000
+	AssertTransferFeeRate(
+		t.t, t.lndHarness.Miner().Client, sendResp, sendInputAmt,
+		highMinRelayFeeRate.FeePerKWeight(),
+	)
+
+	AssertNonInteractiveRecvComplete(t.t, secondTapd, 1)
+	AssertSendEventsComplete(t.t, bobAddr.ScriptKey, sendEvents)
+
+	// Close event stream.
+	err = events.CloseSend()
+	require.NoError(t.t, err)
+}
+
 // testRestartReceiver tests that the receiver node's asset balance after a
 // single asset transfer does not change if the receiver node restarts.
 // Before the addition of this test, after restarting the receiver node

itest/test_list_on_test.go

@@ -81,6 +81,10 @@ var testCases = []*testCase{
 		name: "basic send unidirectional",
 		test: testBasicSendUnidirectional,
 	},
+	{
+		name: "min relay fee bump",
+		test: testMinRelayFeeBump,
+	},
 	{
 		name: "restart receiver check balance",
 		test: testRestartReceiverCheckBalance,

itest/utils.go

@@ -266,6 +266,8 @@ type MintOptions struct {
 	mintingTimeout  time.Duration
 	siblingBranch   *mintrpc.FinalizeBatchRequest_Branch
 	siblingFullTree *mintrpc.FinalizeBatchRequest_FullTree
+	feeRate         uint32
+	errText         string
 }
 
 func DefaultMintOptions() *MintOptions {
@@ -292,6 +294,20 @@ func WithSiblingTree(tree mintrpc.FinalizeBatchRequest_FullTree) MintOption {
 	}
 }
 
+func WithFeeRate(feeRate uint32) MintOption {
+	return func(options *MintOptions) {
+		options.feeRate = feeRate
+	}
+}
+
+// WithError is an option to specify the string that is expected in the error
+// returned by the FinalizeBatch call.
+func WithError(errorText string) MintOption {
+	return func(options *MintOptions) {
+		options.errText = errorText
+	}
+}
+
 func BuildMintingBatch(t *testing.T, tapClient TapdClient,
 	assetRequests []*mintrpc.MintAssetRequest, opts ...MintOption) {
 
@@ -334,9 +350,27 @@ func FinalizeBatchUnconfirmed(t *testing.T, minerClient *rpcclient.Client,
 	if options.siblingFullTree != nil {
 		finalizeReq.BatchSibling = options.siblingFullTree
 	}
+	if options.feeRate > 0 {
+		finalizeReq.FeeRate = options.feeRate
+	}
 
 	// Instruct the daemon to finalize the batch.
 	batchResp, err := tapClient.FinalizeBatch(ctxt, finalizeReq)
+
+	// If we expect an error, check for it and cancel the batch if it's
+	// found.
+	if options.errText != "" {
+		require.ErrorContains(t, err, options.errText)
+		cancelBatchKey, err := tapClient.CancelBatch(
+			ctxt, &mintrpc.CancelBatchRequest{},
+		)
+		require.NoError(t, err)
+		require.NotEmpty(t, cancelBatchKey.BatchKey)
+		return chainhash.Hash{}, nil
+	}
+
+	// If we don't expect an error, we confirm that the batch has been
+	// broadcast.
 	require.NoError(t, err)
 	require.NotEmpty(t, batchResp.Batch)
 	require.Len(t, batchResp.Batch.Assets, len(assetRequests))
@@ -443,6 +477,11 @@ func MintAssetsConfirmBatch(t *testing.T, minerClient *rpcclient.Client,
 	tapClient TapdClient, assetRequests []*mintrpc.MintAssetRequest,
 	opts ...MintOption) []*taprpc.Asset {
 
+	options := DefaultMintOptions()
+	for _, opt := range opts {
+		opt(options)
+	}
+
 	ctxc, streamCancel := context.WithCancel(context.Background())
 	stream, err := tapClient.SubscribeMintEvents(
 		ctxc, &mintrpc.SubscribeMintEventsRequest{},
@@ -457,6 +496,13 @@ func MintAssetsConfirmBatch(t *testing.T, minerClient *rpcclient.Client,
 		t, minerClient, tapClient, assetRequests, opts...,
 	)
 
+	// If we expect an error, we know that the error has successfully
+	// occurred during MintAssetUnconfirmed so we don't need to confirm the
+	// batch and can return here.
+	if options.errText != "" {
+		return nil
+	}
+
 	return ConfirmBatch(
 		t, minerClient, tapClient, assetRequests, sub, mintTXID,
 		batchKey, opts...,