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

CIP-1855: add derivation path standard for forging tokens #96

Merged

Conversation

disassembler
Copy link
Contributor

@disassembler disassembler commented Jun 2, 2021

@disassembler disassembler force-pushed the cip-forge-derivation-path branch from 89df13c to c1fb283 Compare June 2, 2021 16:57
Copy link
Contributor

@rvl rvl left a comment

Choose a reason for hiding this comment

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

Nice, thanks @disassembler !

Comment on lines 41 to 47
To associate policy keys to a wallet, we reserve however `purpose=1855'` to reserve for policy keys for forging tokens. The coin type remains `coin_type=1815'` to identify Ada as registered in [SLIP-0044]. We use a hardened index for each policy key as derivation is not needed.

We can summarize the various paths and their respective domain in the following table:

| `purpose` | `coin_type` | `policy_ix` |
| --- | --- | --- |
| `1855'` | `1815'` | `[2^31 .. 2^32-1]'` |
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose this is where we start the bike shedding... 😁

Are there any downsides to instead using a new role index under m / 1852´ / 1815´ / account´ (i.e. amendment to CIP-1852 scheme)?

This might be easier for wallets to implement. The token policy keys would belong to a specific account of a wallet.

Also, is there a reason to choose a new purpose when this could be a new coin_type under Cardano 1852´? We could use coin_type 2021´ in recognition of the very contemporary surge in popularity of NFTs.

Copy link
Contributor

Choose a reason for hiding this comment

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

@rvl I would avoid changing the coin type because that requires a change to a SLIP to avoid clashing with other cryptocurrencies

RE: using a new role derivation instead
Is creating tokens something that we consider should be namespaced per-account or something specific to the CIP-1852 standard? Not sure if that's necessarily the case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes you are right - the token policy keys aren't necessarily directly related to a wallet account.

- ERC20 Converter IOHK is developing needs to keep track of policy keys. Rather than having randomly generated policy keys, a policy key can be associated with a mnemonic which is easier to backup.
- A 3rd party may want to have multiple tokens tied to same mnemonic, so we allow an index to specify the token.

- We use a different purpose for mainly two reasons:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would help to also state the reason(s) why those particular derivation indices were chosen, as opposed to other possible derivation indices. And why there are 3 levels of derivation when 2 should work equally well.

Copy link
Contributor

Choose a reason for hiding this comment

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

1855 is just the next number in numerical order.

How do you imagine having only 2 derivation levels? By removing the coin type? I guess it could be removed, but no real advantage either.

Copy link
Contributor

Choose a reason for hiding this comment

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

It seemed a bit redundant to have a second branch for coin type that is always 1815´. Especially because these token policies will be used to mint new coin types.

Copy link
Contributor

Choose a reason for hiding this comment

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

I definitely see your point. For #56 we ended up keeping it

Copy link
Member

Choose a reason for hiding this comment

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

@rvl The coin type is really here to prevent clashes across different crypto-currency projects (if someone else uses 1855 for something else for example). While this does not really matter for desktop wallets of our ecosystem like Daedalus who only care about a single crypto, Hardware wallets or generic wallets like Exodus do manipulate multiple currencies and multiple CIP/BIP/TIP/LIP (you name it) proposals.


```
m / purpose' / coin_type' / account_ix' / role / index
```
Copy link
Member

Choose a reason for hiding this comment

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

Actually no 😶, the path considered is:

m / purpose' / coin_type' / policy_ix'

```


To associate policy keys to a wallet, we reserve however `purpose=1855'` to reserve for policy keys for forging tokens. The coin type remains `coin_type=1815'` to identify Ada as registered in [SLIP-0044]. We use a hardened index for each policy key as derivation is not needed.
Copy link
Member

Choose a reason for hiding this comment

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

as derivation is not needed -> as soft derivation is not needed.

Copy link
Member

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

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

The HD derivation path needs to be adjusted*

@crptmppt crptmppt requested a review from dcoutts July 7, 2021 10:46
@crptmppt
Copy link
Contributor

crptmppt commented Jul 7, 2021

6/29 Editors meeting (25) discussed this PR - see notes. (conversation might be ahead of meeting notes at this point, this is a reference)

=> PR to be merged as CIP-1855 (Draft) shortly
(as noted repeatedly, merging does not imply future implementation or agreement, but facilitates conversation with implementers and the community)

@crptmppt crptmppt requested a review from SebastienGllmt July 13, 2021 06:16
@crptmppt crptmppt merged commit cad0ae2 into cardano-foundation:master Jul 20, 2021
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.

5 participants