From fbcf160478a07d51c6edcd8f6e468f742ea58cc4 Mon Sep 17 00:00:00 2001 From: DanS Date: Fri, 21 Aug 2026 16:58:29 -0500 Subject: [PATCH] fix(pow): only dedup RandomX when CheckBlockHeader verified it; drop fake git id Brings onto dev the two fixes that until now existed only on release/1.0.4, so nothing is stranded on a branch we are not shipping from. 1. GUARDED VERIFY-ONCE (main.cpp) 4e67e687d arms the RandomX dedup unconditionally whenever fCheckPOW is set. But CheckBlockHeader returns early -- BEFORE reaching its RandomX check -- for a block whose timestamp is >60s in the future (*futureblockp==1), and CheckBlock deliberately continues on that path. There, hush_checkPOW is the ONLY RandomX verification the block gets, so suppressing it leaves the block unverified. Not a chain-acceptance hole: ConnectBlock rejects futureblock != 0, so such a block never joins the chain. But it silently weakens DoS banning -- an invalid future block gets rejected for its timestamp instead of for bad PoW, which is a regression against the un-deduped behaviour it replaced. ScopedRandomXSkip now takes an `arm` flag and CheckBlock passes fHeaderChecked, so the dedup applies only where the header check actually completed and did the verification. Strictly a tightening: it can only cause MORE verification than before, never less. 2. NO FAKE GIT IDENTITY (clientversion.cpp) A hardcoded `#define GIT_ARCHIVE 1` with GIT_COMMIT_ID "a86845f3dc", dated Feb 2018, is reached whenever build.h supplies no BUILD_DESC -- i.e. any build without git metadata, which is exactly the tarball/CI release case. Such binaries reported themselves as that Komodo commit regardless of content; a build here did precisely that before this was found. The archive substitution placeholders are kept, so a real git-archive export still works; a git-less build now reports "-unk", which is honest and greppable. Both syntax-clean. Rebuild and re-validation on EPYC follows; the earlier validated binary (md5 fe83d70fec5b50c38bf65ea6c733ffa9) predates these. release/1.0.4 is parked, not deleted -- its commit records why the v1.0.2 lineage cannot ship (block-index format incompatibility with dev-written chainstate). Co-Authored-By: Claude Opus 5 (1M context) --- src/clientversion.cpp | 12 ++++++++---- src/main.cpp | 18 ++++++++++++++---- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/clientversion.cpp b/src/clientversion.cpp index 635121bdb..88f03fa47 100644 --- a/src/clientversion.cpp +++ b/src/clientversion.cpp @@ -62,11 +62,15 @@ const std::string CLIENT_NAME = GetArg("-clientname", "DragonX"); #endif //TODO: clean up this stuff -//! git will put "#define GIT_ARCHIVE 1" on the next line inside archives. -#define GIT_ARCHIVE 1 +//! git archives get "#define GIT_ARCHIVE 1" substituted on the next line by export-subst. +//! Do NOT hardcode it: the BUILD_DESC chain below falls back to GIT_COMMIT_ID whenever +//! build.h carries no BUILD_DESC -- any build without git metadata, e.g. from a tarball -- +//! so a hardcoded id makes those binaries claim an identity that is not theirs. Until +//! 2026-08-21 this asserted Komodo commit a86845f3dc, dated Feb 2018, on every such build. +//! With it gone that case reports "-unk", which is honest and greppable. #ifdef GIT_ARCHIVE -#define GIT_COMMIT_ID "a86845f3dc" -#define GIT_COMMIT_DATE "Wed, 21 Feb 2018 16:15:11 +0200" +#define GIT_COMMIT_ID "$Format:%h$" +#define GIT_COMMIT_DATE "$Format:%cD$" #endif #define RENDER_BETA_STRING(num) "-beta" DO_STRINGIZE(num) diff --git a/src/main.cpp b/src/main.cpp index 1375ab064..c15049253 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -5155,10 +5155,15 @@ int32_t hush_checkPOW(int32_t slowflag,CBlock *pblock,int32_t height); // RAII: save+restore the thread-local RandomX-skip flag around the verify-once dedup in CheckBlock, // so it can never clobber the miner's own fSkipRandomXValidation (TestBlockValidity -> ConnectBlock // re-entry) nor leak TRUE on an exception thrown out of hush_checkPOW. +// `arm` is false on paths where CheckBlockHeader did NOT reach its own RandomX check, so the +// dedup can never suppress the only verification a block gets. struct ScopedRandomXSkip { bool prev; - ScopedRandomXSkip() : prev(GetSkipRandomXValidation()) { SetSkipRandomXValidation(true); } - ~ScopedRandomXSkip() { SetSkipRandomXValidation(prev); } + bool armed; + explicit ScopedRandomXSkip(bool arm) : prev(GetSkipRandomXValidation()), armed(arm) { + if (armed) SetSkipRandomXValidation(true); + } + ~ScopedRandomXSkip() { if (armed) SetSkipRandomXValidation(prev); } }; bool CheckBlock(int32_t *futureblockp,int32_t height,CBlockIndex *pindex,const CBlock& block, CValidationState& state, libzcash::ProofVerifier& verifier, @@ -5168,7 +5173,8 @@ bool CheckBlock(int32_t *futureblockp,int32_t height,CBlockIndex *pindex,const C // These are checks that are independent of context. hash = block.GetHash(); // Check that the header is valid (particularly PoW). This is mostly redundant with the call in AcceptBlockHeader. - if (!CheckBlockHeader(futureblockp,height,pindex,block,state,fCheckPOW)) + const bool fHeaderChecked = CheckBlockHeader(futureblockp,height,pindex,block,state,fCheckPOW); + if (!fHeaderChecked) { if ( *futureblockp == 0 ) { @@ -5196,7 +5202,11 @@ bool CheckBlock(int32_t *futureblockp,int32_t height,CBlockIndex *pindex,const C // that dominates IBD). The scoped guard saves/restores the skip flag (never hardcodes false) // so the miner's own skip is preserved and nothing leaks on throw. Equihash + PoW-target in // hush_checkPOW still run. - ScopedRandomXSkip _rxskip; + // ARMED ONLY IF fHeaderChecked: CheckBlockHeader returns early -- BEFORE its RandomX + // check -- for a future-timestamped block (*futureblockp==1), and CheckBlock keeps + // going on that path. There hush_checkPOW is the ONLY RandomX verification, so + // arming unconditionally drops the check for that class of block. + ScopedRandomXSkip _rxskip(fHeaderChecked); if ( hush_checkPOW(1,(CBlock *)&block,height) < 0 ) return state.DoS(100, error("CheckBlock: failed slow_checkPOW"),REJECT_INVALID, "failed-slow_checkPOW"); }