Skip to content

Commit 3004d5a

Browse files
jnewberyjnewbery
committed
[validation] Remove fMissingInputs from AcceptToMemoryPool()
Handle this failure in the same way as all other failures: call Invalid() with the reasons for the failure.
1 parent c428622 commit 3004d5a

11 files changed

+27
-46
lines changed

‎src/bench/block_assemble.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ static void AssembleBlock(benchmark::State& state)
3939

4040
for (const auto& txr : txs) {
4141
TxValidationState state;
42-
bool ret{::AcceptToMemoryPool(::mempool, state, txr, nullptr /* pfMissingInputs */, nullptr /* plTxnReplaced */, false /* bypass_limits */, /* nAbsurdFee */ 0)};
42+
bool ret{::AcceptToMemoryPool(::mempool, state, txr, nullptr /* plTxnReplaced */, false /* bypass_limits */, /* nAbsurdFee */ 0)};
4343
assert(ret);
4444
}
4545
}

‎src/consensus/validation.h

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,7 @@ enum class TxValidationResult {
2727
*/
2828
TX_RECENT_CONSENSUS_CHANGE,
2929
TX_NOT_STANDARD, //!< didn't meet our local policy rules
30-
/**
31-
* transaction was missing some of its inputs
32-
* TODO: ATMP uses fMissingInputs and a valid ValidationState to indicate missing inputs.
33-
* Change ATMP to use TX_MISSING_INPUTS.
34-
*/
35-
TX_MISSING_INPUTS,
30+
TX_MISSING_INPUTS, //!< transaction was missing some of its inputs
3631
TX_PREMATURE_SPEND, //!< transaction spends a coinbase too early, or violates locktime/sequence locks
3732
/**
3833
* Transaction might be missing a witness, have a witness prior to SegWit

‎src/net_processing.cpp

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1836,14 +1836,13 @@ void static ProcessOrphanTx(CConnman* connman, std::set<uint256>& orphan_work_se
18361836
const CTransactionRef porphanTx = orphan_it->second.tx;
18371837
const CTransaction& orphanTx = *porphanTx;
18381838
NodeId fromPeer = orphan_it->second.fromPeer;
1839-
bool fMissingInputs2 = false;
18401839
// Use a new TxValidationState because orphans come from different peers (and we call
18411840
// MaybePunishNodeForTx based on the source peer from the orphan map, not based on the peer
18421841
// that relayed the previous transaction).
18431842
TxValidationState orphan_state;
18441843

18451844
if (setMisbehaving.count(fromPeer)) continue;
1846-
if (AcceptToMemoryPool(mempool, orphan_state, porphanTx, &fMissingInputs2, &removed_txn, false /* bypass_limits */, 0 /* nAbsurdFee */)) {
1845+
if (AcceptToMemoryPool(mempool, orphan_state, porphanTx, &removed_txn, false /* bypass_limits */, 0 /* nAbsurdFee */)) {
18471846
LogPrint(BCLog::MEMPOOL, " accepted orphan tx %s\n", orphanHash.ToString());
18481847
RelayTransaction(orphanHash, *connman);
18491848
for (unsigned int i = 0; i < orphanTx.vout.size(); i++) {
@@ -1856,7 +1855,7 @@ void static ProcessOrphanTx(CConnman* connman, std::set<uint256>& orphan_work_se
18561855
}
18571856
EraseOrphanTx(orphanHash);
18581857
done = true;
1859-
} else if (!fMissingInputs2) {
1858+
} else if (orphan_state.GetResult() != TxValidationResult::TX_MISSING_INPUTS) {
18601859
if (orphan_state.IsInvalid()) {
18611860
// Punish peer that gave us an invalid orphan tx
18621861
if (MaybePunishNodeForTx(fromPeer, orphan_state)) {
@@ -2492,7 +2491,6 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
24922491

24932492
LOCK2(cs_main, g_cs_orphans);
24942493

2495-
bool fMissingInputs = false;
24962494
TxValidationState state;
24972495

24982496
CNodeState* nodestate = State(pfrom->GetId());
@@ -2503,7 +2501,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
25032501
std::list<CTransactionRef> lRemovedTxn;
25042502

25052503
if (!AlreadyHave(inv) &&
2506-
AcceptToMemoryPool(mempool, state, ptx, &fMissingInputs, &lRemovedTxn, false /* bypass_limits */, 0 /* nAbsurdFee */)) {
2504+
AcceptToMemoryPool(mempool, state, ptx, &lRemovedTxn, false /* bypass_limits */, 0 /* nAbsurdFee */)) {
25072505
mempool.check(&::ChainstateActive().CoinsTip());
25082506
RelayTransaction(tx.GetHash(), *connman);
25092507
for (unsigned int i = 0; i < tx.vout.size(); i++) {
@@ -2525,7 +2523,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
25252523
// Recursively process any orphan transactions that depended on this one
25262524
ProcessOrphanTx(connman, pfrom->orphan_work_set, lRemovedTxn);
25272525
}
2528-
else if (fMissingInputs)
2526+
else if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS)
25292527
{
25302528
bool fRejectedParents = false; // It may be the case that the orphans parents have all been rejected
25312529
for (const CTxIn& txin : tx.vin) {

‎src/node/transaction.cpp

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,17 +37,15 @@ TransactionError BroadcastTransaction(const CTransactionRef tx, std::string& err
3737
if (!mempool.exists(hashTx)) {
3838
// Transaction is not already in the mempool. Submit it.
3939
TxValidationState state;
40-
bool fMissingInputs;
41-
if (!AcceptToMemoryPool(mempool, state, std::move(tx), &fMissingInputs,
40+
if (!AcceptToMemoryPool(mempool, state, std::move(tx),
4241
nullptr /* plTxnReplaced */, false /* bypass_limits */, max_tx_fee)) {
42+
err_string = FormatStateMessage(state);
4343
if (state.IsInvalid()) {
44-
err_string = FormatStateMessage(state);
45-
return TransactionError::MEMPOOL_REJECTED;
46-
} else {
47-
if (fMissingInputs) {
44+
if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) {
4845
return TransactionError::MISSING_INPUTS;
4946
}
50-
err_string = FormatStateMessage(state);
47+
return TransactionError::MEMPOOL_REJECTED;
48+
} else {
5149
return TransactionError::MEMPOOL_ERROR;
5250
}
5351
}

‎src/rpc/rawtransaction.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -894,19 +894,20 @@ static UniValue testmempoolaccept(const JSONRPCRequest& request)
894894
result_0.pushKV("txid", tx_hash.GetHex());
895895

896896
TxValidationState state;
897-
bool missing_inputs;
898897
bool test_accept_res;
899898
{
900899
LOCK(cs_main);
901-
test_accept_res = AcceptToMemoryPool(mempool, state, std::move(tx), &missing_inputs,
900+
test_accept_res = AcceptToMemoryPool(mempool, state, std::move(tx),
902901
nullptr /* plTxnReplaced */, false /* bypass_limits */, max_raw_tx_fee, /* test_accept */ true);
903902
}
904903
result_0.pushKV("allowed", test_accept_res);
905904
if (!test_accept_res) {
906905
if (state.IsInvalid()) {
907-
result_0.pushKV("reject-reason", strprintf("%s", state.GetRejectReason()));
908-
} else if (missing_inputs) {
909-
result_0.pushKV("reject-reason", "missing-inputs");
906+
if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) {
907+
result_0.pushKV("reject-reason", "missing-inputs");
908+
} else {
909+
result_0.pushKV("reject-reason", strprintf("%s", state.GetRejectReason()));
910+
}
910911
} else {
911912
result_0.pushKV("reject-reason", state.GetRejectReason());
912913
}

‎src/test/txvalidation_tests.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_reject_coinbase, TestChain100Setup)
3939
BOOST_CHECK_EQUAL(
4040
false,
4141
AcceptToMemoryPool(mempool, state, MakeTransactionRef(coinbaseTx),
42-
nullptr /* pfMissingInputs */,
4342
nullptr /* plTxnReplaced */,
4443
true /* bypass_limits */,
4544
0 /* nAbsurdFee */));

‎src/test/txvalidationcache_tests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ ToMemPool(const CMutableTransaction& tx)
2323
LOCK(cs_main);
2424

2525
TxValidationState state;
26-
return AcceptToMemoryPool(mempool, state, MakeTransactionRef(tx), nullptr /* pfMissingInputs */,
26+
return AcceptToMemoryPool(mempool, state, MakeTransactionRef(tx),
2727
nullptr /* plTxnReplaced */, true /* bypass_limits */, 0 /* nAbsurdFee */);
2828
}
2929

‎src/test/validation_block_tests.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,6 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg)
285285
::mempool,
286286
state,
287287
tx,
288-
/* pfMissingInputs */ &ignored,
289288
&plTxnReplaced,
290289
/* bypass_limits */ false,
291290
/* nAbsurdFee */ 0));

‎src/validation.cpp

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,7 @@ static void UpdateMempoolForReorg(DisconnectedBlockTransactions& disconnectpool,
365365
// ignore validation errors in resurrected transactions
366366
TxValidationState stateDummy;
367367
if (!fAddToMempool || (*it)->IsCoinBase() ||
368-
!AcceptToMemoryPool(mempool, stateDummy, *it, nullptr /* pfMissingInputs */,
368+
!AcceptToMemoryPool(mempool, stateDummy, *it,
369369
nullptr /* plTxnReplaced */, true /* bypass_limits */, 0 /* nAbsurdFee */)) {
370370
// If the transaction doesn't make it in to the mempool, remove any
371371
// transactions that depend on it (which would now be orphans).
@@ -442,7 +442,6 @@ class MemPoolAccept
442442
struct ATMPArgs {
443443
const CChainParams& m_chainparams;
444444
TxValidationState &m_state;
445-
bool* m_missing_inputs;
446445
const int64_t m_accept_time;
447446
std::list<CTransactionRef>* m_replaced_transactions;
448447
const bool m_bypass_limits;
@@ -538,7 +537,6 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
538537

539538
// Copy/alias what we need out of args
540539
TxValidationState &state = args.m_state;
541-
bool* pfMissingInputs = args.m_missing_inputs;
542540
const int64_t nAcceptTime = args.m_accept_time;
543541
const bool bypass_limits = args.m_bypass_limits;
544542
const CAmount& nAbsurdFee = args.m_absurd_fee;
@@ -554,10 +552,6 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
554552
CAmount& nConflictingFees = ws.m_conflicting_fees;
555553
size_t& nConflictingSize = ws.m_conflicting_size;
556554

557-
if (pfMissingInputs) {
558-
*pfMissingInputs = false;
559-
}
560-
561555
if (!CheckTransaction(tx, state))
562556
return false; // state filled in by CheckTransaction
563557

@@ -647,10 +641,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
647641
}
648642
}
649643
// Otherwise assume this might be an orphan tx for which we just haven't seen parents yet
650-
if (pfMissingInputs) {
651-
*pfMissingInputs = true;
652-
}
653-
return false; // fMissingInputs and !state.IsInvalid() is used to detect this condition, don't set state.Invalid()
644+
return state.Invalid(TxValidationResult::TX_MISSING_INPUTS, "bad-txns-inputs-missingorspent");
654645
}
655646
}
656647

@@ -1047,11 +1038,11 @@ bool MemPoolAccept::AcceptSingleTransaction(const CTransactionRef& ptx, ATMPArgs
10471038

10481039
/** (try to) add transaction to memory pool with a specified acceptance time **/
10491040
static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPool& pool, TxValidationState &state, const CTransactionRef &tx,
1050-
bool* pfMissingInputs, int64_t nAcceptTime, std::list<CTransactionRef>* plTxnReplaced,
1041+
int64_t nAcceptTime, std::list<CTransactionRef>* plTxnReplaced,
10511042
bool bypass_limits, const CAmount nAbsurdFee, bool test_accept) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
10521043
{
10531044
std::vector<COutPoint> coins_to_uncache;
1054-
MemPoolAccept::ATMPArgs args { chainparams, state, pfMissingInputs, nAcceptTime, plTxnReplaced, bypass_limits, nAbsurdFee, coins_to_uncache, test_accept };
1045+
MemPoolAccept::ATMPArgs args { chainparams, state, nAcceptTime, plTxnReplaced, bypass_limits, nAbsurdFee, coins_to_uncache, test_accept };
10551046
bool res = MemPoolAccept(pool).AcceptSingleTransaction(tx, args);
10561047
if (!res) {
10571048
// Remove coins that were not present in the coins cache before calling ATMPW;
@@ -1069,11 +1060,11 @@ static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPo
10691060
}
10701061

10711062
bool AcceptToMemoryPool(CTxMemPool& pool, TxValidationState &state, const CTransactionRef &tx,
1072-
bool* pfMissingInputs, std::list<CTransactionRef>* plTxnReplaced,
1063+
std::list<CTransactionRef>* plTxnReplaced,
10731064
bool bypass_limits, const CAmount nAbsurdFee, bool test_accept)
10741065
{
10751066
const CChainParams& chainparams = Params();
1076-
return AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, pfMissingInputs, GetTime(), plTxnReplaced, bypass_limits, nAbsurdFee, test_accept);
1067+
return AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, GetTime(), plTxnReplaced, bypass_limits, nAbsurdFee, test_accept);
10771068
}
10781069

10791070
/**
@@ -4969,7 +4960,7 @@ bool LoadMempool(CTxMemPool& pool)
49694960
TxValidationState state;
49704961
if (nTime + nExpiryTimeout > nNow) {
49714962
LOCK(cs_main);
4972-
AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, nullptr /* pfMissingInputs */, nTime,
4963+
AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, nTime,
49734964
nullptr /* plTxnReplaced */, false /* bypass_limits */, 0 /* nAbsurdFee */,
49744965
false /* test_accept */);
49754966
if (state.IsValid()) {

‎src/validation.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ void PruneBlockFilesManual(int nManualPruneHeight);
273273
/** (try to) add transaction to memory pool
274274
* plTxnReplaced will be appended to with all transactions replaced from mempool **/
275275
bool AcceptToMemoryPool(CTxMemPool& pool, TxValidationState &state, const CTransactionRef &tx,
276-
bool* pfMissingInputs, std::list<CTransactionRef>* plTxnReplaced,
276+
std::list<CTransactionRef>* plTxnReplaced,
277277
bool bypass_limits, const CAmount nAbsurdFee, bool test_accept=false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
278278

279279
/** Get the BIP9 state for a given deployment at the current tip. */

0 commit comments

Comments
 (0)