Import key origin data through descriptors in importmulti #14021

achow101 · 2018-08-22T03:23:16Z

This PR allows for key origin data as defined by the descriptors document to be imported to the wallet when importing a descriptor using importmulti. This allows the walletprocesspsbt to include the BIP 32 derivation paths for keys that it is watching that are from a different HD wallet.

In order to make this easier to use, a new field hdmasterkeyfingerprint has been added to getaddressinfo. Additionally I have removed hdmasterkeyid as was planned. I think that this API change is fine since it was going to be removed in 0.18 anyways. CKeyMetadata has also been extended to store key origin info to facilitate this.

instagibbs · 2018-08-22T13:19:05Z

Nice! strong concept ACK

achow101 · 2018-08-25T15:56:53Z

Fixed a bug where the keymetadata wasn't being written to the wallet file. Also added a test for that case.

achow101 · 2018-08-28T21:48:36Z

Rebased and also changed the keymeta stuff to add the hd master key id at key generation and when the wallet is first loaded.

luke-jr · 2018-08-29T17:48:14Z

src/utilstrencodings.cpp

why not? This is quite standard.

It's fairly standard to use h or H as the indicator, not just '. In fact, the BIP uses H, but as a subscript. ' is not actually used as the indicator for hardened in the BIP even though that is what everyone uses.

The main reason for allowing this is so that you don't have to do any annoying escaping stuff for the ' when entering the keypaths on the command line.

achow101 · 2018-09-04T13:08:16Z

Rebased

laanwj · 2018-09-13T14:54:10Z

Concept ACK, thanks for working on this

meshcollider · 2019-02-14T23:10:25Z

Changes since @sipa's are just nit-fixes, a new test, and a missing brace bugfix 👍

cb3511b Add release notes for importing key origin info change (Andrew Chow) 4c75a69 Test importing descriptors with key origin information (Andrew Chow) 02d6586 Import KeyOriginData when importing descriptors (Andrew Chow) 3d235df Implement a function to add KeyOriginInfo to a wallet (Andrew Chow) eab63bc Store key origin info in key metadata (Andrew Chow) 345bff6 Remove hdmasterkeyid (Andrew Chow) bac8c67 Add a method to CWallet to write just CKeyMetadata (Andrew Chow) e7652d3 Add WriteHDKeypath function and move *HDKeypath to util/bip32.{h,cpp} (Andrew Chow) c45415f Refactor keymetadata writing to a separate method (Andrew Chow) Pull request description: This PR allows for key origin data as defined by the descriptors document to be imported to the wallet when importing a descriptor using `importmulti`. This allows the `walletprocesspsbt` to include the BIP 32 derivation paths for keys that it is watching that are from a different HD wallet. In order to make this easier to use, a new field `hdmasterkeyfingerprint` has been added to `getaddressinfo`. Additionally I have removed `hdmasterkeyid` as was planned. I think that this API change is fine since it was going to be removed in 0.18 anyways. `CKeyMetadata` has also been extended to store key origin info to facilitate this. Tree-SHA512: 9c7794f3c793da57e23c5abbdc3d58779ee9dea3d53168bb86c0643a4ad5a11a446264961e2f772f35eea645048cb60954ed58050002caee4e43cd9f51215097

Summary: Partial backport of Core [[bitcoin/bitcoin#14021 | PR14021]] bitcoin/bitcoin@c45415f Test Plan: ninja ninja check ninja check-functional Reviewers: O1 Bitcoin ABC, #bitcoin_abc, Fabien Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, Fabien Subscribers: Fabien Differential Revision: https://reviews.bitcoinabc.org/D6004

Summary: Creates new files util/bip32.h and util/bip32.cpp for containing BIP 32 stuff. Moves FormatKeyPath from descriptor.cpp to util/bip32. Adds a wrapper around it to prepent the 'm' for when just the BIP 32 style keypath is needed. Partial backport of Core [[bitcoin/bitcoin#14021 | PR14021]] bitcoin/bitcoin@e7652d3 Depends on D6004 Test Plan: ../configure --enable-deprecated-build-system make check ninja ninja check test_runner.py Reviewers: O1 Bitcoin ABC, #bitcoin_abc, Fabien Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, Fabien Subscribers: Fabien Differential Revision: https://reviews.bitcoinabc.org/D6204

Summary: Deprecated return results that were scheduled to be removed in v0.21 Partial backport of Core [[bitcoin/bitcoin#14021 | PR14021]] bitcoin/bitcoin@345bff6 Test Plan: ninja ninja check-functional Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Subscribers: deadalnix Differential Revision: https://reviews.bitcoinabc.org/D5933

Summary: Partial backport of Core [[bitcoin/bitcoin#14021 | PR14021]] bitcoin/bitcoin@bac8c67 Depends on D6204 Test Plan: ninja Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D6227

Summary: Store the master key fingerprint and derivation path in the key metadata. hdKeypath is kept to indicate the seed and for backwards compatibility, but all key derivation path output uses the key origin info instead of hdKeypath. Partial backport of Core [[bitcoin/bitcoin#14021 | PR14021]] bitcoin/bitcoin@eab63bc Depends on D6227 Test Plan: ninja ninja check Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D6228

Summary: Partial backport of Core [[bitcoin/bitcoin#14021 | PR14021]] bitcoin/bitcoin@3d235df Depends on D6228 Test Plan: ninja Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D6229

Summary: Partial backport of Core [[bitcoin/bitcoin#14021 | PR14021]] bitcoin/bitcoin@02d6586 Depends on D6229 Test Plan: ninja ninja check Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D6230

…e notes Summary: Test is separated from the rest of the changes to make review easier. Partial backport of Core [[bitcoin/bitcoin#14021 | PR14021]] bitcoin/bitcoin@4c75a69 bitcoin/bitcoin@cb3511b Depends on D6230 Test Plan: ninja ninja check ninja check-functional Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D6215

Summary: Partial backport of Core [[bitcoin/bitcoin#14021 | PR14021]] bitcoin/bitcoin@c45415f Test Plan: ninja ninja check ninja check-functional Reviewers: O1 Bitcoin ABC, #bitcoin_abc, Fabien Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, Fabien Subscribers: Fabien Differential Revision: https://reviews.bitcoinabc.org/D6004

…portmulti cb3511b Add release notes for importing key origin info change (Andrew Chow) 4c75a69 Test importing descriptors with key origin information (Andrew Chow) 02d6586 Import KeyOriginData when importing descriptors (Andrew Chow) 3d235df Implement a function to add KeyOriginInfo to a wallet (Andrew Chow) eab63bc Store key origin info in key metadata (Andrew Chow) 345bff6 Remove hdmasterkeyid (Andrew Chow) bac8c67 Add a method to CWallet to write just CKeyMetadata (Andrew Chow) e7652d3 Add WriteHDKeypath function and move *HDKeypath to util/bip32.{h,cpp} (Andrew Chow) c45415f Refactor keymetadata writing to a separate method (Andrew Chow) Pull request description: This PR allows for key origin data as defined by the descriptors document to be imported to the wallet when importing a descriptor using `importmulti`. This allows the `walletprocesspsbt` to include the BIP 32 derivation paths for keys that it is watching that are from a different HD wallet. In order to make this easier to use, a new field `hdmasterkeyfingerprint` has been added to `getaddressinfo`. Additionally I have removed `hdmasterkeyid` as was planned. I think that this API change is fine since it was going to be removed in 0.18 anyways. `CKeyMetadata` has also been extended to store key origin info to facilitate this. Tree-SHA512: 9c7794f3c793da57e23c5abbdc3d58779ee9dea3d53168bb86c0643a4ad5a11a446264961e2f772f35eea645048cb60954ed58050002caee4e43cd9f51215097

…portmulti (add)

…portmulti (add 1)

fanquake added Wallet RPC/REST/ZMQ labels Aug 22, 2018

This was referenced Aug 22, 2018

util: Add [[nodiscard]] to all {Decode,Parse}[...](...) functions returning bool #13815

Merged

PSBT key path cleanups #13723

Merged

achow101 force-pushed the import-multi-hd branch from 631a8b7 to 19bd923 Compare August 23, 2018 01:10

DrahtBot mentioned this pull request Aug 23, 2018

Import pubkeys when importing p2sh with importmulti #14019

Closed

achow101 force-pushed the import-multi-hd branch from 19bd923 to 11db726 Compare August 25, 2018 15:55

achow101 force-pushed the import-multi-hd branch 2 times, most recently from 16606ee to 8aba59d Compare August 26, 2018 20:07

DrahtBot mentioned this pull request Aug 26, 2018

Import watch only pubkeys to the keypool if private keys are disabled #14075

Merged

DrahtBot added the Needs rebase label Aug 28, 2018

achow101 force-pushed the import-multi-hd branch from 8aba59d to 1c428a6 Compare August 28, 2018 21:47

achow101 force-pushed the import-multi-hd branch from 1c428a6 to 7433952 Compare August 28, 2018 22:22

DrahtBot removed the Needs rebase label Aug 28, 2018

DrahtBot mentioned this pull request Aug 29, 2018

[wallet] Kill accounts #13825

Closed

achow101 force-pushed the import-multi-hd branch from 1cf56e0 to 8065a31 Compare August 29, 2018 15:21

luke-jr reviewed Aug 29, 2018

View reviewed changes

achow101 force-pushed the import-multi-hd branch from 8065a31 to 4a9be42 Compare August 29, 2018 21:22

DrahtBot added the Needs rebase label Aug 30, 2018

achow101 force-pushed the import-multi-hd branch from 4a9be42 to 44b1725 Compare September 4, 2018 13:08

achow101 mentioned this pull request Sep 4, 2018

Hardware wallet support #14145

Closed

DrahtBot removed the Needs rebase label Sep 4, 2018

Sjors mentioned this pull request Sep 13, 2018

Add key origin support to descriptors #14150

Merged

DrahtBot mentioned this pull request Oct 10, 2018

Add SegWit support to importmulti #14454

Merged

achow101 mentioned this pull request Oct 12, 2018

Add Coldcard bitcoin-dot-org/Bitcoin.org#2612

Closed

achow101 force-pushed the import-multi-hd branch from 1b1c6aa to cb3511b Compare February 14, 2019 23:05

meshcollider merged commit cb3511b into bitcoin:master Feb 14, 2019

This was referenced Feb 15, 2019

Bugfix: RPC/Wallet: Include HD key metadata in dumpwallet #11803

Closed

[wallet] wallet-tool: command to remove key metadata #15424

Closed

jonasschnelli mentioned this pull request Feb 18, 2019

Use a single wallet batch for UpgradeKeyMetadata #15433

Merged

Sjors mentioned this pull request Mar 5, 2019

Key origin metadata, with HD wallet support #8471

Closed

Empact mentioned this pull request Mar 7, 2019

Build: enable -Wdocumentation via isystem #14920

Closed

vijaydasmp mentioned this pull request Sep 28, 2021

Merge #14150: Add key origin support to descriptors dashpay/dash#4457

Closed

vijaydasmp added a commit to vijaydasmp/dash that referenced this pull request Sep 28, 2021

Merge bitcoin#14021: Import key origin data through descriptors in im…

14d28c5

…portmulti (add)

vijaydasmp added a commit to vijaydasmp/dash that referenced this pull request Sep 28, 2021

Merge bitcoin#14021: Import key origin data through descriptors in im…

8a3b0bc

…portmulti (add 1)

6293 mentioned this pull request Nov 29, 2021

Merge 15390, 15426, 15456 dashpay/dash#4591

Closed

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Import key origin data through descriptors in importmulti #14021

Import key origin data through descriptors in importmulti #14021

Uh oh!

achow101 commented Aug 22, 2018 •

edited

Loading

Uh oh!

instagibbs commented Aug 22, 2018

Uh oh!

achow101 commented Aug 25, 2018

Uh oh!

achow101 commented Aug 28, 2018

Uh oh!

luke-jr Aug 29, 2018

Uh oh!

instagibbs Aug 29, 2018

Uh oh!

achow101 Aug 29, 2018

Uh oh!

achow101 commented Sep 4, 2018

Uh oh!

laanwj commented Sep 13, 2018

Uh oh!

meshcollider commented Feb 14, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

Import key origin data through descriptors in importmulti #14021

Import key origin data through descriptors in importmulti #14021

Uh oh!

Conversation

achow101 commented Aug 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

instagibbs commented Aug 22, 2018

Uh oh!

achow101 commented Aug 25, 2018

Uh oh!

achow101 commented Aug 28, 2018

Uh oh!

luke-jr Aug 29, 2018

Choose a reason for hiding this comment

Uh oh!

instagibbs Aug 29, 2018

Choose a reason for hiding this comment

Uh oh!

achow101 Aug 29, 2018

Choose a reason for hiding this comment

Uh oh!

achow101 commented Sep 4, 2018

Uh oh!

laanwj commented Sep 13, 2018

Uh oh!

meshcollider commented Feb 14, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

achow101 commented Aug 22, 2018 •

edited

Loading