revert(net): remove header-accept RandomX check; keep nMinimumChainWork + #8
An adversarial re-review found the header-accept RandomX check (b9fdc7981+7e9b2c661header-PoW +d2124a303defer) to be a persistent source of consensus-liveness bugs: it derives the RandomX key from the ACTIVE chain (hush_chainactive), the wrong branch for reorg/side-branch/catch-up headers, so it repeatedly false-rejected validly-mined headers and DoS(100)-hard-banned honest peers (IBD-tail catch-up and deep-reorg cases); the defer fix and an extend-tip fix each addressed one case while leaving/creating others (an extend-tip variant re-opened an unbounded post-IBD side-branch flood). It only mitigated a low-harm resource DoS -- forged headers bloat mapBlockIndex memory/ disk but are never SELECTED (nMinimumChainWork) and the full RandomX + target check still runs at block-connect. Revert to fCheckPOW=0 at header-accept (original behavior). A comment in AcceptBlockHeader records that any re-attempt must derive the key from the header's OWN ancestry (pindexPrev->GetAncestor), never the active chain. Also hardens two issues the same review found: - #8 IBD header cap now bounds against the VALIDATED chainActive.Height() (attacker-hard) instead of pindexBestHeader, which a forward-extending flood advanced in lockstep, defeating the cap. - opreturn_burn only emits a change output above the dust threshold; a sub-dust change made the returned tx non-standard/unrelayable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+17
-24
@@ -5457,27 +5457,15 @@ bool AcceptBlockHeader(int32_t *futureblockp,const CBlockHeader& block, CValidat
|
||||
}
|
||||
return true;
|
||||
}
|
||||
// SECURITY (header-flood DoS): once synced, verify PoW at header-accept time so a peer cannot
|
||||
// flood unbounded PoW-less headers into mapBlockIndex (they now fail RandomX -> DoS-ban). During
|
||||
// IBD keep fCheckPOW=0 for fast header sync; the full RandomX/target check runs at block connect.
|
||||
// Resolve the parent up-front so CheckBlockHeader receives THIS header's own (still-NULL) pindex
|
||||
// and its CORRECT height (parent height + 1) — never the caller's reused *ppindex, which in a
|
||||
// HEADERS batch aliases the PREVIOUS header. Passing *ppindex here made (a) the height a stale
|
||||
// value (0 for a fresh header, or the prior header's height when aliased) so post-IBD
|
||||
// RandomXValidationRequired() saw a below-activation height and CheckRandomXSolution returned true
|
||||
// WITHOUT verifying, and (b) the (pindex && pindex->fRandomXVerified) short-circuit fire on an
|
||||
// as-yet-unverified header — both silently defeating the post-IBD header-flood PoW gate. The
|
||||
// authoritative parent validation (prev-not-found / prev-invalid) still runs unchanged below; this
|
||||
// lookup is read-only and under cs_main, so it cannot disagree with it. IBD stays fast: fCheckPOW
|
||||
// is still 0 during IBD, so no RandomX is computed here regardless of the height.
|
||||
CBlockIndex* pindexPrevForHeight = NULL;
|
||||
{
|
||||
BlockMap::iterator miPrev = mapBlockIndex.find(block.hashPrevBlock);
|
||||
if (miPrev != mapBlockIndex.end())
|
||||
pindexPrevForHeight = miPrev->second;
|
||||
}
|
||||
int32_t nHeaderHeight = (pindexPrevForHeight != NULL) ? pindexPrevForHeight->GetHeight() + 1 : 0;
|
||||
if (!CheckBlockHeader(futureblockp,nHeaderHeight,pindex, block, state, IsInitialBlockDownload() ? 0 : 1)) {
|
||||
// Header-accept does NOT verify RandomX PoW (fCheckPOW=0). The RandomX key for a header is derived
|
||||
// from the block at keyHeight on the header's OWN branch, which is not reliably resolvable at
|
||||
// header-accept time (reorg / side-branch / catch-up headers are not on the active chain), so a
|
||||
// header-time RandomX check repeatedly false-rejected valid reorg headers and hard-banned honest
|
||||
// peers (see audit notes; reverted b9fdc7981/7e9b2c661/defer). The full RandomX + target check runs
|
||||
// at block-connect with the correct branch key. Fake low-work chains are gated from SELECTION by
|
||||
// nMinimumChainWork; the per-peer IBD header cap bounds flood memory. Do NOT re-enable a header-time
|
||||
// RandomX check without first deriving the key from the header's own ancestry (pindexPrev->GetAncestor).
|
||||
if (!CheckBlockHeader(futureblockp,*ppindex!=0?(*ppindex)->GetHeight():0,*ppindex, block, state, 0)) {
|
||||
if ( *futureblockp == 0 ) {
|
||||
LogPrintf("%s: CheckBlockHeader futureblock=0\n", __func__);
|
||||
return false;
|
||||
@@ -7895,13 +7883,18 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
|
||||
// store while in IBD. Honest headers-first sync needs at most ~chain-length headers from a
|
||||
// peer; one that floods far past 2x the known chain length is only trying to bloat
|
||||
// mapBlockIndex/leveldb (such headers are never selected -- nMinimumChainWork gates that --
|
||||
// but they still cost memory/disk). Cap per-peer and drop the peer. IBD-only: post-IBD the
|
||||
// RandomX check in AcceptBlockHeader already makes forged headers fail RandomX and ban.
|
||||
// but they still cost memory/disk). Cap per-peer and drop the peer. IBD-only: post-IBD there is
|
||||
// no header-accept PoW check (removed as bug-prone), so a post-IBD flood is bounded only by
|
||||
// nMinimumChainWork gating selection -- memory/disk growth there is accepted as low-severity.
|
||||
if (IsInitialBlockDownload()) {
|
||||
CNodeState *hstate = State(pfrom->GetId());
|
||||
if (hstate != NULL) {
|
||||
hstate->nHeadersProcessed += (int64_t)nCount;
|
||||
int knownH = std::max(pindexBestHeader ? (int)pindexBestHeader->GetHeight() : 0,
|
||||
// Cap RELATIVE TO THE VALIDATED ACTIVE-CHAIN HEIGHT (attacker-hard -- advancing it requires
|
||||
// connecting real PoW blocks), NOT pindexBestHeader: a forward-extending header flood advances
|
||||
// pindexBestHeader in lockstep with the attacker, so a pindexBestHeader-relative cap never fires.
|
||||
// The checkpoint height is a fixed floor so honest IBD (blocks still lagging headers) is never capped.
|
||||
int knownH = std::max((int)chainActive.Height(),
|
||||
Checkpoints::GetTotalBlocksEstimate(chainparams.Checkpoints()));
|
||||
int64_t headerCap = 2 * (int64_t)knownH + 200000;
|
||||
if (hstate->nHeadersProcessed > headerCap) {
|
||||
|
||||
Reference in New Issue
Block a user