txncache: spill to disk [wip, don't review]

[ibhatt-jumptrading]

No description provided.

[github-actions]

 ┌─ ⚡ PERF · 4638e05 vs main@78ad777 ─────────────────────────────────
 │ SUITE                               BASELINE          NEW         Δ
 │ replay tps, mainnet               30,712 tps   30,698 tps  ·  -0.04%
 │ bench tps, localnet              763,414 tps  761,905 tps  ·  -0.20%
 │ snapshot load, testnet               14.84 s      14.91 s  ·  +0.46%
+│ mem total, mainnet                158.37 GiB   151.37 GiB  ▼  -4.42%
+│ mem total, testnet                 89.37 GiB    82.37 GiB  ▼  -7.83%
+│ mem total, ag mainnet             190.85 GiB   183.85 GiB  ▼  -3.67%
+│ mem total, ag testnet             121.85 GiB   114.85 GiB  ▼  -5.74%
 │ clean compile, firedancer             3.08 s       3.06 s  ·  -0.52%
 │ binary size, firedancer             56.29 MB     56.31 MB  ·  +0.03%
 ├─────────────────────────────────────────────────────────────────────
@@ 0 REGRESSIONS · 0 WARNINGS · 4 IMPROVED · 5 NOISE @@
 └─────────────────────────────────────────────────────────────────────

history · 11 pushes

 ┌─ HISTORY · Δ vs main, per push, newest first ───────────────────────────────────────────
 │ HEAD         TPS    BENCH     SNAP    MEM·M    MEM·T     AG·M     AG·T  COMPILE   BINARY
+│ 4638e05   -0.04%   -0.20%   +0.46%   -4.42%   -7.83%   -3.67%   -5.74%   -0.52%   +0.03%
+│ 751ddcd   -0.05%   -0.14%   +0.79%   -4.42%   -7.83%   -3.69%   -5.79%   +2.16%   +0.03%
+│ cbed8eb   +0.10%   -0.10%   -0.22%   -4.42%   -7.83%   -3.69%   -5.79%   -1.27%   +0.03%
+│ fa0bd50   +0.14%   -0.23%   +1.30%   -4.42%   -7.83%   -3.69%   -5.79%   -0.13%   +0.03%
+│ 7fea228   -0.73%   +0.34%   +0.94%   -4.42%   -7.83%   -3.69%   -5.79%   +1.92%   +0.03%
+│ 49b6d77   +0.14%   -0.13%   +0.63%   -4.42%   -7.83%   -3.69%   -5.79%   +2.78%   +0.03%
+│ 990ceba   +0.21%   -0.26%   -0.26%   -4.42%   -7.83%   -3.69%   -5.79%   +0.93%   +0.05%
+│ c295f7e   -0.05%   +0.38%   +0.14%   -4.42%   -7.83%   -3.69%   -5.79%   +0.20%   +0.06%
+│ 76720ca   -0.42%   +0.19%   +1.89%   -4.42%   -7.83%   -3.69%   -5.79%   -1.15%   +0.06%
+│ 4bd10c6   +0.89%   -0.26%   +0.08%   -4.42%   -7.83%   -3.69%   -5.79%   +2.56%   +0.06%
+│ d17f440   -0.31%   +0.16%   +0.57%   -4.42%   -7.83%   -3.69%   -5.79%   +0.53%   +0.06%
 └─────────────────────────────────────────────────────────────────────────────────────────

[Copilot]

🔵 Needs a closer look

The concurrent spill path has unresolved descriptor-collision and snapshot-staging failures.

Pull request overview

Adds disk-backed transaction-cache spilling to bound resident memory across Firedancer runtime tiles.

Changes:

• Implements concurrent page spilling and bounded cache sizing.
• Adds spill-file initialization, inheritance, and seccomp permissions.
• Expands unit, fuzz, snapshot, and concurrency coverage.

File summaries

File	Description
src/flamenco/runtime/tests/fd_svm_mini.c	Updates mini-runtime cache construction.
src/flamenco/runtime/test_txncache.c	Adds spill and concurrency tests.
src/flamenco/runtime/fuzz_txncache_fork_graph.c	Fuzzes bounded spill behavior.
src/flamenco/runtime/fd_txncache.h	Extends the public cache API.
src/flamenco/runtime/fd_txncache.c	Implements page residency and spill I/O.
src/flamenco/runtime/fd_txncache_shmem.h	Adds resident-memory configuration.
src/flamenco/runtime/fd_txncache_shmem.c	Sizes and initializes bounded storage.
src/flamenco/runtime/fd_txncache_private.h	Adds shared spill metadata.
src/discof/restore/test_snapin_tile.c	Updates snapshot-loader cache tests.
src/discof/restore/generated/fd_snapin_tile_seccomp.h	Adds generated spill syscall rules.
src/discof/restore/fd_snapin_tile.seccomppolicy	Permits transaction-cache spill I/O.
src/discof/restore/fd_snapin_tile.c	Connects snapin to the spill file.
src/discof/replay/generated/fd_replay_tile_seccomp.h	Adds generated replay spill rules.
src/discof/replay/fd_replay_tile.seccomppolicy	Permits replay spill I/O.
src/discof/replay/fd_replay_tile.c	Connects replay to the spill file.
src/discof/execrp/test_execrp_tile.c	Updates allowed-descriptor tests.
src/discof/execrp/generated/fd_execrp_tile_seccomp.h	Adds generated execrp spill rules.
src/discof/execrp/fd_execrp_tile.seccomppolicy	Permits execrp spill I/O.
src/discof/execrp/fd_execrp_tile.c	Connects execrp to the spill file.
src/discof/execle/test_execle_tile.c	Updates allowed-descriptor tests.
src/discof/execle/generated/fd_execle_tile_seccomp.h	Adds generated execle spill rules.
src/discof/execle/fd_execle_tile.seccomppolicy	Permits execle spill I/O.
src/discof/execle/fd_execle_tile.c	Connects execle to the spill file.
src/discof/backup/test_snap_roundtrip.c	Tests snapshots with forced spilling.
src/discof/backup/generated/fd_snapmk_tile_seccomp.h	Adds generated snapmk spill rules.
src/discof/backup/fd_txncache_writer.c	Reads cache pages through pinned accessors.
src/discof/backup/fd_snapmk_tile.seccomppolicy	Permits snapmk spill I/O.
src/discof/backup/fd_snapmk_tile.c	Connects snapmk to the spill file.
src/app/shared/commands/run/run.h	Declares spill-file initialization.
src/app/shared/commands/run/run.c	Creates and distributes the spill descriptor.
src/app/shared_dev/commands/dev.c	Initializes spilling for development runs.
src/app/shared_dev/commands/bench/bench.c	Initializes spilling for benchmarks.
src/app/firedancer/topology.c	Configures the resident-memory limit.
src/app/firedancer/callbacks.c	Applies bounded cache sizing.
src/app/firedancer-dev/commands/snapshot_load.c	Initializes snapshot-load spilling.
src/app/firedancer-dev/commands/forktest/forktest.c	Initializes forktest spilling.
src/app/firedancer-dev/commands/backtest.c	Initializes backtest spilling.

Review details

• Files reviewed: 32/37 changed files
• Comments generated: 2
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

It changes concurrent shared state, persistent I/O, descriptor inheritance, and multiple seccomp policies.

Review details

Suppressed comments (4)

src/app/firedancer/topology.c:221

• The fixed 640 MiB cap is smaller than snapin's required staging scratch for supported raised limits. fd_txncache_snapin_scratch() now exposes only the resident extent, while snapin requires 151 * config->limits.max_txn_per_slot * 40 bytes (fd_snapin_tile.c:537-542). The checked-in 540M-CU benchmark configuration yields 529,411 transactions/slot, requiring about 3.2 GB, so snapshot loading aborts with “txncache scratch too small.” Size the resident region for this contract or move snapin staging to separately sized storage.

  FD_TEST( fd_pod_insertf_ulong( topo->props, 640UL<<20,        "obj.%lu.resident_bytes",   obj->id ) );

src/flamenco/runtime/fd_txncache.h:113

• 123465 is inside the XDP descriptor range. fd_topo_install_xdp assigns two descriptors per interface starting at 123462 (src/disco/net/fd_net_tile_topo.c:415-420), and the normal physical-interface-plus-loopback setup therefore already uses 123465. initialize_txncache_fd() subsequently dup2s onto this number, closing the loopback XDP program-link fd and detaching/breaking XDP networking. Use an fd outside the documented 123462+ XDP range (the existing reservation is documented in src/flamenco/stakes/fd_stake_delegations.h:75-76).
  src/app/firedancer/topology.c:221
• The fixed 640 MiB resident budget is not sufficient for the benchmark limits this code supports. With development.bench.max_cost_per_block, max_txn_per_slot can be about 529k, while fd_snapin_tile.c:539-542 stages FD_TXNCACHE_MAX_SLOT_DELTAS * max_txn_per_slot 40-byte groups in this scratch region (over 3 GiB), so snapshot loading aborts with “txncache scratch too small” before it can populate the cache. Size the staging scratch independently or reject/disable snapshot loading for limits that exceed the resident budget.

  FD_TEST( fd_pod_insertf_ulong( topo->props, 640UL<<20,        "obj.%lu.resident_bytes",   obj->id ) );

src/app/firedancer/topology.c:221

• The bounded cache now exposes only its resident frames as snapin scratch, but this hard-codes that extent to 640 MiB. That is enough for the default max_txn_per_slot (the staging buffer is 151 * max_txn_per_slot * 40 bytes), but development bench limits can raise config->limits.max_txn_per_slot well above the roughly 111k-entry budget; with the snapshot loader enabled, txncache_staging_scratch then aborts with “scratch too small” before loading. Derive the resident/scratch budget from the configured limit or provide snapin a separate staging allocation.

  FD_TEST( fd_pod_insertf_ulong( topo->props, 640UL<<20,        "obj.%lu.resident_bytes",   obj->id ) );

• Files reviewed: 32/37 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

A missing shared test helper header prevents multiple test and fuzz targets from compiling.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

src/app/shared/commands/run/run.c:1147

• The new backing file is opened with O_TRUNC on every run but is never unlinked, unlike the existing stake-delegation spill file immediately above. After a validator spills pages, the allocated .txncache file remains on disk after normal shutdown (and after crashes), so repeated runs can leave a potentially very large transient cache file behind. Unlink the path after opening/duplicating the descriptor, or otherwise remove it during cleanup.

  int txncache_fd = open( path, O_RDWR|O_CREAT|O_TRUNC, S_IRUSR|S_IWUSR );
  if( FD_UNLIKELY( -1==txncache_fd ) ) FD_LOG_ERR(( "failed to open transaction cache backing file `%s` (%i-%s)", path, errno, fd_io_strerror( errno ) ));

• Files reviewed: 32/37 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

This WIP changes concurrent cache eviction, persistent I/O, and sandbox permissions across consensus-critical runtime paths.

Review details

• Files reviewed: 32/37 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

The WIP spans concurrent eviction, disk I/O, FD lifecycle, and sandbox policy, with unresolved correctness and test-execution gaps.

Review details

Suppressed comments (3)

src/app/shared/commands/run/run.c:1147

• This backing file is transient—the cache metadata is rebuilt and the file is truncated on every run—but it remains named after shutdown or a crash. Since spilled pages can consume many GiB, this leaves persistent disk usage with no cleanup path. Unlink it immediately after opening, as initialize_stake_delegations_fd does for its spill file, while keeping the descriptor alive.

  int txncache_fd = open( path, O_RDWR|O_CREAT|O_TRUNC, S_IRUSR|S_IWUSR );
  if( FD_UNLIKELY( -1==txncache_fd ) ) FD_LOG_ERR(( "failed to open transaction cache backing file `%s` (%i-%s)", path, errno, fd_io_strerror( errno ) ));

src/flamenco/runtime/test_txncache.c:214

• This replaces an exact footprint regression check with only an upper bound, so a layout that silently omits the new resident pages/metadata (or otherwise computes the wrong size) can still pass as long as it remains below 1 GiB. Keep a check tied to the resident-page/layout calculation, rather than only this broad limit.
  src/flamenco/runtime/test_txncache.c:224
• This changes the exact bench-footprint assertion to a one-sided lower bound. An accidental over-allocation or a changed metadata/layout size above 3,197,642,440 bytes will now pass, even though this test is the sizing guard for the supported 540M benchmark configuration. Retain an upper/lower range or an exact calculation that checks both capacity and bounded memory use.

• Files reviewed: 32/37 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

It changes consensus-critical shared-memory concurrency, disk I/O, descriptor lifecycle, and sandbox policies, with unresolved review findings.

Review details

Suppressed comments (1)

src/flamenco/runtime/test_txncache.c:838

• These new spill regressions are only invoked from test_txncache, but src/flamenco/runtime/Local.mk:102 still comments out run-unit-test,test_txncache, so CI never executes them. Re-enable this target or add these checks to an enabled test target before relying on them.

• Files reviewed: 38/43 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

The WIP changes cross-process cache concurrency, persistent I/O, startup descriptor handling, and multiple seccomp sandboxes.

Review details

Suppressed comments (2)

src/flamenco/runtime/test_txncache.c:133

• Truncating the file to zero does not make this read fail: the resident page for BLOCKHASH(2) is dirty, so the query first evicts it and writes it at its nonzero page offset, creating a zero-filled sparse hole at offset 0. The subsequent pread of page 0 therefore succeeds and returns a zero page; its zero blockcache_next makes the query loop forever, so the parent waitpid hangs instead of validating the failure path. Make page 0 resident and clean before ftruncate, then query the page 1 transaction after truncation.
  src/discof/restore/test_snapin_tile.c:990
• The first invocation maps roughly 3.2 GiB to test the 540M benchmark staging size, but this anonymous mapping omits MAP_NORESERVE. On hosts with insufficient commit available, mmap can fail even though the test touches only a few pages; the existing large lazy mapping in test_txncache.c:244 uses MAP_NORESERVE for this reason.

• Files reviewed: 38/43 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

Shared-memory concurrency, disk persistence, sandbox rules, and validator startup behavior require final human validation.

Review details

Suppressed comments (4)

src/app/shared/fd_config.c:549

• This accepts every nonzero cache budget, but snapshot loading requires the resident-page area to hold FD_TXNCACHE_MAX_SLOT_DELTAS * limits.max_txn_per_slot * sizeof(blockhash_group_t) bytes. For example, transaction_cache_size_mib = 1 passes validation and topology setup, then deterministically terminates at txncache_staging_scratch() when the first snapshot starts. Validate the derived cache budget against the staging requirement when snapshots are enabled (after limits.max_txn_per_slot is computed), or separate snapshot scratch from this configurable cache budget.

  CFG_HAS_NON_ZERO( runtime.transaction_cache_size_mib );

src/flamenco/runtime/test_txncache.c:25

• These spill regressions are added to test_txncache, but src/flamenco/runtime/Local.mk:102 still comments out run-unit-test,test_txncache (the target is not executed by the test suite). As a result, the new disk-I/O and concurrency paths are not exercised in CI; enable this target or provide a separate enabled test target before relying on these checks.
  src/app/firedancer/topology.c:1212
• Because cache_footprint is also the only storage returned by fd_txncache_snapin_scratch, a nonzero value can still be too small for snapin's 151 * max_txn_per_slot blockhash-group staging buffer; txncache_staging_scratch then calls FD_LOG_ERR during snapshot startup. The new config validation only rejects zero/overflow, so a value such as 1 MiB passes validation but deterministically aborts a normal snapshot load. Validate this minimum when snapin is enabled, or provide separate staging storage, before creating the topology.

  fd_topo_obj_t * txncache_obj = setup_topo_txncache( topo, "txncache", config->firedancer.runtime.max_live_slots, 2UL*config->limits.max_txn_per_slot /* sig + message hash */, config->firedancer.runtime.transaction_cache_size_mib<<20 );

src/app/shared/fd_config.c:552

• This change makes the transaction-page budget user-configurable, but fd_txncache_shmem_footprint/fd_txncache_shmem_new do not verify that the budget can accommodate snapin's staging scratch. Consequently, a valid nonzero transaction_cache_size_mib can pass configuration validation and only fail later inside the tile with FD_LOG_ERR when a snapshot starts. Reject undersized budgets during topology/config construction (when snapin is enabled), or stop aliasing staging storage with the cache.

  CFG_HAS_NON_ZERO( runtime.transaction_cache_size_mib );
  if( FD_UNLIKELY( config->runtime.transaction_cache_size_mib>(ULONG_MAX>>20) ) ) {
    FD_LOG_ERR(( "runtime.transaction_cache_size_mib is too large: %lu", config->runtime.transaction_cache_size_mib ));
  }

• Files reviewed: 43/48 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

Non-hosted runtime consumers can lose required txncache symbols, while the concurrency and disk-I/O changes require human validation.

Review details

Suppressed comments (3)

src/app/shared/fd_config.c:549

• This validation accepts any nonzero budget, but snapshot loading always requests 151 * limits.max_txn_per_slot * sizeof(blockhash_group_t) bytes from the resident-page area before parsing (see fd_snapin_tile.c:539-542). A smaller configured value therefore passes startup validation and later terminates as soon as a snapshot starts; for the normal 98,039 transaction limit, values below roughly 565 MiB fail this way. Validate the snapshot staging minimum after config->limits.max_txn_per_slot is derived (when snapshot loading is enabled), or allocate that staging storage independently of this tunable cache budget.

  CFG_HAS_NON_ZERO( runtime.transaction_cache_size_mib );

src/flamenco/runtime/test_txncache.c:25

• These new spill, I/O-failure, and concurrency tests are added to test_txncache, but src/flamenco/runtime/Local.mk:105 still comments out run-unit-test,test_txncache because of the existing flakes TODO. As a result, CI will not execute the only tests covering the new disk-backed behavior; please re-enable them once stabilized or add a separate stable test target.
  src/app/shared/fd_config.c:552
• The new nonzero-only validation accepts cache sizes that cannot satisfy snapin's staging requirement. With snapshots enabled, fd_snapin_tile.c:540-542 needs resident space for FD_TXNCACHE_MAX_SLOT_DELTAS * max_txn_per_slot blockhash_group_t records and terminates if that space is unavailable, so a configuration such as transaction_cache_size_mib = 1 passes validation but fails on the first snapshot load. Validate the minimum budget (including alignment) or provide staging storage separately.

  CFG_HAS_NON_ZERO( runtime.transaction_cache_size_mib );
  if( FD_UNLIKELY( config->runtime.transaction_cache_size_mib>(ULONG_MAX>>20) ) ) {
    FD_LOG_ERR(( "runtime.transaction_cache_size_mib is too large: %lu", config->runtime.transaction_cache_size_mib ));
  }

• Files reviewed: 43/48 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

The cross-process spill path has unresolved descriptor-lifecycle and global-locking risks.

Review details

Suppressed comments (2)

src/app/shared/fd_config.c:549

• Nonzero is not sufficient for this setting when snapshot loading is enabled. txncache_staging_scratch requires 151 * limits.max_txn_per_slot * 40 bytes plus alignment; with production limits that is 592,155,567 bytes (565 MiB), so values 1–564 pass validation but every snapshot load terminates. Raised benchmark limits require an even larger minimum. Validate the derived minimum after limits are computed, or allocate snapshot staging independently from the cache budget.

  CFG_HAS_NON_ZERO( runtime.transaction_cache_size_mib );

src/app/shared/fd_config.c:549

• This validation accepts budgets that cannot satisfy the snapin scratch contract. With the default max_txn_per_slot, a setting such as transaction_cache_size_mib=1 leaves only one resident transaction page, while txncache_staging_scratch() needs 151*max_txn_per_slot blockhash groups (about 565 MiB) and terminates via FD_LOG_ERR during snapshot loading. Reject undersized budgets here, or provide separate staging memory, so a valid-looking configuration cannot fail at startup.

  CFG_HAS_NON_ZERO( runtime.transaction_cache_size_mib );

• Files reviewed: 43/48 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

The new concurrent disk-backed cache and sandbox integration require human validation before approval.

Review details

Suppressed comments (1)

src/flamenco/runtime/fd_txncache.c:714

• Each chain record on a spilled page causes a synchronous pread64 while the query holds the shared lock. The hash table targets a load factor of eight, so duplicate checks that reach disk can still require several random reads at transaction rate and delay writers waiting for the lock. Consider a bounded per-join page cache or another lookup layout that avoids repeated synchronous reads for spilled records.

• Files reviewed: 43/48 changed files
• Comments generated: 6
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

The work-in-progress change modifies concurrent cache storage, synchronous disk I/O, file-descriptor lifecycles, and multiple seccomp boundaries.

Review details

Suppressed comments (10)

src/flamenco/runtime/test_txncache.c:894

• These spill and concurrency regressions are compiled into test_txncache, but that binary is still not run by the normal unit-test target because Local.mk:105 keeps run-unit-test,test_txncache commented out. As a result, the primary tests for the new disk path do not execute in CI; re-enable the target or register a non-flaky spill-specific test binary.
  src/flamenco/runtime/fd_txncache.c:676
• Every insertion into a spilled page performs a full-page read in fd_txncache_ensure_txnpage and then rewrites the entire page here. A page is about 240 KiB and holds 8192 transactions, so filling one spilled page transfers roughly 4 GiB while repeatedly holding the global exclusive lock, which can stall all cache queries and inserts. Persist only the page header/new record (or add a bounded page cache) rather than round-tripping the whole page per transaction.
  src/flamenco/runtime/fd_txncache.c:246
• When the current blockhash page is disk-backed but still has free entries, this loads the entire ~245 KB page for every transaction; the write path below then writes the entire page back after that single insert. Once the RAM budget is exhausted, transaction throughput therefore degrades to roughly two full-page I/Os per transaction. Please use an append/record-level update or a properly coordinated page cache for disk-backed inserts.
  src/flamenco/runtime/fd_txncache.c:724
• For a spilled page, this performs a separate 30-byte pread for every record traversed in a query. A single bucket can contain thousands of records in the same page, so duplicate checks can issue thousands of synchronous syscalls and become dominated by I/O overhead. Load each spilled page once per query (with storage safe for concurrent joins) and traverse its records from that buffer.
  src/discof/backup/fd_txncache_writer.c:194
• Snapshot serialization now issues one 30-byte pread for every transaction in a spilled page. Walking a large status cache can therefore perform millions of tiny synchronous reads while holding the shared txncache lock, unnecessarily blocking structural operations. Read each spilled page once per walk (or otherwise batch/cache records) instead of calling page_io for every record.

      page_io( writer->tc, head/FD_TXNCACHE_TXNS_PER_PAGE, offsetof(fd_txncache_txnpage_t, txns)+(head%FD_TXNCACHE_TXNS_PER_PAGE)*sizeof(*txn), txn, sizeof(*txn), 0 );

src/flamenco/runtime/fd_txncache.c:676

• Every insert into a disk-backed current page reaches this branch and writes the entire transaction page after adding one record, while holding the global write lock; the next insert reloads the entire page in fd_txncache_ensure_txnpage. Once the RAM budget is exceeded, inserts and queries are serialized behind full-page disk transfers for each transaction, which makes the high-rate transaction path unusable. Keep dirty disk pages cached/batched or use record-level I/O before relying on this spill path.
  src/flamenco/runtime/fd_txncache.c:724
• For a disk-backed page, this issues one pread for every candidate in the bucket rather than reading a page once. Even normal bucket chains therefore require several syscalls and page-cache lookups per duplicate check; with a cold spilled cache this also becomes repeated random storage I/O. Add a page-level read cache or batch page reads, with coordination for page reuse, before relying on this path.
  src/flamenco/runtime/fd_txncache.c:678
• Every append to a disk-resident page reaches this branch while holding the global write lock and writes the entire fd_txncache_txnpage_t; the preceding ensure path also reads the full page. Thus a single 30-byte transaction causes page-sized synchronous I/O and serializes all disk-backed inserts, which can stall the validator as soon as spill is active. Write only the modified record/metadata or batch page updates while keeping the active page resident.
  src/app/shared/fd_config.c:552
• This validation accepts any nonzero cache size, but snapshot loading later uses the resident pages as scratch for FD_TXNCACHE_MAX_SLOT_DELTAS * limits.max_txn_per_slot blockhash groups and fatally errors when that staging area does not fit. A small, otherwise valid transaction_cache_size_mib therefore passes config validation and only fails during snapshot load. Reject budgets below the staging requirement here, or allocate staging independently of the transaction-cache budget.

  CFG_HAS_NON_ZERO( runtime.program_cache_size_mib );
  CFG_HAS_NON_ZERO( runtime.transaction_cache_size_mib );
  if( FD_UNLIKELY( config->runtime.transaction_cache_size_mib>(ULONG_MAX>>20) ) ) {
    FD_LOG_ERR(( "runtime.transaction_cache_size_mib is too large: %lu", config->runtime.transaction_cache_size_mib ));
  }

src/flamenco/runtime/fd_txncache.c:657

• After dropping the read lock, a concurrent fork removal/root advancement can invalidate or reuse fork_id, but this path reuses the fork pointer captured before the unlock and never revalidates its generation or frozen state. blockhash_on_fork can therefore select a cache for a removed/reused fork (or fail) and the subsequent insert can be attributed to the wrong fork. Keep the fork identity protected across the lock upgrade, or validate the fork generation/state and retry before inserting.

• Files reviewed: 43/48 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

Cross-process spill I/O and global locking introduce unresolved performance risk and require human validation.

Review details

Suppressed comments (5)

Previously missed (1) — in code that hasn't changed since the last review.

src/flamenco/runtime/test_txncache.c:901

• These new spill regressions run only inside test_txncache, but src/flamenco/runtime/Local.mk:105 still comments out run-unit-test,test_txncache. The binary is built but is not executed by the normal test target, so the new disk I/O, concurrency, and failure-path coverage can regress silently. Re-enable this test once its existing flakes are fixed, or move these cases into an executed test target.

src/flamenco/runtime/fd_txncache.c:666

• Typo: “dist” should be “disk”.
  src/app/shared/fd_config.c:549
• This accepts cache budgets that cannot satisfy snapin's staging requirement. For production limits, 512 MiB passes validation, but the rounded resident-page scratch is about 537 MB while snapin requires about 592 MB (151 * 98039 * sizeof(blockhash_group_t)), so snapshot startup later terminates in txncache_staging_scratch. Validate the snapshot-staging minimum (when snapshot loading is enabled), or move that staging allocation out of the configurable page budget.

  CFG_HAS_NON_ZERO( runtime.transaction_cache_size_mib );

src/flamenco/runtime/fd_txncache.c:563

• For a spilled page, this calls page_io once per 30-byte transaction, and each call performs a separate pread. Purging can therefore hold the exclusive txncache lock across thousands of syscalls per 8192-entry page, blocking all inserts and queries. Load each page once into local/scratch storage and walk its records from memory, writing the compacted page back once.
  src/discof/backup/fd_txncache_writer.c:194
• Snapshot serialization has the same per-record I/O problem: a spilled page is read with one pread for every transaction while the writer is walking the cache. A large spilled status cache therefore turns snapshot creation into one syscall per record; cache each logical page during the walk and reuse it for its records.

      fd_txncache_single_txn_t txn[1];
      page_io( writer->tc, head/FD_TXNCACHE_TXNS_PER_PAGE, offsetof(fd_txncache_txnpage_t, txns)+(head%FD_TXNCACHE_TXNS_PER_PAGE)*sizeof(*txn), txn, sizeof(*txn), 0 );

• Files reviewed: 43/48 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

This WIP changes concurrent cache storage, synchronous disk I/O, descriptor inheritance, and seccomp policy across critical runtime components.

Review details

Suppressed comments (9)

Previously missed (1) — in code that hasn't changed since the last review.

src/flamenco/runtime/fd_txncache.c:660

• Correct “dist” to “disk” in this spill-path comment.

src/flamenco/runtime/fd_txncache.c:674

• Each insert into a partially filled spilled page reaches this branch after first reading the entire fd_txncache_txnpage_t at line 238, then writes the entire page again here. With 8,192 entries per page, filling one ~240 KiB page therefore causes roughly 4 GiB of disk I/O and holds the global exclusive txncache lock throughout each transfer. Once spilling starts, this will stall queries and inserts across all joins rather than provide usable overflow capacity. Persist only the free field and newly appended transaction record (or retain a write-back page until full) before publishing the chain head.
  src/app/shared/fd_config.c:552
• This accepts every nonzero budget, but snapshot-enabled topologies require scratch for FD_TXNCACHE_MAX_SLOT_DELTAS * limits.max_txn_per_slot 40-byte blockhash groups. A smaller configured value is rounded to transaction pages successfully here, then txncache_staging_scratch() deterministically terminates while loading a snapshot; this is a functional configuration error, not merely the documented performance risk. Validate the snapshot-staging minimum when snapshots are enabled (or allocate that staging area independently of the cache budget).

  CFG_HAS_NON_ZERO( runtime.transaction_cache_size_mib );
  if( FD_UNLIKELY( config->runtime.transaction_cache_size_mib>(ULONG_MAX>>20) ) ) {
    FD_LOG_ERR(( "runtime.transaction_cache_size_mib is too large: %lu", config->runtime.transaction_cache_size_mib ));
  }

src/flamenco/runtime/fd_txncache.c:656

• The read-to-write lock upgrade drops the lock while retaining fork. A concurrent replay-side fd_txncache_cancel_fork/root operation can remove and recycle this fork ID before the write lock is acquired; blockhash_on_fork then walks the recycled fork->descends, and the insert can be recorded under the wrong generation. Revalidate the fork ID/generation after acquiring the exclusive lock (or use an upgrade/retry path) before looking up the blockcache.
  src/flamenco/runtime/Local.mk:34
• This removes both txncache implementations from every non-hosted fd_flamenco build, but fd_runtime.c is still added unconditionally and calls fd_txncache_insert (src/flamenco/runtime/fd_runtime.c:1223), while the executor calls fd_txncache_query. Non-hosted runtime consumers therefore lose required definitions and cannot link; keep a no-spill txncache implementation available or provide a hosted/non-hosted backend rather than omitting these objects.
  src/flamenco/runtime/Local.mk:34
• This guard removes fd_txncache.c/fd_txncache_shmem.c from every FD_HAS_HOSTED=0 build, but fd_runtime.c is still added unconditionally and calls fd_txncache_insert (src/flamenco/runtime/fd_runtime.c:1223). Any non-hosted link that includes the runtime will therefore have an unresolved txncache symbol; keep these objects available or update all non-hosted callers/build targets together.
  src/flamenco/runtime/fd_txncache.c:674
• Every insert into a disk-resident page reaches this branch and writes the entire ~246 KiB page back with pwrite; the preceding ensure_txnpage also reads the full page for each insert. A single spilled page can therefore cause thousands of full-page reads/writes and syscalls (and a large spill can turn normal replay/snapshot work into effectively unusable I/O). Keep a dirty page resident long enough to batch updates, or write only the changed record/metadata while preserving the publication ordering.
  src/discof/backup/fd_txncache_writer.c:194
• When the cache is spilled, this walks the chain with one pread per transaction record. Snapshotting a large status cache can consequently issue millions of tiny synchronous reads instead of loading each page once, so the disk-spill path can make snapshot creation stall. Buffer a complete page (or otherwise batch records) while walking a blockhash.

    for( uint head=txncache_chain_head( tc, blockhash_desc->blockcache_idx, bucket ); head!=UINT_MAX; ) {
      fd_txncache_single_txn_t txn[1];
      page_io( writer->tc, head/FD_TXNCACHE_TXNS_PER_PAGE, offsetof(fd_txncache_txnpage_t, txns)+(head%FD_TXNCACHE_TXNS_PER_PAGE)*sizeof(*txn), txn, sizeof(*txn), 0 );

src/flamenco/runtime/fd_txncache.c:557

• The purge path now performs a separate pread for every transaction in a spilled page. When capacity pressure triggers purge_stale, a cache with millions of disk-resident entries can spend the entire purge issuing tiny synchronous reads; load each page once (and compact from that buffer) instead of doing record-at-a-time I/O.

• Files reviewed: 43/48 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

The cross-process concurrency, disk I/O, startup, and sandbox changes include an unresolved fork-reuse race.

Review details

Suppressed comments (5)

src/app/shared/fd_config.c:549

• This validation accepts any nonzero budget, but snapshot loading requires scratch for FD_TXNCACHE_MAX_SLOT_DELTAS * limits.max_txn_per_slot blockhash groups. With production limits, values below roughly 565 MiB deterministically hit the fatal check in txncache_staging_scratch; the configuration currently presents smaller values as valid. Reject budgets below the derived staging requirement when snapshot loading is enabled, or move that staging storage outside the cache budget.

  CFG_HAS_NON_ZERO( runtime.transaction_cache_size_mib );

src/flamenco/runtime/fd_txncache.c:662

• Correct the typo “dist” to “disk”.
  src/flamenco/runtime/fd_txncache.c:654
• Use “usable” here; “usage” makes the sentence ungrammatical.
  src/flamenco/runtime/Local.mk:34
• This new hosted-only guard removes both txncache objects from non-hosted builds, but src/flamenco/runtime/Local.mk still unconditionally builds fd_runtime.c, whose commit path calls fd_txncache_insert, and src/discof/backup/Local.mk still unconditionally builds fd_txncache_writer.c, which calls page_io. Those configurations will link with missing txncache symbols; keep a hosted-independent implementation available or guard all dependent objects consistently.
  src/discof/backup/fd_snapmk_tile.seccomppolicy:106
• The snapmk tile only uses fd_txncache_writer, whose sole txncache I/O is the read path (page_io(..., write=0)), but this rule grants it pwrite64 access to the shared txncache file. A compromised snapmk process could therefore overwrite spilled pages and corrupt replay's duplicate-transaction state; allow only pread64 for this tile (and regenerate the generated policy).

pwrite64: (eq (arg 0) txncache_fd)

• Files reviewed: 43/48 changed files
• Comments generated: 1
• Review effort level: Balanced