Merge pull request #1592 from lightninglabs/fix-empty-json-meta

rpc: allow minting assets with empty metadata

Author: ffranr

Makefile

@@ -90,7 +90,7 @@ build-itest:
 	@if [ ! -f itest/chantools/chantools ]; then \
 		$(call print, "Building itest chantools."); \
 		rm -rf itest/chantools; \
-		git clone --depth 1 --branch v0.13.5 https://github.com/lightninglabs/chantools.git itest/chantools; \
+		git clone --depth 1 --branch v0.14.0 https://github.com/lightninglabs/chantools.git itest/chantools; \
 		cd itest/chantools && go build ./cmd/chantools; \
 	else \
 		$(call print, "Chantools is already installed and available in itest/chantools."); \

itest/asset_meta_test.go

@@ -84,6 +84,31 @@ func testAssetMeta(t *harnessTest) {
 				},
 			},
 		},
+
+		// A mint request that doesn't specify asset meta at all should
+		// be permitted.
+		{
+			asset: &mintrpc.MintAssetRequest{
+				Asset: &mintrpc.MintAsset{
+					AssetType: taprpc.AssetType_NORMAL,
+					Name:      "no meta",
+					Amount:    5000,
+				},
+			},
+		},
+
+		// A user should also be able to specify a decimal display, but
+		// not actually specify an asset meta at all.
+		{
+			asset: &mintrpc.MintAssetRequest{
+				Asset: &mintrpc.MintAsset{
+					AssetType:      taprpc.AssetType_NORMAL,
+					Name:           "dec display",
+					Amount:         5000,
+					DecimalDisplay: 6,
+				},
+			},
+		},
 	}
 
 	ctxb := context.Background()
@@ -172,18 +197,9 @@ func testMintAssetWithDecimalDisplayMetaField(t *harnessTest) {
 	_, err = t.tapd.MintAsset(ctxt, secondAssetReq)
 	require.ErrorContains(t.t, err, "decimal display does not match")
 
-	// Requesting a decimal display without specifying the metadata field
-	// with at least the type should fail.
-	secondAssetReq.Asset.DecimalDisplay = firstAsset.DecimalDisplay
-	secondAssetReq.Asset.AssetMeta = nil
-
-	_, err = t.tapd.MintAsset(ctxt, secondAssetReq)
-	require.ErrorContains(
-		t.t, err, "decimal display requires asset metadata",
-	)
-
 	// Attempting to set a different decimal display in the JSON meta data
 	// as in the new RPC request field should give us an error as well.
+	secondAssetReq.Asset.DecimalDisplay = firstAsset.DecimalDisplay
 	secondAssetReq.Asset.AssetMeta = &taprpc.AssetMeta{
 		Type: taprpc.AssetMetaType_META_TYPE_JSON,
 		Data: []byte(`{"foo": "bar", "decimal_display": 3}`),

itest/chantools_harness.go

@@ -66,7 +66,7 @@ func NewChantoolsHarness(t *testing.T) ChantoolsHarness {
 	require.NoError(t, err, "failed to get chantools version")
 
 	versionOutStr := string(versionOut)
-	if !strings.Contains(versionOutStr, "chantools version v0.13.5") {
+	if !strings.Contains(versionOutStr, "chantools version v0.14.0") {
 		t.Fatalf("unexpected chantools version: %v", versionOutStr)
 	}
 

proof/meta.go

@@ -271,17 +271,11 @@ func IsValidMetaType[T SizableInteger](num T) (MetaType, error) {
 // IsValidMetaSize checks if the passed data is non-empty and below the maximum
 // size.
 func IsValidMetaSize(mBytes []byte, maxSize int) error {
-	mSize := len(mBytes)
-	switch {
-	case mSize == 0:
-		return ErrMetaDataMissing
-
-	case mSize > maxSize:
+	if len(mBytes) > maxSize {
 		return ErrMetaDataTooLarge
-
-	default:
-		return nil
 	}
+
+	return nil
 }
 
 // IsValidDecDisplay checks if the decimal display value is below the maximum.
@@ -295,15 +289,13 @@ func IsValidDecDisplay(decDisplay uint32) error {
 
 // DecodeMetaJSON decodes bytes as a JSON object, after checking that the bytes
 // could be valid metadata.
-//
-// TODO(ffranr): Add unit test for `jBytes := []byte{}`.
 func DecodeMetaJSON(jBytes []byte) (map[string]interface{}, error) {
 	jMeta := make(map[string]interface{})
 
 	// These bytes must match our metadata size constraints.
 	err := IsValidMetaSize(jBytes, MetaDataMaxSizeBytes)
 	if err != nil {
-		return nil, fmt.Errorf("%w: %s", ErrInvalidJSON, err.Error())
+		return nil, fmt.Errorf("%w: %w", ErrInvalidJSON, err)
 	}
 
 	// Unmarshal checks internally if the JSON is valid.

proof/meta_test.go

@@ -3,9 +3,12 @@ package proof
 import (
 	"bytes"
 	"encoding/hex"
+	"encoding/json"
+	"errors"
 	"net/url"
 	"os"
 	"path/filepath"
+	"reflect"
 	"strings"
 	"testing"
 
@@ -15,6 +18,7 @@ import (
 	"github.com/lightninglabs/taproot-assets/internal/test"
 	"github.com/lightningnetwork/lnd/tlv"
 	"github.com/stretchr/testify/require"
+	"pgregory.net/rapid"
 )
 
 var (
@@ -71,7 +75,6 @@ func TestValidateMetaReveal(t *testing.T) {
 				Type: MetaOpaque,
 				Data: nil,
 			},
-			expectedErr: ErrMetaDataMissing,
 		},
 		{
 			name: "too much data",
@@ -466,3 +469,162 @@ func TestDecodeNewMetaReveal(t *testing.T) {
 	require.Equal(t, fn.None[[]url.URL](), decoded.CanonicalUniverses)
 	require.Equal(t, fn.None[btcec.PublicKey](), decoded.DelegationKey)
 }
+
+// TestDecodeMetaJSONEmptyBytes tests DecodeMetaJSON with an empty byte slice.
+func TestDecodeMetaJSONEmptyBytes(t *testing.T) {
+	t.Parallel()
+
+	_, err := DecodeMetaJSON([]byte{})
+	require.ErrorIs(t, err, ErrInvalidJSON)
+}
+
+// TestEncodeMetaJSONEmptyOrNilMap tests EncodeMetaJSON with nil and empty maps.
+func TestEncodeMetaJSONEmptyOrNilMap(t *testing.T) {
+	t.Parallel()
+
+	t.Run("nil map", func(t *testing.T) {
+		nilMapBytes, err := EncodeMetaJSON(nil)
+		require.NoError(t, err)
+		require.Equal(
+			t, []byte{}, nilMapBytes,
+			"encoding nil map should produce empty bytes",
+		)
+	})
+
+	t.Run("empty map", func(t *testing.T) {
+		emptyMap := make(map[string]interface{})
+		emptyMapBytes, err := EncodeMetaJSON(emptyMap)
+		require.NoError(t, err)
+		require.Equal(t, []byte("{}"), emptyMapBytes,
+			"Encoding empty map should produce '{}'")
+	})
+}
+
+// simpleJSONMap is a helper struct for generating map[string]interface{}
+// values for property-based testing.
+type simpleJSONMap struct {
+	StringEntries map[string]string
+	FloatEntries  map[string]float64
+	BoolEntries   map[string]bool
+}
+
+// toMap converts a simpleJSONMap to a map[string]interface{}.
+func (s simpleJSONMap) toMap() map[string]interface{} {
+	m := make(map[string]interface{})
+	for k, v := range s.StringEntries {
+		// To avoid key collisions if rapid generates identical keys for
+		// different entry types, we could prefix them, but for
+		// simplicity, we'll assume distinct keys or accept overwrites
+		// if rapid generates identical string keys for different map
+		// types (unlikely to be an issue for typical map generation).
+		m[k] = v
+	}
+	for k, v := range s.FloatEntries {
+		m[k] = v
+	}
+	for k, v := range s.BoolEntries {
+		m[k] = v
+	}
+	return m
+}
+
+// TestMetaJsonEncodeDecodeProperties tests the EncodeMetaJSON and
+// DecodeMetaJSON functions using property-based testing. It ensures that
+// encoding and then decoding a map yields the original map.
+func TestMetaJsonEncodeDecodeProperties(t *testing.T) {
+	t.Parallel()
+
+	rapid.Check(t, func(rt *rapid.T) {
+		// Generate a simpleJSONMap instance.
+		sjm := rapid.Custom(func(t *rapid.T) simpleJSONMap {
+			return simpleJSONMap{
+				StringEntries: rapid.MapOf(
+					rapid.String(), rapid.String(),
+				).Draw(t, "StringEntries"),
+				FloatEntries: rapid.MapOf(
+					rapid.String(), rapid.Float64(),
+				).Draw(t, "FloatEntries"),
+				BoolEntries: rapid.MapOf(
+					rapid.String(), rapid.Bool(),
+				).Draw(t, "BoolEntries"),
+			}
+		}).Draw(rt, "sjm")
+
+		originalMap := sjm.toMap()
+
+		encodedBytes, err := EncodeMetaJSON(originalMap)
+		if err != nil {
+			// If the generated map is too large, EncodeMetaJSON
+			// will return ErrMetaDataTooLarge. This is an expected
+			// behavior, not a property failure.
+			if errors.Is(err, ErrMetaDataTooLarge) {
+				return
+			}
+
+			// Any other encoding error is a failure.
+			rt.Fatalf("EncodeMetaJSON failed for map %#v: %v",
+				originalMap, err)
+		}
+
+		decodedMap, err := DecodeMetaJSON(encodedBytes)
+		if err != nil {
+			rt.Fatalf("DecodeMetaJSON failed for bytes %s "+
+				"(from map %#v): %v",
+				string(encodedBytes), originalMap, err)
+		}
+
+		if !reflect.DeepEqual(originalMap, decodedMap) {
+			rt.Fatalf("Decoded map does not match original map.\n"+
+				"Original: %#v\nEncoded:  %s\nDecoded:  %#v",
+				originalMap, string(encodedBytes), decodedMap)
+		}
+	})
+}
+
+// TestDecodeMetaJSONOversizedProperty tests that DecodeMetaJSON returns an
+// error when the input byte slice is larger than MetaDataMaxSizeBytes.
+func TestDecodeMetaJSONOversizedProperty(t *testing.T) {
+	t.Parallel()
+
+	rapid.Check(t, func(rt *rapid.T) {
+		// Generate a byte slice that is guaranteed to be too large. We
+		// add a small delta to avoid generating slices that are
+		// excessively large and slow down the test.
+		oversizedBytes := rapid.SliceOfN(
+			rapid.Byte(),
+			MetaDataMaxSizeBytes+1,
+			MetaDataMaxSizeBytes+10,
+		).Draw(rt, "oversizedBytes")
+
+		_, err := DecodeMetaJSON(oversizedBytes)
+		require.ErrorIs(rt, err, ErrMetaDataTooLarge)
+	})
+}
+
+// TestDecodeMetaJSONInvalidJSONProperty tests that DecodeMetaJSON returns an
+// error for byte slices that are not valid JSON but are within size limits.
+func TestDecodeMetaJSONInvalidJSONProperty(t *testing.T) {
+	t.Parallel()
+
+	rapid.Check(t, func(rt *rapid.T) {
+		// Generate a byte slice that is within the size limit. There's
+		// a high chance this random byte slice won't be valid JSON.
+		inputBytes := rapid.SliceOfN(
+			rapid.Byte(),
+			0, MetaDataMaxSizeBytes,
+		).Draw(rt, "inputBytes")
+
+		// If, by chance, we generated valid JSON or an empty slice
+		// (which DecodeMetaJSON handles as valid empty JSON), we skip
+		// this iteration as we want to test invalid JSON.
+		// DecodeMetaJSON considers empty bytes as valid (empty map).
+		if json.Valid(inputBytes) {
+			rt.Skip("Generated valid JSON or empty " +
+				"slice, skipping to test invalid case")
+			return
+		}
+
+		_, err := DecodeMetaJSON(inputBytes)
+		require.ErrorIs(rt, err, ErrInvalidJSON)
+	})
+}

rpcserver.go

@@ -505,11 +505,14 @@ func (r *rpcServer) MintAsset(ctx context.Context,
 		return nil, err
 	}
 
-	// If a custom decimal display is set, we require the AssetMeta to be
-	// set. That means the user has to at least specify the meta type.
+	// If a custom decimal display is set, but the user didn't specify an
+	// asset meta, then we'll assume an opaque type.
 	if req.Asset.DecimalDisplay != 0 && req.Asset.AssetMeta == nil {
-		return nil, fmt.Errorf("decimal display requires asset " +
-			"metadata")
+		rpcsLog.Infof("No asset meta specified, using opaque type "+
+			"for decimal display %d", req.Asset.DecimalDisplay)
+
+		req.Asset.AssetMeta = &taprpc.AssetMeta{}
+
 	}
 
 	// Decimal display doesn't really make sense for collectibles.