Skip to content

Conversation

@miketlk
Copy link
Contributor

@miketlk miketlk commented Jun 28, 2024

This is an extension of ELIP 100 specification, adding a new proprietary field to PSET specific to hardware wallets.

This new field, PSBT_ELEMENTS_HWW_GLOBAL_REISSUANCE_TOKEN defines a reissuance token and is supposed to be an optional supplement to the already existing asset metadata field - PSBT_ELEMENTS_HWW_GLOBAL_ASSET_METADATA. Its purpose is to allow the hardware wallet to display textual information related to reissuance token it could encounter while processing the transaction.

The proposed change is non-breaking and should be fully compatible with existing implementations of ELIP 100.

@apoelstra
Copy link
Member

Shouldn't this be input-specific rather than global?

@miketlk
Copy link
Contributor Author

miketlk commented Jun 29, 2024

For sure, there could be benefits if it would be input-specific, like less cluttered global space. But in practice, it will be much less convenient for hardware wallet use. Especially, for very resource-restricted platforms like Ledger Nano S where there is not enough memory to hold the whole PSET or even just a list of its assets and tokens. On this platform, the PSET is converted to Merkle-tree type structure and streamed from the host in small chunks that the wallet processes. Occasionally, the wallet needs to scroll back and request some pieces more than once, but it should be kept to a minimum to not make processing too long.

So in this case, when the wallet would need to display the asset or token information for a specific output, it would need to rescan all the inputs to find it. Storing asset and token data globally allows doing it much quicker using a simple key-value request to host.

@apoelstra
Copy link
Member

Right, I see. And the existing PSBT_ELEMENTS_HWW_GLOBAL_ASSET_METADATA is already a global table for data which is conceptually per-input, I guess for similar reasons.

And for non-resource-constrained systems, if they want to support ELIP-100 they should validate the data, but they are already free to use a per-input model derived from the PSBT_ELEMENTS_IN_ISSUANCE_ASSET_ENTROPY field.

Ok, ACK 1efbac4 except that I'd slightly prefer the text the information stored in the associated asset metadata fields to be more specific about what needs to be computed. (But I don't feel strongly about this.)

I would like somebody closer to PSET and wallet development to also take a look at this -- cc @RCasatta do you know who would be good?

@RCasatta
Copy link
Contributor

RCasatta commented Jul 1, 2024

I and @LeoComandini in particular are following this proposed changes along...

@miketlk
Copy link
Contributor Author

miketlk commented Jul 1, 2024

... except that I'd slightly prefer the text the information stored in the associated asset metadata fields to be more specific about what needs to be computed. (But I don't feel strongly about this.)

Thanks. How about phrasing this piece as follows:

... If any of the reissuance token definition fields are present, the Signer must perform the verification of these fields by computing reisuance tag using the information stored in the associated asset metadata fields and the value of issuanceBlinded flag. Verification is considered successful if the computed value of reissuance token matches the one included in keydata.

I'd like to include a reference to the formulas for this computation, but didn't find the official spec where they are defined. I used comments in Elements core as a reference for my implementation:

https://github.com/ElementsProject/elements/blob/cdcc74bbcc10cec65298626e30eac78c979becf4/src/issuance.cpp#L23-L64

@LeoComandini
Copy link
Contributor

ACK c020b4f, code review (also with the suggested change in a comment above)

Changes are consistent with the existing asset metadata entry.

This change is backward compatible, so it should be harmless for who's already using PSET.

This allows signers to display the reissuance token in a user friendly way, which to me is highly desirable.

@miketlk
Copy link
Contributor Author

miketlk commented Jul 1, 2024

Added 32ab6d5 with improved description of the Signer's role in connection to these changes.

@LeoComandini
Copy link
Contributor

ACK 32ab6d5

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

utACK 32ab6d5

@apoelstra apoelstra merged commit 340cd4c into ElementsProject:main Jul 1, 2024
apoelstra added a commit to ElementsProject/rust-elements that referenced this pull request Jul 5, 2024
1166089 pset: elip100: add and get token metadata (Leonardo Comandini)

Pull request description:

  Following changes in ElementsProject/ELIPs#17

ACKs for top commit:
  RCasatta:
    ACK 1166089

Tree-SHA512: 5e8f1d309c4781f10130f9e6b8f8e6d17582e5e5292921e5a372b34a3d55c9de0a7a1496b47fc51b155a0e8072fb25895f4c05592694d13d09416889cbe40f0f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants