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.

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

Implementation of CAIP-2 for EOSIO blockchains #33

Merged
merged 8 commits into from
Dec 7, 2020

Conversation

sebastianmontero
Copy link
Contributor

Initial draft of the CAIP-2 for EOSIO blockchains.

Copy link
Member

@ligi ligi left a comment

Choose a reason for hiding this comment

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

Please also add your CAIP to the list in the README

Copy link
Member

@ligi ligi left a comment

Choose a reason for hiding this comment

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

LGTM

@ligi
Copy link
Member

ligi commented Dec 3, 2020

LGTM - but would love some more eye-ballz on it before the merge @pedrouid @webmaster128 @rekmarks

Copy link
Contributor

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

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

Oh, and this conflicts with https://github.com/ChainAgnostic/CAIPs/blob/master/CAIPs/caip-2.md#syntax. The reference part must not exceed 47 characters, which is why we use prefixes in https://github.com/ChainAgnostic/CAIPs/blob/master/CAIPs/caip-4.md

@webmaster128
Copy link
Contributor

webmaster128 commented Dec 3, 2020

The alternative to prefixes is to use a different encoding for the 32 bytes. CAIP-2 is case-sensitive and you need 32/47 = 68% efficiency. So candidates are Base58 or Base62 (no idea how this encoding works but promising in theory).

However, I think it is easier for human consumption to use a prefix of the default encoding than to re-encode it.

@sebastianmontero
Copy link
Contributor Author

I've updated the reference definition, to use a 32 character prefix of the Chain ID

Copy link
Contributor

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

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

Some wording 💅 but otherwise 👍

Co-authored-by: Simon Warta <[email protected]>
Copy link
Contributor

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

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

Cool and welcome

@sebastianmontero
Copy link
Contributor Author

Great! Thanks!

@webmaster128
Copy link
Contributor

@ligi can we merge this? I don't have permissions to do so.

@ligi ligi merged commit 076138e into ChainAgnostic:master Dec 7, 2020
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.

3 participants