Merge pull request #1337 from lightninglabs/tlv-refactor-fixes

proof+tapdb: refactor and fix TLV unknown odd types bug for backward compatibility

address/encoding.go

@@ -2,7 +2,6 @@ package address
 
 import (
 	"io"
-	"net/url"
 
 	"github.com/btcsuite/btcd/btcec/v2"
 	"github.com/lightningnetwork/lnd/tlv"
@@ -42,35 +41,6 @@ func compressedPubKeyDecoder(r io.Reader, val any, buf *[8]byte, l uint64) error
 	)
 }
 
-// UrlEncoder encodes a url.URL as a variable length byte slice.
-func UrlEncoder(w io.Writer, val any, buf *[8]byte) error {
-	if t, ok := val.(*url.URL); ok {
-		addrBytes := []byte((*t).String())
-		return tlv.EVarBytes(w, &addrBytes, buf)
-	}
-	return tlv.NewTypeForEncodingErr(val, "*url.URL")
-}
-
-// UrlDecoder decodes a variable length byte slice as an url.URL.
-func UrlDecoder(r io.Reader, val any, buf *[8]byte, l uint64) error {
-	if t, ok := val.(*url.URL); ok {
-		var addrBytes []byte
-		err := tlv.DVarBytes(r, &addrBytes, buf, l)
-		if err != nil {
-			return err
-		}
-
-		addr, err := url.ParseRequestURI(string(addrBytes))
-		if err != nil {
-			return err
-		}
-		*t = *addr
-
-		return nil
-	}
-	return tlv.NewTypeForDecodingErr(val, "*url.URL", l, l)
-}
-
 func VersionEncoder(w io.Writer, val any, buf *[8]byte) error {
 	if t, ok := val.(*Version); ok {
 		return tlv.EUint8T(w, uint8(*t), buf)

address/records.go

@@ -131,6 +131,6 @@ func newProofCourierAddrRecord(addr *url.URL) tlv.Record {
 
 	return tlv.MakeDynamicRecord(
 		addrProofCourierAddrType, addr, recordSize,
-		UrlEncoder, UrlDecoder,
+		asset.UrlEncoder, asset.UrlDecoder,
 	)
 }

asset/records.go

@@ -5,6 +5,7 @@ import (
 	"crypto/sha256"
 	"fmt"
 	"io"
+	"net/url"
 
 	"github.com/btcsuite/btcd/btcec/v2"
 	"github.com/btcsuite/btcd/wire"
@@ -152,7 +153,12 @@ func CombineRecords(records []tlv.Record, unparsed tlv.TypeMap) []tlv.Record {
 		))
 	}
 
-	return append(records, stubRecords...)
+	// Because the map above gives random access to the records, we need to
+	// re-sort them to ensure that the records are in the correct order.
+	combinedRecords := append(records, stubRecords...)
+	tlv.SortRecords(combinedRecords)
+
+	return combinedRecords
 }
 
 // WitnessTlvType represents the different TLV types for Asset Witness TLV
@@ -303,3 +309,32 @@ func NewWitnessSplitCommitmentRecord(commitment **SplitCommitment) tlv.Record {
 		SplitCommitmentEncoder, SplitCommitmentDecoder,
 	)
 }
+
+// UrlEncoder encodes a url.URL as a variable length byte slice.
+func UrlEncoder(w io.Writer, val any, buf *[8]byte) error {
+	if t, ok := val.(*url.URL); ok {
+		addrBytes := []byte((*t).String())
+		return tlv.EVarBytes(w, &addrBytes, buf)
+	}
+	return tlv.NewTypeForEncodingErr(val, "*url.URL")
+}
+
+// UrlDecoder decodes a variable length byte slice as an url.URL.
+func UrlDecoder(r io.Reader, val any, buf *[8]byte, l uint64) error {
+	if t, ok := val.(*url.URL); ok {
+		var addrBytes []byte
+		err := tlv.DVarBytes(r, &addrBytes, buf, l)
+		if err != nil {
+			return err
+		}
+
+		addr, err := url.ParseRequestURI(string(addrBytes))
+		if err != nil {
+			return err
+		}
+		*t = *addr
+
+		return nil
+	}
+	return tlv.NewTypeForDecodingErr(val, "*url.URL", l, l)
+}

itest/assertions.go

@@ -1863,12 +1863,10 @@ func AssertAssetsMinted(t *testing.T, tapClient commands.RpcClientsBundle,
 
 		metaReveal.Type = validMetaType
 		if metaReveal.Type == proof.MetaJson {
-			updatedMeta, err := metaReveal.SetDecDisplay(
+			err := metaReveal.SetDecDisplay(
 				assetRequest.Asset.DecimalDisplay,
 			)
 			require.NoError(t, err)
-
-			metaReveal = updatedMeta
 		}
 
 		metaHash := metaReveal.MetaHash()

itest/asset_meta_test.go

@@ -149,10 +149,10 @@ func testMintAssetWithDecimalDisplayMetaField(t *harnessTest) {
 	}
 
 	// Manually update the requested metadata and compute the expected hash.
-	updatedMeta, err := mintedMeta.SetDecDisplay(firstAsset.DecimalDisplay)
+	err := mintedMeta.SetDecDisplay(firstAsset.DecimalDisplay)
 	require.NoError(t.t, err)
 
-	metaHash := updatedMeta.MetaHash()
+	metaHash := mintedMeta.MetaHash()
 
 	// The meta hash from the minted asset must match the expected hash.
 	genMetaHash := firstAssetMinted.AssetGenesis.MetaHash

itest/utils.go

@@ -423,12 +423,10 @@ func FinalizeBatchUnconfirmed(t *testing.T, minerClient *rpcclient.Client,
 
 		metaReveal.Type = validMetaType
 		if metaReveal.Type == proof.MetaJson {
-			updatedMeta, err := metaReveal.SetDecDisplay(
+			err := metaReveal.SetDecDisplay(
 				assetRequest.Asset.DecimalDisplay,
 			)
 			require.NoError(t, err)
-
-			metaReveal = updatedMeta
 		}
 
 		metaHash := metaReveal.MetaHash()

proof/backward_compat_test.go

@@ -0,0 +1,93 @@
+package proof
+
+import (
+	"context"
+	"encoding/hex"
+	"strings"
+	"testing"
+
+	"github.com/lightninglabs/taproot-assets/internal/test"
+	"github.com/stretchr/testify/require"
+)
+
+// TestBIPTestVectorsBackwardCompatible tests that the BIP test vectors are
+// passing against an older code base (this test file will be copied to an old
+// version of tapd together with the test vectors in the CI).
+func TestBIPTestVectorsBackwardCompatible(t *testing.T) {
+	t.Parallel()
+
+	for idx := range allTestVectorFiles {
+		var (
+			fileName    = allTestVectorFiles[idx]
+			testVectors = &TestVectors{}
+		)
+		test.ParseTestVectors(t, fileName, &testVectors)
+		t.Run(fileName, func(tt *testing.T) {
+			tt.Parallel()
+
+			runBIPTestVectorBackwardCompatible(tt, testVectors)
+		})
+	}
+}
+
+// runBIPTestVectorBackwardCompatible runs the tests in a single BIP test vector
+// file.
+func runBIPTestVectorBackwardCompatible(t *testing.T,
+	testVectors *TestVectors) {
+
+	for _, validCase := range testVectors.ValidTestCases {
+		validCase := validCase
+
+		t.Run(validCase.Comment, func(tt *testing.T) {
+			tt.Parallel()
+
+			// We want to make sure that the proof can be decoded
+			// from the hex string and that the decoded proof's meta
+			// hash matches.
+			decoded := &Proof{}
+			err := decoded.Decode(hex.NewDecoder(
+				strings.NewReader(validCase.Expected),
+			))
+			require.NoError(tt, err)
+
+			if decoded.MetaReveal != nil {
+				metaHash := decoded.MetaReveal.MetaHash()
+				require.Equal(
+					tt,
+					validCase.Proof.Asset.GenesisMetaHash,
+					hex.EncodeToString(metaHash[:]),
+				)
+			}
+
+			// We can't verify the full proof chain but at least we
+			// can verify the inclusion/exclusion proofs.
+			_, err = decoded.VerifyProofs()
+			require.NoError(tt, err)
+
+			// If there is a genesis reveal, we can validate the
+			// full proof chain, as it's the first proof in the
+			// chain.
+			if decoded.GenesisReveal != nil {
+				_, err = decoded.Verify(
+					context.Background(), nil,
+					MockHeaderVerifier,
+					DefaultMerkleVerifier,
+					MockGroupVerifier, MockChainLookup,
+				)
+				require.NoError(tt, err)
+			}
+		})
+	}
+
+	for _, invalidCase := range testVectors.ErrorTestCases {
+		invalidCase := invalidCase
+
+		t.Run(invalidCase.Comment, func(tt *testing.T) {
+			tt.Parallel()
+
+			require.PanicsWithValue(tt, invalidCase.Error, func() {
+				invalidCase.Proof.ToProof(tt)
+			})
+		})
+	}
+}

proof/meta.go

@@ -7,7 +7,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"maps"
 	"math"
 
 	"github.com/lightninglabs/taproot-assets/asset"
@@ -306,66 +305,49 @@ func (m *MetaReveal) DecDisplayOption() (fn.Option[uint32], error) {
 
 // SetDecDisplay attempts to set the decimal display value in existing JSON
 // metadata. It checks that the new metadata is below the maximum metadata size.
-func (m *MetaReveal) SetDecDisplay(decDisplay uint32) (*MetaReveal, error) {
+func (m *MetaReveal) SetDecDisplay(decDisplay uint32) error {
 	err := IsValidDecDisplay(decDisplay)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
-	// Fetch the current decimal display value.
-	currentMetaJSON, currentDecDisplay, err := m.GetDecDisplay()
-	switch {
-	// The current metadata is valid JSON. Either no decimal display value
-	// is present, or it is but doesn't match the desired value.
-	case errors.Is(err, ErrDecDisplayMissing),
-		currentDecDisplay != decDisplay:
-
-		// If the requested decimal display value is 0, we don't need to
-		// add the field at all, as that is the default.
-		if decDisplay == 0 {
-			return &MetaReveal{
-				Type: m.Type,
-				Data: m.Data,
-			}, nil
-		}
-
-		// Otherwise, set the decimal display value and re-validate the
-		// JSON object.
-		updatedJSON := make(map[string]interface{})
-		maps.Copy(updatedJSON, currentMetaJSON)
-
-		updatedJSON[MetadataDecDisplayKey] = decDisplay
+	// If the meta type is not JSON, we can't set the decimal display value.
+	if m.Type != MetaJson {
+		return ErrNotJSON
+	}
 
-		updatedJSONBytes, err := EncodeMetaJSON(updatedJSON)
-		if err != nil {
-			return nil, fmt.Errorf("invalid metadata after "+
-				"setting decimal display: %w", err)
-		}
+	// If the meta type is JSON, we'll also want to set the decimal display
+	// value in the JSON object.
+	metaJSON, err := DecodeMetaJSON(m.Data)
+	switch {
+	// If the metadata is currently empty, we'll just start with an empty
+	// JSON object.
+	case len(m.Data) == 0 || errors.Is(err, ErrMetaDataMissing):
+		metaJSON = make(map[string]interface{})
 
-		return &MetaReveal{
-			Type: m.Type,
-			Data: updatedJSONBytes,
-		}, nil
+	case err != nil:
+		return err
+	}
 
-	// No metadata update needed.
-	case currentDecDisplay == decDisplay:
-		return &MetaReveal{
-			Type: m.Type,
-			Data: m.Data,
-		}, nil
+	metaJSON[MetadataDecDisplayKey] = decDisplay
 
-	// Our metadata is invalid in another way.
-	default:
-		return nil, fmt.Errorf("%w: %d", ErrDecDisplayInvalid,
-			decDisplay)
+	m.Data, err = EncodeMetaJSON(metaJSON)
+	if err != nil {
+		return fmt.Errorf("invalid metadata after setting decimal "+
+			"display: %w", err)
 	}
+
+	return nil
 }
 
 // MetaHash returns the computed meta hash based on the TLV serialization of
 // the meta data itself.
 func (m *MetaReveal) MetaHash() [asset.MetaHashLen]byte {
 	var b bytes.Buffer
-	_ = m.Encode(&b)
+	err := m.Encode(&b)
+	if err != nil {
+		log.Errorf("Unable to encode meta reveal: %v", err)
+	}
 
 	return sha256.Sum256(b.Bytes())
 }

proof/proof_test.go

@@ -1215,6 +1215,24 @@ func runBIPTestVector(t *testing.T, testVectors *TestVectors) {
 			require.NoError(tt, err)
 
 			require.Equal(tt, p, decoded)
+
+			// We can't verify the full proof chain but at least we
+			// can verify the inclusion/exclusion proofs.
+			_, err = decoded.VerifyProofs()
+			require.NoError(tt, err)
+
+			// If there is a genesis reveal, we can validate the
+			// full proof chain, as it's the first proof in the
+			// chain.
+			if decoded.GenesisReveal != nil {
+				_, err = decoded.Verify(
+					context.Background(), nil,
+					MockHeaderVerifier,
+					DefaultMerkleVerifier,
+					MockGroupVerifier, MockChainLookup,
+				)
+				require.NoError(tt, err)
+			}
 		})
 	}
 

rpcserver.go

@@ -540,15 +540,13 @@ func (r *rpcServer) MintAsset(ctx context.Context,
 		// If a custom decimal display was requested, add that to the
 		// metadata and re-validate it.
 		if metaType == proof.MetaJson && req.Asset.DecimalDisplay != 0 {
-			updatedMeta, err := seedlingMeta.SetDecDisplay(
+			err := seedlingMeta.SetDecDisplay(
 				req.Asset.DecimalDisplay,
 			)
 			if err != nil {
 				return nil, err
 			}
 
-			seedlingMeta = updatedMeta
-
 			err = seedlingMeta.Validate()
 			if err != nil {
 				return nil, err