Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions tapdb/addrs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
guggero marked this conversation as resolved.
Show resolved Hide resolved
addr.ScriptKeyDeclaredKnown,
)
addrs = append(addrs, address.AddrWithKeyInfo{
Tap: tapAddr,
ScriptKeyTweak: asset.TweakedScriptKey{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down
113 changes: 113 additions & 0 deletions tapdb/addrs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package tapdb
import (
"context"
"database/sql"
"fmt"
"math/rand"
"testing"
"time"
Expand All @@ -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"
)
Expand Down Expand Up @@ -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)
})
}
6 changes: 4 additions & 2 deletions tapdb/asset_minting.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion tapdb/assets_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions tapdb/assets_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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

Expand Down
10 changes: 9 additions & 1 deletion tapdb/sqlc/assets.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 9 additions & 1 deletion tapdb/sqlc/queries/assets.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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
jharveyb marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
7 changes: 7 additions & 0 deletions tapdb/sqlutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading