Merge pull request #605 from lightninglabs/fee_estimation_testing

multi: anchor fee test coverage

Author: jharveyb

cmd/tapcli/assets.go

@@ -10,6 +10,7 @@ import (
 	"github.com/lightninglabs/taproot-assets/tapcfg"
 	"github.com/lightninglabs/taproot-assets/taprpc"
 	"github.com/lightninglabs/taproot-assets/taprpc/mintrpc"
+	"github.com/lightningnetwork/lnd/lnwallet/chainfee"
 	"github.com/urfave/cli"
 )
 
@@ -50,7 +51,7 @@ var (
 	groupByGroupName             = "by_group"
 	assetIDName                  = "asset_id"
 	shortResponseName            = "short"
-	feeRateName                  = "fee_rate"
+	feeRateName                  = "sat_per_vbyte"
 	assetAmountName              = "amount"
 	burnOverrideConfirmationName = "override_confirmation_destroy_assets"
 )
@@ -261,7 +262,7 @@ var finalizeBatchCommand = cli.Command{
 		},
 		cli.Uint64Flag{
 			Name: feeRateName,
-			Usage: "if set, the fee rate in sat/kw to use for " +
+			Usage: "if set, the fee rate in sat/vB to use for " +
 				"the minting transaction",
 		},
 	},
@@ -270,11 +271,20 @@ var finalizeBatchCommand = cli.Command{
 
 func parseFeeRate(ctx *cli.Context) (uint32, error) {
 	if ctx.IsSet(feeRateName) {
-		feeRate := ctx.Uint64(feeRateName)
-		if feeRate > math.MaxUint32 {
+		userFeeRate := ctx.Uint64(feeRateName)
+		if userFeeRate > math.MaxUint32 {
 			return 0, fmt.Errorf("fee rate exceeds 2^32")
 		}
 
+		// Convert from sat/vB to sat/kw. Round up to the fee floor if
+		// the specified feerate is too low.
+		feeRate := chainfee.SatPerKVByte(userFeeRate * 1000).
+			FeePerKWeight()
+
+		if feeRate < chainfee.FeePerKwFloor {
+			feeRate = chainfee.FeePerKwFloor
+		}
+
 		return uint32(feeRate), nil
 	}
 
@@ -531,7 +541,7 @@ var sendAssetsCommand = cli.Command{
 		},
 		cli.Uint64Flag{
 			Name: feeRateName,
-			Usage: "if set, the fee rate in sat/kw to use for " +
+			Usage: "if set, the fee rate in sat/vB to use for " +
 				"the anchor transaction",
 		},
 		// TODO(roasbeef): add arg for file name to write sender proof

go.mod

@@ -10,6 +10,8 @@ require (
 	github.com/btcsuite/btcd/chaincfg/chainhash v1.0.2
 	github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f
 	github.com/btcsuite/btcwallet v0.16.10-0.20231017144732-e3ff37491e9c
+	github.com/btcsuite/btcwallet/wallet/txrules v1.2.0
+	github.com/btcsuite/btcwallet/wallet/txsizes v1.2.3
 	github.com/caddyserver/certmagic v0.17.2
 	github.com/davecgh/go-spew v1.1.1
 	github.com/decred/dcrd/dcrec/secp256k1/v4 v4.0.1
@@ -60,8 +62,6 @@ require (
 	github.com/andybalholm/brotli v1.0.3 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/btcsuite/btcwallet/wallet/txauthor v1.3.2 // indirect
-	github.com/btcsuite/btcwallet/wallet/txrules v1.2.0 // indirect
-	github.com/btcsuite/btcwallet/wallet/txsizes v1.2.3 // indirect
 	github.com/btcsuite/btcwallet/walletdb v1.4.0 // indirect
 	github.com/btcsuite/btcwallet/wtxmgr v1.5.0 // indirect
 	github.com/btcsuite/go-socks v0.0.0-20170105172521-4720035b7bfd // indirect

itest/assertions.go

@@ -12,6 +12,7 @@ import (
 
 	"github.com/btcsuite/btcd/btcec/v2"
 	"github.com/btcsuite/btcd/btcec/v2/schnorr"
+	"github.com/btcsuite/btcd/btcutil"
 	"github.com/btcsuite/btcd/chaincfg/chainhash"
 	"github.com/btcsuite/btcd/rpcclient"
 	"github.com/btcsuite/btcd/wire"
@@ -24,6 +25,7 @@ import (
 	"github.com/lightninglabs/taproot-assets/universe"
 	"github.com/lightningnetwork/lnd/lnrpc/chainrpc"
 	"github.com/lightningnetwork/lnd/lntest/wait"
+	"github.com/lightningnetwork/lnd/lnwallet/chainfee"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/exp/maps"
 )
@@ -219,6 +221,75 @@ func AssertTxInBlock(t *testing.T, block *wire.MsgBlock,
 	return nil
 }
 
+// AssertTransferFeeRate checks that fee paid for the TX anchoring an asset
+// transfer is close to the expected fee for that TX, at a given fee rate.
+func AssertTransferFeeRate(t *testing.T, minerClient *rpcclient.Client,
+	transferResp *taprpc.SendAssetResponse, inputAmt int64,
+	feeRate chainfee.SatPerKWeight, roundFee bool) {
+
+	txid, err := chainhash.NewHash(transferResp.Transfer.AnchorTxHash)
+	require.NoError(t, err)
+
+	AssertFeeRate(t, minerClient, inputAmt, txid, feeRate, roundFee)
+}
+
+// AssertFeeRate checks that the fee paid for a given TX is close to the
+// expected fee for the same TX, at a given fee rate.
+func AssertFeeRate(t *testing.T, minerClient *rpcclient.Client, inputAmt int64,
+	txid *chainhash.Hash, feeRate chainfee.SatPerKWeight, roundFee bool) {
+
+	var (
+		outputValue                 float64
+		expectedFee, maxOverpayment btcutil.Amount
+		maxVsizeDifference          = int64(2)
+	)
+
+	verboseTx, err := minerClient.GetRawTransactionVerbose(txid)
+	require.NoError(t, err)
+
+	vsize := verboseTx.Vsize
+	for _, vout := range verboseTx.Vout {
+		outputValue += vout.Value
+	}
+
+	t.Logf("TX vsize of %d bytes", vsize)
+
+	btcOutputValue, err := btcutil.NewAmount(outputValue)
+	require.NoError(t, err)
+
+	actualFee := inputAmt - int64(btcOutputValue)
+
+	switch {
+	case roundFee:
+		// Replicate the rounding performed when calling `FundPsbt`.
+		feeSatPerVbyte := uint64(feeRate.FeePerKVByte()) / 1000
+		roundedFeeRate := chainfee.SatPerKVByte(
+			feeSatPerVbyte * 1000,
+		).FeePerKWeight()
+
+		expectedFee = roundedFeeRate.FeePerKVByte().
+			FeeForVSize(int64(vsize))
+		maxOverpayment = roundedFeeRate.FeePerKVByte().
+			FeeForVSize(maxVsizeDifference)
+
+	default:
+		expectedFee = feeRate.FeePerKVByte().
+			FeeForVSize(int64(vsize))
+		maxOverpayment = feeRate.FeePerKVByte().
+			FeeForVSize(maxVsizeDifference)
+	}
+
+	// The actual fee may be higher than the expected fee after
+	// confirmation, as the freighter makes a worst-case estimate of the TX
+	// vsize. The gap between these two fees should still be small.
+	require.GreaterOrEqual(t, actualFee, int64(expectedFee))
+
+	overpaidFee := actualFee - int64(expectedFee)
+	require.LessOrEqual(t, overpaidFee, int64(maxOverpayment))
+
+	t.Logf("Correct fee of %d sats", actualFee)
+}
+
 // WaitForBatchState polls until the planter has reached the desired state with
 // the given batch.
 func WaitForBatchState(t *testing.T, ctx context.Context,
@@ -640,16 +711,16 @@ func ConfirmAndAssertOutboundTransfer(t *testing.T,
 	expectedAmounts []uint64, currentTransferIdx,
 	numTransfers int) *wire.MsgBlock {
 
-	return ConfirmAndAssetOutboundTransferWithOutputs(
+	return ConfirmAndAssertOutboundTransferWithOutputs(
 		t, minerClient, sender, sendResp, assetID, expectedAmounts,
 		currentTransferIdx, numTransfers, 2,
 	)
 }
 
-// ConfirmAndAssetOutboundTransferWithOutputs makes sure the given outbound
+// ConfirmAndAssertOutboundTransferWithOutputs makes sure the given outbound
 // transfer has the correct state and number of outputs before confirming it and
 // then asserting the confirmed state with the node.
-func ConfirmAndAssetOutboundTransferWithOutputs(t *testing.T,
+func ConfirmAndAssertOutboundTransferWithOutputs(t *testing.T,
 	minerClient *rpcclient.Client, sender TapdClient,
 	sendResp *taprpc.SendAssetResponse, assetID []byte,
 	expectedAmounts []uint64, currentTransferIdx,

itest/burn_test.go

@@ -99,7 +99,7 @@ func testBurnAssets(t *harnessTest) {
 		},
 	)
 	require.NoError(t.t, err)
-	ConfirmAndAssetOutboundTransferWithOutputs(
+	ConfirmAndAssertOutboundTransferWithOutputs(
 		t.t, minerClient, t.tapd, sendResp, simpleAssetGen.AssetId,
 		outputAmounts, 0, 1, numOutputs,
 	)

itest/fee_estimation_test.go

@@ -0,0 +1,181 @@
+package itest
+
+import (
+	"context"
+
+	"github.com/btcsuite/btcd/btcutil"
+	"github.com/btcsuite/btcd/wire"
+	"github.com/lightninglabs/taproot-assets/taprpc"
+	"github.com/lightningnetwork/lnd/lnrpc"
+	"github.com/lightningnetwork/lnd/lnwallet/chainfee"
+	"github.com/stretchr/testify/require"
+)
+
+// testFeeEstimation tests that we're able to spend outputs of various script
+// types, and that the fee estimator and TX size estimator used during asset
+// transfers are accurate.
+func testFeeEstimation(t *harnessTest) {
+	var (
+		// Make a ladder of UTXO values so use order is deterministic.
+		anchorAmounts = []int64{10000, 9990, 9980, 9970}
+
+		// The default feerate in the itests is 12.5 sat/vB, but we
+		// define it here explicitly to use for assertions.
+		defaultFeeRate   = chainfee.SatPerKWeight(3125)
+		higherFeeRate    = defaultFeeRate * 2
+		excessiveFeeRate = defaultFeeRate * 8
+		lowFeeRate       = chainfee.SatPerKWeight(500)
+
+		// We will mint assets using the largest NP2WKH output, and then
+		// use all three output types for transfers.
+		initialUTXOs = []*UTXORequest{
+			{
+				Type:   lnrpc.AddressType_NESTED_PUBKEY_HASH,
+				Amount: anchorAmounts[0],
+			},
+			{
+				Type:   lnrpc.AddressType_NESTED_PUBKEY_HASH,
+				Amount: anchorAmounts[1],
+			},
+			{
+				Type:   lnrpc.AddressType_WITNESS_PUBKEY_HASH,
+				Amount: anchorAmounts[2],
+			},
+			{
+				Type:   lnrpc.AddressType_TAPROOT_PUBKEY,
+				Amount: anchorAmounts[3],
+			},
+		}
+	)
+
+	ctxb := context.Background()
+	ctxt, cancel := context.WithTimeout(ctxb, defaultWaitTimeout)
+	defer cancel()
+
+	// 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)
+
+	// Mint some assets with a NP2WPKH input, which will give us an anchor
+	// output to spend for a transfer.
+	rpcAssets := MintAssetsConfirmBatch(
+		t.t, t.lndHarness.Miner.Client, t.tapd, simpleAssets,
+	)
+
+	// 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 the minting TX with a rounded fee rate as the minter does
+	// not adjust the fee rate of the TX after it was funded by our backing
+	// wallet.
+	AssertFeeRate(
+		t.t, t.lndHarness.Miner.Client, anchorAmounts[0],
+		&mintOutpoint.Hash, defaultFeeRate, true,
+	)
+
+	// Split the normal asset to create a transfer with two anchor outputs.
+	normalAssetId := rpcAssets[0].AssetGenesis.AssetId
+	splitAmount := rpcAssets[0].Amount / 2
+	addr, err := t.tapd.NewAddr(
+		ctxt, &taprpc.NewAddrRequest{
+			AssetId: normalAssetId,
+			Amt:     splitAmount,
+		},
+	)
+	require.NoError(t.t, err)
+
+	AssertAddrCreated(t.t, t.tapd, rpcAssets[0], addr)
+	sendResp := sendAssetsToAddr(t, t.tapd, addr)
+
+	transferIdx := 0
+	ConfirmAndAssertOutboundTransfer(
+		t.t, t.lndHarness.Miner.Client, t.tapd, sendResp, normalAssetId,
+		[]uint64{splitAmount, splitAmount}, transferIdx, transferIdx+1,
+	)
+	transferIdx += 1
+	AssertNonInteractiveRecvComplete(t.t, t.tapd, transferIdx)
+
+	sendInputAmt := anchorAmounts[1] + 1000
+	AssertTransferFeeRate(
+		t.t, t.lndHarness.Miner.Client, sendResp, sendInputAmt,
+		defaultFeeRate, false,
+	)
+
+	// Double the fee rate to 25 sat/vB before performing another transfer.
+	t.lndHarness.SetFeeEstimateWithConf(higherFeeRate, 6)
+
+	secondSplitAmount := splitAmount / 2
+	addr2, err := t.tapd.NewAddr(
+		ctxt, &taprpc.NewAddrRequest{
+			AssetId: normalAssetId,
+			Amt:     secondSplitAmount,
+		},
+	)
+	require.NoError(t.t, err)
+
+	AssertAddrCreated(t.t, t.tapd, rpcAssets[0], addr2)
+	sendResp = sendAssetsToAddr(t, t.tapd, addr2)
+
+	ConfirmAndAssertOutboundTransfer(
+		t.t, t.lndHarness.Miner.Client, t.tapd, sendResp, normalAssetId,
+		[]uint64{secondSplitAmount, secondSplitAmount},
+		transferIdx, transferIdx+1,
+	)
+	transferIdx += 1
+	AssertNonInteractiveRecvComplete(t.t, t.tapd, transferIdx)
+
+	sendInputAmt = anchorAmounts[2] + 1000
+	AssertTransferFeeRate(
+		t.t, t.lndHarness.Miner.Client, sendResp, sendInputAmt,
+		higherFeeRate, false,
+	)
+
+	// If we quadruple the fee rate, the freighter should fail during input
+	// selection.
+	t.lndHarness.SetFeeEstimateWithConf(excessiveFeeRate, 6)
+
+	thirdSplitAmount := splitAmount / 4
+	addr3, err := t.tapd.NewAddr(
+		ctxt, &taprpc.NewAddrRequest{
+			AssetId: normalAssetId,
+			Amt:     thirdSplitAmount,
+		},
+	)
+	require.NoError(t.t, err)
+
+	AssertAddrCreated(t.t, t.tapd, rpcAssets[0], addr3)
+	_, err = t.tapd.SendAsset(ctxt, &taprpc.SendAssetRequest{
+		TapAddrs: []string{addr3.Encoded},
+	})
+	require.ErrorContains(t.t, err, "insufficient funds available")
+
+	// The transfer should also be rejected if the manually-specified
+	// feerate fails the sanity check against the fee estimator's fee floor
+	// of 253 sat/kw, or 1.012 sat/vB.
+	_, err = t.tapd.SendAsset(ctxt, &taprpc.SendAssetRequest{
+		TapAddrs: []string{addr3.Encoded},
+		FeeRate:  uint32(chainfee.FeePerKwFloor) - 1,
+	})
+	require.ErrorContains(t.t, err, "manual fee rate below floor")
+	// After failure at the high feerate, we should still be able to make a
+	// transfer at a very low feerate.
+	t.lndHarness.SetFeeEstimateWithConf(lowFeeRate, 6)
+	sendResp = sendAssetsToAddr(t, t.tapd, addr3)
+
+	ConfirmAndAssertOutboundTransfer(
+		t.t, t.lndHarness.Miner.Client, t.tapd, sendResp, normalAssetId,
+		[]uint64{thirdSplitAmount, thirdSplitAmount},
+		transferIdx, transferIdx+1,
+	)
+	transferIdx += 1
+	AssertNonInteractiveRecvComplete(t.t, t.tapd, transferIdx)
+
+	sendInputAmt = anchorAmounts[3] + 1000
+	AssertTransferFeeRate(
+		t.t, t.lndHarness.Miner.Client, sendResp, sendInputAmt,
+		lowFeeRate, false,
+	)
+}