ParseHDKeypath: support h as hardened marker #28192

pull Sjors wants to merge 1 commits into bitcoin:master from Sjors:2023/07/parse-hd-keypath changing 3 files +28 −5

Sjors commented at 1:12 PM on July 31, 2023: member

BIP32 allows both ' and h as hardened derivation marker. Our legacy wallet uses '. Since #26076 our descriptor wallets use h by default.

ParseHDKeypath only supports '. It's currently only used in the legacy wallet context, so this doesn't cause any problems. But it will once #22341 uses it (to parse the RPC path argument for getxpub). Might as well fix it now.

I added a restriction for not combining h and '. Afaik this currently isn't enforced anywhere else in the codebase, including for descriptors, but it seems sane. I've occasionally messed that up in the past.
DrahtBot commented at 1:12 PM on July 31, 2023: contributor



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Code Coverage

For detailed information about the code coverage, see the test coverage report.



Reviews

See the guideline for information on the review process. A summary of reviews will appear here.



Conflicts

No conflicts as of last run.
achow101 commented at 4:28 PM on July 31, 2023: member

I'm not sure if there's any software that enforces the consistent usage of a hardened marker.
Sjors commented at 8:06 AM on August 1, 2023: member

We could always relax the constraint if someone has a good use case for not being consistent. For now this method is only foreseen to be used with getxpub where I can't think of an even remotely sane reason. More likely someone will do this by accident and end up confused why their descriptor checksum doesn't match.
luke-jr commented at 3:08 PM on August 5, 2023: member

Seems likely someone somewhere will want to combine two derivation paths, which might use different markers.
Sjors force-pushed on Aug 21, 2023
Sjors commented at 11:08 AM on August 21, 2023: member

I made the consistency check optional and off by default.
DrahtBot added the label CI failed on Oct 25, 2023

in src/util/bip32.h:18 in 08a8ae9cd0 outdated

  15 | +    *
  16 | +    * @param[in] keypath_str the path, e.g. "m/7'/0/2000" or "m/0h/0h"
  17 | +    * @param[out] keypath the parsed path values
  18 | +    * @param[in] check_hardened_marker Check that hardened deriviation markers h and ' are not mixed.
  19 | +    */
  20 | +

maflcko commented at 7:45 PM on October 25, 2023:

nit: Why the extra newline after the doxygen?

Sjors commented at 6:21 AM on October 26, 2023:

Fixed

Sjors force-pushed on Oct 26, 2023
DrahtBot removed the label CI failed on Oct 26, 2023
DrahtBot added the label CI failed on Jan 12, 2024
ParseHDKeypath: support h as hardened marker
Optionally check for inconsistent use.
777e5f40fe
Sjors force-pushed on Feb 13, 2024
DrahtBot removed the label CI failed on Feb 13, 2024
achow101 closed this on Apr 9, 2024
bitcoin locked this on Apr 9, 2025

Contributors

Linked (view graph)

#22341 rpc: add path to gethdkey #26076 Switch hardened derivation marker to h

ParseHDKeypath: support h as hardened marker #28192

Code Coverage

Reviews

Conflicts