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

CAIP-10: Account ID Specification #10

Merged
merged 4 commits into from Mar 31, 2020
Merged

CAIP-10: Account ID Specification #10

merged 4 commits into from Mar 31, 2020

Conversation

@pedrouid
Copy link
Member

pedrouid commented Mar 13, 2020

Proposing a specification for chain-agnostic account id's

Examples

# Ethereum mainnet
0xab16a96d359ec26a11e2c2b3d8f8b8942d5bfcdb@eip155:1

# Bitcoin mainnet 
128Lkh3S7CkDTBZ8W7BbpsN3YYizJMp8p6@bip122:000000000019d6689c085ae165831e93

# Cosmos Hub (Tendermint + Cosmos SDK)
cosmos1t2uflqwqe0fsj0shcfkrvpukewcw40yjj6hdc0@cosmos:cosmoshub-3
@pedrouid

This comment has been minimized.

Copy link
Member Author

pedrouid commented Mar 13, 2020

I would love your feedback on this! 🙏
cc @ligi @webmaster128

pedrouid added 2 commits Mar 13, 2020
@ligi

This comment has been minimized.

Copy link
Member

ligi commented Mar 13, 2020

Thanks for this PR! I think this is a great idea. Just 2 comments after a first read:

  • I think it should be turned around account@chain instead of chain@account as the account is on the chain and the chain is not on the account
  • Perhaps the CAIP should mention use-case(s).
@pedrouid

This comment has been minimized.

Copy link
Member Author

pedrouid commented Mar 13, 2020

I was also leaning towards having them reversed but my only reason for not doing it is that it would make more sense if the order of CAIP-2 chain_id followed the same pattern

Similar example would be the email addresses.

<address>@<domain>.<tld>

If this pattern would be applied to this CAIP, it would be weirdly ordered

<account_address>@<namespace>:<reference>
@ligi

This comment has been minimized.

Copy link
Member

ligi commented Mar 18, 2020

If this pattern would be applied to this CAIP, it would be weirdly ordered

I think it would be less weird actually. Hoping to get some more input/signaling from others

@pedrouid

This comment has been minimized.

Copy link
Member Author

pedrouid commented Mar 18, 2020

Programmatically speaking it's a very simple change. Example:

const parseAccounts = (accounts: string[]) => {
  const accountMap = accounts.map((account: string) => {
    const params = account.split("@");
    return {
      address: params[0],
      chain: params[1]
    };
  };
};

Thus this is simply a human readability preference

@pedrouid

This comment has been minimized.

Copy link
Member Author

pedrouid commented Mar 18, 2020

I've asked @oed and he also signaled preference to reverse. I'm happy to make this change! 👍

@ligi

This comment has been minimized.

Copy link
Member

ligi commented Mar 18, 2020

Great! Will merge after the change

@pedrouid

This comment has been minimized.

Copy link
Member Author

pedrouid commented Mar 18, 2020

Done! 👍

@ligi

This comment has been minimized.

Copy link
Member

ligi commented Mar 18, 2020

That was fast. Just one more thought: do we want to enforce ERC-55? I think this could make sense here as it would enforce uniqueness.

@pedrouid

This comment has been minimized.

Copy link
Member Author

pedrouid commented Mar 18, 2020

I think that CAIP-10 is more analogous to CAIP-2 while the ECR-55 requirement would be more analogous to CAIP-3 where it define Account Address specification for CAIP-3 chains requiring CAIP-10

@pedrouid

This comment has been minimized.

Copy link
Member Author

pedrouid commented Mar 31, 2020

Good to merge @ligi ?

@ligi
ligi approved these changes Mar 31, 2020
@ligi ligi merged commit 66e10ae into ChainAgnostic:master Mar 31, 2020
@ligi

This comment has been minimized.

Copy link
Member

ligi commented Mar 31, 2020

done - thanks for the ping

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.