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

add get_xpub_at_path function which can handle non-hardened derivations #24

Merged
merged 1 commit into from
Jul 6, 2022

Conversation

scgbckbone
Copy link
Contributor

  • copied from tapsigner branch in HWI as achow does not want to have this code in HWI

cktap/proto.py Outdated Show resolved Hide resolved
cktap/proto.py Outdated Show resolved Hide resolved
cktap/proto.py Outdated Show resolved Hide resolved
cktap/proto.py Outdated Show resolved Hide resolved
cktap/proto.py Outdated
"""
Sign 32 bytes digest and return 65 bytes long recoverable signature.

Uses derivation path based on current set derivation on card plus optional
subpath parameter which if provided, will be added to card derivation path.
Subpath can only be of length 2 and non-hardened components only.
if subpath is specified - use current derivation + derive subpath
if bip32_path is specified - subpath is ignored and derivation goes from root
Copy link
Contributor

Choose a reason for hiding this comment

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

bip32_path => fullpath or subkey_path ... maybe rename subpath too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed:
subpath --> sub_deriv
bip32_path --> fullpath

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed sub_deriv back to subpath --> so we do not "fork" other people code

@scgbckbone scgbckbone force-pushed the xpub_at_path branch 3 times, most recently from 1be1121 to 87881e9 Compare June 30, 2022 14:57
cktap/utils.py Outdated Show resolved Hide resolved
cktap/proto.py Outdated Show resolved Hide resolved
@doc-hex
Copy link
Contributor

doc-hex commented Jun 30, 2022

I think you've changed the (type of) arguments for a few functions here. This makes me "feel bad" since we are breaking working code that others may have written already. Can't we accept both str and Sequence[int] for arguments?

@scgbckbone
Copy link
Contributor Author

only breaking change is renaming of subpath to sub_deriv as that was the only thing in old codebase - otherwise it is a backward compatible change/addition:

sign_digest works as before - if you do not provide fullpath
derive_xpub_at path - is a new function

or am I missing something (besides subpath -> subderiv)?

If subpath/sub_deriv is reverted I do not see any breaking changes there...

@scgbckbone
Copy link
Contributor Author

oh - forgot about changed parameters to set_derivation/get_derivation -> I can also revert that (or we can bump minor version -> 1.1.0)

@scgbckbone
Copy link
Contributor Author

scgbckbone commented Jul 2, 2022

changed few things:

  • sub_deriv renamed back to subpath
  • set_derivation/get_derivation rollback, instead I created same named protected members (_set_derivation/_get_derivation) that accept Sequence[int] instead of str
  • check moved to split function
  • HWI adjusted to work with this branch
    -----> backwards compatible now

@scgbckbone scgbckbone force-pushed the xpub_at_path branch 2 times, most recently from c92a193 to 527899c Compare July 5, 2022 16:08
…tions; sign_digest adjusted to handle fullpath(bip32)
@doc-hex doc-hex merged commit e1b9fea into coinkite:master Jul 6, 2022
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

2 participants