From 6bcbbaab70b5e0f7253b287403f4b72b77822709 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 8 Nov 2024 17:22:43 -0800 Subject: [PATCH 1/3] tapdb: add new extractBool helper function In this commit, we add a new `extractBool` helper function. In an upcoming commit, we'll use this to fix incorrect usage of the `sql.NullBool` type. --- tapdb/sqlutils.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tapdb/sqlutils.go b/tapdb/sqlutils.go index a8ead76b4..570f50bcf 100644 --- a/tapdb/sqlutils.go +++ b/tapdb/sqlutils.go @@ -103,6 +103,13 @@ func extractSqlInt16[T constraints.Integer](num sql.NullInt16) T { return T(num.Int16) } +// extractBool turns a NullBool into a boolean. This can be useful when reading +// directly from the database, as this function handles extracting the inner +// value from the "option"-like struct. +func extractBool(b sql.NullBool) bool { + return b.Bool +} + // readOutPoint reads the next sequence of bytes from r as an OutPoint. // // NOTE: This function is intended to be used along with the wire.WriteOutPoint From d79e1415335045dbf0eb77b0e5a38f09b1b34162 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 8 Nov 2024 17:38:15 -0800 Subject: [PATCH 2/3] tapdb: fix bug with null bool handling in db In this commit, we fix some incorrect handling on null bool in SQL, that looks to be mostly benign. Before this commit, instead of doing `.Bool`, we did `.Valid`. The latter is only useful to check if something is zero, or NULL. In this case, we can just go with the bool value directly. --- tapdb/addrs.go | 12 ++++++++---- tapdb/asset_minting.go | 6 ++++-- tapdb/assets_common.go | 2 +- tapdb/assets_store.go | 6 +++--- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/tapdb/addrs.go b/tapdb/addrs.go index 1270f2d7b..2751018da 100644 --- a/tapdb/addrs.go +++ b/tapdb/addrs.go @@ -467,7 +467,9 @@ func (t *TapAddressBook) QueryAddrs(ctx context.Context, return fmt.Errorf("unable to make addr: %w", err) } - declaredKnown := addr.ScriptKeyDeclaredKnown.Valid + declaredKnown := extractBool( + addr.ScriptKeyDeclaredKnown, + ) addrs = append(addrs, address.AddrWithKeyInfo{ Tap: tapAddr, ScriptKeyTweak: asset.TweakedScriptKey{ @@ -620,9 +622,11 @@ func fetchAddr(ctx context.Context, db AddrBook, params *address.ChainParams, return &address.AddrWithKeyInfo{ Tap: tapAddr, ScriptKeyTweak: asset.TweakedScriptKey{ - RawKey: scriptKeyDesc, - Tweak: dbAddr.ScriptKeyTweak, - DeclaredKnown: dbAddr.ScriptKeyDeclaredKnown.Valid, + RawKey: scriptKeyDesc, + Tweak: dbAddr.ScriptKeyTweak, + DeclaredKnown: extractBool( + dbAddr.ScriptKeyDeclaredKnown, + ), }, InternalKeyDesc: internalKeyDesc, TaprootOutputKey: *taprootOutputKey, diff --git a/tapdb/asset_minting.go b/tapdb/asset_minting.go index 33f1e75a5..779f72137 100644 --- a/tapdb/asset_minting.go +++ b/tapdb/asset_minting.go @@ -666,7 +666,9 @@ func fetchAssetSeedlings(ctx context.Context, q PendingAssetStore, PubKey: scriptKeyInternalPub, } scriptKeyTweak := dbSeedling.ScriptKeyTweak - declaredKnown := dbSeedling.ScriptKeyDeclaredKnown.Valid + declaredKnown := extractBool( + dbSeedling.ScriptKeyDeclaredKnown, + ) seedling.ScriptKey = asset.ScriptKey{ PubKey: tweakedScriptKey, TweakedScriptKey: &asset.TweakedScriptKey{ @@ -800,7 +802,7 @@ func fetchAssetSprouts(ctx context.Context, q PendingAssetStore, Family: keychain.KeyFamily(sprout.ScriptKeyFam), }, } - declaredKnown := sprout.ScriptKeyDeclaredKnown.Valid + declaredKnown := extractBool(sprout.ScriptKeyDeclaredKnown) scriptKey := asset.ScriptKey{ PubKey: tweakedScriptKey, TweakedScriptKey: &asset.TweakedScriptKey{ diff --git a/tapdb/assets_common.go b/tapdb/assets_common.go index 0e66b363b..921716700 100644 --- a/tapdb/assets_common.go +++ b/tapdb/assets_common.go @@ -459,7 +459,7 @@ func fetchScriptKey(ctx context.Context, q FetchScriptKeyStore, Index: uint32(dbKey.KeyIndex), }, }, - DeclaredKnown: dbKey.DeclaredKnown.Valid, + DeclaredKnown: extractBool(dbKey.DeclaredKnown), } return scriptKey, nil diff --git a/tapdb/assets_store.go b/tapdb/assets_store.go index e408508d6..d5ee469ef 100644 --- a/tapdb/assets_store.go +++ b/tapdb/assets_store.go @@ -727,7 +727,7 @@ func (a *AssetStore) dbAssetsToChainAssets(dbAssets []ConfirmedAsset, if err != nil { return nil, err } - declaredKnown := sprout.ScriptKeyDeclaredKnown.Valid + declaredKnown := extractBool(sprout.ScriptKeyDeclaredKnown) scriptKey := asset.ScriptKey{ PubKey: scriptKeyPub, TweakedScriptKey: &asset.TweakedScriptKey{ @@ -2686,7 +2686,7 @@ func fetchAssetTransferOutputs(ctx context.Context, q ActiveAssetsStore, err) } - declaredKnown := dbOut.ScriptKeyDeclaredKnown.Valid + declaredKnown := extractBool(dbOut.ScriptKeyDeclaredKnown) outputAnchor := tapfreighter.Anchor{ Value: btcutil.Amount( dbOut.AnchorValue, @@ -3035,7 +3035,7 @@ func (a *AssetStore) LogAnchorTxConfirm(ctx context.Context, out.OutputType == int16(tappsbt.TypeSplitRoot) isBurn := !isNumsKey && len(witnessData) > 0 && asset.IsBurnKey(scriptPubKey, witnessData[0]) - isKnown := out.ScriptKeyDeclaredKnown.Valid + isKnown := extractBool(out.ScriptKeyDeclaredKnown) skipAssetCreation := !isTombstone && !isBurn && !out.ScriptKeyLocal && !isKnown From d453f81504dfb2684a076268fbf694958ecc82e3 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 8 Nov 2024 17:41:38 -0800 Subject: [PATCH 3/3] tapdb: update InsertScriptKey to allow flipping known to true In this commit, we update `InsertScriptKey` to allow flipping known to true, if it was false before. This is useful as at times a script key from a smart contract might have been inserting on disk, but with declared known as false. Then if we tried to insert it again, with known as true, the upsert logic would end up making no change on disk. --- tapdb/addrs.go | 1 + tapdb/addrs_test.go | 113 ++++++++++++++++++++++++++++++++++ tapdb/sqlc/assets.sql.go | 10 ++- tapdb/sqlc/queries/assets.sql | 10 ++- 4 files changed, 132 insertions(+), 2 deletions(-) diff --git a/tapdb/addrs.go b/tapdb/addrs.go index 2751018da..538514b49 100644 --- a/tapdb/addrs.go +++ b/tapdb/addrs.go @@ -689,6 +689,7 @@ func (t *TapAddressBook) InsertScriptKey(ctx context.Context, return fmt.Errorf("error inserting internal key: %w", err) } + _, err = q.UpsertScriptKey(ctx, NewScriptKey{ InternalKeyID: internalKeyID, TweakedScriptKey: scriptKey.PubKey.SerializeCompressed(), diff --git a/tapdb/addrs_test.go b/tapdb/addrs_test.go index 381f465c5..443197fe4 100644 --- a/tapdb/addrs_test.go +++ b/tapdb/addrs_test.go @@ -3,6 +3,7 @@ package tapdb import ( "context" "database/sql" + "fmt" "math/rand" "testing" "time" @@ -11,9 +12,11 @@ import ( "github.com/btcsuite/btcd/wire" "github.com/lightninglabs/lndclient" "github.com/lightninglabs/taproot-assets/address" + "github.com/lightninglabs/taproot-assets/asset" "github.com/lightninglabs/taproot-assets/internal/test" "github.com/lightninglabs/taproot-assets/tapdb/sqlc" "github.com/lightningnetwork/lnd/clock" + "github.com/lightningnetwork/lnd/keychain" "github.com/lightningnetwork/lnd/lnrpc" "github.com/stretchr/testify/require" ) @@ -644,3 +647,113 @@ func TestAddressEventQuery(t *testing.T) { }) } } + +// randScriptKey makes a random script key with a tweak. +func randScriptKey(t *testing.T) asset.ScriptKey { + scriptKey := asset.RandScriptKey(t) + scriptKey.TweakedScriptKey = &asset.TweakedScriptKey{ + RawKey: keychain.KeyDescriptor{ + PubKey: asset.RandScriptKey(t).PubKey, + }, + } + + return scriptKey +} + +// insertScriptKeyWithNull is a helper function that inserts a script key with a +// a NULL value for declared known. We use this so we can insert a NULL vs an +// actual value. It is identical to the InsertScriptKey. +func insertScriptKeyWithNull(ctx context.Context, key asset.ScriptKey, +) func(AddrBook) error { + + return func(q AddrBook) error { + internalKeyID, err := insertInternalKey( + ctx, q, key.RawKey, + ) + if err != nil { + return fmt.Errorf("error inserting internal key: %w", + err) + } + + _, err = q.UpsertScriptKey(ctx, NewScriptKey{ + InternalKeyID: internalKeyID, + TweakedScriptKey: key.PubKey.SerializeCompressed(), + Tweak: key.Tweak, + DeclaredKnown: sql.NullBool{ + Valid: false, + }, + }) + return err + } +} + +func assertKeyKnowledge(t *testing.T, ctx context.Context, + addrBook *TapAddressBook, scriptKey asset.ScriptKey, known bool) { + + dbScriptKey, err := addrBook.FetchScriptKey(ctx, scriptKey.PubKey) + require.NoError(t, err) + require.Equal(t, known, dbScriptKey.DeclaredKnown) +} + +// TestScriptKeyKnownUpsert tests that we can insert a script key, then insert +// it again declared as known. +func TestScriptKeyKnownUpsert(t *testing.T) { + t.Parallel() + + // First, make a new addr book instance we'll use in the test below. + testClock := clock.NewTestClock(time.Now()) + addrBook, _ := newAddrBook(t, testClock) + + ctx := context.Background() + + // In this test, we insert the known field as false, and make sure we + // can flip it back to true. + t.Run("false_to_true", func(t *testing.T) { + known := false + scriptKey := randScriptKey(t) + + // We'll insert a random script key into the database. We won't + // declare it as known though. + err := addrBook.InsertScriptKey(ctx, scriptKey, known) + require.NoError(t, err) + + // We'll fetch the script key and confirm that it's not known. + assertKeyKnowledge(t, ctx, addrBook, scriptKey, known) + + known = true + + // We'll now insert it again, but this time declare it as known. + err = addrBook.InsertScriptKey(ctx, scriptKey, known) + require.NoError(t, err) + + // We'll fetch the script key and confirm that it's known. + assertKeyKnowledge(t, ctx, addrBook, scriptKey, known) + }) + + // In this test, we insert a NULL value, and make sure that it can still + // be set to true. + t.Run("null_to_true", func(t *testing.T) { + known := false + scriptKey := randScriptKey(t) + + // We'll lift the internal routine of InsertScriptKey so we can + // insert an actual NULL here. + err := addrBook.db.ExecTx( + ctx, &AddrBookTxOptions{}, + insertScriptKeyWithNull(ctx, scriptKey), + ) + require.NoError(t, err) + + // We'll fetch the script key and confirm that it's not known. + assertKeyKnowledge(t, ctx, addrBook, scriptKey, known) + + known = true + + // We'll now insert it again, but this time declare it as known. + err = addrBook.InsertScriptKey(ctx, scriptKey, known) + require.NoError(t, err) + + // We'll fetch the script key and confirm that it's known. + assertKeyKnowledge(t, ctx, addrBook, scriptKey, known) + }) +} diff --git a/tapdb/sqlc/assets.sql.go b/tapdb/sqlc/assets.sql.go index 60b9aea95..fa1ca8f87 100644 --- a/tapdb/sqlc/assets.sql.go +++ b/tapdb/sqlc/assets.sql.go @@ -2880,7 +2880,15 @@ INSERT INTO script_keys ( ) ON CONFLICT (tweaked_script_key) -- As a NOP, we just set the script key to the one that triggered the -- conflict. - DO UPDATE SET tweaked_script_key = EXCLUDED.tweaked_script_key + DO UPDATE SET + tweaked_script_key = EXCLUDED.tweaked_script_key, + -- If the script key was previously unknown, we'll update to the new + -- value. + declared_known = CASE + WHEN script_keys.declared_known IS NULL OR script_keys.declared_known = FALSE + THEN COALESCE(EXCLUDED.declared_known, script_keys.declared_known) + ELSE script_keys.declared_known + END RETURNING script_key_id ` diff --git a/tapdb/sqlc/queries/assets.sql b/tapdb/sqlc/queries/assets.sql index fe8276af8..a16302d27 100644 --- a/tapdb/sqlc/queries/assets.sql +++ b/tapdb/sqlc/queries/assets.sql @@ -854,7 +854,15 @@ INSERT INTO script_keys ( ) ON CONFLICT (tweaked_script_key) -- As a NOP, we just set the script key to the one that triggered the -- conflict. - DO UPDATE SET tweaked_script_key = EXCLUDED.tweaked_script_key + DO UPDATE SET + tweaked_script_key = EXCLUDED.tweaked_script_key, + -- If the script key was previously unknown, we'll update to the new + -- value. + declared_known = CASE + WHEN script_keys.declared_known IS NULL OR script_keys.declared_known = FALSE + THEN COALESCE(EXCLUDED.declared_known, script_keys.declared_known) + ELSE script_keys.declared_known + END RETURNING script_key_id; -- name: FetchScriptKeyIDByTweakedKey :one