Add deriveaddresses RPC util method by Sjors · Pull Request #14667 · bitcoin/bitcoin

Sjors · 2018-11-06T14:25:57Z

Usage:

bitcoin-cli deriveaddresses "wpkh([d34db33f/84h/0h/0h]xpub6DJ2dNUysrn5Vt36jH2KLBT2i1auw1tTSSomg8PhqNiUtx8QX2SvC9nrHu81fT41fvDUnhMjEzQgXnQjKEu3oaqMSzhSrHMxyyoEAmUHQbY/0/0)"
[
  "bc1qg6ucjz7kgdedam7v5yarecy54uqw82yym06z3q"
] // part of the BIP32 test vector

Avoids the need for external (BIP32) libraries to derive an address. Can be used in conjunction with scantxoutset as a poor mans wallet. Might be useful to test more complicated future descriptors.

~~To keep it as simple as possible it only supports descriptors that result in a single address, so no combo() and ranges.~~

As discussed recently on IRC it might make sense to put this in a separate utility along with other descriptor and psbt utility functions which don't need a chain or wallet context. However I prefer to leave that to another PR.

laanwj · 2018-11-06T14:30:22Z

I know, this comes up every time—see for example #14476 (comment) —wasn't the idea to not add more pure-utility RPC calls?
We really need a clear stance on this.

DrahtBot · 2018-11-06T14:44:35Z

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

Conflicts

No conflicts as of last run.

Sjors · 2018-11-06T15:23:36Z

I created an issue to discuss a tool that could replace pure utility RPC calls: #14671

src/rpc/misc.cpp

meshcollider

This looks good, and after the discussion in the meeting this morning I think its worth adding.
utACK 9a55d5f

meshcollider · 2018-11-09T02:20:06Z

src/rpc/misc.cpp

It would be easy enough to add support for picking a specific index from a ranged descriptor, is there a reason you chose to disallow it? Just add a second parameter which specifies the index.
EDIT: thinking about it, it would be just as easy to not use a ranged descriptor if we're only using a single key, don't worry.

It seems a weird anti-feature to me. I don't understand why you're adding extra complexity to the function just to prevent a range. If you only want one, you can still do so.

I initially did that to prevent extra complexity, but I've actually encountered a situation where allowing ranges would be very useful. Will change this to return an array.

@Sjors I assume that also includes allowing combo right? Sounds good 👍

@meshcollider yes, no reason anymore to exclude combo()

sipa · 2018-11-11T03:05:05Z

doc/release-notes-13152.md

This doesn't seem to belong in this file (the file is specific per PR).

listwalletdir was already hijacking that file, so I joined, but I can add a new one.

In which case we could add them to the main file in the first place.

I thought the point of these extra files was mainly to prevent constant rebasing, which seemed mostly a result of different sections interfering, and less when you're just adding one line an existing entry. Though I'm not sure exactly what trips up git.

This file was removed in master. Also, if another patch added a line in the meantime, git(hub) would also report a merge conflict.

Sjors · 2018-11-19T18:58:55Z

Rebased. Result is now an array of addresses, so combo() and ranges can be used. Added range arguments.

Sjors · 2018-11-19T19:15:01Z

Getting a rather cryptic linter error: @MarcoFalke?

Update: #14718 was merged right before I pushed this. I rebased again and now I'm able to reproduce the linter error. It happens on master too.

meshcollider

Looking good

src/rpc/misc.cpp

Sjors · 2018-11-20T13:33:10Z

Rebased so CI works again. Renamed to plural deriveaddresses. Simplified the range arguments (must provide start and end).

meshcollider · 2018-11-22T02:22:54Z

utACK 82d2a8f

One last nit, I would add a comment about the default behavior of 0 for ranged descriptors if start and end aren't provided.

EDIT: Actually, I think the helptext needs to be switched to RPCHelpMan

Sjors · 2018-11-23T11:53:35Z

@meshcollider alternatively I could make the range argument mandatory for ranged descriptors.

What do you mean with RPCHelpMan?

meshcollider · 2018-11-24T10:48:56Z

@Sjors see #14726

Sjors · 2018-11-27T15:31:52Z

Rebased due to #14726 and using RPCHelpMan now.

Range argument is now mandatory for ranged descriptors (typing 0 instead of * is trivial).

maflcko · 2019-01-29T15:58:41Z

Indeed, we could pass a lambda into the RPCHelpMan to validate the value and use the passed in type to validate the type (and thus get rid of RPCTypeCheck completely)

Sjors · 2019-01-29T16:37:53Z

Rebased due to somewhat inexplicable Travis failure.

src/rpc/misc.cpp

instagibbs

Couple of non-blocking nits, tACK 5952838

src/rpc/misc.cpp

instagibbs · 2019-02-06T14:39:56Z

test/functional/rpc_deriveaddresses.py

+
+        bare_multisig_descriptor = "multi(1, tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1/1/0, tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1/1/1)"
+        assert_raises_rpc_error(-5, "Descriptor does not have a corresponding address", self.nodes[0].deriveaddresses, bare_multisig_descriptor)
+


I'd add a wallet roundtrip test like:

wpkh_info = self.nodes[0].getaddressinfo(self.nodes[0].getnewaddress("", "bech32")) # Trim the witness part of descriptor off, check pkh entry in wallet, match descriptor pkh_desc = wpkh_info["desc"][1:] pkh_addr = self.nodes[0].deriveaddresses(pkh_desc) pkh_info = self.nodes[0].getaddressinfo(pkh_addr) assert_equal(wpkh_info["hdkeypath"], pkh_info["hdkeypath"]) assert_equal(wpkh_info["pubkey"], pkh_info["pubkey"])

5952838 [rpc] util: add deriveaddresses method (Sjors Provoost) Pull request description: Usage: ```sh bitcoin-cli deriveaddresses "wpkh([d34db33f/84h/0h/0h]xpub6DJ2dNUysrn5Vt36jH2KLBT2i1auw1tTSSomg8PhqNiUtx8QX2SvC9nrHu81fT41fvDUnhMjEzQgXnQjKEu3oaqMSzhSrHMxyyoEAmUHQbY/0/0)" [ "bc1qg6ucjz7kgdedam7v5yarecy54uqw82yym06z3q" ] // part of the BIP32 test vector ``` Avoids the need for external (BIP32) libraries to derive an address. Can be used in conjunction with `scantxoutset` as a poor mans wallet. Might be useful to test more complicated future descriptors. ~To keep it as simple as possible it only supports descriptors that result in a single address, so no `combo()` and ranges.~ As discussed recently on IRC it might make sense to put this in a separate utility along with other descriptor and psbt utility functions which don't need a chain or wallet context. However I prefer to leave that to another PR. Tree-SHA512: b8e53db11a8fd87638cc98766270cc3be9adc4b3e5085798a6a4e2e6ad252bf6d2189346bbb2da72d04d13f7f1e80b5cb88e8039653bea1f150602a876ef7f34

Summary: This is a backport of Core [[bitcoin/bitcoin#14667 | PR14667]] Test Plan: ninja all check-all Reviewers: #bitcoin_abc, majcosta Reviewed By: #bitcoin_abc, majcosta Differential Revision: https://reviews.bitcoinabc.org/D6214

5952838 [rpc] util: add deriveaddresses method (Sjors Provoost) Pull request description: Usage: ```sh bitcoin-cli deriveaddresses "wpkh([d34db33f/84h/0h/0h]xpub6DJ2dNUysrn5Vt36jH2KLBT2i1auw1tTSSomg8PhqNiUtx8QX2SvC9nrHu81fT41fvDUnhMjEzQgXnQjKEu3oaqMSzhSrHMxyyoEAmUHQbY/0/0)" [ "bc1qg6ucjz7kgdedam7v5yarecy54uqw82yym06z3q" ] // part of the BIP32 test vector ``` Avoids the need for external (BIP32) libraries to derive an address. Can be used in conjunction with `scantxoutset` as a poor mans wallet. Might be useful to test more complicated future descriptors. ~To keep it as simple as possible it only supports descriptors that result in a single address, so no `combo()` and ranges.~ As discussed recently on IRC it might make sense to put this in a separate utility along with other descriptor and psbt utility functions which don't need a chain or wallet context. However I prefer to leave that to another PR. Tree-SHA512: b8e53db11a8fd87638cc98766270cc3be9adc4b3e5085798a6a4e2e6ad252bf6d2189346bbb2da72d04d13f7f1e80b5cb88e8039653bea1f150602a876ef7f34

merge bitcoin#14150, bitcoin#14477, bitcoin#14646, bitcoin#14667, bitcoin#14886, bitcoin#14565, bitcoin#14491, bitcoin#15368: descriptors

fanquake added the RPC/REST/ZMQ label Nov 6, 2018

practicalswift reviewed Nov 7, 2018

View reviewed changes

src/rpc/misc.cpp Outdated Show resolved Hide resolved

practicalswift reviewed Nov 7, 2018

View reviewed changes

src/rpc/misc.cpp Outdated Show resolved Hide resolved

Sjors force-pushed the 2018/11/deriveaddress branch from 4f6fa31 to 9a55d5f Compare November 7, 2018 10:19

meshcollider reviewed Nov 9, 2018

View reviewed changes

sipa reviewed Nov 11, 2018

View reviewed changes

DrahtBot added the Needs rebase label Nov 14, 2018

sipa mentioned this pull request Nov 16, 2018

Tracker for places where descriptors could/should be integrated #14739

Closed

Sjors force-pushed the 2018/11/deriveaddress branch from 9a55d5f to 58d337b Compare November 19, 2018 18:58

Sjors force-pushed the 2018/11/deriveaddress branch from 58d337b to 50c656a Compare November 19, 2018 19:03

DrahtBot removed the Needs rebase label Nov 19, 2018

Sjors force-pushed the 2018/11/deriveaddress branch from 50c656a to f0e881f Compare November 19, 2018 19:09

Sjors force-pushed the 2018/11/deriveaddress branch 2 times, most recently from c71cec5 to 684fdd1 Compare November 19, 2018 19:30

meshcollider reviewed Nov 19, 2018

View reviewed changes

src/rpc/misc.cpp Outdated Show resolved Hide resolved

src/rpc/misc.cpp Outdated Show resolved Hide resolved

Sjors force-pushed the 2018/11/deriveaddress branch 2 times, most recently from 24a58ab to 7eccdde Compare November 20, 2018 13:31

Sjors changed the title ~~Add deriveaddress RPC util method~~ Add deriveaddresses RPC util method Nov 20, 2018

Sjors force-pushed the 2018/11/deriveaddress branch from 7eccdde to 82d2a8f Compare November 20, 2018 13:34

Sjors force-pushed the 2018/11/deriveaddress branch from 82d2a8f to dfad416 Compare November 27, 2018 15:31

Sjors force-pushed the 2018/11/deriveaddress branch from 71eef5a to 7f78582 Compare January 29, 2019 16:37

maflcko reviewed Jan 29, 2019

View reviewed changes

src/rpc/misc.cpp Outdated Show resolved Hide resolved

[rpc] util: add deriveaddresses method

5952838

Sjors force-pushed the 2018/11/deriveaddress branch from 7f78582 to 5952838 Compare January 29, 2019 17:14

instagibbs approved these changes Feb 6, 2019

View reviewed changes

meshcollider merged commit 5952838 into bitcoin:master Feb 7, 2019

Sjors deleted the 2018/11/deriveaddress branch February 8, 2019 09:49

tynes mentioned this pull request Feb 26, 2019

wallet-http: adds new API call to derive address (pubkey and redeemscript) at any index bcoin-org/bcoin#560

Open

Sjors mentioned this pull request Feb 27, 2019

[rpc] deriveaddress: rename range 'begin' argument to 'start' #15496

Closed

nodech mentioned this pull request Mar 23, 2021

wallet-rpc: derive the receive address by derivation index handshake-org/hsd#496

Closed

vijaydasmp mentioned this pull request Sep 16, 2021

Merge bitcoin#19733, bitcoin#19734 dashpay/dash#4430

Merged

kwvg mentioned this pull request Oct 7, 2021

merge bitcoin#14150, #14477, #14646, #14667, #14886, #14565, #14491, #15368: descriptors dashpay/dash#4494

Merged

7 tasks

kwvg added a commit to kwvg/dash that referenced this pull request Oct 28, 2021

merge bitcoin#14667: Add deriveaddresses RPC util method

083bc07

UdjinM6 added a commit to dashpay/dash that referenced this pull request Nov 1, 2021

Merge pull request #4494 from kittywhiskers/descaddr

bbe9b3d

merge bitcoin#14150, bitcoin#14477, bitcoin#14646, bitcoin#14667, bitcoin#14886, bitcoin#14565, bitcoin#14491, bitcoin#15368: descriptors

pravblockc pushed a commit to pravblockc/dash that referenced this pull request Nov 18, 2021

merge bitcoin#14667: Add deriveaddresses RPC util method

6069fcc

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021


		bare_multisig_descriptor = "multi(1, tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1/1/0, tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1/1/1)"
		assert_raises_rpc_error(-5, "Descriptor does not have a corresponding address", self.nodes[0].deriveaddresses, bare_multisig_descriptor)

Conversation

Sjors commented Nov 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laanwj commented Nov 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Nov 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

Sjors commented Nov 6, 2018

Uh oh!

Uh oh!

Uh oh!

meshcollider left a comment

Choose a reason for hiding this comment

Uh oh!

meshcollider Nov 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sjors Nov 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sjors commented Nov 19, 2018

Uh oh!

Sjors commented Nov 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

meshcollider left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Sjors commented Nov 20, 2018

Uh oh!

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

Uh oh!

Uh oh!

Sjors commented Nov 23, 2018

Uh oh!

meshcollider commented Nov 24, 2018

Uh oh!

Sjors commented Nov 27, 2018

Uh oh!

maflcko commented Jan 29, 2019

Uh oh!

Sjors commented Jan 29, 2019

Uh oh!

Uh oh!

instagibbs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Sjors commented Nov 6, 2018 •

edited

Loading

laanwj commented Nov 6, 2018 •

edited

Loading

DrahtBot commented Nov 6, 2018 •

edited

Loading

meshcollider Nov 9, 2018 •

edited

Loading

Sjors Nov 10, 2018 •

edited

Loading

Sjors commented Nov 19, 2018 •

edited

Loading

meshcollider commented Nov 22, 2018 •

edited

Loading