Release notes and followups from 19339 #20109
Conversation
|
Note that |
doc/release-notes.md
Outdated
| if the transaction passes validation. (#19940) | ||
|
|
||
| - The `testmempoolaccept` RPC returns `max-fee-exceeded` rather than `absurdly-high-fee` |
There was a problem hiding this comment.
Given that the criteria to throw this reject reason appears to be the exceeding of a feerate, would it not be more informative to make the reject-reason max-feerate-exceeded instead of max-fee-exceeded? I realize that this would have been a better feedback for #19339, but I only see it now.
There was a problem hiding this comment.
Thanks for reviewing :) my "inspiration" was TransactionError::MAX_FEE_EXCEEDED, hopefully not too big of a deal
There was a problem hiding this comment.
It's a bit of a pet-peeve of mine that fees and feerates are often mixed up in Bitcoin, but the ship has probably sailed for this change? 🤔
I'll have to be faster next time. ;)
There was a problem hiding this comment.
@xekyo You're absolutely right that they're often mixed up, but in this context it is actually referring to an absolute fee (by default 0.1 BTC...) rather than a feerate.
There was a problem hiding this comment.
Okay, thanks for the clarification. I'm a bit confused by this description then:
The
sendrawtransactionerror code for exceedingmaxfeeratehas been changed from-26to-25. The error string has been changed from "absurdly-high-fee" to "Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)." ThetestmempoolacceptRPC returnsmax-fee-exceededrather thanabsurdly-high-feeas thereject-reason. (#19339)
It sounds to me that exceeding maxfeerate causes the reject-reason max-fee-exceeded. If they are referring to two separate error resolutions, it wasn't apparent to me. I'm not overtly invested, though, if that looks fine to more experienced contributors.
There was a problem hiding this comment.
Oh perhaps it is me who is confusing things.
There was a problem hiding this comment.
The maxfeerate passed into testmempoolaccept is a feerate and the wallet -maxtxfee option is a fee amount. I'm not really sure why this is the case 😅
|
Sorry @MarcoFalke I confused myself 😅 , added notes for the changed error codes and strings. |
|
cr re-ACK 88197b0 |
|
|
||
| - To make wallet and rawtransaction RPCs more consistent, the error message for | ||
| exceeding maximum feerate has been changed to "Fee exceeds maximum configured by user | ||
| (e.g. -maxtxfee, maxfeerate)." (#19339) |
| @@ -347,6 +347,16 @@ RPC | |||
| - Fee estimation failed | |||
| - Transaction has too long of a mempool chain | |||
|
|
|||
| - The `sendrawtransaction` error code for exceeding `maxfeerate` has been changed from | |||
| `-26` to `-25`. The error string has been changed from "absurdly-high-fee" to | |||
There was a problem hiding this comment.
I think this should also list the name for these error codes as the number itself is a harder to undertsand.
There was a problem hiding this comment.
We can adjust this in the wiki closer to release.
88197b0 [doc] release notes for max fee checking (gzhao408) c201d73 style and nits for fee-checking in BroadcastTransaction (gzhao408) Pull request description: Pretty trivial... addresses some tiny comments from bitcoin#19339. Also fixes a docs typo from bitcoin#19940 and adds a release note about the error message change for testmempoolaccept. ACKs for top commit: achow101: ACK 88197b0 MarcoFalke: cr re-ACK 88197b0 Tree-SHA512: fff16d731426b9b4db5222df02633983402f4c7241551eec98bb1554145dbdc132f40ed8ca4abd5edcebe1f4d1e879fb6d11bd91730604f6552c10cdf65706a1
Summary: Check absurd fee in BroadcastTransaction and RPC, return TransactionError::MAX_FEE_EXCEEDED instead of TxValidationResult::TX_NOT_STANDARD because this is client preference, not a node-wide policy. This is a backport of [[bitcoin/bitcoin#19339 | core#19339]] [1/3] and [[bitcoin/bitcoin#20109 | core#20109]] bitcoin/bitcoin@8f1290c Depends on D10463 and D10474 Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Subscribers: Fabien Differential Revision: https://reviews.bitcoinabc.org/D10464
Pretty trivial... addresses some tiny comments from #19339. Also fixes a docs typo from #19940 and adds a release note about the error message change for testmempoolaccept.