Merge #10712: Add change output if necessary to reduce excess fee
ClosedPublic
Actions

Authored by jasonbcox on Feb 5 2019, 23:52.

Details

Reviewers

deadalnix
Fabien

Group Reviewers

Restricted Project

Commits

rSTAGING06c232f38061: Merge #10712: Add change output if necessary to reduce excess fee
rABC06c232f38061: Merge #10712: Add change output if necessary to reduce excess fee

Summary

0f402b9 Fix rare edge case of paying too many fees when transaction has no change. (Alex Morcos)
253cd7e Only reserve key for scriptChange once in CreateTransaction (Alex Morcos)

Pull request description:

This is an alternative to #10333

See commit messages.

The first commit is mostly code move, it just moves the change creation code out of the loop.

@instagibbs

Tree-SHA512: f16287ae0f0c6f09cf8b1f0db5880bb567ffa74a50898e3d1ef549ba592c6309ae1a9b251739f63a8bb622d48f03ce2dff9e7a57a6bac4afb4b95b0a86613ea8

Backport of Core PR10712
https://github.com/bitcoin/bitcoin/pull/10712/commits
Completes T534

Test Plan

make check
test_runner.py

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

jasonbcox created this revision.Feb 5 2019, 23:52

Herald added a reviewer: Restricted Project. · View Herald TranscriptFeb 5 2019, 23:52

Herald added a subscriber: schancel. · View Herald Transcript

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B4882: Diff 7192.Feb 6 2019, 00:06

deadalnix requested changes to this revision.Feb 6 2019, 16:04

deadalnix added inline comments.

src/wallet/wallet.cpp
2897 ↗	(On Diff #7192)	This is very strange, but okay. That's how it is in the original diff.
3098 ↗	(On Diff #7192)	Has the indentation level been changed ? If not then why reformat comment blocks ? This doesn't even correspond to what's in the original PR.

This revision now requires changes to proceed.Feb 6 2019, 16:04

Rebase + fix comment linting

jasonbcox added inline comments.Feb 6 2019, 18:56

src/wallet/wallet.cpp
3098 ↗	(On Diff #7192)	I re-ran the linter while working on this diff and may have screwed up something because at a moment in time the indentation was different. Will fix. Edit: I figured out what happened. I expected since the linter passed on the first version, that it must either match the original PR or otherwise have needed line breaks at those locations. Neither of these were the case, but it was a valid linting, so it remained.