Merge pull request #1599 from lightninglabs/wip/asset-group-equality-checks

ffranr · web-flow · commit 5d667f45022c · 2025-06-16T13:11:21.000Z
asset: tighten GroupKey equality checks and update tests
diff --git a/asset/group_key.go b/asset/group_key.go
@@ -975,6 +975,40 @@ func GroupPubKeyV0(rawKey *btcec.PublicKey, singleTweak, tapTweak []byte) (
 	}
 }
 
+// IsEqualCustomTapscriptRoot reports whether the receiver and the provided
+// GroupKey define the same custom tapscript root.
+//
+// Equality semantics:
+//  1. If neither key sets a custom tapscript root (both options are None),
+//     the keys are considered equal.
+//  2. If both keys set a custom tapscript root (both options are Some), the
+//     underlying root hashes must be identical.
+//  3. If the presence bits differ (one Some, the other None) the keys are
+//     unequal.
+//
+// Fields other than CustomTapscriptRoot are intentionally ignored.
+func (g *GroupKey) IsEqualCustomTapscriptRoot(otherGroupKey *GroupKey) bool {
+	// If the two presence flags differ, the roots cannot be equal.
+	if g.CustomTapscriptRoot.IsSome() !=
+		otherGroupKey.CustomTapscriptRoot.IsSome() {
+
+		return false
+	}
+
+	// Both option flags are None -> neither key specifies a custom root;
+	// they match trivially.
+	if g.CustomTapscriptRoot.IsNone() {
+		return true
+	}
+
+	// At this point both option flags are Some. Compare the underlying
+	// hashes.
+	root := g.CustomTapscriptRoot.UnwrapToPtr()
+	otherRoot := otherGroupKey.CustomTapscriptRoot.UnwrapToPtr()
+
+	return root.IsEqual(otherRoot)
+}
+
 // IsEqual returns true if this group key and signature are exactly equivalent
 // to the passed other group key.
 func (g *GroupKey) IsEqual(otherGroupKey *GroupKey) bool {
@@ -986,7 +1020,11 @@ func (g *GroupKey) IsEqual(otherGroupKey *GroupKey) bool {
 		return false
 	}
 
-	equalGroup := g.IsEqualGroup(otherGroupKey)
+	if g.Version != otherGroupKey.Version {
+		return false
+	}
+
+	equalGroup := g.IsSameGroup(otherGroupKey)
 	if !equalGroup {
 		return false
 	}
@@ -995,16 +1033,20 @@ func (g *GroupKey) IsEqual(otherGroupKey *GroupKey) bool {
 		return false
 	}
 
+	if !g.IsEqualCustomTapscriptRoot(otherGroupKey) {
+		return false
+	}
+
 	if len(g.Witness) != len(otherGroupKey.Witness) {
 		return false
 	}
 
 	return slices.EqualFunc(g.Witness, otherGroupKey.Witness, bytes.Equal)
 }
 
-// IsEqualGroup returns true if this group key describes the same asset group
-// as the passed other group key.
-func (g *GroupKey) IsEqualGroup(otherGroupKey *GroupKey) bool {
+// IsSameGroup returns true if this group key refers to the same asset group
+// as the given group key.
+func (g *GroupKey) IsSameGroup(otherGroupKey *GroupKey) bool {
 	// If this key is nil, the other must be nil too.
 	if g == nil {
 		return otherGroupKey == nil
diff --git a/commitment/asset.go b/commitment/asset.go
@@ -89,42 +89,39 @@ type AssetCommitment struct {
 // parseCommon extracts the common fixed parameters of a set of assets to
 // include in the returned commitment.
 func parseCommon(assets ...*asset.Asset) (*AssetCommitment, error) {
+	// Return early if no assets were provided. After this point, we
+	// assume that at least one asset is present.
 	if len(assets) == 0 {
 		return nil, ErrNoAssets
 	}
 
+	// Inspect the first asset to note properties which should be consistent
+	// across all assets.
 	var (
-		assetType        asset.Type
-		tapCommitmentKey [32]byte
-		maxVersion       = asset.Version(0)
-		firstAssetID     = assets[0].Genesis.ID()
-		assetGroupKey    = assets[0].GroupKey
-		assetsMap        = make(CommittedAssets, len(assets))
+		firstAsset = assets[0]
+
+		expectedAssetType        = firstAsset.Type
+		expectedTapCommitmentKey = firstAsset.TapCommitmentKey()
+		expectedAssetID          = firstAsset.Genesis.ID()
+		expectedGroupKey         = firstAsset.GroupKey
 	)
-	for idx, newAsset := range assets {
-		// Inspect the first asset to note properties which should be
-		// consistent across all assets.
-		if idx == 0 {
-			// Set the asset type from the first asset.
-			assetType = newAsset.Type
-
-			// Set the expected tapCommitmentKey from the first
-			// asset.
-			tapCommitmentKey = newAsset.TapCommitmentKey()
-		}
+
+	// Ensure all assets share the same properties as the first.
+	for idx := range assets {
+		newAsset := assets[idx]
 
 		// Return error if the asset type doesn't match the previously
 		// encountered asset types.
-		if assetType != newAsset.Type {
+		if expectedAssetType != newAsset.Type {
 			return nil, ErrAssetTypeMismatch
 		}
 
 		switch {
-		case !assetGroupKey.IsEqualGroup(newAsset.GroupKey):
+		case !expectedGroupKey.IsSameGroup(newAsset.GroupKey):
 			return nil, ErrAssetGroupKeyMismatch
 
-		case assetGroupKey == nil:
-			if firstAssetID != newAsset.Genesis.ID() {
+		case expectedGroupKey == nil:
+			if expectedAssetID != newAsset.Genesis.ID() {
 				return nil, ErrAssetGenesisMismatch
 			}
 		}
@@ -134,36 +131,49 @@ func parseCommon(assets ...*asset.Asset) (*AssetCommitment, error) {
 		//
 		// NOTE: This sanity check executes after the group key check
 		// because it is a less specific check.
-		if tapCommitmentKey != newAsset.TapCommitmentKey() {
+		if expectedTapCommitmentKey != newAsset.TapCommitmentKey() {
 			return nil, fmt.Errorf("inconsistent asset " +
 				"TapCommitmentKey")
 		}
+	}
+
+	// Construct a map from AssetCommitmentKey to asset, validating that
+	// each AssetCommitmentKey value is unique as the map is populated.
+	assetsMap := make(CommittedAssets, len(assets))
+	for idx := range assets {
+		newAsset := assets[idx]
 
 		key := newAsset.AssetCommitmentKey()
 		if _, ok := assetsMap[key]; ok {
 			return nil, fmt.Errorf("%w: %x",
 				ErrAssetDuplicateScriptKey, key[:])
 		}
+		assetsMap[key] = newAsset
+	}
+
+	// Determine the maximum asset version among all assets in the set.
+	maxVersion := asset.Version(0)
+	for idx := range assets {
+		newAsset := assets[idx]
 		if newAsset.Version > maxVersion {
 			maxVersion = newAsset.Version
 		}
-		assetsMap[key] = newAsset
 	}
 
 	// The tapKey here is what will be used to place this asset commitment
 	// into the top-level Taproot Asset commitment. For assets without a
 	// group key, then this will be the normal asset ID. Otherwise, this'll
 	// be the sha256 of the group key.
 	assetSpecifier := asset.NewSpecifierOptionalGroupKey(
-		firstAssetID, assetGroupKey,
+		expectedAssetID, expectedGroupKey,
 	)
 
 	tapKey := asset.TapCommitmentKey(assetSpecifier)
 
 	return &AssetCommitment{
 		Version:   maxVersion,
 		TapKey:    tapKey,
-		AssetType: assetType,
+		AssetType: expectedAssetType,
 		assets:    assetsMap,
 	}, nil
 }
diff --git a/tapdb/assets_store_test.go b/tapdb/assets_store_test.go
@@ -2058,82 +2058,105 @@ func TestAssetGroupComplexWitness(t *testing.T) {
 	require.True(t, groupKey.IsEqual(storedGroup.GroupKey))
 }
 
-// TestAssetGroupV1 tests that we can store and fetch an asset group version 1.
-func TestAssetGroupV1(t *testing.T) {
+// TestStoreFetchAssetGroupV1 tests that we can store and fetch an asset group
+// version 1.
+func TestStoreFetchAssetGroupV1(t *testing.T) {
 	t.Parallel()
 
-	mintingStore, assetStore, db := newAssetStore(t)
-	ctx := context.Background()
+	testCases := []struct {
+		name                string
+		customTapscriptRoot fn.Option[chainhash.Hash]
+		witnessData         [][]byte
+		expectedError       error
+	}{{
+		name:                "group key with custom tapscript root",
+		customTapscriptRoot: fn.Some(test.RandHash()),
+		witnessData: [][]byte{
+			test.RandBytes(32),
+			test.RandBytes(64),
+		},
+		expectedError: nil,
+	}, {
+		name: "group key without custom tapscript " +
+			"root",
+		customTapscriptRoot: fn.None[chainhash.Hash](),
+		witnessData: [][]byte{
+			test.RandBytes(32),
+			test.RandBytes(64),
+		},
+		expectedError: nil,
+	}}
 
-	internalKey := test.RandPubKey(t)
-	groupAnchorGen := asset.RandGenesis(t, asset.RandAssetType(t))
-	groupAnchorGen.MetaHash = [32]byte{}
-	tapscriptRoot := test.RandBytes(32)
-	customTapscriptRoot := test.RandHash()
-	groupSig := test.RandBytes(64)
+	for idx := range testCases {
+		tc := testCases[idx]
 
-	// First, we'll insert all the required rows we need to satisfy the
-	// foreign key constraints needed to insert a new genesis witness.
-	genesisPointID, err := upsertGenesisPoint(
-		ctx, db, groupAnchorGen.FirstPrevOut,
-	)
-	require.NoError(t, err)
+		t.Run(tc.name, func(tt *testing.T) {
+			tt.Parallel()
 
-	genAssetID, err := upsertGenesis(
-		ctx, db, genesisPointID, groupAnchorGen,
-	)
-	require.NoError(t, err)
+			mintingStore, assetStore, db := newAssetStore(tt)
+			ctx := context.Background()
 
-	groupKey := asset.GroupKey{
-		Version: asset.GroupKeyV1,
-		RawKey: keychain.KeyDescriptor{
-			PubKey: internalKey,
-		},
-		GroupPubKey:   *internalKey,
-		TapscriptRoot: tapscriptRoot,
-		CustomTapscriptRoot: fn.Some[chainhash.Hash](
-			customTapscriptRoot,
-		),
-		Witness: fn.MakeSlice(tapscriptRoot, groupSig),
-	}
+			internalKey := test.RandPubKey(tt)
+			groupAnchorGen := asset.RandGenesis(
+				tt, asset.RandAssetType(tt),
+			)
+			groupAnchorGen.MetaHash = [32]byte{}
+			tapscriptRoot := test.RandBytes(32)
+
+			// First, we'll insert all the required rows we need to
+			// satisfy the foreign key constraints needed to insert
+			// a new genesis witness.
+			genesisPointID, err := upsertGenesisPoint(
+				ctx, db, groupAnchorGen.FirstPrevOut,
+			)
+			require.NoError(tt, err)
 
-	// Upsert, fetch, and check the group key.
-	_, err = upsertGroupKey(
-		ctx, &groupKey, assetStore.db, genesisPointID, genAssetID,
-	)
-	require.NoError(t, err)
+			genAssetID, err := upsertGenesis(
+				ctx, db, genesisPointID, groupAnchorGen,
+			)
+			require.NoError(tt, err)
 
-	storedGroup, err := mintingStore.FetchGroupByGroupKey(ctx, internalKey)
-	require.NoError(t, err)
+			groupKey := asset.GroupKey{
+				Version: asset.GroupKeyV1,
+				RawKey: keychain.KeyDescriptor{
+					PubKey: internalKey,
+				},
+				GroupPubKey:         *internalKey,
+				TapscriptRoot:       tapscriptRoot,
+				CustomTapscriptRoot: tc.customTapscriptRoot,
+				Witness:             tc.witnessData,
+			}
 
-	require.Equal(t, groupAnchorGen, *storedGroup.Genesis)
-	require.True(t, groupKey.IsEqual(storedGroup.GroupKey))
+			// Upsert the group key.
+			_, err = upsertGroupKey(
+				ctx, &groupKey, assetStore.db, genesisPointID,
+				genAssetID,
+			)
+			if tc.expectedError != nil {
+				require.ErrorIs(tt, err, tc.expectedError)
+				return
+			}
+			require.NoError(tt, err)
 
-	// Formulate a new group key where the custom tapscript root is None.
-	// Check that we can insert and fetch the group key.
-	groupKeyCustomRootNone := asset.GroupKey{
-		Version: asset.GroupKeyV1,
-		RawKey: keychain.KeyDescriptor{
-			PubKey: internalKey,
-		},
-		GroupPubKey:         *internalKey,
-		TapscriptRoot:       tapscriptRoot,
-		CustomTapscriptRoot: fn.None[chainhash.Hash](),
-		Witness:             fn.MakeSlice(tapscriptRoot, groupSig),
-	}
+			// Fetch and verify the group key.
+			storedGroup, err := mintingStore.FetchGroupByGroupKey(
+				ctx, internalKey,
+			)
+			require.NoError(tt, err)
 
-	// Upsert, fetch, and check the group key.
-	_, err = upsertGroupKey(
-		ctx, &groupKeyCustomRootNone, assetStore.db, genesisPointID,
-		genAssetID,
-	)
-	require.NoError(t, err)
+			// Verify the genesis matches.
+			require.Equal(tt, groupAnchorGen, *storedGroup.Genesis)
 
-	storedGroup2, err := mintingStore.FetchGroupByGroupKey(ctx, internalKey)
-	require.NoError(t, err)
+			// Verify the group key matches.
+			require.True(tt, groupKey.IsEqual(storedGroup.GroupKey))
 
-	require.Equal(t, groupAnchorGen, *storedGroup2.Genesis)
-	require.True(t, groupKeyCustomRootNone.IsEqual(storedGroup2.GroupKey))
+			// Ensure that the group key version is set to V1.
+			require.Equal(
+				tt, asset.GroupKeyV1,
+				storedGroup.GroupKey.Version,
+			)
+		})
+	}
 }
 
 // TestAssetGroupKeyUpsert tests that if you try to insert another asset group
