skalacoin

dcrubro/skalacoin

Fork 0

Commit Graph

Author SHA1 Message Date

Author	SHA1	Message	Date
dcrubro	5eaf0b699c	Let a node that is behind adopt the peer's branch, and fix two bugs that only appear once a reorg actually applies Reported: a node at height N+10 on its own short fork, with the peer at N+500, could never sync. It located the fork point correctly, pooled the branch, and then refused it forever. At 90s blocks a node is 960 blocks behind after a day offline, so this is the normal case, not an edge case. == Why it could never recover == The reorg delay was served by LOCAL chain growth alone: elapsed = tipHeight - observedTip That only makes sense for a node whose tip is advancing. A node that is behind has a frozen tip precisely because it is rejecting the branch, so elapsed stays 0 forever while penalty(5) = 42. Both escape hatches also fail: mining out of it means extending a fork nobody accepts, and the IBD exemption never fires for a node that is mining, because mining keeps its tip fresh. Being 490 blocks behind is being behind, not a reorg contest. Refusing to adopt protects nothing while the node falls a further 40 blocks behind every hour. The delay is now satisfiable by EITHER side making progress: localGrowth = tipHeight - observedTip branchLead = candidateTip - tipHeight (0 if not ahead) elapsed = max(localGrowth, branchLead) A branch already extending `penalty` blocks past our tip has demonstrated exactly what the delay asks for, and every one of those blocks carries PoW we validated ourselves. Requiring us to independently produce the same amount is demanding the same proof twice. Only VALIDATED blocks count -- a peer's advertised height is not evidence and never reaches this code. It degrades correctly in both directions: a mining node's tip advances, so an attacker must outpace it by penalty blocks; a node that is only observing follows the heaviest chain, which is what an observer should do. == The window has to keep pulling the branch == Every forked delivery reset nextReq back to our own tip, so the window re-requested the same eight heights forever and the pool never grew past the window size -- branchLead could not rise even in principle. The backward walk now runs ONCE to establish linkage, then the window marches forward pooling the branch, retrying adoption per window-full with a final attempt when it drains. == sync force == Operator override that skips the delay for one sync, for a node whose chain is known to be the wrong one. Threaded explicitly (Chain_ReplaceBranch gains bypassPenalty, OrphanPool_AttemptAttachForced) rather than through a global, so nothing a peer sends can reach it. Linkage, work comparison and atomicity still apply -- it waives only the waiting, and says so loudly in the log. == Bug found in testing: PoW is branch-relative == A block's epoch seed is the last block of the previous epoch ON ITS OWN BRANCH. Validating a competing branch's block against OUR epoch seed does not merely fail to resolve: when the chains diverge before the boundary it resolves to the WRONG seed and rejects a perfectly valid block. Any fork spanning an epoch boundary was therefore impossible to assemble -- the branch could not grow past the boundary block, so branchLead stalled one short of it. The receive path now does self-contained checks only (Block_HasValidStructure: merkle, transactions, vote, non-empty). Proof of work moved to Chain_AddBlockLocked, at the point a block joins the chain, where the branch context is real -- the rollback has put its ancestors in place by then. That is where the invariant belongs and it removes a duplicate check rather than adding one. Needs Chain_DagParamsForHeightLocked, because Chain_AddBlockLocked already holds chainLock for writing and the lock is not recursive. Consequence worth knowing: the orphan pool can now hold blocks whose work has not been verified, bounded by MAX_ORPHAN_BLOCKS (512). Each still had to pass merkle and full transaction/signature validation, and none can reach the chain unverified. This bug also affected plain forward sync across block 350000; it was masked because appending keeps the boundary block in the chain. == Bug found in testing: stale DAG accepted as current == Block_PowHashHeavy matched on epoch index and size but not the seed. A DAG's content is a function of (seed, size); the epoch index is a label for it. A reorg is exactly the event that changes the seed while leaving index and size untouched, so mid-apply the miner's context still held a DAG built from the PRE-reorg seed, the guard passed, and a valid block was hashed against the wrong lanes. g_dagSeed now records what each DAG was generated from and both Block_EnsureAutolykos2Dag and Block_PowHashHeavy compare it. == Bug found in testing: double free on the failed-apply path == SIGABRT in the allocator: free_tiny_botch -> DynArr_destroy -> Block_Destroy -> Chain_FreeBlockArray -> Chain_ReplaceBranch. DynArr_push_back stores the struct BY VALUE, so the chain's element and the caller's block_t share one transactions pointer. Three places free that array through the chain's copy -- Chain_ClearBlocks, Chain_RollbackToHeightLocked and Chain_SaveToFile -- and each NULLs only the chain's side, leaving any caller wrapper dangling. Whether a caller then had to use free() or Block_Destroy() was a convention carried in comments at every call site plus a consumed-count passed into Chain_FreeBlockArray. Chain_ReplaceBranch reset that count to 0 after rolling back a failed apply, which told the cleanup to Block_Destroy exactly the blocks whose arrays the rollback had just freed. Rather than fix the count, the aliasing is now safe by construction: Chain_AddBlockLocked clears the CALLER's transactions pointer immediately after the push. Since DynArr_destroy(NULL) is a no-op, free(wrapper) and Block_Destroy(wrapper) become equivalent and both safe regardless of what later frees the chain's copy. The consumed-count parameter and both counters are gone -- the thing that could be got wrong no longer exists -- and all call sites are unified on Block_Destroy. Placement is deliberate: immediately after the push, not at the end on success. The ledger pass can fail with the block already in the chain, returning false to a caller that destroys its wrapper on failure -- OrphanPool_ExtendTip does exactly that, a third live instance not yet triggered. Two follow-ons the refactor forced, both improvements anyway: MineAndAppendBlock read the coinbase for its log line after the add (hoisted above it), and the success log printed the caller's block rather than the chain's copy. == Also == The deferral line is rate-limited. The maintenance thread retries pooled branches once a second and elapsed only changes when something moves, so it printed an identical line every second -- forever, on a node that is not mining. It now reports each distinct situation once. == Verification == Synthetic fork, node A 5 deep, node B ~60 ahead, EPOCH_LENGTH=8 so the branch crosses three epoch boundaries: branchLead climbs 8 -> 16 -> 28 -> 34, crosses penalty(5)=42 Adopted competing branch of 50 block(s) at fork height 20 sync complete: localHeight=70 Chain OK Repeated under AddressSanitizer: adopted 47 blocks, 0 ASan errors on both nodes. This matters because the refactor rewrites the exact cleanup path the SIGABRT came from, and a double free that no longer aborts would otherwise pass silently. Unit suites pass, including a new assertion "heavy path refuses a DAG built from a different seed" -- the direct regression for the stale-DAG bug. Harness note: each node needs its OWN wallet. With a shared one both pay the same coinbase address, produce identical merkle roots, and at easy difficulty mine byte-identical blocks -- the fork test silently became a catch-up test. == Still untested == The restore-after-failed-apply path is no longer naturally reachable now that the two bugs above are fixed, so it needs deliberate corruption to exercise. Test B (branch only slightly ahead must still DEFER), test C (sync force), and a TSan pass over the changed paths are outstanding.	2026-08-02 19:36:09 +02:00
dcrubro	1ff2890c0f	Fix the reorg system: make rollback survivable, adopt by cumulative work, and enforce the reorg penalty everywhere (This is a big one, get ready - I told Claude to write the commit message cause I couldn't be bothered) Root cause: reorg was broken at every layer and the failures compounded. Verified with the node's own SKALACOIN_FORCE_ORPHAN_REORG debug mode, which stalled permanently at height 2 on 114 consecutive coinbase-validation failures. A binary built at HEAD behaves identically, so none of this is a regression — the reorg path had simply never worked. Rollback (the keystone): - Chain_RollbackToHeight always returned false on any node that had ever saved or loaded its chain, and only after it had already truncated the chain and destroyed the balance sheet. Chain_RecomputeRuntimeState bails on any header-only block, and Chain_SaveToFile nulls transactions on every in-memory block once persisted, so the failure was universal in practice. Both callers treated the false as "nothing happened" - Added Chain_BorrowBlockTransactions / Chain_ReturnBlockTransactions, which fall back to the on-disk copy when the in-memory block has been compacted to headers - Supply is now accumulated in the rollback's existing balance-sheet replay pass instead of a second Chain_RecomputeRuntimeState pass - Gave Chain_RecomputeRuntimeState the same disk fallback: it had been failing on every restart with an existing chain, silently leaving currentSupply/currentReward at whatever came out of chain.meta Fork choice is now cumulative work, not height: - Added Chain_ComputeBlockWork / Chain_ComputeWorkRange / Chain_ComputeBranchWork, computing 2^256 / (target + 1) per block and summing over a range. Derived on demand from headers — no header, chain.meta or wire-format change - uint256 only had add/sub/cmp, so added uint256_divide (restoring binary long division), uint256_from_be_bytes, uint256_bitwise_not and uint256_is_zero - Comparison is strictly greater, so tied tips do not cause two nodes to keep swapping - Height-based choice was wrong now that difficulty actually varies: a long low-difficulty branch beat a short high-difficulty one Atomic branch replacement: - Added Chain_ReplaceBranch: validate linkage, apply the reorg penalty, compare work, snapshot the outgoing blocks, roll back, apply. On any failure the original chain, balance sheet, supply, reward and difficulty target are restored. The caller keeps ownership of its blocks in every case — the chain applies copies - Split Chain_AddBlock and Chain_RollbackToHeight into locked public wrappers over unlocked internals, so a whole branch swap happens under one lock acquisition and Chain_OnTipAdvanced runs once per reorg rather than once per block - Chain_AddBlock now validates header.prevHash against the tip. It never did — that check lived only in Chain_IsValid and the network path, which is exactly why a rollback-then-reapply could splice blocks from two different forks into a chain that no longer links up - Moved the currentSupply/currentReward update into Chain_AddBlock. Each caller used to do it separately, so the orphan-attach and maintenance-thread paths never did, and the next block's coinbase was then validated against a stale currentReward and rejected forever. This was the height-2 stall Reorg penalty (Horizen-style delayed block submission): - The penalty was only ever reachable from the manual sync command. The P2P broadcast -> orphan pool -> branch adoption path, which is the path an attacker actually uses, had none at all and picked the winner by raw height. It is now enforced inside Chain_ReplaceBranch, the single choke point every adoption passes through - Removed its application to the sync fetch window. The height gap to a peer is not a reorg depth; penalising it only throttled honest catch-up, and for gaps of 4-50 it collapsed the window to one block per pass, defeating MAX_PARALLEL_FETCHES - The depth is stamped once when a branch is first observed (orphan_entry_t.observedAtTipHeight) and never recomputed. Re-deriving it from a moving tip never converges: depth and elapsed both grow by one per block while penalty(depth) grows faster, so a penalized branch could never be adopted at all - The initial-sync exemption now comes from Chain_IsInitialBlockDownload, which uses the local median time past over MEDIAN_TIME_SPAN blocks. It used to key off the peer's advertised height, so any peer claiming localHeight + INITIAL_SYNC_HEIGHT_DIFF could switch reorg handling off for the whole session. A median rather than the tip alone means one backdated block cannot fake it either Orphan pool (largely rewritten): - Added a pool mutex. It had no synchronisation whatsoever while being mutated from the 1 Hz maintenance thread, every per-peer TCP thread and the REPL thread; a concurrent insert could realloc the array while a scan held a raw element pointer. The lock is never held across a call into chain.c - Dedup by block hash, a MAX_ORPHAN_BLOCKS cap with oldest-first eviction, and pruning of entries that can no longer apply. Nothing was ever reaped before, and orphans are reachable before the chain-derived difficulty check, so this is also the memory-exhaustion fix - Candidate branches are now assembled by following prevHash from the fork point. Taking the first orphan found at each successive height could interleave blocks from two competing forks into one incoherent branch - Fixed rollbackHeight = forkHeight - 1. Chain_RollbackToHeight is exclusive, so every non-genesis adoption amputated one block too many and then failed Chain_AddBlock's index check - Fixed a block_t wrapper leak on every successful attach (free the wrapper, not Block_Destroy — the chain owns the transactions after a shallow copy) - Permanently invalid orphans are dropped instead of being retried on every maintenance tick forever Forks below the tip are now discoverable: - A block at blockNumber < chainSize was rejected and freed, so the fork point and the lower half of any competing branch were always thrown away and a sub-tip fork could never be learned. Now the hash is compared: identical means a duplicate and is ignored, different means it goes to the orphan pool - The sync loop probes downwards (RequestForkWindow, bounded by REORG_FETCH_DEPTH and MAX_FORK_PROBE_ROUNDS) when it makes no progress while the peer is ahead. That is the only trigger that fires for a genuine sub-tip fork, because the old divergence check could only see blocks that had already entered our chain. FETCH_BLOCK already answers from the peer's own chain, so no protocol change was needed - Removed the rollback-to-height-0 path. "Could not find the parent" used to wipe the entire local chain, genesis included, and any peer could trigger it with a single unlinked block Floating point removed from consensus and network math: - Chain_ComputeTargetAtHeight (the difficulty retarget) used double ratio arithmetic, and FetchScheduler_ComputeReorgPenaltyBlocks used double/pow/ceil. Both are consensus-critical and are now integer only; float results are not reproducible across platforms and compilers, and a single last-digit difference in a target or a penalty splits the network - The penalty constants became integer rationals (REORG_PENALTY_FACTOR_NUM/DEN, integer EXPONENT and REF_BLOCK_TIME) with saturating exponentiation and explicit ceiling division. Output is unchanged: penalty(4)=10, penalty(8)=39, penalty(10)=60, penalty(50)=1500, penalty(100)=6000 - Removed the unused float macros DAG_MAX_UP/DOWN_SWING_PERCENTAGE and the now-dead math.h include from constants.h. Both were latent: DAG size feeds PoW verification. Replaced with integer numerator/denominator constants and used them at both clamp sites (values verified identical) Other fixes that were blocking fork propagation: - madeProgressOverall was set but never reset, so after one productive pass the "no progress -> stop" guard could never fire again and the sync loop could spin forever holding the REPL - seenBlocks was inserted before/regardless of a successful send, so a block broadcast while no peer was connected was never offered again. It is now recorded only after the block actually goes out - Broadcasts relayed to outbound connections only, so in a two-node setup the dialled node never pushed anything back and the dialer learned of new blocks only via a manual sync. Inbound peers are now relayed to Verified with two-node harnesses at a shortened adjustment interval: - forced-orphan regression: was height 2 with 114 coinbase rejections, now reaches the peer's height with zero rejections and both nodes report Chain OK - sub-tip fork at depth 3: the node discovers the fork below its own tip, discards its three blocks, adopts the heavier five, and both nodes converge on an identical tip hash - deep fork at depth 8: the strictly heavier branch is correctly refused with depth=8 penalty=39 elapsed=0 - uint256 work arithmetic covered by a standalone test (division, big-endian conversion, monotonicity, halved target doubles work) One issue found along the way: the chain in build/chain_data does not pass fullverify. Block 7683 reverts to INITIAL_DIFFICULTY where it should carry 0x1f06df14, i.e. it contains blocks mined before `1288a64` landed. A binary built at HEAD rejects it too, so this is stale data rather than a regression — it needs a wipechain and a re-mine.	2026-07-28 00:24:45 +02:00
dcrubro	3337ac85ab	reorgs, fetch batching (parallel fetch), orphans	2026-05-15 13:01:27 +02:00

dcrubro

5eaf0b699c

Let a node that is behind adopt the peer's branch, and fix two bugs that only appear once a reorg actually applies

Reported: a node at height N+10 on its own short fork, with the peer at N+500,
could never sync. It located the fork point correctly, pooled the branch, and
then refused it forever. At 90s blocks a node is 960 blocks behind after a day
offline, so this is the normal case, not an edge case.

== Why it could never recover ==

The reorg delay was served by LOCAL chain growth alone:

    elapsed = tipHeight - observedTip

That only makes sense for a node whose tip is advancing. A node that is behind
has a frozen tip precisely because it is rejecting the branch, so elapsed stays
0 forever while penalty(5) = 42. Both escape hatches also fail: mining out of it
means extending a fork nobody accepts, and the IBD exemption never fires for a
node that is mining, because mining keeps its tip fresh.

Being 490 blocks behind is being behind, not a reorg contest. Refusing to adopt
protects nothing while the node falls a further 40 blocks behind every hour.

The delay is now satisfiable by EITHER side making progress:

    localGrowth = tipHeight - observedTip
    branchLead  = candidateTip - tipHeight        (0 if not ahead)
    elapsed     = max(localGrowth, branchLead)

A branch already extending `penalty` blocks past our tip has demonstrated
exactly what the delay asks for, and every one of those blocks carries PoW we
validated ourselves. Requiring us to independently produce the same amount is
demanding the same proof twice. Only VALIDATED blocks count -- a peer's
advertised height is not evidence and never reaches this code.

It degrades correctly in both directions: a mining node's tip advances, so an
attacker must outpace it by penalty blocks; a node that is only observing
follows the heaviest chain, which is what an observer should do.

== The window has to keep pulling the branch ==

Every forked delivery reset nextReq back to our own tip, so the window
re-requested the same eight heights forever and the pool never grew past the
window size -- branchLead could not rise even in principle. The backward walk
now runs ONCE to establish linkage, then the window marches forward pooling the
branch, retrying adoption per window-full with a final attempt when it drains.

== sync force ==

Operator override that skips the delay for one sync, for a node whose chain is
known to be the wrong one. Threaded explicitly (Chain_ReplaceBranch gains
bypassPenalty, OrphanPool_AttemptAttachForced) rather than through a global, so
nothing a peer sends can reach it. Linkage, work comparison and atomicity still
apply -- it waives only the waiting, and says so loudly in the log.

== Bug found in testing: PoW is branch-relative ==

A block's epoch seed is the last block of the previous epoch ON ITS OWN BRANCH.
Validating a competing branch's block against OUR epoch seed does not merely
fail to resolve: when the chains diverge before the boundary it resolves to the
WRONG seed and rejects a perfectly valid block. Any fork spanning an epoch
boundary was therefore impossible to assemble -- the branch could not grow past
the boundary block, so branchLead stalled one short of it.

The receive path now does self-contained checks only (Block_HasValidStructure:
merkle, transactions, vote, non-empty). Proof of work moved to
Chain_AddBlockLocked, at the point a block joins the chain, where the branch
context is real -- the rollback has put its ancestors in place by then. That is
where the invariant belongs and it removes a duplicate check rather than adding
one. Needs Chain_DagParamsForHeightLocked, because Chain_AddBlockLocked already
holds chainLock for writing and the lock is not recursive.

Consequence worth knowing: the orphan pool can now hold blocks whose work has
not been verified, bounded by MAX_ORPHAN_BLOCKS (512). Each still had to pass
merkle and full transaction/signature validation, and none can reach the chain
unverified.

This bug also affected plain forward sync across block 350000; it was masked
because appending keeps the boundary block in the chain.

== Bug found in testing: stale DAG accepted as current ==

Block_PowHashHeavy matched on epoch index and size but not the seed. A DAG's
content is a function of (seed, size); the epoch index is a label for it. A
reorg is exactly the event that changes the seed while leaving index and size
untouched, so mid-apply the miner's context still held a DAG built from the
PRE-reorg seed, the guard passed, and a valid block was hashed against the wrong
lanes. g_dagSeed now records what each DAG was generated from and both
Block_EnsureAutolykos2Dag and Block_PowHashHeavy compare it.

== Bug found in testing: double free on the failed-apply path ==

SIGABRT in the allocator: free_tiny_botch -> DynArr_destroy -> Block_Destroy ->
Chain_FreeBlockArray -> Chain_ReplaceBranch.

DynArr_push_back stores the struct BY VALUE, so the chain's element and the
caller's block_t share one transactions pointer. Three places free that array
through the chain's copy -- Chain_ClearBlocks, Chain_RollbackToHeightLocked and
Chain_SaveToFile -- and each NULLs only the chain's side, leaving any caller
wrapper dangling. Whether a caller then had to use free() or Block_Destroy() was
a convention carried in comments at every call site plus a consumed-count passed
into Chain_FreeBlockArray. Chain_ReplaceBranch reset that count to 0 after
rolling back a failed apply, which told the cleanup to Block_Destroy exactly the
blocks whose arrays the rollback had just freed.

Rather than fix the count, the aliasing is now safe by construction:
Chain_AddBlockLocked clears the CALLER's transactions pointer immediately after
the push. Since DynArr_destroy(NULL) is a no-op, free(wrapper) and
Block_Destroy(wrapper) become equivalent and both safe regardless of what later
frees the chain's copy. The consumed-count parameter and both counters are gone
-- the thing that could be got wrong no longer exists -- and all call sites are
unified on Block_Destroy.

Placement is deliberate: immediately after the push, not at the end on success.
The ledger pass can fail with the block already in the chain, returning false to
a caller that destroys its wrapper on failure -- OrphanPool_ExtendTip does
exactly that, a third live instance not yet triggered.

Two follow-ons the refactor forced, both improvements anyway: MineAndAppendBlock
read the coinbase for its log line after the add (hoisted above it), and the
success log printed the caller's block rather than the chain's copy.

== Also ==

The deferral line is rate-limited. The maintenance thread retries pooled
branches once a second and elapsed only changes when something moves, so it
printed an identical line every second -- forever, on a node that is not mining.
It now reports each distinct situation once.

== Verification ==

Synthetic fork, node A 5 deep, node B ~60 ahead, EPOCH_LENGTH=8 so the branch
crosses three epoch boundaries:

  branchLead climbs 8 -> 16 -> 28 -> 34, crosses penalty(5)=42
  Adopted competing branch of 50 block(s) at fork height 20
  sync complete: localHeight=70
  Chain OK

Repeated under AddressSanitizer: adopted 47 blocks, 0 ASan errors on both nodes.
This matters because the refactor rewrites the exact cleanup path the SIGABRT
came from, and a double free that no longer aborts would otherwise pass silently.

Unit suites pass, including a new assertion "heavy path refuses a DAG built from
a different seed" -- the direct regression for the stale-DAG bug.

Harness note: each node needs its OWN wallet. With a shared one both pay the
same coinbase address, produce identical merkle roots, and at easy difficulty
mine byte-identical blocks -- the fork test silently became a catch-up test.

== Still untested ==

The restore-after-failed-apply path is no longer naturally reachable now that
the two bugs above are fixed, so it needs deliberate corruption to exercise.
Test B (branch only slightly ahead must still DEFER), test C (sync force), and a
TSan pass over the changed paths are outstanding.

2026-08-02 19:36:09 +02:00

dcrubro

1ff2890c0f

Fix the reorg system: make rollback survivable, adopt by cumulative work, and enforce the reorg penalty everywhere

(This is a big one, get ready - I told Claude to write the commit message cause I couldn't be bothered)

Root cause: reorg was broken at every layer and the failures compounded. Verified with the node's own
SKALACOIN_FORCE_ORPHAN_REORG debug mode, which stalled permanently at height 2 on 114 consecutive
coinbase-validation failures. A binary built at HEAD behaves identically, so none of this is a regression —
the reorg path had simply never worked.

Rollback (the keystone):
 - Chain_RollbackToHeight always returned false on any node that had ever saved or loaded its chain, and
   only after it had already truncated the chain and destroyed the balance sheet. Chain_RecomputeRuntimeState
   bails on any header-only block, and Chain_SaveToFile nulls transactions on every in-memory block once
   persisted, so the failure was universal in practice. Both callers treated the false as "nothing happened"
 - Added Chain_BorrowBlockTransactions / Chain_ReturnBlockTransactions, which fall back to the on-disk copy
   when the in-memory block has been compacted to headers
 - Supply is now accumulated in the rollback's existing balance-sheet replay pass instead of a second
   Chain_RecomputeRuntimeState pass
 - Gave Chain_RecomputeRuntimeState the same disk fallback: it had been failing on every restart with an
   existing chain, silently leaving currentSupply/currentReward at whatever came out of chain.meta

Fork choice is now cumulative work, not height:
 - Added Chain_ComputeBlockWork / Chain_ComputeWorkRange / Chain_ComputeBranchWork, computing
   2^256 / (target + 1) per block and summing over a range. Derived on demand from headers — no header,
   chain.meta or wire-format change
 - uint256 only had add/sub/cmp, so added uint256_divide (restoring binary long division),
   uint256_from_be_bytes, uint256_bitwise_not and uint256_is_zero
 - Comparison is strictly greater, so tied tips do not cause two nodes to keep swapping
 - Height-based choice was wrong now that difficulty actually varies: a long low-difficulty branch beat a
   short high-difficulty one

Atomic branch replacement:
 - Added Chain_ReplaceBranch: validate linkage, apply the reorg penalty, compare work, snapshot the outgoing
   blocks, roll back, apply. On any failure the original chain, balance sheet, supply, reward and difficulty
   target are restored. The caller keeps ownership of its blocks in every case — the chain applies copies
 - Split Chain_AddBlock and Chain_RollbackToHeight into locked public wrappers over unlocked internals, so a
   whole branch swap happens under one lock acquisition and Chain_OnTipAdvanced runs once per reorg rather
   than once per block
 - Chain_AddBlock now validates header.prevHash against the tip. It never did — that check lived only in
   Chain_IsValid and the network path, which is exactly why a rollback-then-reapply could splice blocks from
   two different forks into a chain that no longer links up
 - Moved the currentSupply/currentReward update into Chain_AddBlock. Each caller used to do it separately, so
   the orphan-attach and maintenance-thread paths never did, and the next block's coinbase was then validated
   against a stale currentReward and rejected forever. This was the height-2 stall

Reorg penalty (Horizen-style delayed block submission):
 - The penalty was only ever reachable from the manual sync command. The P2P broadcast -> orphan pool ->
   branch adoption path, which is the path an attacker actually uses, had none at all and picked the winner
   by raw height. It is now enforced inside Chain_ReplaceBranch, the single choke point every adoption
   passes through
 - Removed its application to the sync fetch window. The height gap to a peer is not a reorg depth;
   penalising it only throttled honest catch-up, and for gaps of 4-50 it collapsed the window to one block
   per pass, defeating MAX_PARALLEL_FETCHES
 - The depth is stamped once when a branch is first observed (orphan_entry_t.observedAtTipHeight) and never
   recomputed. Re-deriving it from a moving tip never converges: depth and elapsed both grow by one per block
   while penalty(depth) grows faster, so a penalized branch could never be adopted at all
 - The initial-sync exemption now comes from Chain_IsInitialBlockDownload, which uses the local median time
   past over MEDIAN_TIME_SPAN blocks. It used to key off the peer's advertised height, so any peer claiming
   localHeight + INITIAL_SYNC_HEIGHT_DIFF could switch reorg handling off for the whole session. A median
   rather than the tip alone means one backdated block cannot fake it either

Orphan pool (largely rewritten):
 - Added a pool mutex. It had no synchronisation whatsoever while being mutated from the 1 Hz maintenance
   thread, every per-peer TCP thread and the REPL thread; a concurrent insert could realloc the array while a
   scan held a raw element pointer. The lock is never held across a call into chain.c
 - Dedup by block hash, a MAX_ORPHAN_BLOCKS cap with oldest-first eviction, and pruning of entries that can
   no longer apply. Nothing was ever reaped before, and orphans are reachable before the chain-derived
   difficulty check, so this is also the memory-exhaustion fix
 - Candidate branches are now assembled by following prevHash from the fork point. Taking the first orphan
   found at each successive height could interleave blocks from two competing forks into one incoherent branch
 - Fixed rollbackHeight = forkHeight - 1. Chain_RollbackToHeight is exclusive, so every non-genesis adoption
   amputated one block too many and then failed Chain_AddBlock's index check
 - Fixed a block_t wrapper leak on every successful attach (free the wrapper, not Block_Destroy — the chain
   owns the transactions after a shallow copy)
 - Permanently invalid orphans are dropped instead of being retried on every maintenance tick forever

Forks below the tip are now discoverable:
 - A block at blockNumber < chainSize was rejected and freed, so the fork point and the lower half of any
   competing branch were always thrown away and a sub-tip fork could never be learned. Now the hash is
   compared: identical means a duplicate and is ignored, different means it goes to the orphan pool
 - The sync loop probes downwards (RequestForkWindow, bounded by REORG_FETCH_DEPTH and MAX_FORK_PROBE_ROUNDS)
   when it makes no progress while the peer is ahead. That is the only trigger that fires for a genuine
   sub-tip fork, because the old divergence check could only see blocks that had already entered our chain.
   FETCH_BLOCK already answers from the peer's own chain, so no protocol change was needed
 - Removed the rollback-to-height-0 path. "Could not find the parent" used to wipe the entire local chain,
   genesis included, and any peer could trigger it with a single unlinked block

Floating point removed from consensus and network math:
 - Chain_ComputeTargetAtHeight (the difficulty retarget) used double ratio arithmetic, and
   FetchScheduler_ComputeReorgPenaltyBlocks used double/pow/ceil. Both are consensus-critical and are now
   integer only; float results are not reproducible across platforms and compilers, and a single last-digit
   difference in a target or a penalty splits the network
 - The penalty constants became integer rationals (REORG_PENALTY_FACTOR_NUM/DEN, integer EXPONENT and
   REF_BLOCK_TIME) with saturating exponentiation and explicit ceiling division. Output is unchanged:
   penalty(4)=10, penalty(8)=39, penalty(10)=60, penalty(50)=1500, penalty(100)=6000
 - Removed the unused float macros DAG_MAX_UP/DOWN_SWING_PERCENTAGE and the now-dead math.h include from
   constants.h. Both were latent: DAG size feeds PoW verification. Replaced with integer numerator/denominator
   constants and used them at both clamp sites (values verified identical)

Other fixes that were blocking fork propagation:
 - madeProgressOverall was set but never reset, so after one productive pass the "no progress -> stop" guard
   could never fire again and the sync loop could spin forever holding the REPL
 - seenBlocks was inserted before/regardless of a successful send, so a block broadcast while no peer was
   connected was never offered again. It is now recorded only after the block actually goes out
 - Broadcasts relayed to outbound connections only, so in a two-node setup the dialled node never pushed
   anything back and the dialer learned of new blocks only via a manual sync. Inbound peers are now relayed to

Verified with two-node harnesses at a shortened adjustment interval:
 - forced-orphan regression: was height 2 with 114 coinbase rejections, now reaches the peer's height with
   zero rejections and both nodes report Chain OK
 - sub-tip fork at depth 3: the node discovers the fork below its own tip, discards its three blocks, adopts
   the heavier five, and both nodes converge on an identical tip hash
 - deep fork at depth 8: the strictly heavier branch is correctly refused with depth=8 penalty=39 elapsed=0
 - uint256 work arithmetic covered by a standalone test (division, big-endian conversion, monotonicity,
   halved target doubles work)

One issue found along the way: the chain in build/chain_data does not pass fullverify. Block 7683 reverts to
INITIAL_DIFFICULTY where it should carry 0x1f06df14, i.e. it contains blocks mined before 1288a64 landed. A
binary built at HEAD rejects it too, so this is stale data rather than a regression — it needs a wipechain
and a re-mine.

2026-07-28 00:24:45 +02:00

dcrubro 3337ac85ab reorgs, fetch batching (parallel fetch), orphans 2026-05-15 13:01:27 +02:00