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

[Feature] Generate Wallet from secret numbers #1799

Merged
merged 35 commits into from
Aug 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
42bb7bb
add the possibility to create a wallet from Secret Numbers
nixer89 Nov 12, 2021
99fb3bd
Merge branch 'develop' into ds/wallet-from-secret-numbers
nixer89 Nov 12, 2021
976a77a
add back xrpl-secret-numbers lib after merge
nixer89 Nov 12, 2021
55245e0
move dependency from top level package.json to xrpl package.json
nixer89 Nov 15, 2021
75ccbbd
Merge branch 'develop' into ds/wallet-from-secret-numbers
nixer89 Nov 15, 2021
02f638f
Merge branch 'develop' into ds/wallet-from-secret-numbers
ledhed2222 Nov 16, 2021
8e14f6a
update package lock
ledhed2222 Nov 16, 2021
01a3938
Merge branch 'ds/wallet-from-secret-numbers' of https://github.com/XR…
ledhed2222 Nov 16, 2021
ce833a6
fix tsc errors in ripple-keypairs
ledhed2222 Nov 16, 2021
ea32c88
run lint --fix
ledhed2222 Nov 16, 2021
e558399
Merge branch 'develop' into ds/wallet-from-secret-numbers
nixer89 Nov 23, 2021
88e9b4d
Update packages/xrpl/src/Wallet/index.ts
nixer89 Nov 23, 2021
63c469f
updates for PR
nixer89 Nov 23, 2021
c655993
fix lint
nixer89 Nov 23, 2021
45b8d91
lint fix #2
nixer89 Nov 23, 2021
22e104e
Regenerated the lockfile
JST5000 Nov 30, 2021
700a18b
use 'const' over 'let' and simply forward opts
nixer89 Dec 16, 2021
7efdbe7
Merge branch 'main' into ds/wallet-from-secret-numbers
nixer89 Dec 16, 2021
15cfee9
fix lint line ending error on windows
nixer89 Dec 16, 2021
7e83e17
Merge branch 'main' into ds/wallet-from-secret-numbers
JST5000 Jul 10, 2023
e50e7a2
Undo lockfile regeneration
JST5000 Jul 10, 2023
d606d9c
Use latest secret-numbers implementation
JST5000 Jul 11, 2023
130594d
Comment broken ed25519 test + add TODO
JST5000 Jul 13, 2023
451cdbf
Merge branch 'main' into ds/wallet-from-secret-numbers
JST5000 Jul 13, 2023
a8d3a93
Lint
JST5000 Jul 13, 2023
0336c26
Simplify default value
JST5000 Jul 18, 2023
ca9cebf
Merge branch 'main' into ds/wallet-from-secret-numbers
JST5000 Jul 29, 2023
d3a467a
Move fromSecretNumbers to its own file
JST5000 Aug 2, 2023
d416f5a
Merge branch 'main' into ds/wallet-from-secret-numbers
JST5000 Aug 3, 2023
91f9f9a
Merge branch 'main' into ds/wallet-from-secret-numbers
JST5000 Aug 4, 2023
74bf3b7
Merge branch 'main' into ds/wallet-from-secret-numbers
JST5000 Aug 7, 2023
db77d01
Separate out lint changes from this PR
JST5000 Aug 7, 2023
61d62fd
Update HISTORY.md
JST5000 Aug 11, 2023
ac17c09
Merge branch 'main' into ds/wallet-from-secret-numbers
JST5000 Aug 11, 2023
0eef2c6
Merge branch 'main' into ds/wallet-from-secret-numbers
ckniffen Aug 14, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 27 additions & 9 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"@types/node": "^14.18.35",
"@types/ws": "^8.2.0",
"@typescript-eslint/eslint-plugin": "^5.28.0",
"@typescript-eslint/parser": "^5.28 .0",
"@typescript-eslint/parser": "^5.28.0",
"@xrplf/eslint-config": "^1.9.1",
"@xrplf/prettier-config": "^1.9.1",
"assert": "^2.0.0",
Expand Down
3 changes: 3 additions & 0 deletions packages/xrpl/HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
Subscribe to [the **xrpl-announce** mailing list](https://groups.google.com/g/xrpl-announce) for release announcements. We recommend that xrpl.js (ripple-lib) users stay up-to-date with the latest stable release.
## Unreleased

### Added
* Add `walletFromSecretNumbers` to derive a wallet from [XLS-12](https://github.com/XRPLF/XRPL-Standards/issues/15). Currently only works with `secp256k1` keys, but will work with `ED25519` keys as part of 3.0 via [#2376](https://github.com/XRPLF/xrpl.js/pull/2376).

## 2.10.0 (2023-08-07)

### Added
Expand Down
3 changes: 2 additions & 1 deletion packages/xrpl/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@
"ripple-address-codec": "^4.3.0",
"ripple-binary-codec": "^1.8.0",
"ripple-keypairs": "^1.3.0",
"ws": "^8.2.2"
"ws": "^8.2.2",
"xrpl-secret-numbers": "^0.3.3"
Copy link
Collaborator

Choose a reason for hiding this comment

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

should this be included in the monorepo?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe wait to bring it up to the top level until another package needs it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

no i mean, should xrpl-secret-numbers's source be in this monorepo. not sure where else it's used or how big it is, but that might make sense

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am sure the XRPL-Labs team uses this lib in their XUMM app. I think @WietseWind is the maintainer?
For now I would leave it as "external" import and would not add it to the monorepo. but maybe Wietse thinks this would be a good idea :)

Copy link
Member

Choose a reason for hiding this comment

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

@ledhed2222 It's a fairly small package, and the (non dev) dependencies overlap with ripple-keypairs (brorand, ripple-keypairs)

Copy link
Collaborator

Choose a reason for hiding this comment

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

that would be a good reason to maintain it within the monorepo :)

IMO, the criteria for whether it should be part of the monorepo:

  1. is the purpose/scope of the package overlapping with the monorepo's? in this case i think it might be
  2. does it depend on other packages w/in this monorepo, potentially requiring work on it and ripple-keypairs simultaneously.

},
"devDependencies": {
"@geut/browser-node-core": "^2.0.13",
Expand Down
37 changes: 37 additions & 0 deletions packages/xrpl/src/Wallet/walletFromSecretNumbers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { Account } from 'xrpl-secret-numbers'

import ECDSA from '../ECDSA'

import { Wallet } from '.'

/**
* Derives a wallet from secret numbers.
* NOTE: This uses a default encoding algorithm of secp256k1 to match the popular wallet
* [Xumm (aka Xaman)](https://xumm.app/)'s behavior.
* This may be different from the DEFAULT_ALGORITHM for other ways to generate a Wallet.
*
* @param secretNumbers - A string consisting of 8 times 6 numbers (whitespace delimited) used to derive a wallet.
* @param opts - (Optional) Options to derive a Wallet.
* @param opts.masterAddress - Include if a Wallet uses a Regular Key Pair. It must be the master address of the account.
* @param opts.algorithm - The digital signature algorithm to generate an address for.
* @returns A Wallet derived from secret numbers.
* @throws ValidationError if unable to derive private key from secret number input.
*/
export function walletFromSecretNumbers(
secretNumbers: string[] | string,
opts?: { masterAddress?: string; algorithm?: ECDSA },
): Wallet {
const secret = new Account(secretNumbers).getFamilySeed()
const updatedOpts: { masterAddress?: string; algorithm?: ECDSA } = {
masterAddress: undefined,
algorithm: undefined,
}
// Use secp256k1 since that's the algorithm used by popular wallets like Xumm when generating secret number accounts
if (opts === undefined) {
updatedOpts.algorithm = ECDSA.secp256k1
} else {
updatedOpts.masterAddress = opts.masterAddress
updatedOpts.algorithm = opts.algorithm ?? ECDSA.secp256k1
}
return Wallet.fromSecret(secret, updatedOpts)
}
2 changes: 2 additions & 0 deletions packages/xrpl/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ export * from './errors'

export { Wallet } from './Wallet'

export { walletFromSecretNumbers } from './Wallet/walletFromSecretNumbers'

export { keyToRFC1751Mnemonic, rfc1751MnemonicToKey } from './Wallet/rfc1751'

export * from './Wallet/signer'
87 changes: 86 additions & 1 deletion packages/xrpl/test/wallet/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import { assert } from 'chai'
import { decode } from 'ripple-binary-codec'

import { NFTokenMint, Payment, Transaction } from '../../src'
import {
NFTokenMint,
Payment,
Transaction,
walletFromSecretNumbers,
} from '../../src'
import ECDSA from '../../src/ECDSA'
import { Wallet } from '../../src/Wallet'
import requests from '../fixtures/requests'
Expand Down Expand Up @@ -297,6 +302,86 @@ describe('Wallet', function () {
})
})

describe('fromSecretNumbers', function () {
const secretNumbersString =
'399150 474506 009147 088773 432160 282843 253738 605430'
const secretNumbersArray = [
'399150',
'474506',
'009147',
'088773',
'432160',
'282843',
'253738',
'605430',
]

const publicKey =
'03BFC2F7AE242C3493187FA0B72BE97B2DF71194FB772E507FF9DEA0AD13CA1625'
const privateKey =
'00B6FE8507D977E46E988A8A94DB3B8B35E404B60F8B11AC5213FA8B5ABC8A8D19'
// TODO: Uncomment when the `deriveKeypair` fix is merged so `deriveSecretNumbers` with ed25519 works.
// const publicKeyED25519 =
// 'ED8079E575450E256C496578480020A33E19B579D58A2DB8FF13FC6B05B9229DE3'
// const privateKeyED25519 =
// 'EDD2AF6288A903DED9860FC62E778600A985BDF804E40BD8266505553E3222C3DA'

it('derives a wallet using default algorithm', function () {
const wallet = walletFromSecretNumbers(secretNumbersString)

assert.equal(wallet.publicKey, publicKey)
assert.equal(wallet.privateKey, privateKey)
})

it('derives a wallet from secret numbers as an array using default algorithm', function () {
const wallet = walletFromSecretNumbers(secretNumbersArray)

assert.equal(wallet.publicKey, publicKey)
assert.equal(wallet.privateKey, privateKey)
})

it('derives a wallet using algorithm ecdsa-secp256k1', function () {
const algorithm = ECDSA.secp256k1
const wallet = walletFromSecretNumbers(secretNumbersString, {
algorithm,
})

assert.equal(wallet.publicKey, publicKey)
assert.equal(wallet.privateKey, privateKey)
})

// TODO: Uncomment this test when the `deriveKeypair` fix is merged
// it('derives a wallet using algorithm ed25519', function () {
// const algorithm = ECDSA.ed25519
// const wallet = Wallet.fromSecretNumbers(secretNumbersString, {
// algorithm,
// })

// assert.equal(wallet.publicKey, publicKeyED25519)
// assert.equal(wallet.privateKey, privateKeyED25519)
// })
Comment on lines +353 to +362
Copy link
Contributor

Choose a reason for hiding this comment

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

This test fails because of this issue: #2376

Commented with a TODO to re-add the test once that PR is resolved. (Open to other ideas about what's the best way to handle this :) )


it('derives a wallet using a Regular Key Pair', function () {
const masterAddress = 'rUAi7pipxGpYfPNg3LtPcf2ApiS8aw9A93'
const regularKeyPair = {
secretNumbers:
'399150 474506 009147 088773 432160 282843 253738 605430',
publicKey:
'03BFC2F7AE242C3493187FA0B72BE97B2DF71194FB772E507FF9DEA0AD13CA1625',
privateKey:
'00B6FE8507D977E46E988A8A94DB3B8B35E404B60F8B11AC5213FA8B5ABC8A8D19',
}

const wallet = walletFromSecretNumbers(regularKeyPair.secretNumbers, {
masterAddress,
})

assert.equal(wallet.publicKey, regularKeyPair.publicKey)
assert.equal(wallet.privateKey, regularKeyPair.privateKey)
assert.equal(wallet.classicAddress, masterAddress)
})
})

describe('fromEntropy', function () {
let entropy: number[]
const publicKey =
Expand Down