Merge pull request #1316 from lightninglabs/no-asset-id-fix

universe: sync "missing universe id" when syncing unknown asset ID

itest/universe_test.go

@@ -688,6 +688,55 @@ func testUniverseFederation(t *harnessTest) {
 	)
 	require.NoError(t.t, err)
 	require.Equal(t.t, 0, len(fedNodes.Servers))
+
+	// When querying for a universe root that doesn't exist, we currently
+	// get an empty response. In a future version, this should be a
+	// universe.ErrNoUniverseRoot error.
+	dummyAssetIDBytes := fn.ByteSlice([32]byte{0x01})
+	rootResp, err := bob.QueryAssetRoots(ctxt, &unirpc.AssetRootQuery{
+		Id: &unirpc.ID{
+			Id: &unirpc.ID_AssetId{
+				AssetId: dummyAssetIDBytes,
+			},
+		},
+	})
+	require.NoError(t.t, err)
+
+	// The following checks show the problem: The actual roots are not nil
+	// but all the fields are empty. So they're basically useless. To not
+	// break the sync for older clients, we can't just change this right
+	// away. But we're going to change this in the future.
+	require.NotNil(t.t, rootResp.IssuanceRoot)
+	require.Nil(t.t, rootResp.IssuanceRoot.Id)
+	require.Nil(t.t, rootResp.IssuanceRoot.MssmtRoot)
+	require.Empty(t.t, rootResp.IssuanceRoot.AssetName)
+	require.Empty(t.t, rootResp.IssuanceRoot.AmountsByAssetId)
+	require.NotNil(t.t, rootResp.TransferRoot)
+	require.Nil(t.t, rootResp.TransferRoot.Id)
+	require.Nil(t.t, rootResp.TransferRoot.MssmtRoot)
+	require.Empty(t.t, rootResp.TransferRoot.AssetName)
+	require.Empty(t.t, rootResp.TransferRoot.AmountsByAssetId)
+
+	// The sync logic should detect that the response from the universe
+	// server was empty and just skip that root, not resulting in an error
+	// anymore.
+	dummyID := &unirpc.ID{
+		Id: &unirpc.ID_AssetId{
+			AssetId: dummyAssetIDBytes,
+		},
+		ProofType: unirpc.ProofType_PROOF_TYPE_ISSUANCE,
+	}
+	syncResp, err := bob.SyncUniverse(ctxt, &unirpc.SyncRequest{
+		UniverseHost: t.tapd.rpcHost(),
+		SyncMode:     unirpc.UniverseSyncMode_SYNC_ISSUANCE_ONLY,
+		SyncTargets: []*unirpc.SyncTarget{
+			{
+				Id: dummyID,
+			},
+		},
+	})
+	require.NoError(t.t, err)
+	require.Empty(t.t, syncResp.SyncedUniverses)
 }
 
 // testFederationSyncConfig tests that we can properly set and query the

rpcserver.go

@@ -4946,8 +4946,7 @@ func (r *rpcServer) QueryAssetRoots(ctx context.Context,
 		return assetRoots, nil
 
 	case err != nil:
-		return nil, fmt.Errorf("asset group lookup failed: %w",
-			err)
+		return nil, fmt.Errorf("asset group lookup failed: %w", err)
 
 	// We found the correct group for this asset; fetch the universe
 	// roots for the group.
@@ -4979,10 +4978,12 @@ func (r *rpcServer) QueryAssetRoots(ctx context.Context,
 // specific asset, for both proof types. The asset can be identified by its
 // asset ID or group key.
 func (r *rpcServer) queryAssetProofRoots(ctx context.Context,
-	id universe.Identifier) (*unirpc.QueryRootResponse, error) {
+	uniID universe.Identifier) (*unirpc.QueryRootResponse, error) {
 
-	var issuanceRootRPC, transferRootRPC *unirpc.UniverseRoot
-	uniID := id
+	var (
+		resp unirpc.QueryRootResponse
+		err  error
+	)
 
 	issuanceRoot, issuanceErr := r.cfg.UniverseArchive.RootNode(ctx, uniID)
 	if issuanceErr != nil {
@@ -4994,7 +4995,7 @@ func (r *rpcServer) queryAssetProofRoots(ctx context.Context,
 		}
 	}
 
-	issuanceRootRPC, err := marshalUniverseRoot(issuanceRoot)
+	resp.IssuanceRoot, err = marshalUniverseRoot(issuanceRoot)
 	if err != nil {
 		return nil, err
 	}
@@ -5015,15 +5016,12 @@ func (r *rpcServer) queryAssetProofRoots(ctx context.Context,
 		}
 	}
 
-	transferRootRPC, err = marshalUniverseRoot(transferRoot)
+	resp.TransferRoot, err = marshalUniverseRoot(transferRoot)
 	if err != nil {
 		return nil, err
 	}
 
-	return &unirpc.QueryRootResponse{
-		IssuanceRoot: issuanceRootRPC,
-		TransferRoot: transferRootRPC,
-	}, nil
+	return &resp, nil
 }
 
 // DeleteAssetRoot attempts to locate the current Universe root for a specific

universe/syncer.go

@@ -12,6 +12,7 @@ import (
 	"github.com/lightninglabs/taproot-assets/fn"
 	"github.com/lightninglabs/taproot-assets/mssmt"
 	"github.com/lightninglabs/taproot-assets/proof"
+	"github.com/lightninglabs/taproot-assets/taprpc/universerpc"
 	"golang.org/x/sync/errgroup"
 )
 
@@ -195,11 +196,32 @@ func fetchRootsForIDs(ctx context.Context, idsToSync []Identifier,
 		ctx, idsToSync,
 		func(ctx context.Context, id Identifier) error {
 			root, err := diffEngine.RootNode(ctx, id)
-			if err != nil {
+			switch {
+			// We're potentially calling an RPC endpoint, so the
+			// error cannot always be mapped directly using
+			// errors.Is, so we use fn.IsRpcErr. If we do get the
+			// ErrNoUniverseRoot it means the remote universe
+			// doesn't know about that root, which is okay and not
+			// a reason to abort the sync. This could either be the
+			// case because the asset ID was configured manually
+			// and isn't present in all universes, or because it's
+			// actually an incorrect asset ID.
+			case fn.IsRpcErr(err, ErrNoUniverseRoot):
+				log.Debugf("UniverseRoot(%v) not found in "+
+					"remote universe", id.String())
+				return nil
+
+			case err != nil:
 				return err
 			}
 
-			rootsToSync <- root
+			// Older versions of the universe didn't return an error
+			// when the root wasn't found. But the returned root is
+			// empty in that case, so we can check for that.
+			if !IsEmptyRoot(root) {
+				rootsToSync <- root
+			}
+
 			return nil
 		},
 	)
@@ -223,8 +245,7 @@ func (s *SimpleSyncer) syncRoot(ctx context.Context, remoteRoot Root,
 	// If we don't have this root, then we don't have anything to compare
 	// to, so we'll proceed as normal.
 	case errors.Is(err, ErrNoUniverseRoot):
-		// TODO(roasbeef): abstraction leak, error should be in
-		// universe package
+		// Continue below, we don't have this root locally.
 
 	// If the local root matches the remote root, then we're done here.
 	case err == nil && mssmt.IsEqualNode(localRoot, remoteRoot):
@@ -595,3 +616,38 @@ func (s *SimpleSyncer) fetchAllLeafKeys(ctx context.Context,
 
 	return leafKeys, nil
 }
+
+// IsEmptyRoot return true if the given root does not have any values set.
+func IsEmptyRoot(root Root) bool {
+	return root.ID == Identifier{} && root.Node == nil &&
+		root.AssetName == "" && len(root.GroupedAssets) == 0
+}
+
+// IsEmptyRootResponse returns true if the given root response does not have
+// any values set.
+func IsEmptyRootResponse(resp *universerpc.QueryRootResponse) bool {
+	// If the response is nil, then it's empty by definition.
+	if resp == nil {
+		return true
+	}
+
+	// If none of the roots are set, then the response is empty. This is
+	// not expected to be the case with the current version, as the roots
+	// are always set, but their content is empty. But future versions might
+	// set it that way.
+	if resp.IssuanceRoot == nil && resp.TransferRoot == nil {
+		return true
+	}
+
+	// If both roots are set, but their IDs are nil, then the response is
+	// empty. This is what the current version returns when the roots are
+	// empty.
+	if resp.IssuanceRoot != nil && resp.TransferRoot != nil {
+		return resp.IssuanceRoot.Id == nil &&
+			resp.TransferRoot.Id == nil
+	}
+
+	// If only one of the roots is set, then the response is likely not
+	// empty, or at least not as per our definition.
+	return false
+}

universe_rpc_diff.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/lightninglabs/taproot-assets/fn"
 	"github.com/lightninglabs/taproot-assets/mssmt"
 	"github.com/lightninglabs/taproot-assets/taprpc/universerpc"
 	unirpc "github.com/lightninglabs/taproot-assets/taprpc/universerpc"
@@ -109,10 +110,24 @@ func (r *RpcUniverseDiff) RootNode(ctx context.Context,
 	}
 
 	universeRoot, err := r.conn.QueryAssetRoots(ctx, rootReq)
-	if err != nil {
+	switch {
+	// We're calling using the RPC endpoint, so the error cannot be mapped
+	// directly using errors.Is.
+	case fn.IsRpcErr(err, universe.ErrNoUniverseRoot):
+		return universe.Root{}, universe.ErrNoUniverseRoot
+
+	case err != nil:
 		return universe.Root{}, err
 	}
 
+	// Old universe servers will return an empty response instead of the
+	// above error. But our sync engine now understands the error, so we can
+	// transform the empty response to the error. Future servers will return
+	// the error directly, which can be handled by newer clients.
+	if universe.IsEmptyRootResponse(universeRoot) {
+		return universe.Root{}, universe.ErrNoUniverseRoot
+	}
+
 	if id.ProofType == universe.ProofTypeIssuance {
 		return unmarshalUniverseRoot(universeRoot.IssuanceRoot)
 	}