Page MenuHomePhabricator

[Apps][Examples] Create wallet
ClosedPublic

Authored by emack on Jun 30 2023, 13:32.

Details

Reviewers
bytesofman
Group Reviewers
Restricted Project
Commits
rABC36f0fb5bdcbf: [Apps][Examples] Create wallet
Summary

Example using bitgo and bip39 libraries to create a new wallet and derivate its attributes

Test Plan

npm i
npm test
npm run createWallet

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

emack requested review of this revision.Jun 30 2023, 13:32
bytesofman added inline comments.
apps/examples/package.json
19 ↗(On Diff #41121)

nit for all of these: you don't need to include .js here

apps/examples/scripts/createWallet.js
10–15 ↗(On Diff #41121)

nit depending on your IDE

in VS code, you can just type /** and push enter, it will auto generate code comments including the params of your function if you type this above a defined function, e.g. doing so for this function would give

/**
 *  
 * @returns
 */

which you could then fill in. If you have params, it would include those.
This also provides a unique comment format for functions (which is something of a standard across JS, in that I've seen it some places and VS code supports it, although it's certainly not universal) as opposed to general comments.

Not a blocker but imo it's a timesaver and nice to have for the JS apps in the monorepo, should converge on this as a standard.

17 ↗(On Diff #41121)

Should include some comments here as we are writing these scripts as examples for developers. Discussion of how english is typically the recommended default but it is possible to use various languages, ref bip39 documentation

20 ↗(On Diff #41121)

tough var name but we should definitely avoid stringing numbers together like this.

In this case, I think it's ok to just call it mnemonic, as the comment explains exactly what kind of mnemonic it is

Useful coment would be to say that seed and mnemonic are often used interchangeably. We're talking about a human-readable private key backup here.

26 ↗(On Diff #41121)

hm there is a ton of stuff we could say about this. We should probably just link to some documentation about HD wallets and derivation paths, e.g. https://github.com/bitcoin/bips/blob/master/bip-0039.mediawiki

34 ↗(On Diff #41121)

Need some discussion about this one. minimum, "For eCash wallets, Path 1899 is used for a wallet that supports eTokens. Path 899 is used for a wallet that does not support eTokens."

37 ↗(On Diff #41121)

Some comments about the available types would be useful, e.g. "p2pkh - standard for most wallets; p2sh - multisig or other advanced features"

This revision now requires changes to proceed.Jun 30 2023, 16:50
emack marked 7 inline comments as done.

added contextual comments

resolved merge conflicts in package-lock.json

apps/examples/scripts/createWallet.js
10–15 ↗(On Diff #41121)

yup good call, I use atom ide but it seems to only be smart enough to split out the comment structure without the params. Might switch over to VSc at some stage.

bytesofman requested changes to this revision.Jul 1 2023, 05:38
bytesofman added inline comments.
apps/examples/scripts/createWallet.js
12 ↗(On Diff #41147)
37 ↗(On Diff #41147)
48 ↗(On Diff #41147)

latest version of ecashaddrjs accepts lowercase address type input. worth using imo since chronik-client always accepts lowercase address type input

This revision now requires changes to proceed.Jul 1 2023, 05:38
emack marked 3 inline comments as done.

updated comments

bytesofman requested changes to this revision.Jul 1 2023, 21:09
bytesofman added inline comments.
apps/examples/scripts/createWallet.js
51 ↗(On Diff #41153)

Need up upgrade ecashaddrjs to 1.5.2 for this support, npm i ecashaddrjs@latest

This revision now requires changes to proceed.Jul 1 2023, 21:09
emack marked an inline comment as done.

Upgraded ecashaddrjs

bytesofman requested changes to this revision.Jul 2 2023, 03:34
bytesofman added inline comments.
apps/examples/test/createWallet.test.js
11–20 ↗(On Diff #41174)

Since there are no expected error conditions, just do one test that checks everything, i.e.

  • keep assert.strictEqual(isValidAddress, true); and assert.strictEqual(isValidMnemonic, true);
  • Confirm that Object.keys(wallet) contains address, publicKey, mnemonic, derivationPath
  • Also confirm derivation path is 1899.
This revision now requires changes to proceed.Jul 2 2023, 03:34
emack marked an inline comment as done.

updated unit tests

This revision is now accepted and ready to land.Jul 2 2023, 12:56
This revision was automatically updated to reflect the committed changes.