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.

Sign up for GitHub

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

taprpc: decimal display requires JSON meta #983

Merged
merged 1 commit into from
Jul 1, 2024
Merged

taprpc: decimal display requires JSON meta #983

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 11 additions & 2 deletions itest/asset_meta_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,11 +172,20 @@ 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 type as
// JSON 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 JSON")
jharveyb marked this conversation as resolved.
Show resolved Hide resolved

// If we update the decimal display to match the group anchor, minting
// should succeed. We also unset the metadata to ensure that the decimal
// display is set as the sole JSON object if needed.
secondAssetReq.Asset.AssetMeta.Data = []byte{}
secondAssetReq.Asset.DecimalDisplay = firstAsset.DecimalDisplay
secondAssetReq.Asset.AssetMeta = &taprpc.AssetMeta{
Type: taprpc.AssetMetaType_META_TYPE_JSON,
}
MintAssetsConfirmBatch(
t.t, t.lndHarness.Miner.Client, t.tapd,
[]*mintrpc.MintAssetRequest{secondAssetReq},
Expand Down
8 changes: 8 additions & 0 deletions rpcserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,14 @@ func (r *rpcServer) MintAsset(ctx context.Context,
}

var seedlingMeta *proof.MetaReveal

// If a custom decimal display is set, the meta type must also be set to
// JSON.
if req.Asset.DecimalDisplay != 0 && req.Asset.AssetMeta == nil {
return nil, fmt.Errorf("decimal display requires JSON asset " +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be better to instead bootstrap the meta json with just the decimal display field? otherwise a user will have to provide an empty json {} for this to work, which might not be intuitive

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with setting to an empty meta json if the meta field nil in the request but a decimal display is given.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The AssetMeta field here is both Data and Type. So we require Type to be set t JSON for decimal display to get set as the only JSON field if the metadata data field was empty.

Was implemented in #979 , seen here:

taproot-assets/rpcserver.go

Line 533 in 311231d

if metaType == proof.MetaJson && req.Asset.DecimalDisplay != 0 {

This check is added before that bootstrapping to enforce that the user at least set the correct metadata type.

"metadata")
}

if req.Asset.AssetMeta != nil {
// Ensure that the meta type is valid.
metaType, err := proof.IsValidMetaType(req.Asset.AssetMeta.Type)
Expand Down
Loading