Merge pull request #1185 from Roasbeef/script-key-upsert

tapdb: fix bug w.r.t null bool handling, allow InsertScriptKey to allow flipping known to true

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,
@@ -685,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(),

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)
+	})
+}

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{

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

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
 

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

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