Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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
CIP-0014: User-facing Asset Fingerprint #64
CIP-0014: User-facing Asset Fingerprint #64
Changes from all commits
088daec
069a1e5
645243e
2e0638b
0154d08
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KtorZ To make it clear - even though the hash digest was set to 20 bytes, the current bech32 assetId length exceeds Ledger's screen capacity of 20 characters (it has the "asset1" prefix, checksum at the end and one bech32 character is not the same as 1 byte, obviously) - so users will indeed have to scroll on Ledger to see it fully.
However, given that we chose to go with a hash of the policyId and assetName concatenation, even the first screenful of the id should be a reasonable assurance that given assetId is correct, though it's definitely recommendable to scroll through the whole assetId to be completely sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know :( .. bech32 isn't the "smallest" encoding possible, base64 with no padding would produce shorter strings but they'd also be too long. My hope is that this is short-enough to still allow users to check them, if not fully, at least as much as possible from what they get on the first screen 😬
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about more descriptive such as
humanReadablePart = 'fngrprnt'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Referring to it as fingerprint would actually be less descriptive 😶 There are many things that can be a fingerprint of something. Yet here, it is used to referred to an asset, hence the prefix. In the same way we don't use
hash
as a prefix for key hashes for instance, but we use:addr_vkh
orstake_vkh
👍