To check if a single raw transaction makes it into the current transaction pool, one had to call sendrawtransaction. However, on success, this adds the transaction to the mempool with no easy way to undo.
The call testmempoolaccept is introduced to provide a way to solely check the result without changing the mempool state.
996+ "testmempoolaccept \"hexstring\" ( allowhighfees )\n"
997+ "\nReturns if raw transaction (serialized, hex-encoded) would be accepted by mempool.\n"
998+ "\nAlso see sendrawtransaction call.\n"
999+ "\nArguments:\n"
1000+ "1. \"hexstring\" (string, required) The hex string of the raw transaction)\n"
1001+ "2. allowhighfees (boolean, optional, default=false) Allow high fees\n"
1035+ }
1036+
1037+ UniValue result(UniValue::VOBJ);
1038+ result.push_back(Pair("hex", tx_hash.GetHex()));
1039+
1040+ {
1042+ const CCoinsViewCache& view = *pcoinsTip;
1043+ for (size_t o = 0; o < tx->vout.size(); o++) {
1044+ bool is_tx_in_chain = !view.AccessCoin(COutPoint(tx_hash, o)).IsSpent();
1045+ if (is_tx_in_chain) {
1046+ result.push_back(Pair("allowed", false));
1047+ result.push_back(Pair("error", "transaction already in block chain"));
4394@@ -4386,7 +4395,8 @@ bool LoadMempool(void)
4395 if (nTime + nExpiryTimeout > nNow) {
4396 LOCK(cs_main);
4397 AcceptToMemoryPoolWithTime(chainparams, mempool, state, tx, nullptr /* pfMissingInputs */, nTime,
4398- nullptr /* plTxnReplaced */, false /* bypass_limits */, 0 /* nAbsurdFee */);
4399+ nullptr /* plTxnReplaced */, false /* bypass_limits */, 0 /* nAbsurdFee */,
4400+ nullptr /* test_accept */);
testmempoolaccept in a short period can return different values?
1077@@ -998,6 +1078,7 @@ static const CRPCCommand commands[] =
1078 { "rawtransactions", "sendrawtransaction", &sendrawtransaction, {"hexstring","allowhighfees"} },
1079 { "rawtransactions", "combinerawtransaction", &combinerawtransaction, {"txs"} },
1080 { "rawtransactions", "signrawtransaction", &signrawtransaction, {"hexstring","prevtxs","privkeys","sighashtype"} }, /* uses wallet if enabled */
1081+ { "rawtransactions", "testmempoolaccept", &testmempoolaccept, {"hexstring","allowhighfees"} },
1008+ "\nExamples:\n"
1009+ "\nCreate a transaction\n"
1010+ + HelpExampleCli("createrawtransaction", "\"[{\\\"txid\\\" : \\\"mytxid\\\",\\\"vout\\\":0}]\" \"{\\\"myaddress\\\":0.01}\"") +
1011+ "Sign the transaction, and get back the hex\n"
1012+ + HelpExampleCli("signrawtransaction", "\"myhex\"") +
1013+ "\nSend the transaction (signed hex)\n"
1001+ "2. allowhighfees (boolean, optional, default=false) Allow high fees\n"
1002+ "\nResult:\n"
1003+ "{\n"
1004+ " \"hex\" (string) The transaction hash in hex\n"
1005+ " \"allowed\" (boolean) If the mempool allows this tx to be inserted\n"
1006+ " \"reject-reason\" (string) rejection string (if any)\n"
577@@ -578,7 +578,7 @@ class CTxMemPool
578 /** Populate setDescendants with all in-mempool descendants of hash.
579 * Assumes that setDescendants includes all in-mempool descendants of anything
580 * already in it. */
581- void CalculateDescendants(txiter it, setEntries &setDescendants);
582+ void CalculateDescendants(const txiter it, setEntries& setDescendants) const;
const has no effect.
Nice. Finally. utACK fa12a9481ea7d930c75f109f3c10b200db70e501
A comment for the new parameter of AcceptToMemoryPool in validation.h would be nice.
1003+ "\nArguments:\n"
1004+ "1. \"hexstring\" (string, required) The hex string of the raw transaction)\n"
1005+ "2. allowhighfees (boolean, optional, default=false) Allow high fees\n"
1006+ "\nResult:\n"
1007+ "{\n"
1008+ " \"hex\" (string) The transaction hash in hex\n"
txid and the parameter to rawtx to make clear that they are different.
1040+
1041+ UniValue result(UniValue::VOBJ);
1042+ result.push_back(Pair("hex", tx_hash.GetHex()));
1043+
1044+ LOCK(cs_main);
1045+ const CCoinsViewCache& view = *pcoinsTip;
I guess this was borrowed from sendrawtransaction.
There, there is an extra test bool fHaveMempool = mempool.exists(hashTx),
but it could be replaced by checking the ATMP error txn-already-in-mempool.
However in other PR I recall @sipa said it was kind of bad thing to rely on these rejection reasons.
missing_inputs, which is correct but might not be helpful at first.
1010+ " \"reject-reason\" (string) rejection string (only present when 'allowed'==false)\n"
1011+ "}\n"
1012+ "\nExamples:\n"
1013+ "\nCreate a transaction\n"
1014+ + HelpExampleCli("createrawtransaction", "\"[{\\\"txid\\\" : \\\"mytxid\\\",\\\"vout\\\":0}]\" \"{\\\"myaddress\\\":0.01}\"") +
1015+ "Sign the transaction, and get back the hex\n"
\n.
1005+ "2. allowhighfees (boolean, optional, default=false) Allow high fees\n"
1006+ "\nResult:\n"
1007+ "{\n"
1008+ " \"hex\" (string) The transaction hash in hex\n"
1009+ " \"allowed\" (boolean) If the mempool allows this tx to be inserted\n"
1010+ " \"reject-reason\" (string) rejection string (only present when 'allowed'==false)\n"
when 'allowed' is false
@instagibbs Revelant history #7552 from @laanwj
I think this one can work because it has a better name. verifyrawtransaction is complex because there is lot’s of bike-shed about the args and what verifying means. The meaning of testmempoolaccept is obvious and can’t be misinterpreted, nor would it make sense to have more parameters.
1055+ if (state.IsInvalid()) {
1056+ result.push_back(Pair("reject-reason", strprintf("%i: %s", state.GetRejectCode(), state.GetRejectReason())));
1057+ } else if (missing_inputs) {
1058+ result.push_back(Pair("reject-reason", "Missing inputs"));
1059+ } else {
1060+ result.push_back(Pair("reject-reason", state.GetRejectReason()));
IsInvalid but it would reject it?
@instagibbs Previous pulls were:
verifytransaction tells you everything is fine, but your mempool rejects). Also ended in a state {WIP, needs review, will revisit later}…I think this pull (testmempoolaccept) nicely exploits the existing checks for consensus/standardness that are called in ATMP, without touching too much of validation code. A future pull can extend this rpc to take lists of more than one raw transaction and feed them into an ephemeral mempool (e.g. based on the current mempool).
997+ if (request.fHelp || request.params.size() < 1 || request.params.size() > 2) {
998+ throw std::runtime_error(
999+ // clang-format off
1000+ "testmempoolaccept [\"rawtxs\"] ( allowhighfees )\n"
1001+ "\nReturns if raw transaction (serialized, hex-encoded) would be accepted by mempool.\n"
1002+ "\nThis checks if the transaction violates our consesus or policy rules.\n"
Type: consesus.
Nit, s/our/the.
998+ throw std::runtime_error(
999+ // clang-format off
1000+ "testmempoolaccept [\"rawtxs\"] ( allowhighfees )\n"
1001+ "\nReturns if raw transaction (serialized, hex-encoded) would be accepted by mempool.\n"
1002+ "\nThis checks if the transaction violates our consesus or policy rules.\n"
1003+ "\nAlso see sendrawtransaction call.\n"
See sendrawtransaction.
1029+
1030+ ObserveSafeMode();
1031+
1032+ RPCTypeCheck(request.params, {UniValue::VARR, UniValue::VBOOL});
1033+ if (request.params[0].get_array().size() != 1) {
1034+ throw JSONRPCError(RPC_TYPE_ERROR, "Array must contain exactly one raw transaction for now");
RPC_INVALID_PARAMETER instead? Type is already correct (it’s an array at least).
1045+ if (!request.params[1].isNull() && request.params[1].get_bool()) {
1046+ max_raw_tx_fee = 0;
1047+ }
1048+
1049+ UniValue result(UniValue::VARR);
1050+
1009+ "[ (array) The result of the mempool acceptance test for each raw transaction in the input array.\n"
1010+ " Length is exactly one for now.\n"
1011+ " {\n"
1012+ " \"txid\" (string) The transaction hash in hex\n"
1013+ " \"allowed\" (boolean) If the mempool allows this tx to be inserted\n"
1014+ " \"reject-reason\" (string) rejection string (only present when 'allowed' is false)\n"
Rejection.
1027+ );
1028+ }
1029+
1030+ ObserveSafeMode();
1031+
1032+ RPCTypeCheck(request.params, {UniValue::VARR, UniValue::VBOOL});
request.params[0].get_array() and request.params[1].get_bool().
1054+ LOCK(cs_main);
1055+
1056+ CValidationState state;
1057+ bool missing_inputs;
1058+ bool test_accept_res;
1059+ bool res = AcceptToMemoryPool(mempool, state, std::move(tx), &missing_inputs,
Nit, we could avoid loose locks, I mean, it’s more clear where the lock/unlock happens:
0{
1 LOCK(cs_main);
2 bool res = AcceptToMemoryPool(...);
3 assert(!res);
4}
1062+ result_0.pushKV("allowed", test_accept_res);
1063+ if (!test_accept_res) {
1064+ if (state.IsInvalid()) {
1065+ result_0.pushKV("reject-reason", strprintf("%i: %s", state.GetRejectCode(), state.GetRejectReason()));
1066+ } else if (missing_inputs) {
1067+ result_0.pushKV("reject-reason", "Missing inputs");
missing-inputs?
Looks like tests are failing because getnewaddress has changed to default to p2sh-segwit, changing virtual tx size, and some of the fail cases dealing with large txs are no longer triggered. I was able to get everything passing again with trivial changes but you may want to tweak those numbers to line up with expected boundaries.
Tests also pass again if specifically using “legacy” addresses like:
0node.getnewaddress("", "legacy")
Perhaps the test can be updated to handle both cases?
936@@ -934,6 +937,11 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
937 }
938 }
939
940+ if (test_accept) {
941+ *test_accept = true;
942+ return false;
1089+ LOCK(cs_main);
1090+ bool res = AcceptToMemoryPool(mempool, state, std::move(tx), &missing_inputs,
1091+ nullptr /* plTxnReplaced */, false /* bypass_limits */, max_raw_tx_fee, &test_accept_res);
1092+ assert(!res);
1093+ }
1094+ result_0.pushKV("allowed", test_accept_res);
pushKV actually doesn’t work with bool… :/
Guess we need push_back(Pair()) for this.
1023@@ -1024,6 +1024,88 @@ UniValue sendrawtransaction(const JSONRPCRequest& request)
1024 return hashTx.GetHex();
1025 }
1026
1027+UniValue testmempoolaccept(const JSONRPCRequest& request)
1028+{
1029+ if (request.fHelp || request.params.size() < 1 || request.params.size() > 2) {
1030+ throw std::runtime_error(
1031+ // clang-format off
1032+ "testmempoolaccept [\"rawtxs\"] ( allowhighfees )\n"
Needs release note.
utACK b55555d.
1143+ "\nReturns if raw transaction (serialized, hex-encoded) would be accepted by mempool.\n"
1144+ "\nThis checks if the transaction violates the consensus or policy rules.\n"
1145+ "\nSee sendrawtransaction call.\n"
1146+ "\nArguments:\n"
1147+ "1. [\"rawtxs\"] (array, required) An array of hex strings of raw transactions.\n"
1148+ " Length must be one for now.\n"
1196+ bool missing_inputs;
1197+ bool test_accept_res;
1198+ {
1199+ LOCK(cs_main);
1200+ test_accept_res = AcceptToMemoryPool(mempool, state, std::move(tx), &missing_inputs,
1201+ nullptr /* plTxnReplaced */, false /* bypass_limits */, max_raw_tx_fee, /* test_accpet */ true);
test_accpet
AcceptToMemoryPool
AcceptToMemoryPoolWithTime
AcceptToMemoryPoolWorker
Have the following parameter in common
test_accept, nAbsurdFee, bypass_limits, nAcceptTime
It would be fine to extract this into a structure like CAcceptToMemoryPoolOptions rather than adding yet one more parameter.
Anyway, not blocking, CodeReview ACK, will test soon.
Post-merge utACK https://github.com/bitcoin/bitcoin/pull/11742/commits/b55555da3e25a47f1e7fced7f09d4f0bf8198624
Change looks useful and well implemented. The tests are great! In some sense it’s sad that the functional tests you’ve introduced for this particular RPC call are the only place where we’re testing basic mempool rejection logic (e.g. scriptsig-not-pushonly, multi-op-return), but it’s great that we’ve got explicit tests for those cases now.
1171+
1172+ ObserveSafeMode();
1173+
1174+ RPCTypeCheck(request.params, {UniValue::VARR, UniValue::VBOOL});
1175+ if (request.params[0].get_array().size() != 1) {
1176+ throw JSONRPCError(RPC_INVALID_PARAMETER, "Array must contain exactly one raw transaction for now");
testmempoolaccept accept multiple transactions at some point?
