Bound fluxnode block-index memory with a file-backed arena - #285
Closed
MorningLightMountain713 wants to merge 50 commits into
Closed
Bound fluxnode block-index memory with a file-backed arena#285MorningLightMountain713 wants to merge 50 commits into
MorningLightMountain713 wants to merge 50 commits into
Conversation
Reverts 85d252e to make way for the incremental PersistToDisk approach with full crash recovery. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…odes Fluxnode cache dirty entries were accumulated for up to 24 hours and flushed in a single operation holding cs_main. On memory-constrained CUMULUS nodes the fluxnode maps get swapped out, causing thousands of page faults during the flush — stalling RPC for minutes and triggering watchdog kills. Two changes: 1. Incremental fluxnode persistence: new PersistToDisk() writes dirty entries to LevelDB every 10 blocks using a batched write. Only holds the fluxnode lock, not cs_main. DumpFluxnodeCache is removed from the periodic flush path (kept only for shutdown via fForce). 2. Block index solution pruning: on fluxnodes, clear equihash solutions from PoW block index entries after load. Serialization paths (RPC, REST, P2P getheaders) fall back to reading from block files. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The block index solution pruning needs fFluxnode to be set before LoadBlockIndexDB runs. Move the assignment from after genesis wait to before LoadBlockIndex in the init sequence. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move nNonce, nSolution, nodesCollateral, and vchBlockSig out of CBlockIndex into a separately-allocated HeaderData struct. On fluxnodes, this allocation is freed for all block index entries after load, saving ~116 bytes per entry across 2.5M blocks (~290 MB). hashMerkleRoot and hashFinalSaplingRoot remain in the core struct as they are accessed during consensus validation and reorgs. All serialization paths (RPC, REST, P2P getheaders) fall back to reading from block files when HeaderData is null. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move hashSproutAnchor, hashFinalSproutRoot, nSproutValue, nChainSproutValue, nSaplingValue, nChainSaplingValue, and nCachedBranchId into CBlockIndex::HeaderData alongside the proof fields. On fluxnodes, HeaderData is freed for blocks deeper than 100, saving ~244 bytes per entry across 2.5M blocks (~610 MB). The 100-block retention depth provides margin beyond the max reorg depth of 40 blocks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Missed access sites for hashFinalSproutRoot, nChainSproutValue, nSproutValue, nChainSaplingValue, nSaplingValue in getblock and getblockchaininfo RPCs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The pruning was inside LoadBlockIndexDB which runs before block rewinding. Rewinding accesses HeaderData fields on blocks that had already been pruned, causing a crash loop. Move to init.cpp after ActivateBestChain completes so all chain operations are done before we free the data. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
jemalloc is significantly better than glibc malloc at returning freed memory to the OS and avoiding fragmentation. This is critical for the HeaderData split where 2.5M allocations are freed after load — glibc retains the freed pages in its arena, jemalloc returns them. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
These fields are only accessed during reorgs (DisconnectBlock) and wallet merkle verification — both limited to recent blocks where HeaderData is retained. Moving them saves 64 bytes per entry across 2.5M blocks (~160 MB) on fluxnodes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…Data split Replace individual new/delete of CBlockIndex with a contiguous mmap-backed pool allocator. Benefits: - Zero per-element malloc overhead across 2.5M+ entries - Contiguous layout: old blocks at low addresses, recent at high - MADV_COLD hint tells the kernel which pages are cold after sync - OS manages working set: pages out old blocks under memory pressure - No heap fragmentation from millions of small allocations This reverts the HeaderData struct split (commits 1798652..7ed2691) which added null checks on every access site and caused build/crash issues. The mmap pool achieves the same goal (OS-managed memory for old blocks) without changing the CBlockIndex field layout. Also: - Reserve mapBlockIndex capacity before loading to avoid rehashes - Store block hashes in parallel pool array instead of relying on pointer-into-map-key (safer, pool-controlled memory) - Graceful fallback to heap allocation if mmap fails Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The git checkout reverted chain.h but main.cpp retained pHeaderData references from the earlier HeaderData commits. Restore direct field access on CBlockIndex. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Combines the mmap pool allocator with the HeaderData struct separation. The pool provides contiguous allocation with OS-managed paging for old blocks. HeaderData provides deterministic memory reduction by freeing the extended fields for buried blocks on fluxnodes. Together: the pool eliminates malloc overhead and fragmentation, jemalloc returns freed HeaderData pages to the OS, and MADV_COLD hints the kernel about cold pool pages. CBlockIndex shrinks from 424 to ~112 bytes for pruned entries. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use AC_CHECK_LIB to probe for jemalloc at configure time. Enabled by default, falls back gracefully with a warning if not found. Follows the same pattern as the existing Proton and ZMQ dependencies. Build with --disable-jemalloc to explicitly skip it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…very CMainCleanup's destructor called delete on CBlockIndex pointers that were placement-new'd into mmap pool memory, not heap-allocated. This caused SEGV/SIGABRT in jemalloc/glibc during process exit. Fix by using DestroyAll to run destructors on pool entries, then freeing the pool itself. Add RecoverFluxnodeCache startup check: compares the FluxnodeSyncState marker (written by PersistToDisk) to the chain tip. On mismatch (unclean shutdown, power cut, OOM kill), disconnects the stale blocks and lets ActivateBestChain reconnect them through normal ConnectBlock, rebuilding the fluxnode cache correctly. Cost: a few seconds to replay ≤10 blocks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When the sync state block is on a different fork (crash happened mid-reorg), walk back from the sync state block through pprev to find the common ancestor with the active chain, then disconnect to there. ActivateBestChain reconnects everything through normal ConnectBlock, rebuilding the cache. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Build jemalloc 5.3.0 as a static library through the depends system so it's available when configure checks for it. Linux only — Windows and macOS fall back to their system allocator via the existing configure --disable-jemalloc path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Build jemalloc without --with-jemalloc-prefix so it replaces global malloc/free. Update configure.ac to check for malloc_stats_print (unprefixed) instead of je_malloc_stats_print. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
jemalloc as a malloc replacement needs dynamic linking for proper symbol interposition. Use the system libjemalloc.so (from the libjemalloc2 apt package) instead of building a static library in depends. The deb package will declare libjemalloc2 as a dependency. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PersistToDisk was writing a sync state referencing the current chain tip, but the block index is flushed to disk on a different schedule. On crash, the sync state could reference a block not yet in the persisted block index, making recovery impossible. Track hashLastBlockIndexWrite — updated when FlushStateToDisk writes the block index. PersistToDisk uses this for the sync state, so it can never reference a block that isn't on disk. The cache may be ahead of the sync state, but RecoverFluxnodeCache handles that by disconnecting back to the sync state and replaying forward. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PersistToDisk was called independently from ConnectTip/DisconnectTip, which allowed the fluxnode cache to be persisted ahead of the block index. On crash, the cache referenced blocks not yet in the persisted index, causing validation failures on restart. Move PersistToDisk into FlushStateToDisk, right after the block index write. The cache and block index are now always flushed together — the sync state can never reference a block that isn't on disk. Benchmarked: batched write of 2,000 entries takes <3ms even on CUMULUS under memory pressure. No RPC impact from holding cs_main. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fields moved to pHeaderData in the HeaderData split were still accessed directly in AcceptBlock and ConnectBlock — auto-merged without conflict markers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…; fix pruning audit bugs Replace the fixed 5M-capacity anonymous mmap pool with a sparse, file-backed arena (MAP_SHARED to scratch files in the datadir): - Cold block-index pages evict to the backing file, not swap, so RSS stays bounded to the working set on low-RAM nodes with no swap dependency. - The capacity reservation is a sparse ftruncate (100M entries): no disk/RAM until pages are touched, removing the hard 5M ceiling. Exhaustion now falls back to per-entry heap allocation instead of aborting (with a warning), and a 90%-full warning lets a capacity bump be scheduled. - The arena (and the existing header-data pruning) are now gated on fFluxnode, so non-fluxnodes use plain heap allocation (master behavior) and never create backing files. - Backing files are scratch: O_TRUNC at startup, unlinked on shutdown. Fix audit findings from the header-data pruning (pHeaderData == nullptr for buried blocks): - P2P getheaders served zeroed merkle roots / nonces for pruned blocks on both the compact and regular paths. New GetFullBlockHeader() re-reads the header from disk when pruned. - DisconnectBlock dereferenced pprev->pHeaderData->hashFinalSaplingRoot without a null check; restore it from disk first. - ConnectBlock and the ReceivedBlockTransactions descendant loop dereferenced pHeaderData on potentially-pruned blocks; ensure it is allocated (and header fields restored from the block in hand) before use. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…uring load Two related changes that make header-data pruning correct-by-construction and bound the init memory transient: 1. Move nSproutValue/nChainSproutValue/nSaplingValue/nChainSaplingValue out of the prunable HeaderData and onto CBlockIndex directly. These cannot be rebuilt from the block on disk (the cumulative fields accumulate across the whole chain) and feed ZIP209 turnstile enforcement when enabled, so they must stay resident. They are small and live in the arena, so cold entries still page out. On-disk serialization is unchanged (same bytes/order, just read into the new field location) — no reindex. This removes the prior reliance on ZIP209 being disabled. 2. Free header data during the block-index load (LoadBlockIndexGuts), right after the per-entry consistency checks, instead of accumulating every entry's header data and pruning only after ActivateBestChain. That accumulation was the multi-GB init RSS spike. Header data is rebuilt from disk on demand (connect/disconnect/serve). Fluxnode-gated. The post-ActivateBestChain prune loop stays as a backstop for header data rehydrated during a long reconnect. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…h-loop)
The previous commit freed HeaderData during load but left nCachedBranchId,
hashSproutAnchor, and hashFinalSproutRoot in the prunable HeaderData. These are
read by init-time checks that run after load but before ActivateBestChain —
notably RewindBlockIndex, which compares each block's nCachedBranchId against
the expected branch id. With header data freed during load, every block looked
unvalidated, triggering a full-chain rewind and abort ('Please restart with
-reindex'), crash-looping on startup.
Move all three onto CBlockIndex directly, alongside the value-pool fields, per
the principle: HeaderData holds only block-file data that can be rebuilt from
disk (merkle/saplingroot/nonce/solution/collateral/sig); everything the node
computes or validates about a block stays resident. On-disk serialization
order is unchanged (nCachedBranchId and hashSproutAnchor were already serialized
in that position) — no reindex.
Also rehydrate hashFinalSaplingRoot from disk in the wallet rescan path, the
last unguarded prunable-field access.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace the named blockindex.arena/.hashes scratch files with anonymous O_TMPFILE inodes on the datadir's filesystem: - No directory entry, so invisible to ls/du and backup/snapshot tooling (nothing to remember to exclude), and the alarming sparse apparent-size no longer shows anywhere. - The kernel reclaims the disk blocks automatically when the process exits, clean or crashed — no orphaned scratch files (previously a crash left them behind to clean up manually), no O_TRUNC-on-startup. - Same memory behavior: still a real disk-backed inode, so cold dirty pages evict to it (not swap) and RSS stays bounded with no swap dependency. If O_TMPFILE is unsupported (old kernel/exotic FS), open() fails, Initialize() returns false, and the caller falls through to the existing heap allocation path — no separate open+unlink fallback needed. Also trim POOL_CAPACITY from 100M to 30M entries (~25+ years of headroom), shrinking the virtual reservation (and the heap fallback still prevents any hard ceiling). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- main.cpp: statvfs free-space check before creating the arena. The sparse
ftruncate can't surface low disk at allocation time (it allocates no blocks),
so guard the startup case: if the datadir filesystem has <2 GiB free, use heap
allocation instead of the arena. Runtime disk-fill remains a benign residual
(SIGBUS -> crash -> rebuild from leveldb on ext4/xfs, same as any node unable
to write chainstate on a full disk).
- New gtests (src/gtest/test_blockindexpool.cpp):
* CBlockIndexPool alloc/Contains/HashAt, exhaustion->nullptr (the heap-fallback
trigger), DestroyAll, and Initialize-failure->false (the other fallback path).
* Prune frees only block-file HeaderData and KEEPS resident consensus state
(nCachedBranchId, value pools, sprout anchors) — RewindBlockIndex crash-loop
regression test.
* GetBlockHeader zeroed-after-prune contract; copy ctor deep-copy.
* CDiskBlockIndex serialization round-trip — guards no-on-disk-format-change.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ization test The hand-built CBlockIndex needs a valid phashBlock (CDiskBlockIndex ctor reads pindex->GetBlockHash()), and since CBlockIndex carries prev via pprev rather than storing hashPrevBlock, the fixture's header prev must be null to match the null pprev for the reconstructed-hash check. All 6 arena/HeaderData tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…unks Replace the single big sparse O_TMPFILE mapping with a segmented, named scratch file grown on demand: - blockindex.arena grows ~128 MiB at a time (ftruncate) and is mapped as fixed mmap windows at increasing offsets. Existing windows never move, so the millions of CBlockIndex* into them stay valid. File size and virtual size now track actual chain size — no fixed reservation, no hard ceiling, and VSZ is ~hundreds of MiB instead of an 8.6 GiB up-front mapping. - Each slot inlines the 32-byte block hash right after the CBlockIndex, so a single file carries both (hash shares a page with its index) and the public API (AllocateEntry/HashAt/Contains/...) is unchanged — InsertBlockIndex does not change. - Named (not O_TMPFILE) so the file is visible/debuggable and df/du agree; segmentation removed the alarming apparent-size that O_TMPFILE was hiding. Cleanup: opened O_TRUNC at startup (discards any file left by a crash; the arena is always rebuilt from leveldb) and unlinked on clean shutdown. - A chunk that can't be mapped (disk full / unsupported FS) -> AllocateEntry returns nullptr / Initialize false -> heap fallback, no abort. gtests updated for cross-chunk allocation; all 6 arena/HeaderData tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…entries Fixes M2 and M6 from the memory-branch adversarial review (MEM_REVIEW_FINDINGS.md): M2: FlushStateToDisk could null-deref pHeaderData when a dirty entry had been pruned (CDiskBlockIndex serializes through pHeaderData). Reachable via invalidateblock/reconsiderblock on a block buried >100 deep, and via startup catch-up when the chainstate lags the block index. The flush loop now restores header data from disk for dirty-but-pruned entries before serialization, and aborts the node if the block cannot be read — never writing zeroed header fields over a good record. Entries restored only for the write are re-pruned afterwards so a deep invalidateblock cannot re-accumulate header data in memory. M6: EnsureHeaderDataFromDisk fabricated an empty HeaderData when the disk read failed, which silently popped a zero Sapling anchor in DisconnectBlock and, once the entry was dirtied, wrote zeroed fields over the good leveldb record. It now returns failure and DisconnectBlock aborts (matching the missing-undo-data treatment). Both prune sites (load-time and post-init) now skip entries lacking BLOCK_HAVE_DATA: header-only entries cannot be rebuilt from disk so they must stay resident. The field-copy restore is factored into CBlockIndex::RestoreHeaderData (also used by the header ctor) with a gtest asserting a pruned-then- restored entry serializes byte-identically to a never-pruned one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rker Fixes M7 from the memory-branch adversarial review (MEM_REVIEW_FINDINGS.md): PersistToDisk never read its fForce parameter and returned early whenever the dirty sets were empty. The stale-marker repair in RecoverFluxnodeCache calls PersistToDisk(tip, true) at init — when the cache is clean that silently no-op'd, leaving the stale marker in place, so the node re-entered recovery on every restart. The forced path now proceeds to the batch write (marker plus any dirty data) even when nothing is dirty. M1's recovery rewind depends on this semantics. Also deletes the dead no-arg DumpFluxnodeCache(): it persisted cache data WITHOUT the sync marker — exactly the marker/data divergence the recovery design forbids — and had no callers. The default argument on fForce is removed; every call site passes it explicitly. New gtest covers both paths: clean+unforced writes no marker, clean+forced writes the marker. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fixes M11 from the memory-branch adversarial review (MEM_REVIEW_FINDINGS.md): the undo record was written only when one of vecExpiredDosData / vecExpiredConfirmedData / mapUpdateLastConfirmHeight / mapLastPaidHeights was non-empty. mapOldDelegates (populated when a START tx updates delegates for a node with pre-existing delegates, and serialized in the undo record since its introduction) was not checked. A block whose only fluxnode content is such a delegate update wrote NO undo record; on disconnect the mapOldDelegates lookup miss takes the "new addition -> erase" branch and permanently erases delegates it should have restored. mapLastIpAddress is added to the condition for symmetry (today it is only populated alongside mapUpdateLastConfirmHeight, so it is already covered, but the condition should not silently rely on that pairing). This hole pre-exists on master (same condition, same serialized fields); it matters more on this branch because the recovery design relies on "missing undo record == legitimately empty". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…chain hashes Fixes M12 from the memory-branch adversarial review (MEM_REVIEW_FINDINGS.md): CleanupOldFluxnodeData kept only undo records whose block hash appeared in the last ONE_WEEK_OF_BLOCK_COUNT (5040) ACTIVE-chain blocks. After a reorg away from a fork, the next cleanup pass (runs roughly every 10 blocks) erased the fork side's undo records — possibly minutes after the reorg and before the next periodic sync- marker write. A crash in that window leaves the marker pointing at the fork with its undo records gone, so the fork-case recovery rewind (M1) would silently under-rewind. Retention is now by height: a record survives while its block (looked up in mapBlockIndex, any chain) is within 5040 blocks of the tip. Records for blocks unknown to the block index are still erased — such a block can never be disconnected, so its record is unreachable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…active chain Fixes M1 (HIGH) from the memory-branch adversarial review; implements the design validated 2026-06-10 (MEM_REVIEW_FINDINGS.md §M1). The periodic write path commits the fluxnode DB and its sync marker at the in-memory tip every 15-20 minutes, while the coins DB flushes ~24h apart. A crash in that window restarts with chainActive BEHIND the marker. Old recovery treated marker-ahead as success (common ancestor == tip -> nothing to disconnect -> return true), so ActivateBestChain replayed blocks onto a cache that already contained their effects: UPDATE_CONFIRM undo records rebuilt from the already-updated cache overwrote the correct on-disk records (permanent corruption), starts were re-inserted into the tracker, and the confirm path reset nLastPaidHeight (payout-order divergence). The fork case had the same hole: fork-side effects were never rewound. Recovery now: - factors the fluxnode portion of DisconnectBlock into DisconnectFluxnodeOnly (undo-record read -> AddBackUndoData -> CheckForUndoExpiredStartTx -> reverse-tx loop -> delegate push, in exactly that order; no coins/chainstate access); - walks the MARKER's chain from the marker down to the common ancestor with the active chain, undoing each block's fluxnode effects into a fresh local cache flushed per block (per-block Flush is required: setAddToConfirmHeight semantics and AddBackUndoData's already-in- local guard both assume it, mirroring DisconnectTip); - runs the existing chainstate disconnect loop unchanged (verified convergent on a cache that never applied those blocks); - persists the repaired state with PersistToDisk(tip, fForce=true) (depends on the M7 fix) and clears the RPC list cache; - hard-fails (was: return true with a corrupt cache) on missing common ancestor, unreadable block, or a rewind deeper than the 5040-block undo retention window (matching M12), telling the operator to -reindex. init now aborts BEFORE ActivateBestChain on recovery failure - previously the error string was only checked afterwards, by which point ABC had already replayed onto the corrupt cache and overwritten good undo records. Crash mid-recovery is safe: phase 1 mutates only the in-memory cache (disk marker unchanged until the final atomic batch), so recovery re-runs identically on the next start. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fixes M3 from the memory-branch adversarial review (MEM_REVIEW_FINDINGS.md): GetDepthInMainChainINTERNAL compared the tx's merkle branch against pHeaderData->hashMerkleRoot, substituting uint256() when the header data had been pruned. CheckMerkleBranch never returns zero for a real tx (nIndex == -1 already early-returns), so the check always failed for txs buried in pruned-header blocks: depth 0, and GetDepthInMainChain turned that into -1 — every confirmed wallet tx reported as CONFLICTED. fMerkleVerified is memory-only and vMerkleBranch is serialized, so this re-fired on every restart of a fluxnode with a non-empty wallet (the wallet is not disabled by -zelnode, only by -prune). Uses the same read-from-disk fallback as the rescan path (wallet.cpp ChainTipAdded caller); the read happens once per wtx per session (fMerkleVerified caches the result). A failed disk read keeps depth 0 rather than verifying against garbage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…reads Fixes M4 (HIGH) from the memory-branch adversarial review (MEM_REVIEW_FINDINGS.md): serving getheaders (160/message) or cmpheaders (2000/message) from a fluxnode's pruned block index called GetFullBlockHeader per entry, which fell back to ReadBlockFromDisk — deserializing EVERY transaction in each block and re-running the Equihash/PoW or PON proof check — all under cs_main. A single syncing peer could pin cs_main for seconds per message and turn a tiny request into tens-to-hundreds of MB of disk reads (cheap remote amplifier). The compact path was reading full blocks to recover an nSolution that CCompactBlockHeader then omits. A CBlock on disk serializes its CBlockHeader base first, so the new ReadBlockHeaderFromDisk deserializes only the header prefix at nDataPos: no transaction parsing, no proof recheck (the block was fully validated at accept; the reconstructed hash is still verified against the index entry, the same integrity check the full read performed). GetFullBlockHeader and EnsureHeaderDataFromDisk now use it, which also removes the full-block read from DisconnectBlock's pruned-pprev anchor restore and the M2 flush-side guard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ives pruning Fixes M5 from the memory-branch adversarial review (MEM_REVIEW_FINDINGS.md): CBlockIndexWorkComparator computed GetPONHash(GetBlockHeader()) per comparison. For entries whose header data was pruned, GetBlockHeader() returns a zeroed nodesCollateral — the distinguishing PON input — so after a restart (load prunes ALL entries before they are inserted into setBlockIndexCandidates), equal-work PON ties could resolve opposite the rest of the network. Worse, ConnectBlock/DisconnectBlock restore header data on entries that may still sit in the candidate set: an in-place comparator-key change violates strict weak ordering and lets erase-by-key silently fail when an equal-chainwork same-height sibling coexists. The PON hash (32 bytes) is now a resident memory-only CBlockIndex member, computed where the header is guaranteed complete: at index load (it was already being computed there for the proof check, before the prune) and at entry creation (AddToBlockIndex, cmpheaders). The comparator reads the cached value, so its key never depends on pHeaderData and never mutates. No disk-format change: CDiskBlockIndex serialization is untouched (round-trip gtest unchanged and green). Note: trial/mem-on-pr284 is immune (resident nodesVrfOutput); this fix is specific to this branch's HeaderData split. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…op stale arena warning Fixes M8 and M9 from the memory-branch adversarial review (MEM_REVIEW_FINDINGS.md). M9: the pruned-header disk fallback was hand-rolled in five places with already-divergent behavior (rest.cpp best-effort, getblockheader hex-mode throwing only on one branch, blockheaderToJSON field-by-field with up to two full-block reads per header, wallet assert-style). GetFullBlockHeader is now exported via main.h with one deliberate contract: it fills the header (re-reading the header prefix from disk when pruned or when nSolution was omitted) and returns false on a failed read, leaving the partial in-memory view in place. P2P/REST serving stays best-effort; getblockheader hex mode throws RPC_INTERNAL_ERROR on any failed read; blockheaderToJSON emits empty strings (as before); the wallet sites fail their respective checks. All callers now benefit from the M4 header-prefix read instead of full-block deserialization. M8: delete the "arena over 90% full, bump POOL_CAPACITY" warning. It fired once on every mainnet fluxnode at ~90% of the FIRST 128MiB chunk and named a constant that no longer exists — the segmented arena grows on demand. Real exhaustion still warns via the heap-fallback path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Exercises RecoverFluxnodeCache end-to-end by rewriting the sync marker directly in determ_zelnodes (plyvel) between restarts: - clean restart skips recovery - stale marker is repaired exactly once (M7: the forced persist must write the marker even with a clean cache — asserted by reading the marker back from leveldb) and the next restart skips recovery - marker behind the active tip triggers the chainstate disconnect and the node replays back to the same tip - marker on a stale fork triggers the fluxnode-only rewind along the marker's chain (M1 phase 1) plus the chainstate disconnect, and the node converges to the best tip with the marker re-anchored there Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… completion fails Fixes a regression introduced by the M9 consolidation, caught by the per-fix gtest run (five wallet tests newly failing: FindUnspentSproutNotes, SproutNullifierIsSpent, SaplingNullifierIsSpent, NavigateFromSaplingNullifierToNote, SpentSaplingNoteIsFromMe). The consolidated GetFullBlockHeader returned false whenever any disk read failed — including the nSolution-completion read for a RESIDENT POW header with an empty solution. Callers that only need the header fields (the wallet merkle-depth check, the rescan sapling-anchor lookup) then treated a perfectly valid resident header as missing. Before M9 those sites read the resident fields directly and never touched disk. The gtest wallet fixtures (in-memory blocks, empty nSolution, no block files) hit exactly this. GetFullBlockHeader now reports failure only when the header FIELDS are unavailable (pruned entry and the disk read failed); a resident header whose omitted nSolution could not be completed is returned as success with the solution left empty — matching the pre-M9 per-field behavior of every call site. The two wallet sites go back to using the resident fields directly (no disk access at all when resident), falling back to the cheap ReadBlockHeaderFromDisk only when pruned. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…B snapshots The first version rewrote the sync marker with plyvel, but writes from a modern plyvel/leveldb produce a MANIFEST the daemon's older bundled leveldb silently ignores — fluxd never saw the rewritten marker (reads are compatible, writes are not; verified by writing ff..ff, reading it back with plyvel, and watching the daemon still report "sync state matches chain tip"). The test now uses directory snapshots of determ_zelnodes taken between restarts (only fluxd's own writes), a second never-connected node to supply the unknown-marker DB for the stale-marker scenario, and keeps plyvel strictly read-only for the marker assertions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Caught by the new crash-recovery regtest: every startup "recovered" cleanly even when the on-disk marker pointed somewhere else entirely. FlushStateToDisk passed fForce=true to PersistToDisk. Now that fForce is honored, that combination writes the sync marker even when the fluxnode cache is clean — and FlushStateToDisk also runs during init BEFORE RecoverFluxnodeCache (RewindBlockIndex ends with an unconditional FLUSH_STATE_ALWAYS). The forced write overwrote the on-disk marker with the current in-memory tip, destroying the marker/chain divergence that tells recovery a crash happened and leaving the stale fluxnode DB state in place unrepaired. The flush path now persists unforced: dirty data still goes out with the marker in the same atomic batch, and a clean cache leaves the marker untouched. The forced write remains where it is the point — recovery's stale-marker repair and post-rewind persist, and the manual RPC flush. Also rewords test comments to drop review-shorthand and historical phrasing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Follow-up to the previous commit: leaving the flush path permanently unforced meant that on a chain with no fluxnode transactions (regtest, quiet chains) the cache is never dirty, so the sync marker would never be written at all — recovery then has nothing to verify and the marker-tracking guarantees degrade to nothing. A new fFluxnodeCacheRecovered flag is set right after RecoverFluxnodeCache succeeds at init. Flushes before that point persist unforced (they must not overwrite the marker the recovery is about to read); flushes after it force the marker write, keeping the marker at the tip even when no fluxnode data is dirty. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nale Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… plyvel Merely OPENING a leveldb directory with modern plyvel compacts the write-ahead log into snappy-compressed table files. The daemon's bundled leveldb is built without snappy, so on the next start it dies in AppInit with "corrupted compressed block contents" — printed only to stderr, which the test framework swallows, leaving -rpcwait hanging forever. (Diagnosed from the file modes: the poisoned table was 0644, plyvel's umask, among the daemon's 0600 files.) Marker assertions now copy the DB directory to a throwaway path and open the copy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rges Two isolated regtest nodes mine identical block hashes, so node1's "foreign" tip was actually a block on node0's active chain — recovery correctly treated the marker as merely behind (disconnect/replay) instead of taking the stale-marker path the scenario asserts. Running node1 on -mocktime gives its blocks different timestamps and therefore hashes node0 has never seen. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… fork Deterministic regtest mining strikes again: re-mining at the height of a just-invalidated block reproduces that exact block (same parent, same coinbase, same timestamp), which the daemon rejects as already-invalid. Mock the clock forward for chain B's blocks so they hash differently. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ives restart Blocks marked invalid by invalidateblock do not survive a restart in the block index: LoadBlockIndexDB skips nCachedBranchId reconstruction for blocks failing IsValid(BLOCK_VALID_CONSENSUS), so RewindBlockIndex deems them insufficiently validated and erases them. The fork scenario then exercised the stale-marker repair path instead of the marker-chain rewind. A real crash-during-reorg leaves the losing fork's blocks fully valid, so reconsiderblock the fork tip after mining the heavier chain — the failure flags clear, no reorg happens (chain B has more work), and the fork block survives the restart as recovery expects. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ardown restart
node1's blocks carry far-future timestamps from its mocked clock; the
final courtesy restart for framework teardown launched it with the real
clock, so startup verification rejected its own chain ("Corrupted block
database detected") and the framework's -rpcwait hung until timeout.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
MorningLightMountain713
marked this pull request as draft
June 12, 2026 06:32
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Keeps a fluxnode's resident memory bounded as the chain grows, with no swap dependency and no on-disk-format change. The block index (one
CBlockIndexper block, ~2.67 M and counting) currently lives in anonymous heap memory that can only be reclaimed to swap and grows linearly forever. This moves the block index into a segmented, file-backed arena (a single named scratch file,blockindex.arena, grown ~128 MiB at a time), so cold index pages evict to disk under memory pressure instead of being pinned in RAM. All memory-optimization behavior is gated onfFluxnode.Measured (same height, 62 GB host, uncapped):
salad)squidward)VmRSSRssAnon(pinned, needs swap)RssFile(reclaimable, no swap)Under a 500 MB
MemoryHighcgroup cap withMemorySwapMax=0, this branch ran fully synced with zero swap used; a stock node cannot fit in that envelope.Motivation
mapBlockIndexholds every block'sCBlockIndexfor the process lifetime. On a stock build these are heap allocations — anonymous memory — so:Allocator tuning can't help (the memory is live). The only way to bound it without a full handle-based rewrite is to let the OS page cold entries to a backing store — and to avoid swap, that store must be a file.
What changed
All memory-optimization paths are gated on
fFluxnode.CBlockIndexsplit into resident state vs. rebuildable block-file data.n[Chain]SproutValue/n[Chain]SaplingValue),nCachedBranchId, Sprout anchors (hashSproutAnchor/hashFinalSproutRoot).HeaderData(rebuildable from the block on disk): merkle root, final sapling root, nonce, solution, nodes-collateral, block-sig.Skeleton allocated from a segmented, file-backed arena. A single named scratch file (
blockindex.arena,MAP_SHARED); each slot is aCBlockIndex+ its 32-byte hash. Cold pages evict to the file (not swap). The file grows ~128 MiB at a time (ftruncate+ fixed mmap windows at increasing offsets; windows never move, so pointers stay valid), so file size and VSZ track actual usage — no fixed reservation, no hard ceiling (VSZ ~1.9 GiB vs ~8.6 GiB for an earlier single big sparse mapping). Cleanup:O_TRUNCat startup (discards crash leftovers; always rebuilt from leveldb),unlinkon clean shutdown. A chunk that can't be mapped (disk full / unsupported FS) → heap fallback, no abort.Header data pruned during index load (not after
ActivateBestChain), freeing each entry'sHeaderDataright after its consistency check. Cuts the init memory transient ~2.1 GB → ~1.4 GB. Rebuilt from disk on demand.Correctness fixes folded in (from a pre-merge audit of the prior memory branch)
getheaders(compact + regular) served zeroed merkle/nonce for pruned blocks. NewGetFullBlockHeader()re-reads from disk.HeaderDatainDisconnectBlock,ConnectBlock,ReceivedBlockTransactions, and a wallet rescan path — now rehydrate/guard.Post-review hardening (2026-06-10/11 multi-agent adversarial review)
A 7-angle adversarial review of the branch (findings M1–M12, see
MEM_REVIEW_FINDINGS.md) was followed by a 20-commit fix stack (86485820e..0c12c4922) — one commit per finding, plus a new crash-recovery regtest and the fixes that test caught:ActivateBestChainre-applied blocks onto a cache already containing their effects (corrupted undo records, re-inserted starts,nLastPaidHeightresets). Recovery now rewinds fluxnode state along the marker's chain (DisconnectFluxnodeOnly, factored fromDisconnectBlock), hard-fails instead of continuing with a corrupt cache, caps rewind depth at the undo retention window, and init aborts before ABC on failure.EnsureHeaderDataFromDiskpropagates failure; prunes skip header-only entries.PersistToDiskhonorsfForceso the stale-marker repair actually writes; dead no-markerDumpFluxnodeCache()deleted.mapOldDelegateswas missing from the write condition — pre-exists on master) and retained by height across all chains so fork-side records survive reorgs for recovery.ReadBlockHeaderFromDisk(header-prefix read, no tx parse, no proof recheck) replaces full-block reads in header serving; a 2000-entrycmpheadersrequest no longer does 2000 full block deserializations + Equihash rechecks undercs_main.CBlockIndex(32 B, memory-only); the comparator key no longer depends on prunable data and can no longer mutate in-place insidesetBlockIndexCandidates.POOL_CAPACITYwarning removed; the five divergent pruned-header fallbacks consolidated through the exportedGetFullBlockHeader.The new recovery regtest also caught a real bug beyond the findings list:
RewindBlockIndex's unconditional init-timeFlushStateToDisk(ALWAYS)— with M7'sfForcenow honored — overwrote the on-disk sync marker with the in-memory tip before recovery could read it, neutering recovery exactly when it was needed. Fixed with anfFluxnodeCacheRecoveredgate: flushes before recovery completes never force the marker; flushes after always keep it at the tip (4577feae6+f3e4d2296).M10 (runtime allocations bypass the arena, ~0.5–0.7 MB/day RssAnon drift) is deliberately deferred to a follow-up PR.
Compatibility / risk
CDiskBlockIndexserialization is byte-identical (moved fields keep their serialized position/order); memory-only fields stay memory-only.HeaderDatasplit (two allocations/block instead of one, ~50–85 MB at current height + a pointer indirection). No functional change.invalidateblockinto pruned territory now triggers on-demand disk reads (rehydration) rather than crashes.Testing
flux-gtesttargeted run (Validation/Wallet/value-pool/upgrades): 64/64 passed, includingValidation.ReceivedBlockTransactions(exercises the moved value-pool propagation) and Sprout/Sapling wallet tests (the rescan path).flux-gtestfull suite at head0c12c4922: 238/239 passed. The one failure (WalletTests.CachedWitnessesCleanIndex) is pre-existing — it fails identically on the master baseline (verified against a master-built gtest binary) and the witness code is unchanged here, so it is not a regression.squidward): clean init/sync, value pools intact (getblockchaininfo: sprout 19 291, sapling 77 189), noRewindBlockIndex/abort, ~500 MB under a 500 MB no-swap cap, no visible backing files.CDiskBlockIndexserialization round-trip (no-reindex guard), prune→restore serializes byte-identically (M2/M6), resident state incl.nCachedBranchIdand the cached PON hash survives the prune (RewindBlockIndex + M5 regressions),CBlockIndexPoolalloc/exhaustion/Contains/DestroyAll, forcedPersistToDiskwrites the sync marker on a clean cache (M7).qa/rpc-tests/fluxnode_cache_recovery.py(green): rewrites the sync marker in leveldb between restarts and asserts all four recovery shapes — clean restart skips recovery; a stale marker is repaired exactly once (the M7 fForce assertion, verified by reading the marker back); a marker behind the tip disconnects/replays to the same tip; a marker on a stale fork triggers the M1 fluxnode-only rewind along the marker's chain and converges to the best tip.26a00f328:RssAnonflat at 402–500 MB,VmSwap0 on every node, all at tip.0c12c4922(same 6 nodes, v9.1.0, deployed 2026-06-11 ~08:37 UTC, checked 2026-06-12 ~06:30 UTC):RssAnon356–467 MB (charlie 373, squidward 467, cindy 356, mule 404, sandwich 378, cabbage 411) — flat vs. the fresh-boot baseline of 320–422 MB;VmSwap0 kB on every node; all six at tip (2678998/2678999). One node (cabbage) had two host reboots during the soak; every fluxd start ran the new recovery path and loggedRecoverFluxnodeCache: sync state matches chain tip … no recovery needed— no recovery loops, no stale markers.Reviewer guidance — scrutinize
CDiskBlockIndex::SerializationOpfield order vs. master (must be byte-identical).pHeaderData->access: guarded, rehydrated, or pre-prune (seeEnsureHeaderDataFromBlock/FromDisk,GetFullBlockHeader).fFluxnodegating points (arena create, both prune sites).CBlockIndexPoollifetime:Contains()-based pool-vs-heap discrimination in cleanup paths; chunk-map failure → heap fallback;O_TRUNC/unlinkcleanup; mmap-window offsets stay valid as the file grows.Deployment notes
dbcache=200(bounds the anonymous UTXO cache, the dominantRssAnon).blockindex.arenafrom datadir backups/snapshots — it's a visible scratch file regenerated from leveldb each run.zramdevice for the anonymous portion under pressure (operator choice, not in fluxd).🤖 Generated with Claude Code