wallet, RPC: Default BIP125 signal to -mempoolfullrbf #25602
Conversation
maflcko
force-pushed
the
2207-rpc-wallet-replace-
branch
from
faffb06 to
fac6ce4
Compare
glozow
left a comment
glozow
left a comment
There was a problem hiding this comment.
Concept ACK based on the description, will review more closely soon
|
Tend to NACK I would expect that |
| if (!request.params[3].isNull()) { | ||
| rbf = request.params[3].isTrue(); | ||
| } | ||
| const bool rbf{request.params[3].isNull() ? args.GetBoolArg("-mempoolfullrbf", DEFAULT_MEMPOOL_FULL_RBF) : request.params[3].getBool()}; |
There was a problem hiding this comment.
I think the following works, though I would rather do some wallet options encapsulations like done with MemPoolOptions. I believe the wallet could have its own default transaction-relay policy settings in case the mempool ones are bigger (e.g if local limitdescendantsize=100, still makes sense to propagate chain of transactions of max size=25)
diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
index 263baa3c9..dc9f02160 100644
--- a/src/rpc/rawtransaction.cpp
+++ b/src/rpc/rawtransaction.cpp
@@ -305,8 +305,9 @@ static RPCHelpMan createrawtransaction()
}, true
);
const ArgsManager& args{EnsureAnyArgsman(request.context)};
+ const NodeContext& node = EnsureAnyNodeContext(request.context);
- const bool rbf{request.params[3].isNull() ? args.GetBoolArg("-mempoolfullrbf", DEFAULT_MEMPOOL_FULL_RBF) : request.params[3].getBool()};
+ const bool rbf{request.params[3].isNull() ? node.mempool->m_full_rbf : request.params[3].getBool()};
CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], rbf);
return EncodeHexTx(CTransaction(rawTx));
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
6 participants
-mempoolfullrbfallows valid replacement transactions into the mempool, even if they do not signal for BIP125. If it is set, it would be confusing if the wallet and RPC didn't signal for BIP125 replacement:So to prevent those issues in the normal case, adjust the default for
-walletrbfas well as the RPC raw transactions API.It is still possible for users to set
-mempoolfullrbf=1 -walletrbf=0, as well as passing in thereplaceable=Falseoption to each RPC, if they wish. Or to leave the-mempoolfullrbfdefault value: off.