ElementsProject / ELIPs

Elements Improvement proposals
12 stars 10 forks source link

ELIP 100: New field of PSET for reissuance token #17

Closed miketlk closed 3 months ago

miketlk commented 3 months ago

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 commented 3 months ago

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

miketlk commented 3 months ago

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 commented 3 months ago

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 1efbac441b1e7f6b123a4238b032ea6cc450ddfd 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 commented 3 months ago

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

miketlk commented 3 months ago

... 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 commented 3 months ago

ACK c020b4f29c732a8592b91a98c58efa901eb6fb10, 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 commented 3 months ago

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

LeoComandini commented 3 months ago

ACK 32ab6d5bc6f47b9ce9b07e81576cb3c30810b628