Use wtxid for peer state management

This commit is contained in:
Jack Grigg 2021-09-09 00:43:59 +01:00
parent d503691778
commit fc8726e6a4
5 changed files with 94 additions and 24 deletions

View File

@ -8,3 +8,4 @@
- [Design](design.md)
- [Chain state](design/chain-state.md)
- ["Coins" view](design/coins-view.md)
- [P2P data propagation](design/p2p-data-propagation.md)

View File

@ -0,0 +1,43 @@
# P2P data propagation
This page contains notes about how block and transaction data is tracked and propagated by
`zcashd`. Most of this behaviour is inherited from Bitcoin Core, but some differences have
developed.
Some of this content is duplicated from in-code comments, but assembling this summary in
one place is generally useful for understanding the overall dynamic :)
## `recentRejects`
When a transaction is rejected by `AcceptToMemoryPool`, the transaction is added to the
`recentRejects` Bloom filter, so that we don't process it again. The Bloom filter resets
whenever the chain tip changes, as previously invalid transactions might become valid.
To prevent DoS attacks against wallets submitting transactions, `recentRejects` needs to
store a commitment to the entire transaction. This ensures that if a transaction is
malleated by a network peer to invalidate its authorizing data, the node will ignore
future advertisements of that specific transaction, but still request alternative versions
of the same txid (which might have valid authorizing data).
- For pre-v5 transactions, the txid commits to the entire transaction, and the wtxid is
the txid with a globally-fixed (all-ones) suffix.
- For v5+ transactions, the wtxid commits to the entire transaction.
## `mapOrphanTransactions`
Upstream uses this map to store transactions that are rejected by `AcceptToMemoryPool`
because the node doesn't have their transparent inputs. `zcashd` inherits this behaviour
but limits it to purely-transparent transactions (that is, if a transaction contains any
shielded components, the node rejects it as invalid and adds it to `recentRejects`).
`mapOrphanTransactions` indexes transactions by txid. This means that if an orphan
transaction is received (spending transparent UTXOs the node does not know about), and it
also happens to be invalid for other reasons (subsequent `AcceptToMemoryPool` rules that
haven't yet been checked), then the node will not request any v5+ transactions with the
same txid but different authorizing data. This does not create a DoS problem for wallets,
because an adversary that manipulated an orphan transaction to be invalid under the above
constraints would also need to prevent the orphan's parent from entering the mempool, and
eventually a parent is reached that is not an orphan. Once the orphan's direct parent is
accepted, the orphan is re-evaluated, and if it had been manipulated to be invalid, its
wtxid is added to `recentRejects` while its txid is removed from `mapOrphanTransactions`,
enabling the wallet to rebroadcast the unmodified transaction.

View File

@ -240,6 +240,11 @@ namespace {
* million to make it highly unlikely for users to have issues with this
* filter.
*
* We only need to add wtxids to this filter. For pre-v5 transactions, the
* txid commits to the entire transaction, and the wtxid is the txid with a
* globally-fixed (all-ones) suffix. For v5+ transactions, the wtxid commits
* to the entire transaction.
*
* Memory used: 1.3 MB
*/
boost::scoped_ptr<CRollingBloomFilter> recentRejects;
@ -630,6 +635,8 @@ CBlockTreeDB *pblocktree = NULL;
bool AddOrphanTx(const CTransaction& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
// See doc/book/src/design/p2p-data-propagation.md for why mapOrphanTransactions uses
// txid to index transactions instead of wtxid.
uint256 hash = tx.GetHash();
if (mapOrphanTransactions.count(hash))
return false;
@ -6034,7 +6041,11 @@ bool static AlreadyHave(const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
recentRejects->reset();
}
return recentRejects->contains(inv.hash) ||
// We only need to use wtxid with recentRejects. Orphan map entries
// are re-validated once their parents have arrived, and all other
// locations are only possible if the transaction has already been
// validated (we don't care about alternative authorizing data).
return recentRejects->contains(inv.GetWideHash()) ||
mempool.exists(inv.hash) ||
mapOrphanTransactions.count(inv.hash) ||
pcoinsTip->HaveCoins(inv.hash);
@ -6552,7 +6563,7 @@ bool static ProcessMessage(const CChainParams& chainparams, CNode* pfrom, string
}
else
{
pfrom->AddKnownTx(inv.hash);
pfrom->AddKnownTx(WTxId(inv.hash, inv.hashAux));
if (fBlocksOnly)
LogPrint("net", "transaction (%s) inv sent in violation of protocol peer=%d\n", inv.hash.ToString(), pfrom->id);
else if (!fAlreadyHave && !IsInitialBlockDownload(chainparams.GetConsensus()))
@ -6696,22 +6707,28 @@ bool static ProcessMessage(const CChainParams& chainparams, CNode* pfrom, string
CTransaction tx;
vRecv >> tx;
CInv inv(MSG_TX, tx.GetHash());
pfrom->AddKnownTx(inv.hash);
const uint256& txid = tx.GetHash();
const WTxId& wtxid = tx.GetWTxId();
LOCK(cs_main);
pfrom->AddKnownTx(wtxid);
bool fMissingInputs = false;
CValidationState state;
pfrom->setAskFor.erase(inv.hash);
mapAlreadyAskedFor.erase(inv.hash);
pfrom->setAskFor.erase(wtxid);
mapAlreadyAskedFor.erase(wtxid);
if (!AlreadyHave(inv) && AcceptToMemoryPool(chainparams, mempool, state, tx, true, &fMissingInputs))
// We do the AlreadyHave() check using a MSG_WTX inv unconditionally,
// because for pre-v5 transactions wtxid.authDigest is set to the same
// placeholder as is used for the CInv.hashAux field for MSG_TX.
if (!AlreadyHave(CInv(MSG_WTX, txid, wtxid.authDigest)) &&
AcceptToMemoryPool(chainparams, mempool, state, tx, true, &fMissingInputs))
{
mempool.check(pcoinsTip);
RelayTransaction(tx);
vWorkQueue.push_back(inv.hash);
vWorkQueue.push_back(txid);
LogPrint("mempool", "AcceptToMemoryPool: peer=%d %s: accepted %s (poolsz %u txn, %u kB)\n",
pfrom->id, pfrom->cleanSubVer,
@ -6762,8 +6779,12 @@ bool static ProcessMessage(const CChainParams& chainparams, CNode* pfrom, string
// Probably non-standard or insufficient fee/priority
LogPrint("mempool", " removed orphan tx %s\n", orphanHash.ToString());
vEraseQueue.push_back(orphanHash);
// Add the wtxid of this transaction to our reject filter.
// Unlike upstream Bitcoin Core, we can unconditionally add
// these, as they are always bound to the entirety of the
// transaction regardless of version.
assert(recentRejects);
recentRejects->insert(orphanHash);
recentRejects->insert(orphanTx.GetWTxId().ToBytes());
}
mempool.check(pcoinsTip);
}
@ -6786,8 +6807,12 @@ bool static ProcessMessage(const CChainParams& chainparams, CNode* pfrom, string
if (nEvicted > 0)
LogPrint("mempool", "mapOrphan overflow, removed %u tx\n", nEvicted);
} else {
// Add the wtxid of this transaction to our reject filter.
// Unlike upstream Bitcoin Core, we can unconditionally add
// these, as they are always bound to the entirety of the
// transaction regardless of version.
assert(recentRejects);
recentRejects->insert(tx.GetHash());
recentRejects->insert(tx.GetWTxId().ToBytes());
if (pfrom->fWhitelisted && GetBoolArg("-whitelistforcerelay", DEFAULT_WHITELISTFORCERELAY)) {
// Always relay transactions received from whitelisted peers, even
@ -6816,7 +6841,7 @@ bool static ProcessMessage(const CChainParams& chainparams, CNode* pfrom, string
FormatStateMessage(state));
if (state.GetRejectCode() < REJECT_INTERNAL) // Never send AcceptToMemoryPool's internal codes over P2P
pfrom->PushMessage("reject", strCommand, (unsigned char)state.GetRejectCode(),
state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), inv.hash);
state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), txid);
if (nDoS > 0)
Misbehaving(pfrom->GetId(), nDoS);
}
@ -7646,7 +7671,7 @@ bool SendMessages(const Consensus::Params& params, CNode* pto)
}
} else {
//If we're not going to ask, don't expect a response.
pto->setAskFor.erase(inv.hash);
pto->setAskFor.erase(WTxId(inv.hash, inv.hashAux));
}
pto->mapAskFor.erase(pto->mapAskFor.begin());
}

View File

@ -73,7 +73,7 @@ std::string strSubVersion;
vector<CNode*> vNodes;
CCriticalSection cs_vNodes;
limitedmap<uint256, int64_t> mapAlreadyAskedFor(MAX_INV_SZ);
limitedmap<WTxId, int64_t> mapAlreadyAskedFor(MAX_INV_SZ);
static deque<string> vOneShots;
static CCriticalSection cs_vOneShots;
@ -2034,7 +2034,7 @@ void RelayTransaction(const CTransaction& tx)
LOCK(cs_vNodes);
for (CNode* pnode : vNodes)
{
pnode->PushTxInventory(tx.GetHash());
pnode->PushTxInventory(tx.GetWTxId());
}
}
@ -2279,14 +2279,15 @@ void CNode::AskFor(const CInv& inv)
{
if (mapAskFor.size() > MAPASKFOR_MAX_SZ || setAskFor.size() > SETASKFOR_MAX_SZ)
return;
WTxId wtxid(inv.hash, inv.hashAux);
// a peer may not have multiple non-responded queue positions for a single inv item
if (!setAskFor.insert(inv.hash).second)
if (!setAskFor.insert(wtxid).second)
return;
// We're using mapAskFor as a priority queue,
// the key is the earliest time the request can be sent
int64_t nRequestTime;
limitedmap<uint256, int64_t>::const_iterator it = mapAlreadyAskedFor.find(inv.hash);
limitedmap<WTxId, int64_t>::const_iterator it = mapAlreadyAskedFor.find(wtxid);
if (it != mapAlreadyAskedFor.end())
nRequestTime = it->second;
else
@ -2305,7 +2306,7 @@ void CNode::AskFor(const CInv& inv)
if (it != mapAlreadyAskedFor.end())
mapAlreadyAskedFor.update(it, nRequestTime);
else
mapAlreadyAskedFor.insert(std::make_pair(inv.hash, nRequestTime));
mapAlreadyAskedFor.insert(std::make_pair(wtxid, nRequestTime));
mapAskFor.insert(std::make_pair(nRequestTime, inv));
}

View File

@ -164,7 +164,7 @@ extern int nMaxConnections;
extern std::vector<CNode*> vNodes;
extern CCriticalSection cs_vNodes;
extern limitedmap<uint256, int64_t> mapAlreadyAskedFor;
extern limitedmap<WTxId, int64_t> mapAlreadyAskedFor;
extern std::vector<std::string> vAddedNodes;
extern CCriticalSection cs_vAddedNodes;
@ -346,7 +346,7 @@ public:
// and in the order requested.
std::vector<uint256> vInventoryBlockToSend;
CCriticalSection cs_inventory;
std::set<uint256> setAskFor;
std::set<WTxId> setAskFor;
std::multimap<int64_t, CInv> mapAskFor;
int64_t nNextInvSend;
// Used for BIP35 mempool sending, also protected by cs_inventory
@ -466,19 +466,19 @@ public:
}
void AddKnownTx(const uint256& hash)
void AddKnownTx(const WTxId& wtxid)
{
{
LOCK(cs_inventory);
filterInventoryKnown.insert(hash);
filterInventoryKnown.insert(wtxid.ToBytes());
}
}
void PushTxInventory(const uint256& hash)
void PushTxInventory(const WTxId& wtxid)
{
LOCK(cs_inventory);
if (!filterInventoryKnown.contains(hash)) {
setInventoryTxToSend.insert(hash);
if (!filterInventoryKnown.contains(wtxid.ToBytes())) {
setInventoryTxToSend.insert(wtxid.hash);
}
}