Skip to content

0.18: wallet: Fix -maxtxfee check by moving it to CWallet::CreateTransaction #16414

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Sep 23, 2019
Merged

0.18: wallet: Fix -maxtxfee check by moving it to CWallet::CreateTransaction #16414

merged 3 commits into from
Sep 23, 2019

Conversation

promag
Copy link
Contributor

@promag promag commented Jul 18, 2019 •
edited by fanquake
Loading

Backports #16322 and #16257.

@promag promag force-pushed the 2019-07-backport-16322 branch 2 times, most recently from 49a65ca to 2e236ef Compare July 18, 2019 15:53
@fanquake fanquake added this to the 0.18.1 milestone Jul 18, 2019
@fanquake fanquake requested review from Sjors and maflcko July 19, 2019 00:48
@Sjors
Copy link
Member

Sjors commented Jul 20, 2019

ACK modulo the wip: commit messages. Description should mention this also backports #16257.

@promag
Copy link
Contributor Author

promag commented Jul 20, 2019

Is this 0.18.2 then?

This was referenced Jul 25, 2019
Merged
Merged
@fanquake
Copy link
Member

Is this 0.18.2 then?

Unless there happens to be an rc2, and this is merged in the interim, I think that will likely be the case.

Given that this is backporting to the 0.18 branch, instead of adding the doc/release-notes-0.18.1-16257.md file, can you just add the changes directly to release-notes.md? You might have to rebase, as the release notes have just been updated for 0.18.1.

@fanquake fanquake modified the milestones: 0.18.1, 0.18.2 Aug 2, 2019
@meshcollider
Copy link
Contributor

Why is the commit message wip? Is this ready for review?

@Sjors Sjors mentioned this pull request Aug 17, 2019
Merged
@promag promag force-pushed the 2019-07-backport-16322 branch from 2e236ef to 7397275 Compare August 18, 2019 23:54
@promag
Copy link
Contributor Author

promag commented Aug 18, 2019

Dropped wip prefix.

@Sjors
Copy link
Member

Sjors commented Aug 19, 2019

ACK 7397275

@fanquake
Copy link
Member

This needs a rebase and the commits need the backport metadata (Github-Pull: & Rebased-From:) added. See the backport.py script.

Sjors and others added 2 commits August 25, 2019 02:48
@Sjors @promag
0.18: [wallet] abort when attempting to fund a transaction above maxt…
8f354ce 
…xfee

FundTransaction calls GetMinimumFee which, when the fee rate is absurdly high, quietly reduced the fee to -maxtxfee. Becaue an absurdly high fee rate is usually the result of a fat finger, aborting seems safer behavior.

Github-Pull: bitcoin#16257
Rebased-From: 806b005
@promag
0.18: wallet: Fix -maxtxfee check by moving it to CWallet::CreateTran…
a11dbaa 
…saction

Github-Pull: bitcoin#16322
Rebased-From: 5c1b971
@promag promag force-pushed the 2019-07-backport-16322 branch from 7397275 to d3b3bb8 Compare August 25, 2019 01:52
laanwj added a commit that referenced this pull request Sep 12, 2019
@laanwj
Merge #16639: [0.17] abort when attempting to fund a transaction abov…
1659b9b 
…e -maxtxfee

0e7c746 wallet: Fix -maxtxfee check by moving it to CWallet::CreateTransaction (João Barbosa)
e9adb96 [wallet] abort when attempting to fund a transaction above maxtxfee (Sjors Provoost)

Pull request description:

  Backport #16257.

  Cherry-picked from the 0.18 backport in #16414, but without the [wip] messages and without the last commit (which adds a test in a file that didn't exist in 0.17).

ACKs for top commit:
  laanwj:
    ACK 0e7c746

Tree-SHA512: a0fd603518487854be0b3815f34a8dabd2ed258850c032b08894a7c55cb135df5d3c103f76c5294e8065fec6d610e064acf01d24c77f02eaf996698a1e45d512
@promag
Copy link
Contributor Author

promag commented Sep 22, 2019

Ping.

@Sjors
Copy link
Member

Sjors commented Sep 23, 2019

re-ACK d3b3bb8

@maflcko maflcko removed the Tests label Sep 23, 2019
@maflcko
Copy link
Member

maflcko commented Sep 23, 2019

ACK d3b3bb8 (did the backport myself, arrived at the same result)

Show signature and timestamp

Signature:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK d3b3bb8c9fb3a2ff1cd3a8776552c649aaf19dc2 (did the backport myself, arrived at the same result)
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUgocwv/bfvARaya5du9b8Tamq0D+dRpoy6FdepDukCHC8NvuekeIxucmHUz7LhJ
UdDwzVSL8v+VGSJ2G6FtLfEzMVJzYdZDak08yarUxJT5vHnxI91fULAvNbXG/+7l
3silywwBoei32t3v3WV/Lgth6xHIWHVumpyjLCJjvB3gp7amEbcKc9KqlvSTvYfg
EWRrtO2RVt7MRduI99m/b+FMuMBY83lkhMBjyFJpD7cmqGtN3WGxxw/IiY0Xu78y
bMt/ELGHTS3AgelOj4rEZwAiRHW+iCgEzHp0Mzn2OvaI6LzsNL0cU/5MKw+sFJzy
zTGOfWpKj+bXlZtpYwr2F9WDh0GF8Fw9RO/w6pjtulTxGbAeD6jtrz8ko99md8kh
RzzNaJrnlddu3fNPe0wGYDIG04eEJPl4ZLBmjmJ44F2m/A2o+/GOJdvs2kOPgfhL
4/JZb32CxJmFQwbYhd85TE3PBFtGQpUalKu/MS1Tx/TGI16kSdztgqkdkGbFWDsN
2zqsw7x9
=F6GX
-----END PGP SIGNATURE-----

Timestamp of file with hash 95092eee457eb227f864af6a5b5dcb82a1ac2386c8b2dffd2bd4ed4ff7edade7 -

maflcko pushed a commit that referenced this pull request Sep 23, 2019
Merge #16414: 0.18: wallet: Fix -maxtxfee check by moving it to CWall…
3b04221 
…et::CreateTransaction

d3b3bb8 0.18: test: Add test for maxtxfee option (MarcoFalke)
a11dbaa 0.18: wallet: Fix -maxtxfee check by moving it to CWallet::CreateTransaction (João Barbosa)
8f354ce 0.18: [wallet] abort when attempting to fund a transaction above maxtxfee (Sjors Provoost)

Pull request description:

  Backports #16322 and #16257.

ACKs for top commit:
  Sjors:
    re-ACK d3b3bb8
  MarcoFalke:
    ACK d3b3bb8 (did the backport myself, arrived at the same result)

Tree-SHA512: 8b0ca15fc7e893af80239afecf8ff1018d6f249f2fa530babe61ec34ede6103b9b60909259abb730ebf1d54789aceed94b136600158dc3d6c5505b07f28189e5
@maflcko maflcko merged commit d3b3bb8 into bitcoin:0.18 Sep 23, 2019
@promag promag deleted the 2019-07-backport-16322 branch September 23, 2019 13:45
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Sjors Sjors Awaiting requested review from Sjors

@maflcko maflcko Awaiting requested review from maflcko

@meshcollider meshcollider Awaiting requested review from meshcollider

@achow101 achow101 Awaiting requested review from achow101

Assignees
No one assigned
Labels
Backport Wallet
Projects
None yet
Milestone
0.18.2
Development

Successfully merging this pull request may close these issues.
6 participants
@promag @Sjors @fanquake @meshcollider @maflcko @DrahtBot