test: check_mempool_result negative feerate #29459

pull kevkevinpal wants to merge 1 commits into bitcoin:master from kevkevinpal:mempoolAcceptNegativeValues changing 1 files +6 −0

kevkevinpal commented at 3:18 AM on February 21, 2024: contributor

Adds test coverage in mempool_accept.py to check if a negative maxfeerate is input into check_mempool_result Asserts "Amount out of range" error message and -3 error code

Motivated by this comment
DrahtBot commented at 3:18 AM on February 21, 2024: contributor



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Code Coverage

For detailed information about the code coverage, see the test coverage report.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK maflcko, brunoerg, davidgumberg

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
DrahtBot added the label Tests on Feb 21, 2024
in test/functional/mempool_accept.py:99 in 93e6247382 outdated
```
  96 | @@ -97,6 +97,13 @@ def run_test(self):
  97 |              maxfeerate=1,
  98 |          ))
  99 |          # ... 0.99 passes
```
maflcko commented at 8:22 AM on February 21, 2024:

The comment is now wrongly placed

kevkevinpal commented at 4:08 PM on February 21, 2024:

oops fixed in bf264e0
maflcko changes_requested

Type	Reviewers
ACK	maflcko, brunoerg, davidgumberg

test: check_mempool_result negative feerate

Adds test in mempool_accept to check if a negative maxfeerate is inputed
into check_mempool_result, asserts "Amount out of range" error message
and -3 error code

bf264e0598

kevkevinpal force-pushed on Feb 21, 2024
maflcko commented at 4:12 PM on February 21, 2024: member

lgtm ACK bf264e05981e3809715f34f548138d53991db6f2
brunoerg commented at 1:54 PM on February 22, 2024: contributor

nice, utACK bf264e05981e3809715f34f548138d53991db6f2
davidgumberg commented at 8:00 PM on March 11, 2024: contributor
Looks great, ACK https://github.com/bitcoin/bitcoin/pull/29459/commits/bf264e05981e3809715f34f548138d53991db6f2

If you need to rebase you could also add a check that the same error is thrown when maxfeerate exceeds MAX_MONEY:
```
inline bool MoneyRange(const CAmount& nValue) { return (nValue >= 0 && nValue <= MAX_MONEY); }

CAmount AmountFromValue(const UniValue& value, int decimals)
{
    // [...]
    if (!MoneyRange(amount))
        throw JSONRPCError(RPC_TYPE_ERROR, "Amount out of range");
    // [...]
}
```
The same tests should probably be added for sendrawtransaction but that could be done in a follow up.
fanquake assigned glozow on Mar 12, 2024
glozow merged this on Mar 14, 2024
glozow closed this on Mar 14, 2024
glozow unassigned glozow on Mar 14, 2024
bitcoin locked this on Mar 14, 2025

Contributors

Labels

Linked (view graph)

#29434 rpc: Fixed signed integer overflow for large feerates

test: check_mempool_result negative feerate #29459

Code Coverage

Reviews