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

Deterministic secrets and restore from seed phrase 🔥 #91

Merged
merged 24 commits into from
Jan 18, 2024
Merged

Conversation

gandlafbtc
Copy link
Collaborator

@gandlafbtc gandlafbtc commented Oct 4, 2023

new feature: Deterministic secrets and restore from seed phrase 🔥

Description

This PR introduces the functionality for creating ecash derived from a seed. This allows for a backup and restore scheme where the wallet owner only needs to store the 12 words seed phrase to restore his ecash.

Changes

External changes

  • Add and expose function generateNewMnemonic() for creating new mnemonics
  • Add optional mnemonic param to pass mnemonic to CashuWallet constructor
  • Add optional count param to all functions that create tokens, to pass the current state of the derivation path
  • Add restore() function that recreates ecash that has previously been signed by the mint (doesn't check for validity!)

Internal changes

  • Add paths for deterministic secret and blinding factor if mnemonic and count are present
  • Implementation secret and blinding factor derivation from seed
  • refactored createblankOutputs to reduce duplicate code

There are no breaking changes, deterministic secrets is opt in. By ommiting count or mnemonic params, ecash will be generated randomly.

how it works

presentation slides --> https://det-sec.gandlaf.com/

Usage

import { CashuMint, CashuWallet, generateNewMnemonic } from '@cashu/cashu-ts';

//generate seed phrase
const seed = generateNewMnemonic();

const cashuMint = new CashuMint('https://.........');

// initialize wallet with seed phrase
const wallet = new CashuWallet(cashuMint, mint.keys, seed);

// as the implementer we have to keep track of the current counter of a keyset as an example, 
// we asume we store the counter in keysetID.counter. (The counter needs to be persistent)
// pass counter into function that creates proofs. 
const {proofs, newKeys} = await wallet.receive({{ some token }}, undefined, keysetID.counter) 

// You need to update your wallet internal counter,
// for the keysetID by the number of proofs that were created
keysetID.counter += proofs.length

// restore. can be called multiple times as long as we find new signatures. 
//In this case it would recreate the blinded messages from 100 to 200
const { proofs, newKeys } = await wallet.restore(100, 100);

// check for valid ecash after creating proofs
const {proofs: validProofs} = await wallet.checkProofsSpent(proofs)

PR Tasks

  • Open PR
  • run npm run test --> no failing unit tests
  • run npm run format

@dipunm
Copy link
Contributor

dipunm commented Nov 4, 2023

First note, can we update the example in the description? I don't really know where keysetID.COUNTER comes from. I can't see any references.

test/wallet.test.ts Outdated Show resolved Hide resolved
src/model/types/index.ts Outdated Show resolved Hide resolved
src/CashuWallet.ts Outdated Show resolved Hide resolved
src/CashuWallet.ts Outdated Show resolved Hide resolved
src/CashuWallet.ts Outdated Show resolved Hide resolved
src/CashuWallet.ts Outdated Show resolved Hide resolved
src/CashuWallet.ts Outdated Show resolved Hide resolved
src/secrets.ts Outdated Show resolved Hide resolved
src/secrets.ts Outdated Show resolved Hide resolved
src/secrets.ts Outdated Show resolved Hide resolved
src/secrets.ts Outdated Show resolved Hide resolved
src/CashuWallet.ts Outdated Show resolved Hide resolved
src/CashuWallet.ts Outdated Show resolved Hide resolved
@dipunm
Copy link
Contributor

dipunm commented Nov 4, 2023

I was getting confused with some of the variable names, help me out here:

  • count: is this really like a deterministicNoteIndex? (dni?). It represents the index used to generate both blinding factor and secret used for an ecash token.
  • amount: when restoring, it seems amount 0 means unknown. The server provides the amount in its response.

One thing to note, what happens if there is a conflict? I imagine that the same note can be signed for with different amounts (i.e. count=1,amount=1 and count=1,amount=4 can both exist. So if you send a count=1,amount=0 token to a mint, how should it respond if it finds multiple signatures for that token?

Should mints be guarding against this case themselves?

src/CashuWallet.ts Outdated Show resolved Hide resolved
src/CashuWallet.ts Outdated Show resolved Hide resolved
src/CashuWallet.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@callebtc callebtc left a comment

Choose a reason for hiding this comment

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

Great work! Only nitpicks I have are about variable naming!

src/CashuWallet.ts Outdated Show resolved Hide resolved
src/CashuWallet.ts Show resolved Hide resolved
src/CashuWallet.ts Outdated Show resolved Hide resolved
src/CashuWallet.ts Outdated Show resolved Hide resolved
src/secrets.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@dipunm dipunm left a comment

Choose a reason for hiding this comment

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

Awesome work. Only nitpick is that count variable, but consider this my ACK.

@prusnak
Copy link

prusnak commented Nov 5, 2023

What bip32 paths were used for secrets and blinding factors?

@callebtc
Copy link
Contributor

callebtc commented Nov 6, 2023

What bip32 paths were used for secrets and blinding factors?

We haven't officially specced this in a NUT yet but we're now using the same path for two implementations.

image

Source: cashubtc/nutshell#131

@KKA11010 KKA11010 mentioned this pull request Nov 24, 2023
3 tasks
@minibits-cash
Copy link
Contributor

One remark to the example code:

const wallet = new CashuWallet(cashuMint, mint.keys, seed);

It is unclear where the mint.keys comes from, yet it is crucial for the code to run. Initializing wallet without keys is possible and if seed is provided causes low-level failure:

SyntaxError: can't convert string to bigint

Suggestion is that keys param is validated for presence if seed is provided.

@minibits-cash
Copy link
Contributor

For deterministic seed based wallet, there seems to be a bug when creating blinded messages with counter that is passed into the derivation path. Bug causes the messages to keep and send having the duplicate _B, subsequently being refused by the mint validation when passed to the /split service.

Log showing the issue:

 LOG  14:13:51:104 | TRACE : [getWallet] Returning new cashuWallet instance with seed
 **LOG  [createRandomBlindedMessages] {"amounts": [2, 4, 8]}**
 LOG  [createBlindedMessages] {"amounts": [2, 4, 8], **"startingCounter": 5}**
 LOG  [createBlindedMessages] {"amount": 2, "currentCounter": 5, "i": 0}
 LOG  [derive] {"derivationPath": "m/129372'/0'/361559222'/5'/0", "keysetId": "9mlfd5vCzgGl", "pk": [**252, 217, 215, 94, 173, 32, 148, 197, 221, 239, 14, 188, 31, 46, 237, 30, 208, 215, 147, 5, 57, 169, 243, 64, 120, 140, 183, 247, 8, 192, 52, 156**]}
 LOG  [derive] {"derivationPath": "m/129372'/0'/361559222'/5'/1", "keysetId": "9mlfd5vCzgGl", "pk": [**111, 177, 58, 44, 199, 61, 91, 207, 164, 190, 189, 196, 76, 20, 119, 239, 43, 122, 246, 56, 248, 23, 150, 24, 199, 28, 42, 119, 243, 58, 148, 184**]}
 LOG  [createBlindedMessages] {"amount": 4, "currentCounter": 6, "i": 1}
 LOG  [derive] {"derivationPath": "m/129372'/0'/361559222'/6'/0", "keysetId": "9mlfd5vCzgGl", "pk": [210, 70, 85, 192, 144, 130, 26, 20, 169, 87, 162, 200, 96, 2, 43, 74, 64, 55, 20, 51, 75, 208, 101, 176, 46, 231, 130, 81, 219, 236, 86, 130]}
 LOG  [derive] {"derivationPath": "m/129372'/0'/361559222'/6'/1", "keysetId": "9mlfd5vCzgGl", "pk": [198, 89, 92, 11, 104, 2, 206, 90, 64, 201, 128, 209, 16, 148, 131, 178, 224, 85, 3, 122, 91, 179, 107, 192, 113, 101, 135, 46, 180, 205, 77, 131]}
 LOG  [createBlindedMessages] {"amount": 8, "currentCounter": 7, "i": 2}
 LOG  [derive] {"derivationPath": "m/129372'/0'/361559222'/7'/0", "keysetId": "9mlfd5vCzgGl", "pk": [42, 60, 214, 220, 23, 253, 190, 99, 4, 248, 30, 29, 13, 222, 189, 56, 250, 78, 137, 63, 12, 40, 28, 25, 201, 229, 156, 51, 236, 73, 180, 59]}
 LOG  [derive] {"derivationPath": "m/129372'/0'/361559222'/7'/1", "keysetId": "9mlfd5vCzgGl", "pk": [86, 165, 204, 60, 55, 203, 83, 4, 230, 26, 124, 147, 163, 215, 110, 85, 95, 132, 10, 127, 82, 146, 74, 209, 230, 200, 216, 19, 251, 130, 106, 16]}
 **LOG  [createRandomBlindedMessages] {"amounts": [2]}**
 LOG  [createBlindedMessages] {"amounts": [2], **"startingCounter": 5**}
 LOG  [createBlindedMessages] {"amount": 2, "currentCounter": 5, "i": 0}
 LOG  [derive] {"derivationPath": "m/129372'/0'/361559222'/5'/0", "keysetId": "9mlfd5vCzgGl", "pk": **[252, 217, 215, 94, 173, 32, 148, 197, 221, 239, 14, 188, 31, 46, 237, 30, 208, 215, 147, 5, 57, 169, 243, 64, 120, 140, 183, 247, 8, 192, 52, 156**]}
 LOG  [derive] {"derivationPath": "m/129372'/0'/361559222'/5'/1", "keysetId": "9mlfd5vCzgGl", "pk": [**111, 177, 58, 44, 199, 61, 91, 207, 164, 190, 189, 196, 76, 20, 119, 239, 43, 122, 246, 56, 248, 23, 150, 24, 199, 28, 42, 119, 243, 58, 148, 184**]}
 ERROR  14:13:53:093 | ERROR : [sendFromMint] MINT_ERROR The mint could not return proofs necessary for this transaction. **duplicate outputs**.

@minibits-cash
Copy link
Contributor

Performance issue - creating a CashuWallet instance with seed takes 9 - 10 seconds on mobile device.

 LOG  2023-12-01T19:45:46.462Z CashuWallet.constructor deriving seed from mnemonic
 LOG  2023-12-01T19:45:55.831Z CashuWallet.constructor deriving seed from mnemonic - complete

caused by:

this._seed = deriveSeedFromMnemonic(mnemonic);

In case of more then one instances for different mints, method runs mutiple times even seed is the same.

@gandlafbtc
Copy link
Collaborator Author

Wow that's pretty long. I guess we should give the option to instantiate from seed instead from mnemonic. Or what do you suggest?

@minibits-cash
Copy link
Contributor

It is measired in dev mode, but still will be half of that in prod.
I would suggest to accept seed instead of mnemonic in the wallet constructor and if there is a reverse function in deps cashu-ts already uses, (seed2mnemonic) use it to show the mnemonic on the UI which is a rare usecase.
Just a note, to store uint seed in mobile secure storage, it needs to be converted to string and back but this is not a perf issue.

@minibits-cash
Copy link
Contributor

It is measired in dev mode, but still will be half of that in prod. I would suggest to accept seed instead of mnemonic in the wallet constructor and if there is a reverse function in deps cashu-ts already uses, (seed2mnemonic) use it to show the mnemonic on the UI which is a rare usecase. Just a note, to store uint seed in mobile secure storage, it needs to be converted to string and back but this is not a perf issue.

Actually seed2mnemonic makes no sense as the seed is derived using PBKDF function. Sorry for confusion. So perhaps to store both the seed and the derived key in wallet secure store is a way to go.

@gandlafbtc
Copy link
Collaborator Author

Yes i think we have to store both in that case. Mnemonic for showing, seed for improved performance.

@gandlafbtc
Copy link
Collaborator Author

That means, the wallet needs to be initialized with seed, and not mnemonic, and we need to expose the seed somehow, maybe as a secondary step after creating a mnemonic as a separate function

@minibits-cash
Copy link
Contributor

I found out the hopefully final issue causing the duplicate _B messages passed from wallet to the mint causing failure.

In case there are gaps in proof sequence vs the counter it is not enough to set the initial counter after the restore to the number of recreated proofs (spent or not).

To be safe against gaps, I believe the counter needs to be set to upper limit of the used recovery interval (e.g. 50 for 1...50) and then further incremented by the number of recovered (only unspent/pending) proofs.

If ack, this should be explicitly named / mentioned in the docs as not following it breaks further wallet operations.

There is related fix on the nutshell mint side pending (to prevent such duplicate messages account for as spent by mint)

@gandlafbtc
Copy link
Collaborator Author

@callebtc there are still requested changes pending from you. Can you take a look and check if it's ok now? And then let's merge this badboy!

@gandlafbtc gandlafbtc merged commit e092562 into main Jan 18, 2024
@gandlafbtc gandlafbtc deleted the det-sec branch February 19, 2024 05:57
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.

None yet

5 participants