#25434, #25532 and #25570 add some parameters to bypass some mempool checks. The motivations for each parameter can be found in these PRs.
#25434 (review) suggested to allocate the existing mempool parameters bypass_limits and test_accept in the same structure.
This PR makes that change without introducing new behaviors or new assumptions. It is basically a refactor and will be used as basis for the new parameters in the PRs mentioned above.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, this pull request conflicts with the following ones:
desriptor_file to importdescriptors RPC by w0xlt)save_to_file to listdescriptors RPC by w0xlt)If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
154 | - CFeeRate(AmountFromValue(request.params[1])); 155 | + CFeeRate max_raw_tx_fee_rate = DEFAULT_MAX_RAW_TX_FEE_RATE; 156 | + 157 | + if (!request.params[1].isNull()) { 158 | + const UniValue& options = request.params[1]; 159 | + RPCTypeCheckObj(options,
Should have backward compatibility for now
515 | @@ -518,18 +516,16 @@ class MemPoolAccept 516 | // Private ctor to avoid exposing details to clients and allowing the possibility of 517 | // mixing up the order of the arguments. Use static functions above instead. 518 | ATMPArgs(const CChainParams& chainparams, 519 | - int64_t accept_time, 520 | - bool bypass_limits, 521 | + const int64_t accept_time, 522 | + const std::optional<MemPoolBypass>& mempool_bypass,
Probably better to just have a constexpr noop MemPoolBypass
Could you elaborate more ? I don´t see how constexpr fits in this case.
242 | + //! This does not prevent mempool from accepting the transaction. 243 | + bool m_bypass_limits{false}; 244 | + 245 | + MemPoolBypass() {} 246 | + 247 | + MemPoolBypass(bool test_accept, bool bypass_limits) :
Prefer limiting this to designated initializers, or otherwise explicit assignment.
I originally used only C++ 20 designated initializers in this PR, but it crashes on one (and only one) CI check (error C7555 on a Windows machine, if I remember correctly).
Something like:
error C7555: use of designated initializers requires at least '/std:c++20'
We now use C++20 on that CI, so they should work fine.
@luke-jr Thanks for the review. #25577 (review) and #25577 (review) have been addressed in the new push.
97 | @@ -98,7 +98,15 @@ static RPCHelpMan testmempoolaccept() 98 | }, 99 | }, 100 | {"maxfeerate", RPCArg::Type::AMOUNT, RPCArg::Default{FormatMoney(DEFAULT_MAX_RAW_TX_FEE_RATE.GetFeePerK())}, 101 | - "Reject transactions whose fee rate is higher than the specified value, expressed in " + CURRENCY_UNIT + "/kvB\n"}, 102 | + "(deprecated. Use \"options\" instead) Reject transactions whose fee rate is higher than the specified value, expressed in " + CURRENCY_UNIT + "/kvB\n"},
Replacing the option is fine. Only need to tolerate the old param type.
Might need to pull in 09964963a1f7748fc1abc9471cbae8dab1061eb1
Are you suggesting something like the dummy argument in getbalance() ?
RPCHelpMan getbalance()
{
....
{"dummy", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Remains for backward compatibility. Must be excluded or set to \"*\"."},
No, more like this:
{"options|maxfeerate", {RPCArg::Type::OBJ, RPCArg::Type::AMOUNT}, RPCArg::Optional::OMITTED_NAMED_ARG, "",
Author: Luke Dashjr <luke-jr+git@utopios.org>
`maxfeerate` becomes a member of an "options" object rather than
a positional argument.
The idea is that any new parameters in the future will also go into
options.
Unclear to me how beneficial this refactor is, see contrib guidelines on refactoring.