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

Let wallet importmulti RPC accept labels for standard scriptPubKeys #9332

Merged
merged 1 commit into from Sep 10, 2018

Conversation

Projects
None yet
6 participants
@ryanofsky
Copy link
Contributor

ryanofsky commented Dec 12, 2016

Allow importmulti RPC to apply address labels when importing standard scriptPubKeys. This makes the importmulti RPC less finnicky about import formats and also simpler internally.

@ryanofsky ryanofsky force-pushed the ryanofsky:pr/script-label branch from a766541 to df9e95d Dec 12, 2016

@ryanofsky ryanofsky force-pushed the ryanofsky:pr/script-label branch 3 times, most recently from 0f0abc6 to ef25b9a Dec 13, 2016

@ryanofsky ryanofsky changed the title Let wallet importmulti RPC accept labels for standard scriptPubKeys (on top of #9331) Let wallet importmulti RPC accept labels for standard scriptPubKeys Dec 15, 2016

@ryanofsky ryanofsky force-pushed the ryanofsky:pr/script-label branch 2 times, most recently from 4f1abbd to 21c179a Mar 1, 2017

@ryanofsky ryanofsky force-pushed the ryanofsky:pr/script-label branch from 21c179a to c2ec9bf Jun 2, 2017

@ryanofsky ryanofsky force-pushed the ryanofsky:pr/script-label branch from c2ec9bf to 49c711c Sep 20, 2017

@ryanofsky ryanofsky force-pushed the ryanofsky:pr/script-label branch from 49c711c to 6b1996a Oct 19, 2017

@ryanofsky ryanofsky force-pushed the ryanofsky:pr/script-label branch from 6b1996a to 9c32fd9 Jan 25, 2018

Let wallet importmulti RPC accept labels for standard scriptPubKeys
Allow importmulti RPC to apply address labels when importing standard
scriptPubKeys. This makes the importmulti RPC less finnicky about import
formats and also simpler internally.

@ryanofsky ryanofsky force-pushed the ryanofsky:pr/script-label branch from 9c32fd9 to 98ea64c Feb 23, 2018

@jamesob
Copy link
Member

jamesob left a comment

utACK 98ea64c

I don't see anything in the tests exercising the application of a label to a scriptPubKey import; if we're indeed lacking that I'm happy to either add a commit here or file a follow-up PR.

@@ -166,7 +166,7 @@ def run_test(self):

# Create new transactions sending to each address.
for i, variant in enumerate(IMPORT_VARIANTS):
variant.sent_amount = 10 - (2 * i + 1) / 8.0
variant.sent_amount = 1 - (2 * i + 1) / 128

This comment has been minimized.

@jamesob

jamesob Mar 7, 2018

Member

Curious what's prompting these value changes.

This comment has been minimized.

@ryanofsky

ryanofsky Mar 8, 2018

Contributor

Curious what's prompting these value changes.

This is just reducing the amount sent each iteration to prevent the wallet running out of funds because there are more test variants now.

@ryanofsky
Copy link
Contributor

ryanofsky left a comment

I don't see anything in the tests exercising the application of a label to a scriptPubKey import; if we're indeed lacking that I'm happy to either add a commit here or file a follow-up PR.

This is tested in wallet_import_rescan.py on line 61 when the call type is Call.multiscript. (If you revert the code changes and run the test you should see this case fail.)

@@ -166,7 +166,7 @@ def run_test(self):

# Create new transactions sending to each address.
for i, variant in enumerate(IMPORT_VARIANTS):
variant.sent_amount = 10 - (2 * i + 1) / 8.0
variant.sent_amount = 1 - (2 * i + 1) / 128

This comment has been minimized.

@ryanofsky

ryanofsky Mar 8, 2018

Contributor

Curious what's prompting these value changes.

This is just reducing the amount sent each iteration to prevent the wallet running out of funds because there are more test variants now.

@DrahtBot

This comment has been minimized.

Copy link
Contributor

DrahtBot commented Jul 21, 2018

The last travis run for this pull request was 148 days ago and is thus outdated. To trigger a fresh travis build, this pull request should be closed and re-opened.

@DrahtBot DrahtBot closed this Jul 21, 2018

@DrahtBot DrahtBot reopened this Jul 21, 2018

@sipa

This comment has been minimized.

Copy link
Member

sipa commented Jul 21, 2018

utACK 98ea64c. Looks like a nice simplification too.

@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented Sep 10, 2018

utACK 98ea64c

@laanwj laanwj merged commit 98ea64c into bitcoin:master Sep 10, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Sep 10, 2018

Merge #9332: Let wallet importmulti RPC accept labels for standard sc…
…riptPubKeys

98ea64c Let wallet importmulti RPC accept labels for standard scriptPubKeys (Russell Yanofsky)

Pull request description:

  Allow importmulti RPC to apply address labels when importing standard scriptPubKeys. This makes the importmulti RPC less finnicky about import formats and also simpler internally.

Tree-SHA512: 102426b21239f1fa5f38162dc3f4145572caef76e63906afd786b7aff1670d6cd93456f8d85f737588eedc49c11bef2e1e8019b8b2cbf6097c77b3501b0cab1f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment