boxtec
/
bitcoin
mirror of https://github.com/bitcoin/bitcoin
4
1
Fork 0
Code Issues Projects Releases Wiki Activity

[refactor] move new tx logic to txdownload

Browse Source 
Also delete external RecentRejectsReconsiderableFilter() access since it
is no longer necessary.
pull/30110/head
glozow 7 months ago
parent 257568eab5
commit 1e08195135
4 changed files with 75 additions and 56 deletions
Show Stats Download Patch File Download Diff File

61
src/net_processing.cpp
Unescape View File

@ -808,12 +808,6 @@ private:
/** Stalling timeout for blocks in IBD */ /** Stalling timeout for blocks in IBD */
std::atomic<std::chrono::seconds> m_block_stalling_timeout{BLOCK_STALLING_TIMEOUT_DEFAULT}; std::atomic<std::chrono::seconds> m_block_stalling_timeout{BLOCK_STALLING_TIMEOUT_DEFAULT};
CRollingBloomFilter& RecentRejectsReconsiderableFilter() EXCLUSIVE_LOCKS_REQUIRED(m_tx_download_mutex)
{
AssertLockHeld(m_tx_download_mutex);
return m_txdownloadman.RecentRejectsReconsiderableFilter();
}
/** /**
* For sending `inv`s to inbound peers, we use a single (exponentially * For sending `inv`s to inbound peers, we use a single (exponentially
* distributed) timer for all peers. If we used a separate timer for each * distributed) timer for all peers. If we used a separate timer for each
@ -4239,24 +4233,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
LOCK2(cs_main, m_tx_download_mutex); LOCK2(cs_main, m_tx_download_mutex);
auto& m_txrequest = m_txdownloadman.GetTxRequestRef(); const auto& [should_validate, package_to_validate] = m_txdownloadman.ReceivedTx(pfrom.GetId(), ptx);
if (!should_validate) {
m_txrequest.ReceivedResponse(pfrom.GetId(), txid);
if (tx.HasWitness()) m_txrequest.ReceivedResponse(pfrom.GetId(), wtxid);
// We do the AlreadyHaveTx() check using wtxid, rather than txid - in the
// absence of witness malleation, this is strictly better, because the
// recent rejects filter may contain the wtxid but rarely contains
// the txid of a segwit transaction that has been rejected.
// In the presence of witness malleation, it's possible that by only
// doing the check with wtxid, we could overlook a transaction which
// was confirmed with a different witness, or exists in our mempool
// with a different witness, but this has limited downside:
// mempool validation does its own lookup of whether we have the txid
// already; and an adversary can already relay us old transactions
// (older than our recency filter) if trying to DoS us, without any need
// for witness malleation.
if (m_txdownloadman.AlreadyHaveTx(GenTxid::Wtxid(wtxid), /*include_reconsiderable=*/true)) {
if (pfrom.HasPermission(NetPermissionFlags::ForceRelay)) { if (pfrom.HasPermission(NetPermissionFlags::ForceRelay)) {
// Always relay transactions received from peers with forcerelay // Always relay transactions received from peers with forcerelay
// permission, even if they were already in the mempool, allowing // permission, even if they were already in the mempool, allowing
@ -4271,37 +4249,18 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
} }
} }
if (RecentRejectsReconsiderableFilter().contains(wtxid)) { if (package_to_validate) {
// When a transaction is already in m_lazy_recent_rejects_reconsiderable, we shouldn't submit const auto package_result{ProcessNewPackage(m_chainman.ActiveChainstate(), m_mempool, package_to_validate->m_txns, /*test_accept=*/false, /*client_maxfeerate=*/std::nullopt)};
// it by itself again. However, look for a matching child in the orphanage, as it is LogDebug(BCLog::TXPACKAGES, "package evaluation for %s: %s\n", package_to_validate->ToString(),
// possible that they succeed as a package. package_result.m_state.IsValid() ? "package accepted" : "package rejected");
LogDebug(BCLog::TXPACKAGES, "found tx %s (wtxid=%s) in reconsiderable rejects, looking for child in orphanage\n", ProcessPackageResult(package_to_validate.value(), package_result);
txid.ToString(), wtxid.ToString());
if (auto package_to_validate{m_txdownloadman.Find1P1CPackage(ptx, pfrom.GetId())}) {
const auto package_result{ProcessNewPackage(m_chainman.ActiveChainstate(), m_mempool, package_to_validate->m_txns, /*test_accept=*/false, /*client_maxfeerate=*/std::nullopt)};
LogDebug(BCLog::TXPACKAGES, "package evaluation for %s: %s\n", package_to_validate->ToString(),
package_result.m_state.IsValid() ? "package accepted" : "package rejected");
ProcessPackageResult(package_to_validate.value(), package_result);
}
} }
// If a tx is detected by m_lazy_recent_rejects it is ignored. Because we haven't
// submitted the tx to our mempool, we won't have computed a DoS
// score for it or determined exactly why we consider it invalid.
//
// This means we won't penalize any peer subsequently relaying a DoSy
// tx (even if we penalized the first peer who gave it to us) because
// we have to account for m_lazy_recent_rejects showing false positives. In
// other words, we shouldn't penalize a peer if we aren't *sure* they
// submitted a DoSy tx.
//
// Note that m_lazy_recent_rejects doesn't just record DoSy or invalid
// transactions, but any tx not accepted by the mempool, which may be
// due to node policy (vs. consensus). So we can't blanket penalize a
// peer simply for relaying a tx that our m_lazy_recent_rejects has caught,
// regardless of false positives.
return; return;
} }
// ReceivedTx should not be telling us to validate the tx and a package.
Assume(!package_to_validate.has_value());
const MempoolAcceptResult result = m_chainman.ProcessTransaction(ptx); const MempoolAcceptResult result = m_chainman.ProcessTransaction(ptx);
const TxValidationState& state = result.m_state; const TxValidationState& state = result.m_state;

6
src/node/txdownloadman.h
Unescape View File

@ -125,7 +125,6 @@ public:
// temporary and removed later once logic has been moved internally. // temporary and removed later once logic has been moved internally.
TxOrphanage& GetOrphanageRef(); TxOrphanage& GetOrphanageRef();
TxRequestTracker& GetTxRequestRef(); TxRequestTracker& GetTxRequestRef();
CRollingBloomFilter& RecentRejectsReconsiderableFilter();
// Responses to chain events. TxDownloadManager is not an actual client of ValidationInterface, these are called through PeerManager. // Responses to chain events. TxDownloadManager is not an actual client of ValidationInterface, these are called through PeerManager.
void ActiveTipChange(); void ActiveTipChange();
@ -171,6 +170,11 @@ public:
/** Respond to package rejected from mempool */ /** Respond to package rejected from mempool */
void MempoolRejectedPackage(const Package& package); void MempoolRejectedPackage(const Package& package);
/** Marks a tx as ReceivedResponse in txrequest and checks whether AlreadyHaveTx.
* Return a bool indicating whether this tx should be validated. If false, optionally, a
* PackageToValidate. */
std::pair<bool, std::optional<PackageToValidate>> ReceivedTx(NodeId nodeid, const CTransactionRef& ptx);
}; };
} // namespace node } // namespace node
#endif // BITCOIN_NODE_TXDOWNLOADMAN_H #endif // BITCOIN_NODE_TXDOWNLOADMAN_H

62
src/node/txdownloadman_impl.cpp
Unescape View File

@ -27,10 +27,6 @@ TxRequestTracker& TxDownloadManager::GetTxRequestRef()
{ {
return m_impl->m_txrequest; return m_impl->m_txrequest;
} }
CRollingBloomFilter& TxDownloadManager::RecentRejectsReconsiderableFilter()
{
return m_impl->RecentRejectsReconsiderableFilter();
}
void TxDownloadManager::ActiveTipChange() void TxDownloadManager::ActiveTipChange()
{ {
m_impl->ActiveTipChange(); m_impl->ActiveTipChange();
@ -83,6 +79,10 @@ void TxDownloadManager::MempoolRejectedPackage(const Package& package)
{ {
m_impl->MempoolRejectedPackage(package); m_impl->MempoolRejectedPackage(package);
} }
std::pair<bool, std::optional<PackageToValidate>> TxDownloadManager::ReceivedTx(NodeId nodeid, const CTransactionRef& ptx)
{
return m_impl->ReceivedTx(nodeid, ptx);
}
// TxDownloadManagerImpl // TxDownloadManagerImpl
void TxDownloadManagerImpl::ActiveTipChange() void TxDownloadManagerImpl::ActiveTipChange()
@ -450,4 +450,58 @@ void TxDownloadManagerImpl::MempoolRejectedPackage(const Package& package)
{ {
RecentRejectsReconsiderableFilter().insert(GetPackageHash(package)); RecentRejectsReconsiderableFilter().insert(GetPackageHash(package));
} }
std::pair<bool, std::optional<PackageToValidate>> TxDownloadManagerImpl::ReceivedTx(NodeId nodeid, const CTransactionRef& ptx)
{
const uint256& txid = ptx->GetHash();
const uint256& wtxid = ptx->GetWitnessHash();
// Mark that we have received a response
m_txrequest.ReceivedResponse(nodeid, txid);
if (ptx->HasWitness()) m_txrequest.ReceivedResponse(nodeid, wtxid);
// First check if we should drop this tx.
// We do the AlreadyHaveTx() check using wtxid, rather than txid - in the
// absence of witness malleation, this is strictly better, because the
// recent rejects filter may contain the wtxid but rarely contains
// the txid of a segwit transaction that has been rejected.
// In the presence of witness malleation, it's possible that by only
// doing the check with wtxid, we could overlook a transaction which
// was confirmed with a different witness, or exists in our mempool
// with a different witness, but this has limited downside:
// mempool validation does its own lookup of whether we have the txid
// already; and an adversary can already relay us old transactions
// (older than our recency filter) if trying to DoS us, without any need
// for witness malleation.
if (AlreadyHaveTx(GenTxid::Wtxid(wtxid), /*include_reconsiderable=*/true)) {
if (RecentRejectsReconsiderableFilter().contains(wtxid)) {
// When a transaction is already in m_lazy_recent_rejects_reconsiderable, we shouldn't submit
// it by itself again. However, look for a matching child in the orphanage, as it is
// possible that they succeed as a package.
LogDebug(BCLog::TXPACKAGES, "found tx %s (wtxid=%s) in reconsiderable rejects, looking for child in orphanage\n",
txid.ToString(), wtxid.ToString());
return std::make_pair(false, Find1P1CPackage(ptx, nodeid));
}
// If a tx is detected by m_lazy_recent_rejects it is ignored. Because we haven't
// submitted the tx to our mempool, we won't have computed a DoS
// score for it or determined exactly why we consider it invalid.
//
// This means we won't penalize any peer subsequently relaying a DoSy
// tx (even if we penalized the first peer who gave it to us) because
// we have to account for m_lazy_recent_rejects showing false positives. In
// other words, we shouldn't penalize a peer if we aren't *sure* they
// submitted a DoSy tx.
//
// Note that m_lazy_recent_rejects doesn't just record DoSy or invalid
// transactions, but any tx not accepted by the mempool, which may be
// due to node policy (vs. consensus). So we can't blanket penalize a
// peer simply for relaying a tx that our m_lazy_recent_rejects has caught,
// regardless of false positives.
return {false, std::nullopt};
}
return {true, std::nullopt};
}
} // namespace node } // namespace node

2
src/node/txdownloadman_impl.h
Unescape View File

@ -167,6 +167,8 @@ public:
void MempoolAcceptedTx(const CTransactionRef& tx); void MempoolAcceptedTx(const CTransactionRef& tx);
RejectedTxTodo MempoolRejectedTx(const CTransactionRef& ptx, const TxValidationState& state, NodeId nodeid, bool first_time_failure); RejectedTxTodo MempoolRejectedTx(const CTransactionRef& ptx, const TxValidationState& state, NodeId nodeid, bool first_time_failure);
void MempoolRejectedPackage(const Package& package); void MempoolRejectedPackage(const Package& package);
std::pair<bool, std::optional<PackageToValidate>> ReceivedTx(NodeId nodeid, const CTransactionRef& ptx);
}; };
} // namespace node } // namespace node
#endif // BITCOIN_NODE_TXDOWNLOADMAN_IMPL_H #endif // BITCOIN_NODE_TXDOWNLOADMAN_IMPL_H

Write Preview
Loading…
Cancel
Save