diff --git a/src/bench/block_assemble.cpp b/src/bench/block_assemble.cpp --- a/src/bench/block_assemble.cpp +++ b/src/bench/block_assemble.cpp @@ -44,7 +44,6 @@ for (const auto &txr : txs) { CValidationState vstate; bool ret{::AcceptToMemoryPool(config, ::g_mempool, vstate, txr, - nullptr /* pfMissingInputs */, false /* bypass_limits */, /* nAbsurdFee */ Amount::zero())}; assert(ret); diff --git a/src/consensus/validation.h b/src/consensus/validation.h --- a/src/consensus/validation.h +++ b/src/consensus/validation.h @@ -55,7 +55,7 @@ // Only loose txn: //! didn't meet our local policy rules TX_NOT_STANDARD, - //! a transaction was missing some of its inputs + //! transaction was missing some of its inputs TX_MISSING_INPUTS, //! transaction spends a coinbase too early, or violates locktime/sequence //! locks @@ -64,8 +64,6 @@ * Tx already in mempool or conflicts with a tx in the chain * TODO: Currently this is only used if the transaction already exists in * the mempool or on chain, - * TODO: ATMP's fMissingInputs and a valid CValidationState being used to - * indicate missing inputs */ TX_CONFLICT, //! violated mempool's fee/size/descendant/etc limits diff --git a/src/net_processing.cpp b/src/net_processing.cpp --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2165,7 +2165,6 @@ const CTransactionRef porphanTx = orphan_it->second.tx; const CTransaction &orphanTx = *porphanTx; NodeId fromPeer = orphan_it->second.fromPeer; - bool fMissingInputs2 = false; // Use a new CValidationState because orphans come from different peers // (and we call MaybePunishNode based on the source peer from the orphan // map, not based on the peer that relayed the previous transaction). @@ -2178,7 +2177,7 @@ } if (AcceptToMemoryPool(config, g_mempool, orphan_state, porphanTx, - &fMissingInputs2, false /* bypass_limits */, + false /* bypass_limits */, Amount::zero() /* nAbsurdFee */)) { LogPrint(BCLog::MEMPOOL, " accepted orphan tx %s\n", orphanTxId.ToString()); @@ -2194,7 +2193,8 @@ } EraseOrphanTx(orphanTxId); done = true; - } else if (!fMissingInputs2) { + } else if (orphan_state.GetReason() != + ValidationInvalidReason::TX_MISSING_INPUTS) { if (orphan_state.IsInvalid()) { // Punish peer that gave us an invalid orphan tx MaybePunishNode(fromPeer, orphan_state, @@ -2965,7 +2965,6 @@ LOCK2(cs_main, g_cs_orphans); - bool fMissingInputs = false; CValidationState state; CNodeState *nodestate = State(pfrom->GetId()); @@ -2974,7 +2973,7 @@ EraseTxRequest(txid); if (!AlreadyHave(inv) && - AcceptToMemoryPool(config, g_mempool, state, ptx, &fMissingInputs, + AcceptToMemoryPool(config, g_mempool, state, ptx, false /* bypass_limits */, Amount::zero() /* nAbsurdFee */)) { g_mempool.check(pcoinsTip.get()); @@ -3000,7 +2999,8 @@ // Recursively process any orphan transactions that depended on this // one ProcessOrphanTx(config, connman, pfrom->orphan_work_set); - } else if (fMissingInputs) { + } else if (state.GetReason() == + ValidationInvalidReason::TX_MISSING_INPUTS) { // It may be the case that the orphans parents have all been // rejected. bool fRejectedParents = false; diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -46,20 +46,16 @@ if (!g_mempool.exists(txid)) { // Transaction is not already in the mempool. Submit it. CValidationState state; - bool fMissingInputs; if (!AcceptToMemoryPool(config, g_mempool, state, std::move(tx), - &fMissingInputs, false /* bypass_limits */, - max_tx_fee)) { + false /* bypass_limits */, max_tx_fee)) { + err_string = FormatStateMessage(state); if (state.IsInvalid()) { - err_string = FormatStateMessage(state); + if (state.GetReason() == + ValidationInvalidReason::TX_MISSING_INPUTS) { + return TransactionError::MISSING_INPUTS; + } return TransactionError::MEMPOOL_REJECTED; } - - if (fMissingInputs) { - return TransactionError::MISSING_INPUTS; - } - - err_string = FormatStateMessage(state); return TransactionError::MEMPOOL_ERROR; } diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1037,22 +1037,24 @@ result_0.pushKV("txid", txid.GetHex()); CValidationState state; - bool missing_inputs; bool test_accept_res; { LOCK(cs_main); test_accept_res = AcceptToMemoryPool( - config, g_mempool, state, std::move(tx), &missing_inputs, - false /* bypass_limits */, max_raw_tx_fee, true /* test_accept */); + config, g_mempool, state, std::move(tx), false /* bypass_limits */, + max_raw_tx_fee, true /* test_accept */); } result_0.pushKV("allowed", test_accept_res); if (!test_accept_res) { if (state.IsInvalid()) { - result_0.pushKV("reject-reason", - strprintf("%i: %s", state.GetRejectCode(), - state.GetRejectReason())); - } else if (missing_inputs) { - result_0.pushKV("reject-reason", "missing-inputs"); + if (state.GetReason() == + ValidationInvalidReason::TX_MISSING_INPUTS) { + result_0.pushKV("reject-reason", "missing-inputs"); + } else { + result_0.pushKV("reject-reason", + strprintf("%i: %s", state.GetRejectCode(), + state.GetRejectReason())); + } } else { result_0.pushKV("reject-reason", state.GetRejectReason()); } diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp --- a/src/test/txvalidation_tests.cpp +++ b/src/test/txvalidation_tests.cpp @@ -41,7 +41,6 @@ BOOST_CHECK_EQUAL(false, AcceptToMemoryPool(GetConfig(), *m_node.mempool, state, MakeTransactionRef(coinbaseTx), - nullptr /* pfMissingInputs */, true /* bypass_limits */, Amount::zero() /* nAbsurdFee */)); diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -35,8 +35,7 @@ CValidationState state; return AcceptToMemoryPool( GetConfig(), *m_node.mempool, state, MakeTransactionRef(tx), - nullptr /* pfMissingInputs */, true /* bypass_limits */, - Amount::zero() /* nAbsurdFee */); + true /* bypass_limits */, Amount::zero() /* nAbsurdFee */); }; // Create a double-spend of mature coinbase txn: diff --git a/src/txmempool.cpp b/src/txmempool.cpp --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1394,7 +1394,6 @@ CValidationState stateDummy; if (!fAddToMempool || tx->IsCoinBase() || !AcceptToMemoryPool(config, g_mempool, stateDummy, tx, - nullptr /* pfMissingInputs */, true /* bypass_limits */, Amount::zero() /* nAbsurdFee */)) { // If the transaction doesn't make it in to the mempool, remove any diff --git a/src/validation.h b/src/validation.h --- a/src/validation.h +++ b/src/validation.h @@ -411,8 +411,8 @@ */ bool AcceptToMemoryPool(const Config &config, CTxMemPool &pool, CValidationState &state, const CTransactionRef &tx, - bool *pfMissingInputs, bool bypass_limits, - const Amount nAbsurdFee, bool test_accept = false) + bool bypass_limits, const Amount nAbsurdFee, + bool test_accept = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** diff --git a/src/validation.cpp b/src/validation.cpp --- a/src/validation.cpp +++ b/src/validation.cpp @@ -339,13 +339,11 @@ * to optionally remove the cache additions if the associated transaction ends * up being rejected by the mempool. */ -static bool -AcceptToMemoryPoolWorker(const Config &config, CTxMemPool &pool, - CValidationState &state, const CTransactionRef &ptx, - bool *pfMissingInputs, int64_t nAcceptTime, - bool bypass_limits, const Amount nAbsurdFee, - std::vector &coins_to_uncache, - bool test_accept) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { +static bool AcceptToMemoryPoolWorker( + const Config &config, CTxMemPool &pool, CValidationState &state, + const CTransactionRef &ptx, int64_t nAcceptTime, bool bypass_limits, + const Amount nAbsurdFee, std::vector &coins_to_uncache, + bool test_accept) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); const Consensus::Params &consensusParams = @@ -357,9 +355,6 @@ // mempool "read lock" (held through // GetMainSignals().TransactionAddedToMempool()) LOCK(pool.cs); - if (pfMissingInputs) { - *pfMissingInputs = false; - } // Coinbase is only valid in a block, not as a loose transaction. if (!CheckRegularTransaction(tx, state)) { @@ -435,13 +430,9 @@ // Otherwise assume this might be an orphan tx for which we just // haven't seen parents yet. - if (pfMissingInputs) { - *pfMissingInputs = true; - } - - // fMissingInputs and !state.IsInvalid() is used to detect this - // condition, don't set state.Invalid() - return false; + return state.Invalid(ValidationInvalidReason::TX_MISSING_INPUTS, + REJECT_INVALID, + "bad-txns-inputs-missingorspent"); } } @@ -636,14 +627,14 @@ static bool AcceptToMemoryPoolWithTime(const Config &config, CTxMemPool &pool, CValidationState &state, const CTransactionRef &tx, - bool *pfMissingInputs, int64_t nAcceptTime, - bool bypass_limits, const Amount nAbsurdFee, - bool test_accept) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { + int64_t nAcceptTime, bool bypass_limits, + const Amount nAbsurdFee, bool test_accept) + EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); std::vector coins_to_uncache; - bool res = AcceptToMemoryPoolWorker( - config, pool, state, tx, pfMissingInputs, nAcceptTime, bypass_limits, - nAbsurdFee, coins_to_uncache, test_accept); + bool res = AcceptToMemoryPoolWorker(config, pool, state, tx, nAcceptTime, + bypass_limits, nAbsurdFee, + coins_to_uncache, test_accept); if (!res) { // Remove coins that were not present in the coins cache before calling // ATMPW; this is to prevent memory DoS in case we receive a large @@ -666,11 +657,10 @@ bool AcceptToMemoryPool(const Config &config, CTxMemPool &pool, CValidationState &state, const CTransactionRef &tx, - bool *pfMissingInputs, bool bypass_limits, - const Amount nAbsurdFee, bool test_accept) { - return AcceptToMemoryPoolWithTime(config, pool, state, tx, pfMissingInputs, - GetTime(), bypass_limits, nAbsurdFee, - test_accept); + bool bypass_limits, const Amount nAbsurdFee, + bool test_accept) { + return AcceptToMemoryPoolWithTime(config, pool, state, tx, GetTime(), + bypass_limits, nAbsurdFee, test_accept); } /** @@ -5520,8 +5510,7 @@ if (nTime + nExpiryTimeout > nNow) { LOCK(cs_main); AcceptToMemoryPoolWithTime( - config, pool, state, tx, nullptr /* pfMissingInputs */, - nTime, false /* bypass_limits */, + config, pool, state, tx, nTime, false /* bypass_limits */, Amount::zero() /* nAbsurdFee */, false /* test_accept */); if (state.IsValid()) { ++count; diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py --- a/test/functional/rpc_rawtransaction.py +++ b/test/functional/rpc_rawtransaction.py @@ -210,8 +210,10 @@ rawtx = self.nodes[2].signrawtransactionwithwallet(rawtx) # This will raise an exception since there are missing inputs - assert_raises_rpc_error( - -25, "Missing inputs", self.nodes[2].sendrawtransaction, rawtx['hex']) + assert_raises_rpc_error(-25, + "bad-txns-inputs-missingorspent", + self.nodes[2].sendrawtransaction, + rawtx['hex']) ##################################### # getrawtransaction with block hash #