Avoid creating unnecessary change #892
Merged
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
Contributor
Author
|
User error on my part. I made the change on my master branch which seemed to cause problems. See above where it says: "dooglus merged commit da9ab62 into bitcoin:master from dooglus:master" I don't have write access to bitcoin:master, so I don't know why it thinks I did that. I opened a new pull request, 898, for the same issue. |
destenson
pushed a commit
to destenson/bitcoin--bitcoin
that referenced
this pull request
Jun 26, 2016
c8d3b55 Update dnsseeds - removed darkcoin.qa - added dashdot.io
ptschip
added a commit
to ptschip/bitcoin
that referenced
this pull request
Jan 9, 2018
…oin#838) (bitcoin#892) There is no need to append all the addresses for each output to the transaction destination address which is displayed in the ui. This is because each output for a multi-output transaction is already displayed as an individual transactions. Furthermore, concatenating all the output addresses only make it impossible to know for which output this transaction is for without spending the time to open the transaction details and search through the list of outputs to find the one associated with this amount; and again if two outputs had the same amount you would not be able to make the differentiation. This commit reverts to the previous correct behavior when displaying mulit-output transactions.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
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.
If we have three 50BTC outputs and try to spend 100BTC, we should use only two of the outputs. Previously we were using all three. Similarly if we have outputs of 50, 50, and 0.01 and want to spend 100BTC, we shouldn't include the 0.01 in the transaction.
http://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf shows a transaction where MtGox was trying to consolidate ten 50k outputs into a single 500k output. The wallet had at least 11 50k outputs in it. Because of this code in wallet.cpp:
if (nTotalLower >= nTargetValue + CENT)
nTargetValue += CENT;
the target value was increased from 500k to 500k plus a cent, which couldn't be found using just ten 50k outputs, and so an eleventh 50k output became involved, to avoid sub-cent change. There was no need for any change at all. (I notice the 50k change hasn't yet been redeemed. I hope it isn't lost!)
These transactions show other common cases where extra 0.01 coins are tacked on to both sides of transactions for no good reason:
http://blockexplorer.com/tx/a9d50d41be42beb278f5830d75aee06216b0a9c93d5eb5a62b6d30bec8c272f3
http://blockexplorer.com/tx/a2f748139f90b5063aace9ab18cb0ceb011a440a5a51888e0ec6c7b71a12aed6