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

ADR 0008: Standard Account Key Generation implementation #3918

Merged
merged 2 commits into from
May 20, 2021

Conversation

tjanez
Copy link
Member

@tjanez tjanez commented May 4, 2021

Implementation of ADR 0008.

@tjanez tjanez added c:apps Category: application support c:keymgmt Category: key management c:staking Category: staking labels May 4, 2021
@tjanez tjanez force-pushed the tjanez/adr-0008-implementation branch from 0da0e00 to a32a46c Compare May 6, 2021 16:33
@tjanez tjanez changed the base branch from master to yawning/feature/curve25519-voi May 6, 2021 16:34
Base automatically changed from yawning/feature/curve25519-voi to master May 7, 2021 03:16
@tjanez tjanez changed the title ADR 0008: Standard Account Key Generation Process implementation ADR 0008: Standard Account Key Generation implementation May 7, 2021
@tjanez tjanez force-pushed the tjanez/adr-0008-implementation branch from a32a46c to dd28f18 Compare May 7, 2021 12:22
@tjanez tjanez changed the base branch from master to tjanez/adr-key-derivation May 7, 2021 12:22
@tjanez tjanez force-pushed the tjanez/adr-0008-implementation branch from dd28f18 to 3eeb698 Compare May 7, 2021 12:24
@tjanez tjanez changed the base branch from tjanez/adr-key-derivation to master May 7, 2021 12:25
@tjanez tjanez force-pushed the tjanez/adr-0008-implementation branch from 3eeb698 to 3243f71 Compare May 7, 2021 12:25
@tjanez tjanez marked this pull request as ready for review May 7, 2021 12:26
Copy link
Member

@kostko kostko left a comment

Choose a reason for hiding this comment

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

Very nice!

docs/adr/0008-standard-account-key-generation.md Outdated Show resolved Hide resolved
@kostko kostko added the c:docs/adr Category: documentation/ADR label May 7, 2021
@tjanez tjanez removed the c:docs/adr Category: documentation/ADR label May 7, 2021
@tjanez tjanez force-pushed the tjanez/adr-0008-implementation branch from 3243f71 to 2c15fc9 Compare May 7, 2021 19:22
go/common/crypto/sakg/sakg.go Outdated Show resolved Hide resolved
## Standard Account Key Generation

When generating an [account]'s private/public key pair, follow [ADR 0008:
Standard Account Key Generation][ADR 0008].
Copy link
Contributor

Choose a reason for hiding this comment

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

How do we intend to handle "nothing up until this point (strictly speaking even after this point), used this method"?
Does it not matter? Migration tools? Should the oasis-core tooling be updated to use this stuff?

Copy link
Member Author

Choose a reason for hiding this comment

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

Upcoming wallets using ts-web JavaScript SDK will be the first users of this: oasisprotocol/oasis-sdk#119.

We should extend the Oasis Node CLI to support generating an account key (i.e. entity) from a mnemonic.

Upcoming TrustWallet integration will use ADR 0008.

The Oasis' Ledger app will need to support both the existing (legacy) account key generation used now and the new standard account key generation defined in ADR 0008.

@tjanez tjanez force-pushed the tjanez/adr-0008-implementation branch 2 times, most recently from 4c6ef43 to 330b82b Compare May 12, 2021 12:10
go/common/crypto/sakg/bip32.go Show resolved Hide resolved
go/common/crypto/sakg/bip32.go Outdated Show resolved Hide resolved
go/common/crypto/sakg/bip32.go Outdated Show resolved Hide resolved
go/common/crypto/sakg/sakg.go Outdated Show resolved Hide resolved
@tjanez tjanez force-pushed the tjanez/adr-0008-implementation branch 2 times, most recently from c68dd86 to 3df0ae2 Compare May 19, 2021 12:00
@tjanez tjanez force-pushed the tjanez/adr-0008-implementation branch from 3df0ae2 to 88fb230 Compare May 19, 2021 12:21
@tjanez tjanez enabled auto-merge May 19, 2021 12:31
@tjanez tjanez force-pushed the tjanez/adr-0008-implementation branch from 88fb230 to ce2a227 Compare May 20, 2021 16:30
@tjanez tjanez merged commit 9864bcd into master May 20, 2021
@tjanez tjanez deleted the tjanez/adr-0008-implementation branch May 20, 2021 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c:apps Category: application support c:keymgmt Category: key management c:staking Category: staking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants