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

BIP 44 derivation path #42

Merged
merged 6 commits into from
Feb 12, 2021
Merged

BIP 44 derivation path #42

merged 6 commits into from
Feb 12, 2021

Conversation

armaniferrante
Copy link
Contributor

@armaniferrante armaniferrante commented Nov 12, 2020

Addresses https://github.com/project-serum/spl-token-wallet/issues/9.

BIP 44 derivation path update.

All newly created wallets will use BIP 44, defaulting to the path m/44'/501'/0'/0'. Existing wallets are unchanged. And restored (from mnemonic) wallets have the option to choose between m/44'/501'/0'/0', m/44'/501'/0', and the old deprecated path.

Screenshot from 2020-11-12 13-54-40
Screenshot from 2021-02-12 13-48-26

@armaniferrante armaniferrante force-pushed the armani/bip44 branch 3 times, most recently from f590350 to fcb5d2a Compare November 12, 2020 22:17
@@ -112,10 +112,11 @@ const useStyles = makeStyles((theme) => ({
},
}));

function BalanceListItem({ publicKey }) {
export function BalanceListItem({ publicKey, expandable }) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Re-used this component for the derivable accounts component, but without the ability to expand into details.

successMessage: 'Wallet created',
});
callAsync(
storeMnemonicAndSeed(mnemonic, seed, password, DERIVATION_PATH.bip44),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All newly created wallets will use BIP 44.

@armaniferrante armaniferrante marked this pull request as ready for review November 12, 2020 22:18
src/utils/wallet.js Outdated Show resolved Hide resolved
@@ -6,7 +6,7 @@ import { EventEmitter } from 'events';

export async function generateMnemonicAndSeed() {
const bip39 = await import('bip39');
const mnemonic = bip39.generateMnemonic(128);
const mnemonic = bip39.generateMnemonic(256);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switching to use 24 word mnemonics so that we derive the same keys for ledger.

Copy link
Contributor

@nathanielparke nathanielparke left a comment

Choose a reason for hiding this comment

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

LGTM

@armaniferrante armaniferrante merged commit 3fc7be0 into master Feb 12, 2021
@armaniferrante armaniferrante deleted the armani/bip44 branch February 12, 2021 09:16
@huazhouwang
Copy link

Why not derive to the address level?

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