wallet: Fix -maxtxfee check by moving it to CWallet::CreateTransaction #16322
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
|
@Sjors you promised a review 😄 |
|
tACK a2f23b8 This also makes the QT message the same as RPC. Afaik there's no way to reach the |
maflcko
reviewed
Jul 2, 2019
There was a problem hiding this comment.
Concept ACK.
Maybe revert the docstring in the gui to say "belt-and-suspenders" again?
See https://github.com/bitcoin/bitcoin/pull/16257/files#diff-2e3836af182cfb375329c3463ffd91f8
|
Looks good, I'll write some tests and send them over in a bit. |
|
ACK, tested by writing a test: maflcko@9ff66a1 |
|
Thanks for the
Maybe that is indeed slightly safer than dropping it completely. |
|
@MarcoFalke great stuff, pulled both commits. |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
|
ACK 9ff66a1 (read code, wrote test case) Show signature and timestampSignature: Timestamp of file with hash |
|
Concept ACK |
|
@meshcollider ping. |
|
The previous pull was reviewed by @kallewoof @jonasschnelli @laanwj @achow101. Would be helpful to get your eyes on this as well. |
test/functional/wallet_create_tx.py
Outdated
| lambda: self.nodes[0].fundrawtransaction(hexstring=raw_tx), | ||
| ) | ||
|
|
||
| self.log.info('Check maxtxfee in combination with settxfee'.format(fee_setting)) |
There was a problem hiding this comment.
.format(fee_setting) part is a mistake? :-)
There was a problem hiding this comment.
Yap, @MarcoFalke I'll amend.
9ff66a1 to
0d101a3
Compare
|
re-ACK 0d101a3, only change is small test fixup Show signature and timestampSignature: Timestamp of file with hash |
|
ACK 0d101a3 |
laanwj
added a commit
that referenced
this pull request
Jul 10, 2019
…eateTransaction 0d101a3 test: Add test for maxtxfee option (MarcoFalke) 1775501 wallet: Remove unreachable code in CreateTransaction (MarcoFalke) 5c1b971 wallet: Fix -maxtxfee check by moving it to CWallet::CreateTransaction (João Barbosa) Pull request description: Follow up to #16257, this PR makes `bumpfee` aware of `-maxtxfee`. It also prevents dangling locked unspents when calling `fundrawtransaction` - because the previous check was after `LockCoin`. ACKs for top commit: MarcoFalke: re-ACK 0d101a3, only change is small test fixup Tree-SHA512: 3464b24ae7cd4e72ed41438c6661828ba1304af020f05da62720b23668ae734e16cf47c6d97e150cc84ef631ee099b16fc786c858f3d089905845437338fd512
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Jul 10, 2019
…let::CreateTransaction 0d101a3 test: Add test for maxtxfee option (MarcoFalke) 1775501 wallet: Remove unreachable code in CreateTransaction (MarcoFalke) 5c1b971 wallet: Fix -maxtxfee check by moving it to CWallet::CreateTransaction (João Barbosa) Pull request description: Follow up to bitcoin#16257, this PR makes `bumpfee` aware of `-maxtxfee`. It also prevents dangling locked unspents when calling `fundrawtransaction` - because the previous check was after `LockCoin`. ACKs for top commit: MarcoFalke: re-ACK 0d101a3, only change is small test fixup Tree-SHA512: 3464b24ae7cd4e72ed41438c6661828ba1304af020f05da62720b23668ae734e16cf47c6d97e150cc84ef631ee099b16fc786c858f3d089905845437338fd512
|
Being backported in #16414. |
Sjors
pushed a commit
to Sjors/bitcoin
that referenced
this pull request
Aug 19, 2019
Github-Pull: bitcoin#16322 Rebased-From: 5c1b971
promag
added a commit
to promag/bitcoin
that referenced
this pull request
Aug 25, 2019
…saction Github-Pull: bitcoin#16322 Rebased-From: 5c1b971
promag
pushed a commit
to promag/bitcoin
that referenced
this pull request
Aug 25, 2019
Github-Pull: bitcoin#16322 Rebased-From: 0d101a3
maflcko
pushed a commit
that referenced
this pull request
Sep 23, 2019
…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
deadalnix
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Jun 22, 2020
…:CreateTransaction Summary: wallet: Remove unreachable code in CreateTransaction test: Add test for maxtxfee option --- Backport of Core [[bitcoin/bitcoin#16322 | PR16322]] Test Plan: ninja all check ./test/functional/test_runner.py wallet_create_tx Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D6664
Munkybooty
pushed a commit
to Munkybooty/dash
that referenced
this pull request
Nov 4, 2021
…let::CreateTransaction 0d101a3 test: Add test for maxtxfee option (MarcoFalke) 1775501 wallet: Remove unreachable code in CreateTransaction (MarcoFalke) 5c1b971 wallet: Fix -maxtxfee check by moving it to CWallet::CreateTransaction (João Barbosa) Pull request description: Follow up to bitcoin#16257, this PR makes `bumpfee` aware of `-maxtxfee`. It also prevents dangling locked unspents when calling `fundrawtransaction` - because the previous check was after `LockCoin`. ACKs for top commit: MarcoFalke: re-ACK 0d101a3, only change is small test fixup Tree-SHA512: 3464b24ae7cd4e72ed41438c6661828ba1304af020f05da62720b23668ae734e16cf47c6d97e150cc84ef631ee099b16fc786c858f3d089905845437338fd512
Munkybooty
pushed a commit
to Munkybooty/dash
that referenced
this pull request
Nov 4, 2021
…let::CreateTransaction 0d101a3 test: Add test for maxtxfee option (MarcoFalke) 1775501 wallet: Remove unreachable code in CreateTransaction (MarcoFalke) 5c1b971 wallet: Fix -maxtxfee check by moving it to CWallet::CreateTransaction (João Barbosa) Pull request description: Follow up to bitcoin#16257, this PR makes `bumpfee` aware of `-maxtxfee`. It also prevents dangling locked unspents when calling `fundrawtransaction` - because the previous check was after `LockCoin`. ACKs for top commit: MarcoFalke: re-ACK 0d101a3, only change is small test fixup Tree-SHA512: 3464b24ae7cd4e72ed41438c6661828ba1304af020f05da62720b23668ae734e16cf47c6d97e150cc84ef631ee099b16fc786c858f3d089905845437338fd512
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Follow up to #16257, this PR makes
bumpfeeaware of-maxtxfee.It also prevents dangling locked unspents when calling
fundrawtransaction- because the previous check was afterLockCoin.