Eliminate fee overpaying edge case when subtracting fee from recipients #10942
Conversation
src/wallet/wallet.cpp
Outdated
There was a problem hiding this comment.
Nit, nSubtractFeeFromAmount > 0.
There was a problem hiding this comment.
nFeeRet == 0 should not be necessary? So the first iteration should have enough inputs for when nSubtractFeeFromAmount > 0, or am I missing something?
There was a problem hiding this comment.
Actually, because of
// Never create dust outputs; if we would, just
// add the dust to the fee.
if (IsDust(newTxOut, discard_rate))
{
nChangePosInOut = -1;
nFeeRet += nChange;
}nFeeRet can be > 0, and in that case there is no need to pick new inputs too.
There was a problem hiding this comment.
Good catch. The nFeeRet == 0 check wasn't really doing anything useful anyway, because pick_new_inputs doesn't ever get reset to true. But the prior check that I edited the comment on asserts that we are always on our final pass if pick_new_inputs is false.
|
utACK 49d903e. |
|
utACK 49d903e |
|
Concept ACK |
1 similar comment
|
Concept ACK |
|
utACK 49d903e, looks sane to me |
|
utACK; agree this would be good to include in 0.15 as a bugfix. Perhaps edit the PR description to explain this a bit better (as that is now included in the merge commit)? |
|
utACK, it would be good to clean this function up to calculate change output after calculating the fee at some point post-15. |
|
utACK 49d903e |
… from recipients 49d903e Eliminate fee overpaying edge case when subtracting fee from recipients (Alex Morcos) Pull request description: I'm not sure if this is the cause of the issue in #10034 , but this was a known edge case. I just didn't realize how simple the fix is. Could use a couple more eyes to make sure nothing silly can go wrong here, but if we all agree it's this simple, we can add this as another 0.15 bug fix. Tree-SHA512: db1dd1e83363a3c231267b626d3a388893ee70ba1972056fe2c339c5c9e4fbfd30f7fe837c30cc7be884d454797fd4c619b9d631a8d5eeb55cdb07402a83acb3
…ing fee from recipients 49d903e Eliminate fee overpaying edge case when subtracting fee from recipients (Alex Morcos) Pull request description: I'm not sure if this is the cause of the issue in bitcoin#10034 , but this was a known edge case. I just didn't realize how simple the fix is. Could use a couple more eyes to make sure nothing silly can go wrong here, but if we all agree it's this simple, we can add this as another 0.15 bug fix. Tree-SHA512: db1dd1e83363a3c231267b626d3a388893ee70ba1972056fe2c339c5c9e4fbfd30f7fe837c30cc7be884d454797fd4c619b9d631a8d5eeb55cdb07402a83acb3
11 participants
I'm not sure if this is the cause of the issue in #10034 , but this was a known edge case. I just didn't realize how simple the fix is.
Could use a couple more eyes to make sure nothing silly can go wrong here, but if we all agree it's this simple, we can add this as another 0.15 bug fix.