Skip to content

feat: follow deterministic derivation spec#68

Closed
lescuer97 wants to merge 6 commits intocashubtc:masterfrom
lescuer97:derivation_test
Closed

feat: follow deterministic derivation spec#68
lescuer97 wants to merge 6 commits intocashubtc:masterfrom
lescuer97:derivation_test

Conversation

@lescuer97
Copy link

changes the derivation path to be the same as cashubtc/nuts#331

@github-project-automation github-project-automation bot moved this to Backlog in coco Jan 22, 2026
@Egge21M Egge21M moved this from Backlog to In Progress in coco Jan 23, 2026
const seed = await this.seedService.getSeed();
const hdKey = HDKey.fromMasterSeed(seed);
const derivationPath = `m/129373'/10'/0'/0'/${nextDerivationIndex}`;
const derivationPath = `m/129372'/10'/0'/0'/${nextDerivationIndex}`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

The derivation path can not simply be changes like that. We need a backwards compatible way, like deriving both paths at the same time and checking proofs against both

Copy link
Collaborator

Choose a reason for hiding this comment

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

@lescuer97 you marked this as resolved, but I am unable to see how it was resolved.

@changeset-bot
Copy link

changeset-bot bot commented Jan 26, 2026

⚠️ No Changeset found

Latest commit: 6883df6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@lescuer97
Copy link
Author

@Egge21M I'm getting type errors on build but I can't seem to manage to fix this. completely, the same command in my pc is passing correctly.

@Egge21M
Copy link
Collaborator

Egge21M commented Jan 26, 2026

TBH I am not sure what changed. Maybe we need to pin tsc to stop it from changing under our feet. I have the same problem and fixed it in another PR. Should be merged tomorrow. Then a rebase will fix this

@Egge21M
Copy link
Collaborator

Egge21M commented Jan 27, 2026

Rebasing to master should fix this now

@lescuer97 lescuer97 force-pushed the derivation_test branch 2 times, most recently from 913c194 to 640d568 Compare January 27, 2026 14:06
@lescuer97 lescuer97 requested a review from Egge21M January 28, 2026 16:41
@ye0man ye0man added this to the stable v1 milestone Feb 6, 2026
@lescuer97
Copy link
Author

@Egge21M is this ready to be reviewed after the v1 prospection?

@ye0man ye0man moved this from In Progress to In Review in coco Mar 10, 2026
Copy link
Collaborator

@Egge21M Egge21M left a comment

Choose a reason for hiding this comment

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

Thank you @lescuer97 for taking the lead on this one! I might be missing something, but I don't think the concerns about backwards compatibility where addressed in the latest fixes. Also the new bun-sqlite adapter is not wired up.

const seed = await this.seedService.getSeed();
const hdKey = HDKey.fromMasterSeed(seed);
const derivationPath = `m/129373'/10'/0'/0'/${nextDerivationIndex}`;
const derivationPath = `m/129372'/10'/0'/0'/${nextDerivationIndex}`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@lescuer97 you marked this as resolved, but I am unable to see how it was resolved.

@lescuer97 lescuer97 requested a review from Egge21M March 17, 2026 12:08
@lescuer97 lescuer97 mentioned this pull request Mar 20, 2026
@lescuer97
Copy link
Author

replaced by: #121

@lescuer97 lescuer97 closed this Mar 20, 2026
@github-project-automation github-project-automation bot moved this from In Review to Done in coco Mar 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants