Skip to content

Commit

Permalink
[validation] Remove absurdfee from accepttomempool
Browse files Browse the repository at this point in the history
Mempool behavior should not be user-specific.
Checking that txfee is acceptable should be
the responsibility of the wallet or client, not
the mempool.
  • Loading branch information
jnewbery authored and glozow committed Oct 5, 2020
1 parent 932564b commit b048b27
Show file tree
Hide file tree
Showing 9 changed files with 16 additions and 23 deletions.
2 changes: 1 addition & 1 deletion src/bench/block_assemble.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ static void AssembleBlock(benchmark::Bench& bench)

for (const auto& txr : txs) {
TxValidationState state;
bool ret{::AcceptToMemoryPool(*test_setup.m_node.mempool, state, txr, nullptr /* plTxnReplaced */, false /* bypass_limits */, /* nAbsurdFee */ 0)};
bool ret{::AcceptToMemoryPool(*test_setup.m_node.mempool, state, txr, nullptr /* plTxnReplaced */, false /* bypass_limits */)};
assert(ret);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/net_processing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2058,7 +2058,7 @@ void PeerManager::ProcessOrphanTx(std::set<uint256>& orphan_work_set)
TxValidationState state;
std::list<CTransactionRef> removed_txn;

if (AcceptToMemoryPool(m_mempool, state, porphanTx, &removed_txn, false /* bypass_limits */, 0 /* nAbsurdFee */)) {
if (AcceptToMemoryPool(m_mempool, state, porphanTx, &removed_txn, false /* bypass_limits */)) {
LogPrint(BCLog::MEMPOOL, " accepted orphan tx %s\n", orphanHash.ToString());
RelayTransaction(orphanHash, porphanTx->GetWitnessHash(), m_connman);
for (unsigned int i = 0; i < porphanTx->vout.size(); i++) {
Expand Down Expand Up @@ -3014,7 +3014,7 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat
// (older than our recency filter) if trying to DoS us, without any need
// for witness malleation.
if (!AlreadyHaveTx(GenTxid(/* is_wtxid=*/true, wtxid), m_mempool) &&
AcceptToMemoryPool(m_mempool, state, ptx, &lRemovedTxn, false /* bypass_limits */, 0 /* nAbsurdFee */)) {
AcceptToMemoryPool(m_mempool, state, ptx, &lRemovedTxn, false /* bypass_limits */)) {
m_mempool.check(&::ChainstateActive().CoinsTip());
RelayTransaction(tx.GetHash(), tx.GetWitnessHash(), m_connman);
for (unsigned int i = 0; i < tx.vout.size(); i++) {
Expand Down
4 changes: 2 additions & 2 deletions src/node/transaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,15 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
// First, call ATMP with test_accept and check the fee. If ATMP
// fails here, return error immediately.
if (!AcceptToMemoryPool(*node.mempool, state, tx,
nullptr /* plTxnReplaced */, false /* bypass_limits */, /* absurdfee*/ 0, /* test_accept */ true, &fee)) {
nullptr /* plTxnReplaced */, false /* bypass_limits */, /* test_accept */ true, &fee)) {
return HandleATMPError(state, err_string);
} else if (fee > max_tx_fee) {
return TransactionError::MAX_FEE_EXCEEDED;
}
}
// Try to submit the transaction to the mempool.
if (!AcceptToMemoryPool(*node.mempool, state, tx,
nullptr /* plTxnReplaced */, false /* bypass_limits */, max_tx_fee)) {
nullptr /* plTxnReplaced */, false /* bypass_limits */)) {
return HandleATMPError(state, err_string);
}

Expand Down
2 changes: 1 addition & 1 deletion src/rpc/rawtransaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -951,7 +951,7 @@ static RPCHelpMan testmempoolaccept()
{
LOCK(cs_main);
test_accept_res = AcceptToMemoryPool(mempool, state, std::move(tx),
nullptr /* plTxnReplaced */, false /* bypass_limits */, max_raw_tx_fee, /* test_accept */ true, &fee);
nullptr /* plTxnReplaced */, false /* bypass_limits */, /* test_accept */ true, &fee);
}

// Check that fee does not exceed maximum fee
Expand Down
3 changes: 1 addition & 2 deletions src/test/txvalidation_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_reject_coinbase, TestChain100Setup)
false,
AcceptToMemoryPool(*m_node.mempool, state, MakeTransactionRef(coinbaseTx),
nullptr /* plTxnReplaced */,
true /* bypass_limits */,
0 /* nAbsurdFee */));
true /* bypass_limits */));

// Check that the transaction hasn't been added to mempool.
BOOST_CHECK_EQUAL(m_node.mempool->size(), initialPoolSize);
Expand Down
2 changes: 1 addition & 1 deletion src/test/txvalidationcache_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, TestChain100Setup)

TxValidationState state;
return AcceptToMemoryPool(*m_node.mempool, state, MakeTransactionRef(tx),
nullptr /* plTxnReplaced */, true /* bypass_limits */, 0 /* nAbsurdFee */);
nullptr /* plTxnReplaced */, true /* bypass_limits */);
};

// Create a double-spend of mature coinbase txn:
Expand Down
3 changes: 1 addition & 2 deletions src/test/validation_block_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg)
state,
tx,
&plTxnReplaced,
/* bypass_limits */ false,
/* nAbsurdFee */ 0));
/* bypass_limits */ false));
}
}

Expand Down
17 changes: 6 additions & 11 deletions src/validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ static void UpdateMempoolForReorg(CTxMemPool& mempool, DisconnectedBlockTransact
TxValidationState stateDummy;
if (!fAddToMempool || (*it)->IsCoinBase() ||
!AcceptToMemoryPool(mempool, stateDummy, *it,
nullptr /* plTxnReplaced */, true /* bypass_limits */, 0 /* nAbsurdFee */)) {
nullptr /* plTxnReplaced */, true /* bypass_limits */)) {
// If the transaction doesn't make it in to the mempool, remove any
// transactions that depend on it (which would now be orphans).
mempool.removeRecursive(**it, MemPoolRemovalReason::REORG);
Expand Down Expand Up @@ -463,7 +463,6 @@ class MemPoolAccept
const int64_t m_accept_time;
std::list<CTransactionRef>* m_replaced_transactions;
const bool m_bypass_limits;
const CAmount& m_absurd_fee;
/*
* Return any outpoints which were not previously present in the coins
* cache, but were added as a result of validating the tx for mempool
Expand Down Expand Up @@ -558,7 +557,6 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
TxValidationState &state = args.m_state;
const int64_t nAcceptTime = args.m_accept_time;
const bool bypass_limits = args.m_bypass_limits;
const CAmount& nAbsurdFee = args.m_absurd_fee;
std::vector<COutPoint>& coins_to_uncache = args.m_coins_to_uncache;

// Alias what we need out of ws
Expand Down Expand Up @@ -729,9 +727,6 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
// blocks
if (!bypass_limits && !CheckFeeRate(nSize, nModifiedFees, state)) return false;

if (nAbsurdFee && nFees > nAbsurdFee)
LogPrintf("Ignoring Absurdfee\n");

const CTxMemPool::setEntries setIterConflicting = m_pool.GetIterSet(setConflicts);
// Calculate in-mempool ancestors, up to a limit.
if (setConflicts.size() == 1) {
Expand Down Expand Up @@ -1064,10 +1059,10 @@ bool MemPoolAccept::AcceptSingleTransaction(const CTransactionRef& ptx, ATMPArgs
/** (try to) add transaction to memory pool with a specified acceptance time **/
static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPool& pool, TxValidationState &state, const CTransactionRef &tx,
int64_t nAcceptTime, std::list<CTransactionRef>* plTxnReplaced,
bool bypass_limits, const CAmount nAbsurdFee, bool test_accept, CAmount* fee_out=nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
bool bypass_limits, bool test_accept, CAmount* fee_out=nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
std::vector<COutPoint> coins_to_uncache;
MemPoolAccept::ATMPArgs args { chainparams, state, nAcceptTime, plTxnReplaced, bypass_limits, nAbsurdFee, coins_to_uncache, test_accept, fee_out };
MemPoolAccept::ATMPArgs args { chainparams, state, nAcceptTime, plTxnReplaced, bypass_limits, coins_to_uncache, test_accept, fee_out };
bool res = MemPoolAccept(pool).AcceptSingleTransaction(tx, args);
if (!res) {
// Remove coins that were not present in the coins cache before calling ATMPW;
Expand All @@ -1086,10 +1081,10 @@ static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPo

bool AcceptToMemoryPool(CTxMemPool& pool, TxValidationState &state, const CTransactionRef &tx,
std::list<CTransactionRef>* plTxnReplaced,
bool bypass_limits, const CAmount nAbsurdFee, bool test_accept, CAmount* fee_out)
bool bypass_limits, bool test_accept, CAmount* fee_out)
{
const CChainParams& chainparams = Params();
return AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, GetTime(), plTxnReplaced, bypass_limits, nAbsurdFee, test_accept, fee_out);
return AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, GetTime(), plTxnReplaced, bypass_limits, test_accept, fee_out);
}

CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMemPool* const mempool, const uint256& hash, const Consensus::Params& consensusParams, uint256& hashBlock)
Expand Down Expand Up @@ -5078,7 +5073,7 @@ bool LoadMempool(CTxMemPool& pool)
if (nTime + nExpiryTimeout > nNow) {
LOCK(cs_main);
AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, nTime,
nullptr /* plTxnReplaced */, false /* bypass_limits */, 0 /* nAbsurdFee */,
nullptr /* plTxnReplaced */, false /* bypass_limits */,
false /* test_accept */);
if (state.IsValid()) {
++count;
Expand Down
2 changes: 1 addition & 1 deletion src/validation.h
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ void PruneBlockFilesManual(int nManualPruneHeight);
* @param[out] fee_out optional argument to return tx fee to the caller **/
bool AcceptToMemoryPool(CTxMemPool& pool, TxValidationState &state, const CTransactionRef &tx,
std::list<CTransactionRef>* plTxnReplaced,
bool bypass_limits, const CAmount nAbsurdFee, bool test_accept=false, CAmount* fee_out=nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
bool bypass_limits, bool test_accept=false, CAmount* fee_out=nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

/** Get the BIP9 state for a given deployment at the current tip. */
ThresholdState VersionBitsTipState(const Consensus::Params& params, Consensus::DeploymentPos pos);
Expand Down

0 comments on commit b048b27

Please sign in to comment.