fundrawtransaction: Keep change-output keys by default, make it optional #9377
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
Thanks! Concept ACK. |
Contributor
|
Concept ACK. Thanks! |
jtimon
reviewed
Jan 13, 2017
qa/rpc-tests/fundrawtransaction.py
Outdated
Contributor
There was a problem hiding this comment.
why += instead of a regular assignment? What if there's more outputs than one with value > 1?
Contributor
|
utACK 161af5b modulo the tests which I'm not sure I'm fully understanding. Needs rebase. |
Contributor
|
Needs rebase (but is a bugfix, so can go in for 0.14 after the feature freeze on Monday). |
jonasschnelli
force-pushed
the
2016/12/fix_frt_adr
branch
from
161af5b to
c9f3062
Compare
Contributor
Author
|
Rebased. |
Member
Member
|
Not going to block this on a test nit - that can be fixed later. |
laanwj
added a commit
that referenced
this pull request
Jan 20, 2017
…make it optional c9f3062 Add fundrawtransactions new reserveChangeKey option to the release notes (Jonas Schnelli) 9eb325d [QA] Add test for fundrawtransactions new reserveChangeKey option (Jonas Schnelli) 9aa4e6a [Wallet] Add an option to keep the change address key, true by default (Jonas Schnelli)
|
Just a quick question to confirm: so in the latest master there is no more problem at fundrawtransaction() with address reuse? We hit an issue when sending transaction via fundrawtransaction change was sent to the existing user account address and thus credited our user with balance. sendtoaddress always creates a new change address I suppose. |
Contributor
Author
Yes. This is exactly what this PR does fix. |
laanwj
added a commit
that referenced
this pull request
Jul 18, 2017
…erving them cf82a9e Do not allow users to get keys from keypool without reserving them (Matt Corallo) Pull request description: fundrawtransaction allows users to add a change output and then not have it removed from keypool. While it would be nice to have users follow the normal CreateTransaction/CommitTransaction process we use internally, there isnt much benefit in exposing this option, especially with HD wallets, while there is ample room for users to misunderstand or misuse this option. This partially reverts #9377. Would be nice to get this for 15 since its kinda crazy we have this option to begin with IMO, will need release notes as an RPC option is now ignored. Tree-SHA512: 72b5ee9c4a229b84d799dfb00c56fe80d8bba914ce81a433c3f5ab325bf9bf2b839ee658c261734f0ee183ab19435039481014d09c41dbe155e6323e63beb01d
codablock
pushed a commit
to codablock/dash
that referenced
this pull request
Jan 21, 2018
…fault, make it optional c9f3062 Add fundrawtransactions new reserveChangeKey option to the release notes (Jonas Schnelli) 9eb325d [QA] Add test for fundrawtransactions new reserveChangeKey option (Jonas Schnelli) 9aa4e6a [Wallet] Add an option to keep the change address key, true by default (Jonas Schnelli)
andvgal
pushed a commit
to energicryptocurrency/gen2-energi
that referenced
this pull request
Jan 6, 2019
…fault, make it optional c9f3062 Add fundrawtransactions new reserveChangeKey option to the release notes (Jonas Schnelli) 9eb325d [QA] Add test for fundrawtransactions new reserveChangeKey option (Jonas Schnelli) 9aa4e6a [Wallet] Add an option to keep the change address key, true by default (Jonas Schnelli)
CryptoCentric
pushed a commit
to absolute-community/absolute
that referenced
this pull request
Feb 27, 2019
…fault, make it optional c9f3062 Add fundrawtransactions new reserveChangeKey option to the release notes (Jonas Schnelli) 9eb325d [QA] Add test for fundrawtransactions new reserveChangeKey option (Jonas Schnelli) 9aa4e6a [Wallet] Add an option to keep the change address key, true by default (Jonas Schnelli)
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Sep 8, 2019
…out reserving them cf82a9e Do not allow users to get keys from keypool without reserving them (Matt Corallo) Pull request description: fundrawtransaction allows users to add a change output and then not have it removed from keypool. While it would be nice to have users follow the normal CreateTransaction/CommitTransaction process we use internally, there isnt much benefit in exposing this option, especially with HD wallets, while there is ample room for users to misunderstand or misuse this option. This partially reverts bitcoin#9377. Would be nice to get this for 15 since its kinda crazy we have this option to begin with IMO, will need release notes as an RPC option is now ignored. Tree-SHA512: 72b5ee9c4a229b84d799dfb00c56fe80d8bba914ce81a433c3f5ab325bf9bf2b839ee658c261734f0ee183ab19435039481014d09c41dbe155e6323e63beb01d
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Alternative solution for #9370, fixes #9362.
This adds
reserveChangeKey, a new boolean option tofundrawtransaction.The default is
trueresulting in always removing the change output address key from the keypool.Includes test and documentation in the release-notes.