Repository: lightninglabs/taproot-assets

Issue:
[feature]: Remove rounding of fees after bumping to LND `v0.19.0`
LND will introduce support for `sat_per_kw` with the `FundPsbt` RPC endpoint in `v0.19.0`. (See: https://github.com/lightningnetwork/lnd/pull/9013)

We could use this in `tapd` which would remove the need for rounding up fees to keep them above the `min_relay_fee`. This rounding fix was introduced with #1354.

This issue is a todo for once v0.19.0 is released. 

Be careful to also bump the minimum version or make the rounding dependent on the version of `lnd` being used.


Discussion:
@Roasbeef: Can we close this now?@guggero: No, not yet. But created a fix in #1600.

Solution patch:
diff --git a/tapcfg/config.go b/tapcfg/config.go
index b9441e7f..d7b5ed0e 100644
--- a/tapcfg/config.go
+++ b/tapcfg/config.go
@@ -195,8 +195,8 @@ var (
 	// required in lnd to run tapd.
 	minimalCompatibleVersion = &verrpc.Version{
 		AppMajor: 0,
-		AppMinor: 18,
-		AppPatch: 4,
+		AppMinor: 19,
+		AppPatch: 0,
 
 		// We don't actually require the invoicesrpc calls. But if we
 		// try to use lndclient on an lnd that doesn't have it enabled,
diff --git a/tapdb/postgres.go b/tapdb/postgres.go
index a069a9cb..654c1381 100644
--- a/tapdb/postgres.go
+++ b/tapdb/postgres.go
@@ -171,7 +171,7 @@ func (s *PostgresStore) ExecuteMigrations(target MigrationTarget,
 
 // NewTestPostgresDB is a helper function that creates a Postgres database for
 // testing.
-func NewTestPostgresDB(t *testing.T) *PostgresStore {
+func NewTestPostgresDB(t testing.TB) *PostgresStore {
 	t.Helper()
 
 	t.Logf("Creating new Postgres DB for testing")
@@ -189,7 +189,7 @@ func NewTestPostgresDB(t *testing.T) *PostgresStore {
 
 // NewTestPostgresDBWithVersion is a helper function that creates a Postgres
 // database for testing and migrates it to the given version.
-func NewTestPostgresDBWithVersion(t *testing.T, version uint) *PostgresStore {
+func NewTestPostgresDBWithVersion(t testing.TB, version uint) *PostgresStore {
 	t.Helper()
 
 	t.Logf("Creating new Postgres DB for testing, migrating to version %d",
diff --git a/tapdb/postgres_fixture.go b/tapdb/postgres_fixture.go
index 7907b2af..8cddc583 100644
--- a/tapdb/postgres_fixture.go
+++ b/tapdb/postgres_fixture.go
@@ -35,7 +35,7 @@ type TestPgFixture struct {
 // NewTestPgFixture constructs a new TestPgFixture starting up a docker
 // container running Postgres 11. The started container will expire in after
 // the passed duration.
-func NewTestPgFixture(t *testing.T, expiry time.Duration,
+func NewTestPgFixture(t testing.TB, expiry time.Duration,
 	autoRemove bool) *TestPgFixture {
 
 	// Use a sensible default on Windows (tcp/http) and linux/osx (socket)
@@ -122,13 +122,13 @@ func (f *TestPgFixture) GetConfig() *PostgresConfig {
 }
 
 // TearDown stops the underlying docker container.
-func (f *TestPgFixture) TearDown(t *testing.T) {
+func (f *TestPgFixture) TearDown(t testing.TB) {
 	err := f.pool.Purge(f.resource)
 	require.NoError(t, err, "Could not purge resource")
 }
 
 // ClearDB clears the database.
-func (f *TestPgFixture) ClearDB(t *testing.T) {
+func (f *TestPgFixture) ClearDB(t testing.TB) {
 	dbConn, err := sql.Open("postgres", f.GetDSN())
 	require.NoError(t, err)
 
diff --git a/tapdb/sqlite.go b/tapdb/sqlite.go
index b5ac730c..0d107da4 100644
--- a/tapdb/sqlite.go
+++ b/tapdb/sqlite.go
@@ -256,7 +256,7 @@ func (s *SqliteStore) ExecuteMigrations(target MigrationTarget,
 
 // NewTestSqliteDB is a helper function that creates an SQLite database for
 // testing.
-func NewTestSqliteDB(t *testing.T) *SqliteStore {
+func NewTestSqliteDB(t testing.TB) *SqliteStore {
 	t.Helper()
 
 	// TODO(roasbeef): if we pass :memory: for the file name, then we get
@@ -269,7 +269,7 @@ func NewTestSqliteDB(t *testing.T) *SqliteStore {
 
 // NewTestSqliteDbHandleFromPath is a helper function that creates a SQLite
 // database handle given a database file path.
-func NewTestSqliteDbHandleFromPath(t *testing.T, dbPath string) *SqliteStore {
+func NewTestSqliteDbHandleFromPath(t testing.TB, dbPath string) *SqliteStore {
 	t.Helper()
 
 	sqlDB, err := NewSqliteStore(&SqliteConfig{
@@ -287,7 +287,7 @@ func NewTestSqliteDbHandleFromPath(t *testing.T, dbPath string) *SqliteStore {
 
 // NewTestSqliteDBWithVersion is a helper function that creates an SQLite
 // database for testing and migrates it to the given version.
-func NewTestSqliteDBWithVersion(t *testing.T, version uint) *SqliteStore {
+func NewTestSqliteDBWithVersion(t testing.TB, version uint) *SqliteStore {
 	t.Helper()
 
 	t.Logf("Creating new SQLite DB for testing, migrating to version %d",
diff --git a/wallet_anchor.go b/wallet_anchor.go
index 78015102..2669dd28 100644
--- a/wallet_anchor.go
+++ b/wallet_anchor.go
@@ -76,17 +76,11 @@ func (l *LndRpcWalletAnchor) FundPsbt(ctx context.Context, packet *psbt.Packet,
 		}
 	}
 
-	// We'll convert the fee rate to sat/vbyte as that's what the FundPsbt
-	// expects. We round up to the nearest whole unit to prevent issues
-	// where the fee doesn't meet the min_relay_fee because of rounding
-	// down.
-	satPerVByte := uint64(math.Ceil(float64(feeRate.FeePerKVByte()) / 1000))
-
 	pkt, changeIndex, leasedUtxos, err := l.lnd.WalletKit.FundPsbt(
 		ctx, &walletrpc.FundPsbtRequest{
 			Template: fundTemplate,
-			Fees: &walletrpc.FundPsbtRequest_SatPerVbyte{
-				SatPerVbyte: satPerVByte,
+			Fees: &walletrpc.FundPsbtRequest_SatPerKw{
+				SatPerKw: uint64(feeRate),
 			},
 			MinConfs:   int32(minConfs),
 			ChangeType: defaultChangeType,



Test patch:
diff --git a/itest/send_test.go b/itest/send_test.go
index 8f446f42..fc087cfe 100644
--- a/itest/send_test.go
+++ b/itest/send_test.go
@@ -160,13 +160,6 @@ func testMinRelayFeeBump(t *harnessTest) {
 	harnessMinRelayFeeRate := chainfee.SatPerKVByte(1000)
 	defaultFeeRate := chainfee.SatPerKWeight(3125)
 
-	// roundUpMinRelayFeeRate is the converted fee rate to sat/vbyte as
-	// that's what the FundPsbt expects. We add 999 to round up to the
-	// nearest 1000. We'll use this with `AssertFeerate`.
-	roundUpMinRelayFeeRate := chainfee.SatPerVByte(
-		(realWorldMinRelayFeeRate + 999) / 1000,
-	)
-
 	t.lndHarness.SetFeeEstimateWithConf(belowFloorFeeRate, 6)
 	t.lndHarness.SetMinRelayFeerate(realWorldMinRelayFeeRate)
 
@@ -205,7 +198,7 @@ func testMinRelayFeeBump(t *harnessTest) {
 	// 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, roundUpMinRelayFeeRate.FeePerKWeight(),
+		&mintOutpoint.Hash, realWorldMinRelayFeeRate.FeePerKWeight(),
 	)
 
 	// Now that we have the asset created, we'll make a new node that'll
@@ -258,7 +251,7 @@ func testMinRelayFeeBump(t *harnessTest) {
 	sendInputAmt := initialUTXOs[1].Amount + 1000
 	AssertTransferFeeRate(
 		t.t, t.lndHarness.Miner().Client, sendResp, sendInputAmt,
-		roundUpMinRelayFeeRate.FeePerKWeight(),
+		realWorldMinRelayFeeRate.FeePerKWeight(),
 	)
 
 	AssertNonInteractiveRecvComplete(t.t, secondTapd, 1)
diff --git a/tapdb/test_postgres.go b/tapdb/test_postgres.go
index 4e284e10..ffb638f4 100644
--- a/tapdb/test_postgres.go
+++ b/tapdb/test_postgres.go
@@ -7,18 +7,18 @@ import (
 )
 
 // NewTestDB is a helper function that creates a Postgres database for testing.
-func NewTestDB(t *testing.T) *PostgresStore {
+func NewTestDB(t testing.TB) *PostgresStore {
 	return NewTestPostgresDB(t)
 }
 
 // NewTestDbHandleFromPath is a helper function that creates a new handle to an
 // existing SQLite database for testing.
-func NewTestDbHandleFromPath(t *testing.T, dbPath string) *PostgresStore {
+func NewTestDbHandleFromPath(t testing.TB, dbPath string) *PostgresStore {
 	return NewTestPostgresDB(t)
 }
 
 // NewTestDBWithVersion is a helper function that creates a Postgres database
 // for testing and migrates it to the given version.
-func NewTestDBWithVersion(t *testing.T, version uint) *PostgresStore {
+func NewTestDBWithVersion(t testing.TB, version uint) *PostgresStore {
 	return NewTestPostgresDBWithVersion(t, version)
 }
diff --git a/tapdb/test_sqlite.go b/tapdb/test_sqlite.go
index 40e2b367..96d67687 100644
--- a/tapdb/test_sqlite.go
+++ b/tapdb/test_sqlite.go
@@ -7,18 +7,18 @@ import (
 )
 
 // NewTestDB is a helper function that creates an SQLite database for testing.
-func NewTestDB(t *testing.T) *SqliteStore {
+func NewTestDB(t testing.TB) *SqliteStore {
 	return NewTestSqliteDB(t)
 }
 
 // NewTestDbHandleFromPath is a helper function that creates a new handle to an
 // existing SQLite database for testing.
-func NewTestDbHandleFromPath(t *testing.T, dbPath string) *SqliteStore {
+func NewTestDbHandleFromPath(t testing.TB, dbPath string) *SqliteStore {
 	return NewTestSqliteDbHandleFromPath(t, dbPath)
 }
 
 // NewTestDBWithVersion is a helper function that creates an SQLite database for
 // testing and migrates it to the given version.
-func NewTestDBWithVersion(t *testing.T, version uint) *SqliteStore {
+func NewTestDBWithVersion(t testing.TB, version uint) *SqliteStore {
 	return NewTestSqliteDBWithVersion(t, version)
 }
diff --git a/tapdb/universe_perf_test.go b/tapdb/universe_perf_test.go
index 7a99d93e..3e5c1c5c 100644
--- a/tapdb/universe_perf_test.go
+++ b/tapdb/universe_perf_test.go
@@ -37,7 +37,7 @@ const (
 )
 
 // executeSQLStatements executes a list of SQL statements with logging.
-func executeSQLStatements(t *testing.T, db *BaseDB, statements []string,
+func executeSQLStatements(t testing.TB, db *BaseDB, statements []string,
 	action string) {
 
 	t.Logf("%s indices...", action)
@@ -52,7 +52,7 @@ func executeSQLStatements(t *testing.T, db *BaseDB, statements []string,
 }
 
 // createIndices creates all required indices on the database.
-func createIndices(t *testing.T, db *BaseDB) {
+func createIndices(t testing.TB, db *BaseDB) {
 	statements := []string{
 		createUniverseRootIndex,
 		createUniverseLeavesIndex,
@@ -63,7 +63,7 @@ func createIndices(t *testing.T, db *BaseDB) {
 }
 
 // dropIndices removes all custom indices from the database.
-func dropIndices(t *testing.T, db *BaseDB) {
+func dropIndices(t testing.TB, db *BaseDB) {
 	statements := []string{
 		`DROP INDEX IF EXISTS idx_universe_roots_composite`,
 		`DROP INDEX IF EXISTS idx_universe_leaves_asset`,
@@ -178,17 +178,17 @@ var testQueries = []queryTest{
 }
 
 // logPerformanceAnalysis prints performance metrics for all test queries.
-func logPerformanceAnalysis(t *testing.T, results map[string]*queryStats) {
-	t.Log("\n=== Performance Analysis ===")
+func logPerformanceAnalysis(b *testing.B, results map[string]*queryStats) {
+	b.Log("\n=== Performance Analysis ===")
 
 	for name, result := range results {
-		t.Logf("\nQuery: %s (%d runs)", name, result.queries)
+		b.Logf("\nQuery: %s (%d runs)", name, result.queries)
 
-		t.Log("Query Plan:")
-		t.Log(result.queryPlan)
+		b.Log("Query Plan:")
+		b.Log(result.queryPlan)
 
-		t.Logf("No indices exec time: %v", result.withoutIndices)
-		t.Logf("With indices exec time: %v", result.withIndices)
+		b.Logf("No indices exec time: %v", result.withoutIndices)
+		b.Logf("With indices exec time: %v", result.withIndices)
 
 		// Calculate improvement factor.
 		var improvement float64
@@ -197,12 +197,12 @@ func logPerformanceAnalysis(t *testing.T, results map[string]*queryStats) {
 				float64(result.withIndices)
 		}
 
-		t.Logf("Improvement: %.2fx", improvement)
+		b.Logf("Improvement: %.2fx", improvement)
 	}
 }
 
 // getQueryPlan retrieves the execution plan for a query from the database.
-func getQueryPlan(ctx context.Context, t *testing.T, db *BaseDB,
+func getQueryPlan(ctx context.Context, t testing.TB, db *BaseDB,
 	q queryTest, h *uniStatsHarness) string {
 
 	var (
@@ -253,7 +253,7 @@ func getQueryPlan(ctx context.Context, t *testing.T, db *BaseDB,
 }
 
 // executeQuery runs a query multiple times and measures execution time.
-func executeQuery(ctx context.Context, t *testing.T, db *BaseDB,
+func executeQuery(ctx context.Context, t testing.TB, db *BaseDB,
 	q queryTest, h *uniStatsHarness) time.Duration {
 
 	start := time.Now()
@@ -291,11 +291,9 @@ type queryStats struct {
 	queryPlan      string
 }
 
-// TestUniverseIndexPerformance tests the performance impact
-// of database indices.
-func TestUniverseIndexPerformance(t *testing.T) {
-	t.Parallel()
-
+// BenchmarkUniverseIndexPerformance tests the performance impact of database
+// indices.
+func BenchmarkUniverseIndexPerformance(b *testing.B) {
 	// Create context with reasonable timeout.
 	ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
 	defer cancel()
@@ -305,7 +303,7 @@ func TestUniverseIndexPerformance(t *testing.T) {
 	// setupDB creates a new database and harness for testing, either
 	// creating or dropping the supporting indices.
 	setupDB := func(withIndices bool) (*BaseDB, *uniStatsHarness) {
-		db := NewTestDB(t)
+		db := NewTestDB(b)
 		sqlDB := db.BaseDB
 
 		// Set reasonable connection limits.
@@ -317,11 +315,11 @@ func TestUniverseIndexPerformance(t *testing.T) {
 		statsDB, _ := newUniverseStatsWithDB(db.BaseDB, testClock)
 
 		// Add progress tracking.
-		t.Logf("Gen test data: %d assets, %d leaves/tree",
+		b.Logf("Gen test data: %d assets, %d leaves/tree",
 			numAssets, numLeavesPerTree)
 
-		h := newUniStatsHarness(t, numAssets, db.BaseDB, statsDB)
-		require.NotNil(t, h)
+		h := newUniStatsHarness(b, numAssets, db.BaseDB, statsDB)
+		require.NotNil(b, h)
 
 		// Generate some events for all assets.
 		for range 10 {
@@ -329,32 +327,32 @@ func TestUniverseIndexPerformance(t *testing.T) {
 		}
 
 		if withIndices {
-			createIndices(t, sqlDB)
+			createIndices(b, sqlDB)
 		} else {
-			dropIndices(t, sqlDB)
+			dropIndices(b, sqlDB)
 		}
 
 		return sqlDB, h
 	}
 
 	// runTest executes a query with or without indices.
-	runTest := func(t *testing.T, q queryTest, withIndices bool,
+	runTest := func(b *testing.B, q queryTest, withIndices bool,
 		sqlDB *BaseDB, h *uniStatsHarness) {
 
-		t.Logf("\n=== Query Plan for %s ===", q.name)
+		b.Logf("\n=== Query Plan for %s ===", q.name)
 
 		// Skip unsupported queries.
 		if !supportsQuery(sqlDB.Backend(), q.dbTypes) {
-			t.Skipf("Query %s unsupported by backend %v",
+			b.Skipf("Query %s unsupported by backend %v",
 				q.name, sqlDB.Backend())
 			return
 		}
 
-		plan := getQueryPlan(ctx, t, sqlDB, q, h)
-		t.Logf("Query Plan:\n%s", plan)
+		plan := getQueryPlan(ctx, b, sqlDB, q, h)
+		b.Logf("Query Plan:\n%s", plan)
 
 		// Execute the query repeatedly.
-		queryTime := executeQuery(ctx, t, sqlDB, q, h)
+		queryTime := executeQuery(ctx, b, sqlDB, q, h)
 
 		// Record results.
 		stat, ok := testResults[q.name]
@@ -373,7 +371,7 @@ func TestUniverseIndexPerformance(t *testing.T) {
 			stat.withoutIndices = queryTime
 		}
 
-		t.Logf("%s executed in: %v", q.name, queryTime)
+		b.Logf("%s executed in: %v", q.name, queryTime)
 	}
 
 	// Run tests with and without indices.
@@ -383,11 +381,11 @@ func TestUniverseIndexPerformance(t *testing.T) {
 		for _, q := range testQueries {
 			testName := fmt.Sprintf("name=%v,indices=%v", q.name,
 				withIndices)
-			t.Run(testName, func(t *testing.T) {
-				runTest(t, q, withIndices, sqlDB, h)
+			b.Run(testName, func(b *testing.B) {
+				runTest(b, q, withIndices, sqlDB, h)
 			})
 		}
 
-		logPerformanceAnalysis(t, testResults)
+		logPerformanceAnalysis(b, testResults)
 	}
 }
diff --git a/tapdb/universe_stats_test.go b/tapdb/universe_stats_test.go
index 7e824284..4e144bb8 100644
--- a/tapdb/universe_stats_test.go
+++ b/tapdb/universe_stats_test.go
@@ -43,10 +43,10 @@ type uniStatsHarness struct {
 
 	db *UniverseStats
 
-	t *testing.T
+	t testing.TB
 }
 
-func newUniStatsHarness(t *testing.T, numAssets int, db *BaseDB,
+func newUniStatsHarness(t testing.TB, numAssets int, db *BaseDB,
 	statsDB *UniverseStats) *uniStatsHarness {
 
 	stats := &uniStatsHarness{
@@ -100,7 +100,7 @@ func (u *uniStatsHarness) logSyncEventByIndex(i int) {
 	require.NoError(u.t, err)
 }
 
-func (u *uniStatsHarness) assertUniverseStatsEqual(t *testing.T,
+func (u *uniStatsHarness) assertUniverseStatsEqual(t testing.TB,
 	stats universe.AggregateStats) {
 
 	var (
diff --git a/tapdb/universe_test.go b/tapdb/universe_test.go
index 0ce04d60..597961b0 100644
--- a/tapdb/universe_test.go
+++ b/tapdb/universe_test.go
@@ -68,7 +68,7 @@ func randUniverseID(t testing.TB, forceGroup bool,
 	return id
 }
 
-func newTestUniverse(t *testing.T,
+func newTestUniverse(t testing.TB,
 	id universe.Identifier) (*BaseUniverseTree, sqlc.Querier) {
 
 	db := NewTestDB(t)
@@ -82,7 +82,7 @@ func newTestUniverse(t *testing.T,
 	return NewBaseUniverseTree(dbTxer, id), db
 }
 
-func newTestMultiverse(t *testing.T) (*MultiverseStore, sqlc.Querier) {
+func newTestMultiverse(t testing.TB) (*MultiverseStore, sqlc.Querier) {
 	db := NewTestDB(t)
 
 	dbTxer := NewTransactionExecutor(
@@ -116,7 +116,7 @@ func newTestUniverseWithDb(db *BaseDB,
 	return NewBaseUniverseTree(dbTxer, id), db
 }
 
-func assertIDInList(t *testing.T, leaves []universe.MultiverseLeaf,
+func assertIDInList(t testing.TB, leaves []universe.MultiverseLeaf,
 	id universe.Identifier) {
 
 	require.True(t, fn.Any(leaves, func(l universe.MultiverseLeaf) bool {
@@ -155,14 +155,14 @@ func TestUniverseEmptyTree(t *testing.T) {
 	require.ErrorIs(t, err, universe.ErrNoUniverseRoot)
 }
 
-func randLeafKey(t *testing.T) universe.LeafKey {
+func randLeafKey(t testing.TB) universe.LeafKey {
 	return universe.BaseLeafKey{
 		OutPoint:  test.RandOp(t),
 		ScriptKey: fn.Ptr(asset.NewScriptKey(test.RandPubKey(t))),
 	}
 }
 
-func randProof(t *testing.T, argAsset *asset.Asset) *proof.Proof {
+func randProof(t testing.TB, argAsset *asset.Asset) *proof.Proof {
 	proofAsset := *asset.RandAsset(t, asset.Normal)
 	if argAsset != nil {
 		proofAsset = *argAsset
@@ -200,7 +200,7 @@ func randProof(t *testing.T, argAsset *asset.Asset) *proof.Proof {
 	}
 }
 
-func randMintingLeaf(t *testing.T, assetGen asset.Genesis,
+func randMintingLeaf(t testing.TB, assetGen asset.Genesis,
 	groupKey *btcec.PublicKey) universe.Leaf {
 
 	randProof := randProof(t, nil)
@@ -490,7 +490,7 @@ func TestUniverseMetaBlob(t *testing.T) {
 	require.Equal(t, assetGen.ID(), uniProof.Leaf.Genesis.ID())
 }
 
-func insertRandLeaf(t *testing.T, ctx context.Context, tree *BaseUniverseTree,
+func insertRandLeaf(t testing.TB, ctx context.Context, tree *BaseUniverseTree,
 	assetGen *asset.Genesis) (*universe.Proof, error) {
 
 	var targetGen asset.Genesis