refactor(storage)!: remove the LMDB chunk store, and never refuse a start over what it left behind by grumbach · Pull Request #218 · WithAutonomi/ant-node

grumbach · 2026-08-28T07:35:24Z

Restores the unheld-chunk penalty, and removes the migration signal with the store.
On 2026-10-01 the decision was to give the nodes still migrating more time and to ship this
"full throttle": the trust penalty for not holding a close-group chunk, suspended since
v0.18.1, comes back in this release rather than the one after, the switch and its override
go with it, and the migration signal goes because nothing is left to gate on it. A node
still unmigrated when this lands is penalised like any node that cannot serve its chunks and
can lose its routing slots; its old store stays on its disk untouched. Six commits on top of the
removal carry that (44de0b4f..HEAD), and ADR-0022 records it. Which release carries it is
the release manager's call.

Rebased onto main at d41a3535 (2026-10-08). Main had merged two store changes written
against the two-store facade this PR deletes: #242's corrupt-chunk recheck queue
(report_corrupt / run_corrupt_rechecks) and #233's exactly-sized chunk reads. Both are
ported onto the single store in the deletion commit, with their tests, and main's four new
file_store::CHUNKS_DIR_NAME imports now name chunk_store. Every other commit replays
unchanged or with context-only changes.

Reviewing the store. file_store.rs folded into chunk_store.rs, and git diff -M does
not pick that up as a rename, because chunk_store.rs already existed: it was the facade
over the two stores, and its contents were replaced rather than moved. So the raw diff shows
~8.3k deleted and ~3.5k added across the two files. What to read instead:
git diff d41a3535:src/storage/file_store.rs HEAD:src/storage/chunk_store.rs   # +1036 -381
That is the store's actual change, including the ~440 lines of #242 and #233 that lived in
the facade on main. Elsewhere under src/storage/, file_store.rs, lmdb.rs, migration.rs
and migration_signal.rs are deleted, and legacy_artifacts.rs (the leftover cleanup) is new.

Linear issue

Closes V2-1482

Risk tier

T0 — docs / tooling / CI / pure UX-output. Repo CI only.
T1 — client-only, no network-facing behavior change. CI + prod compat smoke.
T2 — node/client logic with behavioral surface, no protocol/format/economics change. Dev testnet + ADR.
T3 — protocol / storage format / payments / routing. T2 evidence + adversarial testing.

Compatibility

Wire: no message shape, field, or protocol version change. The user agent changes value:
node/<version>, without the migration/<state> token fix(storage): stop the migration denying a promise a node already made, and report what the neighbours say #223 added. saorsa-core admits DHT
participants on the node/ prefix alone, which stays (pinned by a test). Peers on earlier
releases read the shorter agent as a node that does not report, which only changes their own
log lines.
Behaviour: changes. The trust penalty for not holding a close-group chunk is restored on
every lane that charged it before v0.18.1, at the same weights: the responsible-chunk audit
(weight 5, every failure reason including a timeout), the possession check (5), the prune audit
(5), a sole-source replica hint its sender then denies holding (1), and a fetch from a verified
source answered NotFound (1). One weight-5 failure takes a neutral peer below saorsa-core's
swap threshold at that auditor. Commitment-bound subtree audits were never suspended and are
unchanged. Nodes no longer log migration_event = "signal" or peer_state lines, and opening
the chunk store no longer sweeps the node root for the migration marker's temporaries: nothing
in the root is deleted by the store any more.
Storage: changes. chunks.mdb can no longer be read at all. Nothing here refuses a
start — an earlier draft refused on an unmigrated store and that design was rejected: a node
that refuses serves nothing, and it cannot be held on the previous release, because
build_upgrade_monitor is unconditional and UpgradeConfig has no field that disables it. A
leftover is removed only when it carries the RETIRED mark the previous release wrote
inside it, or is empty; both are re-checked on the deleting thread immediately before it
unlinks. Anything else is kept and named once under migration_event = "legacy_store_left".
Links are neither followed nor unlinked. Kept is not the same as available: there is no reader for the old
chunks.mdb store in this build, so a node that keeps a directory keeps its bytes and can serve none of
them.
Storage, second axis — values over the 4 MB ceiling are not chunks, and none is copied or kept
as a readable chunk or sidecar.
Every released ingress enforces MAX_CHUNK_SIZE before storing anything (the protocol handler
on a paid store; replication on receive and on fetch), so no over-ceiling value ever entered
this network as a chunk. The one way one could reach a disk was the bridge's own put, which
wrote to LMDB first — that store has no size ceiling — and only then offered the same bytes to
the file store, which refused them; the key was then recorded legacy-only and the copier's size
arm deleted it. This PR removes the bridge, so it removes that hole: one store, whose put
refuses over the ceiling before it writes, and no LmdbStorage::put left to take an unbounded
value at all. No sidecar is built and nothing is copied out — there is no valid chunk there to be
the only copy of. A legacy directory containing such a value is kept or removed on its mark like
any other; this build never looks inside one.
API: breaking. Removed from ant_node::storage, in full because the list matters to
anyone compiling against this crate: LmdbStorage, LmdbStorageConfig, MigrationConfig,
MigrationPhase, MigrationState, the storage::migration module, FileStore,
FileStoreConfig, StoreLayout, VerifyReport and LEGACY_ENV_DIR. The FileStore family
and LEGACY_ENV_DIR are easy to miss because the type survives under another name; they were
public and they are not any more. ChunkStore and ChunkStoreConfig keep their
names and the file store's core methods (flush_namespace, stored_len, health_generation
and invalidate_capacity_cache are not carried over); they now name the file store directly rather than a facade
over two stores. What the facade added for the migration goes with it: ChunkStoreConfig's
max_map_size and migration fields, and the ChunkStore methods that read or drove the
migration (migration_phase, migration_state, migration_config, has_legacy,
legacy_only_keys, legacy_bytes, copy_batch, commit_to_files, committable_keys,
note_commitment_rebuilt, retirement_blocker, verify_before_retire, retire_legacy and the
rest of that family), as does the test-utils-public storage::file_store module. Also removed: from ant_node::replication::config, the penalty switch
(RELEASE_SUSPEND_CLOSE_GROUP_STORAGE_PENALTY, SUSPEND_CLOSE_GROUP_STORAGE_PENALTY_ENV,
apply_close_group_storage_penalty_policy, set_close_group_storage_penalty_suspended,
close_group_storage_penalty_suspended, penalise_unheld_close_group_chunk); from
ReplicationEngine, sync_state, audit_challenge_coordinator and config; from
ResponderCommitmentState, note_commitment_delivered, current_delivered_peer_count and
current_delivered_peers. ANT_SUSPEND_UNHELD_CHUNK_PENALTY has no effect.
storage.migration and storage.db_size_gb are gone from the config file. An
operator's existing config still loads with both keys present, because nothing declares
deny_unknown_fields, and there is a test holding that true.

Semver impact

breaking
feature
fix

Test evidence

At 41a9c82d on d41a3535, macOS arm64, Rust 1.99 (CI's stable) with RUSTFLAGS=-D warnings
as CI sets it. The long suites ran on 2a79e308, whose tree differs from the head only by
comment wording and one blank line in Cargo.toml; the fast gates and the unit tests were rerun
on the head itself.

cargo test --lib --features test-utils                                     1165 passed, 0 failed
cargo test --lib --no-default-features                                     1120 passed, 0 failed
cargo check --lib --no-default-features --locked                           clean
cargo build --release --no-default-features                                clean
strings target/debug/ant-node | grep -c ANT_HALT_                          0 (default-feature build)
cargo test --test e2e --features test-utils                                116 passed, 0 failed, 3 ignored (968 s)
cargo test --test chunk_store_crash_safety --features test-utils           2 passed, 1 child ignored
cargo test --test storage_scale --features test-utils                      2 passed
cargo test --test webrtc_direct_devnet                                     3 passed
cargo test --test webrtc_direct_devnet --features test-utils -- --ignored  1 passed (five-node)
cargo test --test poc_commitment_audit_attacks --features test-utils       19 passed
cargo test --test poc_audit_handler_live --features test-utils             16 passed
cargo test --test poc_bootstrap_stall --features test-utils                3 passed
cargo test --test pointer_convergence --features test-utils                15 passed
cargo clippy --all-targets --all-features -- -D warnings                   clean (the CI invocation)
cargo +1.95.0 check --all-targets --all-features --locked                  clean (MSRV)
RUSTDOCFLAGS=-D warnings cargo doc --all-features --no-deps                clean
cargo fmt --all -- --check                                                 clean
cfd (local 1.98: both clippy forms, fmt, doc)                              clean
python3 scripts/adr-governance.py                                          passed, 19 ADRs checked

Not run locally: the ext4 / XFS / btrfs storage matrix (no Linux loop mounts on macOS) and the
third-party notices job; both run on hosted CI.

33 of the 34 commits type-check on their own (cargo check --all-targets --all-features). The
exception is "let the cleanup and the fleet signal be one reading of one directory", which leaves
a LEGACY_ENV_DIR re-export dangling until the next commit; it did not build alone before this
rebase either.

The restored penalty is asserted by the possession-check e2e tests, which now charge an absent
peer with no switch left to set.

Four properties are newly pinned, none of which had a test before:

for ordinary shapes, the cleanup removes exactly what classify calls harmless, checked in
both directions (the_cleanup_removes_exactly_what_classify_calls_harmless), so the deleter and
the classifier cannot drift apart; the deleter's own refusals (a subdirectory inside a leftover,
an implausibly wide one) have tests of their own;
all sixty-six leftovers a root can hold — the live directory, the unnumbered tombstone and
sixty-four numbered ones — are removed by one start. The implementation removes them one after
another on one thread; the test checks the removal, not the thread count;
a root that cannot be listed has nothing removed from it, which is the case that used to
return silently while the decision record promised a warning;
a value over the ceiling is refused before anything is written — not claimed, no file left
under its name, and nothing for a restart's scan to index. Addressed to its own bytes so the
refusal is the size arm and not the content-address arm, and mutation-checked: with the size
branch deleted the test fails.

Three adversarial review rounds with codex xhigh — the release graph and design before any
editing, the exact diff afterwards, and a third against the deleter written to answer the human
review. The third round found two blockers in that new code: the
mark's type test accepted a symlink or a socket (S_IFLNK and S_IFSOCK each contain every bit
of S_IFREG, so it has to mask with S_IFMT), and O_NOFOLLOW does not refuse a mount point,
so a subdirectory inside a leftover is now refused rather than entered. Both have tests, both
mutation-checked.

The six commits that restore the penalty and remove the signal went through codex xhigh
review commit by commit and then as a series, fixing what each round found, until a round came
back clean. What those rounds caught: comments still describing a marked directory as holding
nothing (it can hold shed chunks the close group proved it holds), an off-Unix durability gap
described as covered by replication when recovery is conditional (it predates this PR and is now
named in the ADR), and documentation claiming the cleanup deletes exactly what the classifier
calls harmless when the deleter also refuses shapes a retired environment never has, and a
startup sweep of the node root for the migration marker's temporaries that had outlived the
marker.

The rebase onto d41a3535 had one final exact-diff codex xhigh review. It found the port
faithful, and no interaction between the restored penalty and #242's quarantine, #238/#240's
pointer audits or #233's send path. It raised two points. Retention comments in
commitment_state.rs, replication/mod.rs and pruning.rs still describe a two-slot model,
while the code keeps every root gossiped in the last three hours, up to 16. Those comments are
main's and this PR does not touch them, so they are left for a separate fix. A comment in the
port gave an imprecise reason for never cancelling a recheck; that was corrected, and a scoped
second pass agreed.

New dependency

One, added in review: rustix (unix targets only, fs feature). It supplies safe
openat/unlinkat, so the cleanup deletes relative to a directory handle it opened
O_NOFOLLOW instead of re-resolving a path it has already checked — see Mitigation below.
The alternative was hand-written unsafe around the same libc calls, in a code path whose job
is deleting things, which seemed the worse trade.

It adds nothing to the build. tempfile, already a direct dependency, pulls in this exact
crate and version with the fs feature on; cargo tree shows one rustix v1.1.4 before and
after, and Cargo.lock gains no new package entry. Flagging it anyway because the template asks for
explicit acknowledgement of new dependencies, and this is a new line in Cargo.toml.

heed stays in the dependency list: the paid-key list has its own LMDB environment, which this
change does not touch.

ADR

https://github.com/grumbach/ant-node/blob/41a9c82d20f6cd2c4cdd3e553c2d005f3c24165a/docs/adr/ADR-0022-remove-the-lmdb-chunk-store.md

Mitigation / rollback

The blast radius is bounded at the point of deletion rather than after it. A directory is removed
only when the previous release had finished with it — it carries the RETIRED mark retirement
wrote once every kept chunk was in the file store and every shed chunk was proven held by the
close group — or when it is empty, and both conditions are re-checked on the deleting thread,
against a handle on the directory it is about to empty, immediately before it does. Everything else is kept, and the node runs and serves what it
did migrate.

Operator-facing, and it belongs in the release notes rather than only in the ADR: the mark
is believed and never re-verified, because this build cannot read the environment to compare.
An operator who restores a backup of chunks.mdb must remove the RETIRED file from it
first. Left in place, the next start reads it as the previous release's record that the
directory was finished with, and deletes it. No release produces that state; a restore can.

There is no fleet-side lever any more. The penalty is restored here and
ANT_SUSPEND_UNHELD_CHUNK_PENALTY is removed with it, so suspending the accusation again takes a
release, as undoing the store removal always did: the on-disk format rolls back cleanly, but a node
put back on the previous binary is dragged forward again by an upgrade monitor that cannot be
switched off: it rediscovers the upgrade at its next check and applies it after any staged-rollout
delay. Adding a disable or a version ceiling is an upgrade-subsystem change and is
deliberately not in this release; it is named as an open gap in the ADR.

Two consequences to watch once it lands, both intended. A node that arrives still holding a store
it cannot read is charged on every lane that asks it for one of those chunks, and once below the
swap threshold it can lose routing slots to peers that can serve them; the store itself stays on its disk. And the responsible-chunk
audit charges a timeout at weight 5 again, as it did before v0.18.1, so a slow or overloaded peer
loses trust on that lane again.

Fleet gate: answered by decision, not by a count reaching zero (ADR-0022, The fleet gate, and
how it was answered). On 2026-10-01 every node the project runs reported from its own disk that it
had finished. A minority of the community peers visible to them still announced an old store, most
of them as of a start days earlier, and the v0.21.0 rollout restarting them was converting most of
those it reached to finished. The decision owner held this back two more weeks for the rest;
whatever has not finished by then is penalised as above.

Re-measured 2026-10-08, read-only, over 2026-10-07T04:21:50Z to 2026-10-08T04:21:50Z:

Nodes the project runs: 774 of 774 report a finished store from their own disk, all on
v0.21.0, every reading under 10 minutes old.
Community nodes, from what they announce: 1,081 distinct identities opened a new
connection to those nodes. 1,022 (94.5%) announce a finished store, 58 announce an old one,
and 1 announces node/0.27.3, most likely v0.18.1, which has no migration at all. 46 of the
58 announced it at a start at least three days earlier, so some will have finished since
without restarting.
Down from 92 of 1,018 on 2026-10-01. No node was seen going back from finished to old.

That is short of 99% of visible nodes on any reading that does not discard evidence. Even
setting aside the 46 stale announcements, it is 98.7%.

Any devnet or canary run before it, and the final go, are the release manager's call and not this
PR's.

dirvine

Review — LMDB chunk store removal (final migration release)

Six-seat independent review against the PR head (94e247b) plus the previous release's store for provenance. CI green across macos/ubuntu/windows and ext4/xfs/btrfs; ADR-governance, clippy, fmt, doc, security-audit all pass. Data-loss, concurrency, API/semver, and replication/audit lanes are sound. One security finding to address before merge; release-gating handled separately (below).

MAJOR — path-based deletion in `delete_mark_last` enables symlink-swap escalation outside the data dir (`src/storage/legacy_artifacts.rs:223–265`)

delete_mark_last re-checks symlink_metadata(dir) is a real directory at line 229, but then re-opens the same path with read_dir(dir) at lines 240 and 247. read_dir follows a symlink. A local actor with write access to the node's data dir who wins the window between the metadata check and the read_dir can swap the marked directory for a symlink to an arbitrary target, redirecting the node's remove_file / remove_dir_all (lines 252–255) at the symlink target's contents — files the attacker cannot reach but the node process can (e.g. the root-run node under the terraform unit's ProtectSystem=strict). The ADR's accepted-risk paragraph (":214–222: whoever can win that race already has write access to this node's data directory") only covers deleting things inside the data dir; it does not address this escalation-outward variant. This is real privilege escalation, not just data-dir data loss.

The fix is available in this same codebase: open_regular already uses libc::O_NOFOLLOW, "checked on the handle, not the path" (src/storage/chunk_store.rs:2479–2487). The same pattern — open the directory with O_NOFOLLOW | O_DIRECTORY, then unlinkat/fdopendir relative to that handle — closes the window. Please either apply that (unix path; keep the existing portable fallback) or amend the ADR's accepted-risk paragraph to explicitly name and justify the external-target escalation. Not blocking the code otherwise.

Release gate — do not publish until the fleet gate in ADR-0015 is actually met

Consistent across the PR body, the ADR and the code: the migration signal shipped under a day ago (2026-09-09), staged migrations haven't started, and community/NTFS hosts aren't reporting. ADR-0015 is explicit that no pre-staged-migration count is progress and "this release is not published on the strength of one." Given the no-rollback asymmetry (upgrade monitor drags nodes forward; marked directories are deleted finally), holding publication is the only defensible call. Merging the code is fine; publishing is not yet safe. This is an operational gate, not a code defect.

Fleet-ops MAJOR (roster gap): the positive half of the gate — "roster member finished" — is not implementable from what the code emits. Per-peer migration_event="peer_state" lines name only peers that are not Files (src/storage/migration_signal.rs:368–379); Files peers appear only as an aggregate count (:464–470), so a roster node that finished is indistinguishable, in the logs, from one that never connected. The negative half (no outstanding peers) is fully implementable. Recommend a low-rate named peer_state line for files peers (or a peers_seen list) before the gate is claimed met, or satisfy the roster check out-of-band.

Minor / follow-ups (none blocking)

config/production.toml still ships [upgrade] enabled = false, but UpgradeConfig has no enabled field — stale config telling operators they can disable upgrades when they can't. Remove or annotate.
pub struct StoreLayout sits in a pub(crate) module under non-test cfg — a future cfg flip would silently re-publish it. Consider pub(crate).
ADR-0015 "values over 4 MB never existed" is overstated — they could exist via the bridge's ceiling-free LmdbStorage::put. Conclusion holds (they are not chunks, safe to delete), but reword to "can exist but are not chunks."
The RETIRED-in-a-restored-backup data-loss path is accepted in the ADR; make it prominent in release notes / operator docs, not buried in the ADR.
NTFS: tombstone matching is case-sensitive-by-string but case-folded-by-filesystem; one sentence in the ADR noting this would close the gap.
Version lineage: Cargo.toml is 0.18.1 while ADR-0015 cites v0.19.0-beta.1. Bump to 0.19.x, call the break out in release notes.
Concurrency seat flagged an ordering point worth the payments seat's eye: in the client-PUT handler, holds_verified runs (step 3) before payment verification (step 5), and its Wrong branch can write/replace a file on disk before any payment gate. Not a data-integrity race, but an unauthenticated remote reaching the write path.

Verdict

No data-loss, concurrency, API, or replication blockers. The only must-address item is the delete_mark_last TOCTOU (MAJOR). Everything else is minor or release-timing. The author's claims check out against the code; the "no wire/protocol change" and "replication routes through ChunkStore" claims verified. Requesting changes on the security finding; otherwise this is approve-quality once the gate elapses.

dirvine

Re-review — fix verified, approving

Reviewed the two fix commits (efd9289, cd5e91d) at head cd5e91d against my prior findings. CI fully green (all platforms + storage fs matrix); the 21 legacy_artifacts tests pass locally.

MAJOR (delete_mark_last privilege escalation) — resolved, and resolved well.
delete_mark_last now opens the directory once with O_NOFOLLOW | O_DIRECTORY and makes every unlink against that handle (unlinkat/openat via rustix, unix only). A link can never produce a handle, so no unlink can be redirected. The follow-up details are the right ones: the mark's type test is masked with S_IFMT (the prior S_IFREG test wrongly accepted a symlink/socket — a data-deletion-direction bug, now caught and pinned with per-type tests); a subdirectory inside a leftover is refused rather than entered (O_NOFOLLOW doesn't stop a mount-point walk or bind-mount cycle); the mark is removed through the same handle (closing a second re-open window); the handle is re-asked immediately before the irreversible step (a listing is not a snapshot); and an implausibly wide listing is refused rather than read into memory unbounded. The improved symlink test now marks the target so it satisfies every gate, plus a test that a link never yields a handle. The off-Unix path-based fallback and its exposure are now stated honestly in the ADR. The author's own third-round adversarial pass also fixed a data-loss bug their first fix introduced — exactly the right discipline on a destructive path.

Release gate — unchanged, still correct to wait. The migration signal shipped under a day ago; staged migrations haven't started; community/NTFS hosts aren't reporting; no rollback once published. Do not publish until the ADR-0015 gate is actually met. Merging the code is fine.

Standing follow-ups (not blocking this merge):

Roster half of the gate (MAJOR, gate-mechanics): Files peers still aren't named individually — only as an aggregate count — so "roster member finished" can't be confirmed from the logs. Needs a low-rate named peer_state line for files peers (or a peers_seen list), or an out-of-band roster check, before the gate can be claimed met.
Version: Cargo.toml is still 0.18.1 while ADR-0015 cites v0.19.0-beta.1; bump to 0.19.x and call the break out in release notes at release time.
Payments-seat flag: in the client-PUT handler holds_verified (step 3) runs before payment verification (step 5) and its Wrong branch can hit the write path before any payment gate — worth a look from the payments side (not a data-integrity race).
NIT: pub struct StoreLayout remains in a pub(crate) module under non-test cfg (now documented in the ADR's API-narrowing caveat).

No remaining code-level blockers. Approving the code; publish timing governed by the fleet gate.

Once a node reaches `Committed`, the copier moves only the keys the rank check refuses to shed. A chunk that turns up in the legacy environment after that, and that this node never agreed to give up, is on no such list: nothing copies it, the shed gate correctly refuses to prove a single copy exists elsewhere, and the startup reconciliation only returns a node to `Bridging` when the file store holds less than it recorded keeping, which the file store never does. The node is terminal in that state. Free disk does not change it and neither does a restart. Seen on one node of 38 in the 2026-09-08 beta cohort: it committed with "nothing has to be shed", then pre-retirement verification found 317 chunks in the legacy environment that were in neither view and re-queued them. It has refused retirement every 30 seconds since. Once #218 removes the LMDB reader those 317 stop being servable, and on the evidence they are the network's only copy. The fix is a third arm in the reconciliation block, next to the two of the same shape: when the marker says `Committed` and there are more legacy-only keys than the node ever approved for shedding, go back to `Bridging`, clear `committed_at_unix` and reset `rebuilds_since_commit`. `open_legacy` already computes the legacy-only set before the marker is read, so this is a comparison against a number that is already in memory. No new I/O, no marker format change. The count, not emptiness, is the test. A node that legitimately shed keeps exactly the keys it is giving up in the legacy environment until they stop being answerable, so a non-empty legacy-only set is its normal state; bouncing it to `Bridging` on every restart would reset its retention clock each time and a regularly restarted node would never retire. The arm fires only on `legacy_only > shed_key_count`. It logs `migration_event = "back_to_bridging"` with both numbers so the case is visible in the beta watch. The upgrade's own restart is enough to take an affected node out of the loop. Tests: `cargo test --lib` 1104 passed, 0 failed; fmt clean; clippy with `-D clippy::panic -D clippy::unwrap_used -D clippy::expect_used` clean on lib and tests. Two tests added, and both were mutation-checked: with the discriminator replaced by `legacy_only > 0`, the new regression test and the existing `the_migration_marker_survives_a_restart` both fail; with the arm disabled, the fire-case test fails. Closes V2-1232 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UrjwPBwPqMi3F2sT1yVYEa

jacderida · 2026-09-15T20:07:13Z

T3 testnet evidence — V2-1213, DEV-01 run 583 (2026-09-11/12)

What ran. This PR's head cd5e91d, built from grumbach:storage/lmdb-removal-release-3 (binary sha256 bba27f6136924215300cd183bb4d058a625a6877164add77ba33e16f2f6f6a63, verified on 73/73 node+bootstrap VMs; every ant-node starting line carries commit=cd5e91d), on the staging-short profile: 990 nodes (66 VMs × 15 across DO/Vultr/OVH/OVH-3AZ, 30 % NAT), 7 bootstraps, 10 uploaders + 2 downloaders, 6 hours, nodes at info. Adversarial cohorts planted with services stopped, then restarted twice (T+1:00 and T+3:30): marked env (A), tombstones + migration-state.json (B), unmarked store with data on a public and a NAT VM (C), plain file / symlink to a decoy / four near-miss names / marked store with a subdirectory / marked store bind-mounted read-only (D), sixty-six marked leftovers and symlink/socket marks (H), ANT_SUSPEND_UNHELD_CHUNK_PENALTY=0 drop-in (E), 15 chunk files overwritten in place (K), 150 nodes restarted with measured stop cost (R), and 30 nodes on a previous-release config file with db_size_gb + [storage.migration] + [upgrade] enabled = false (G). Full procedure, numbers and per-node sha256 are in the two comments on the Linear issue.

Result: all cleanup, corrupt-chunk, config, data-loss and hot-path gates passed.

Gate	Result
Cleaned leftovers go (A, B, H×66, D read-only after `umount`)	PASS — all 282 directories gone within 0.6 s of start (1 GiB in ≤0.6 s, 66 dirs in ≤0.1 s, in sequence), exactly one `space_returned` each, `df` back within 2 min, marker file untouched
Kept leftovers stay (C, plain file, symlink + decoy, near-misses, subdirectory, symlink/socket marks)	PASS — all 132 kept files byte-identical after both starts; exactly one `legacy_store_left` per leftover per start with the shape's reason text
Mark last (read-only store)	PASS — `RETIRED` present and dir non-empty after the first start; gone with one `space_returned` after `umount` + second start
Exact names only	PASS — near-misses never touched or named; fleet sweep of 66 VMs found nothing created/removed outside the cohorts; 0 cleanup lines on untouched nodes
Second start idempotent	PASS — cleaned cohorts logged nothing; kept cohorts logged the same single warning and deleted nothing
Stranded nodes serve	PASS — 59/59 C-held keys fetched at every check, C nodes seen by ≥946 observers throughout
Planted nodes read right	PASS — C/symlink/false-mark → `legacy`, plain file → `unknown`, all others `files`; the fleet-wide `peer_state` set was exactly the 42 planted nodes, 0 false positives, 0 misses
Corrupt chunks	PASS — 15/15 detected on first read (`does not match its name` → `Removed corrupt chunk file`), no restart or stall, all re-replicated byte-identical in 12 s–7 min
Old config loads	PASS — 30/30 started with `--config`, 0 parse errors
Penalty policy	PASS — suspension line on 1395/1395 starts, override VM logged `Ignoring it` 15/15, 0 `APPLIED`
Hot path / shutdown	PASS — `systemctl stop` p95 5.15 s, max 8.6 s over 375 restarts; 0 store-lock failures; upload p50 within +2 % of Phase 0 outside the restart hour
Stability / joins / transfers	PASS — 0 panics, 0 unplanned restarts; join p50 21.7 s; uploads 3729/3729, downloads 777/778 (one transient datamap miss in the restart hour)
No data loss	PASS — 535-chunk canary set 535/535 at all five checks
`UnknownCommitment`	0 across 4 099 audit failures in Phases 2–4

Two caveats.

"Clients never counted" (S3) failed by one edge. On one of 21 402 signal ticks, one observer reported peers_unreported=1 and a peer_state … agent=none line for a client/0.27.3 connection. The NotANode filter and the agent string were correct (the same client was skipped correctly 780 times); the leak was a peer whose user agent had gone from the transport while its connection was still in the tally. This has since been traced (V2-1260) and addressed by fix(storage): stop counting a peer that disconnects mid-tally as unreported #228 (stop counting a peer that disconnects mid-tally as unreported) and fix(transport): record and drop a peer's user agent under its connection entry saorsa-core#163 (record and drop the user agent under the connection entry).
The audit-rate gate was not measurable as written on this build: commitment rotation is a 3600 s constant and the start-time build retires an empty store, so Phase 0 has no commitment-bound audits to baseline against. What could be measured — UnknownCommitment = 0 throughout, hourly rotation on every node — was clean.

Deviations from the planned manifest: uploader eth funding raised per the gas preflight (4.8 → 6.05 ETH) and ant pinned to 0.3.6; nothing else.

…p chunk Suspended for two releases so the fleet could move off a chunk store that never returned disk. The penalty is the auditor's decision, so a node that has to give up chunks cannot stop its peers punishing it for that; the peers had to stop first, one release ahead, and the nodes moved in the next one. That is done, so the accusation means what it always meant and is enforced again. The switch and its environment override stay. Restoring the penalty is the moment most likely to need undoing in a hurry, and this is the cheapest way to do it. It suspends only the penalties a node hands out, so an emergency suspension has to reach the fleet rather than the node being penalised. A test now pins the value this release ships. The existing tests set the switch both ways on purpose and so never noticed which way it was compiled, which is how a suspension outlives the thing it was suspended for.

…t replaced it Two releases ago every chunk lived in an LMDB environment that never returned a freed page to the filesystem: the fleet deleted 2.29M chunks and recovered nothing. The release before this one copied every chunk into a file of its own and deleted that environment. This one removes the code that did it. What goes: the LMDB chunk store, the migration driver, and the facade that presented both stores as one while the copying was in flight. About 5,600 lines of bridge and driver, plus the harnesses that existed to prove the bridge worked. What is left is one store, one file per chunk, and it is called `ChunkStore` because that is what every caller already called it. `heed` stays in the dependency list: the paid-key list has its own LMDB environment, which this does not touch. A node that starts with an unretired environment still on disk refuses to start, and says which directory and what to do about it. Starting anyway was the tempting answer and it is wrong: those chunks are unreachable to this build, but the commitment this node published before the upgrade claimed them, and a commitment is good to its neighbours for two hours. The accusation the first release suspended was "you did not have a chunk you were supposed to hold"; the commitment-bound audit was never suspended in any release. So a node that starts half-migrated spends hours failing audits at full weight on the one lane that always counted, for keys it cannot read. Refusing everything would be wrong too. A migration that finished and then failed to delete the directory leaves one behind that is safe to ignore, and a node whose only fault is a failed `remove_dir_all` should not be held offline for it. So the question is not whether an environment is there but whether it was retired, and the evidence is the mark the retirement wrote inside it. Three states, not two: a mark that cannot be read is neither permission to start nor a reason to stay down forever, and it says which case it is. Tombstones are checked as well as the live name, because a crash between the rename and the mark leaves an intact environment wearing a retired-looking name. Nothing is deleted; this build has no migration code and no business deciding that a directory it cannot read is safe to remove. The deployment settings for the per-volume migration lock go with the migration. They configured an environment variable that no longer exists. The loopback filesystem CI job now runs the storage tests rather than the deleted harnesses, so ext4, XFS and btrfs keep covering what the store does on them: publish through a temporary and a rename, flush, delete, and rebuild an index from the names. BREAKING CHANGE: `LmdbStorage`, `LmdbStorageConfig`, `MigrationConfig`, `MigrationPhase` and `MigrationState` are removed from the public API. `storage.migration` and `storage.db_size_gb` are removed from the node configuration; the second capped a memory map that no longer exists. A config file written by the previous release still loads with both keys present, because nothing declares `deny_unknown_fields`, and there is now a test holding that true.

Two of these are the same failure as the one already caught here: a removal that took something load-bearing with it. The workflow lost two job headers. Removing the deleted harnesses' steps by matching on step boundaries also swallowed the `filesystems:` and `doc:` declarations, so their steps were absorbed into the per-OS test job. That job then referenced a matrix key it does not declare and ran `apt-get`, `mkfs` and `mount` on macOS and Windows, the filesystem coverage never ran as its own job, and documentation sat behind three failing jobs. Every matrix reference now belongs to the job that declares it. Shutdown stopped aborting the protocol routing task. That went with the migration ordering it was written next to. The routing loop has no cancellation branch of its own: it waits on `events.recv()` while holding an `Arc` on the P2P node that keeps the sender alive, so nothing left would ever wake it. It would sit there holding the chunk store and its single-process lock open after the node returned. A node with an unmigrated store could start by turning storage off. The refusal lived only in the store's constructor, and a node configured with `storage.enabled = false` never builds a store. Turning storage off is not consent to run beside chunks whose commitment is still live, so the question is now asked before anything is built. The test that covers it goes through `build()` both ways, because the failure worth catching is a route into the node walking past the check, which is exactly what happened. Finding the leftovers folded unreadable into absent. A `try_exists` that failed, a node root that could not be listed, and an unreadable entry inside it were all read as "nothing here", so a root that permits traversal but not listing would hide an unmigrated store and the node would start. That is the same fail-open the classifier itself is three-state to avoid, one step earlier in the same file. Each is now a refusal that says which question could not be answered. Also: the shipped production config still advertised the database cap and the whole migration section, describing a copier that no longer exists; the two tests that mutate the process-wide penalty switch now both serialise rather than one of them; `page_size` was left as a direct dependency for LMDB map alignment that no longer happens; a failpoint for an operation that no longer exists is gone; and the startup narration still told operators the penalty was suspended.

Making the leftover check refuse on an unreadable root made it refuse on a missing one too, because reading a directory that is not there fails like any other read. That is every node starting for the first time. The whole suite passed with it, because every caller in the tree happens to create the root before opening a store. Nothing depended on that being true, and nothing said so. A root that is not there holds nothing, which is an answer rather than a failure to get one. Every other read failure still refuses.

…ng nodes that are fine The facade held a per-key critical section across whole logical transitions, and deleting it took two requirements with it that had nothing to do with the migration. A delete no longer waited for a write already in flight for that key. A write's blocking half outlives the future that started it, so a cancelled put can still be queued when a delete arrives, and the write then lands afterwards and puts back a chunk the node had decided to prune. The key ends up in a store that no longer claims it. Both regression tests for this ordering were deleted with the facade even though the requirement was not. One is back, and it reproduces the resurrection when the wait is removed. The check that decides whether an offered copy is already held no longer excluded deletion. A prune could remove the file between that read and the answer, so the caller was told the chunk was already held while the good copy it was offering was discarded. Both now take the key's lock for the whole operation. Two ways this refused nodes that are fine. An empty leftover directory is what the previous release's cleanup leaves when it is interrupted between removing the mark and removing the directory: fully migrated, nothing in it, and that release recognised the state and tidied it up. Holding a node offline for a directory with nothing in it is an outage for bookkeeping. And a root that does not exist yet is every node starting for the first time, which the unreadable-root refusal had swept up with it. The startup check also ran after the transport was built, so a bind failure could mask it and a node that did see it had already been charged for a transport it was about to throw away. It runs as soon as the root is known. The test proves the ordering by staging a port an ordinary user cannot bind: moving the check back after the transport returns the bind error instead.

…t was not the bridge's The key's lane was taken by the delete and by the check that answers whether a chunk is already held, but not by the put. That is not enough. A put does a lot before it registers itself as in flight: it checks the address, reads to see whether the name is taken, and reserves capacity. A delete arriving in that window finds nothing registered, waits for nothing, and goes ahead, and the put then registers and publishes afterwards. The node keeps a chunk its pruner had already given up. The put now holds the lane for its whole transition. Repair is split into a public entry that takes the lane and a body for the three callers that already hold it, because the lane is not reentrant. Two harnesses were deleted as migration machinery and were not entirely that. A process killed mid-publish leaving no chunk the store cannot serve, and an interrupted write's leftovers being swept, are about the store's own publish path, which is now the only one there is. They are back as `tests/chunk_store_crash_safety.rs` and run in CI. A third property, that engine shutdown waits for a store write whose awaiter was dropped, went with a harness written against the old store; it needs a live P2P node to stage, so the record names it as missing rather than this change pretending otherwise. Three explanations claimed more than the code does. The non-atomic rewrite off Unix was justified by the old store still being there to repair from; the argument now is that every caller reaches it only after a read proved those bytes wrong, so a crash leaves wrong bytes where wrong bytes were. The already-held check linearises a question about the store, and does not follow its answer out to the wire, where a prune can still land before the peer hears it. Aborting the protocol task asked it to stop without establishing that it had; the handle is awaited now, which is what actually releases the store. The record said commitments stay answerable for two hours. The constant says three.

The put-against-delete test did not test what it said. It started a put and a delete together, accepted either ordering, and asserted only that the index and the disk agreed about the outcome. The bug it was written for satisfies that: a delete that finishes and a put that publishes afterwards leaves both agreeing that the chunk is there. Removing the lane it was supposed to be guarding left it green. Staging it properly needed a hook that did not exist. The existing gate parks a put inside its blocking closure, which is after the write has registered itself, so a delete blocks on waiting for that registration and the test cannot tell which mechanism stopped it. The new gate parks a put after it has taken the key's lane and before it registers, which is exactly the window the lane exists for. It is an async lock: a synchronous one there blocks the runtime the put is on, and the first attempt deadlocked rather than observing anything. The restored crash test could pass having checked nothing. The child discards its put results and the failpoint counts arrivals rather than successes, so every publish before the kill could have failed, leaving an empty store and a serviceability loop with nothing to iterate. It now requires the store to hold something first. `repair` claimed atomic replacement and an untouched old file on every error. Off Unix neither is true: there is no durable rename there, so it truncates and rewrites in place. What makes that acceptable is a precondition nothing enforces, so the contract now states it, and the note about which callers establish it counts them correctly. The rest is the rename's prose finishing: comments describing chunk operations as LMDB, and explanations of the health counter and the directory flush that were written for a verification pass that ran before a store that no longer exists.

`fsync_path` and `write_file_durably` existed for the retirement: marking a directory before deleting it, and writing that mark durably. Both lost their only callers when the migration went and neither is reachable now. Nothing local caught them. `clippy --all-features` cannot: with every feature on, the module is public, so a `pub fn` nobody calls is still reachable and not dead. The build CI runs is `--no-default-features`, where the module is crate-private and the same function is dead, and it runs with warnings denied. That combination is what turns this into an error, and it is now part of what gets run here before a push rather than after one.

…ts, and stop overclaiming Both put gates were compiled under `test-utils`, which the e2e suite enables, so every write in that suite went through an await production will not have. That await is the pre-registration gate, and nothing outside the unit tests uses it, so it is `cfg(test)` now and e2e exercises the same path a node does. The put gate stays under `test-utils`: taking it is an uncontended read on the blocking thread rather than an await, and the browser devnet harness parks a real write through it. The gate handshake was a hundred-millisecond sleep, which makes the staging a guess and the word "deterministic" in the comment above it untrue. It waits on a counter now. Four claims were larger than what is actually checked. The rollback note said nothing on disk is deleted or rewritten, which is only true of the old store: the node still writes, repairs and prunes its own chunks, and opening the store still creates the store's own files and sweeps orphaned temporaries. The record said a node with a retired leftover "says so" and an unclassifiable one "says which", when only the refusals' messages are asserted. The filesystem CI job said it watched space come back; it watches an unlink. And the config compatibility test read one table out of a file rather than loading a whole previous config through the loader a node uses, which is now what it does.

A node must never be unable to start because of what the previous release left behind. The earlier shape refused to open an unfinished legacy store, and that cannot be recovered from: auto-upgrade is unconditional and has no enable field, so an operator cannot hold a node on an older build to let it finish. Under the deployed unit's Restart=always that is a restart loop until a person intervenes. Two drafts were wrong in opposite directions and both are worth knowing about. Refusing to start serves nothing, not even the chunks that migrated fine. Deleting whatever is found is data loss for the node that most needs the data, because the upgrade monitor picks the newest eligible release rather than the next one, so a node offline through the previous release arrives with everything it owns still in that directory. Cleanup now removes only a leftover that carries the RETIRED mark or one that is empty. The mark is deleted last, so an interrupted cleanup still reads as interrupted rather than as never having migrated. Names are matched exactly, so an operator's chunks.mdb.retired-keep is not claimed. Links are neither followed nor unlinked, and the mark must be a regular file at the exact name: try_exists follows links and says nothing about kind, and this answer authorises deleting every chunk beneath it. Anything else is kept and named once. Cleanup runs after the file store opens, and not at all when storage is disabled. Deleting first and letting the constructor fail behind it leaves a node with neither store.

The cleanup matches the old store's exact names and nothing else, and that is a decision rather than an implementation detail: it is what keeps a `chunks.mdb.retired-keep-this` somebody put there on purpose, and what keeps `.007` and `.65`, which retirement cannot have produced. Anything that widened the match later would be a silent data loss rather than an obvious one, so what is outside the set is written down along with why. The half of this change that touched the emptiness predicate is gone. The release before this one removed that branch outright rather than fixing its test again, and the reasoning it left behind is better than what this was going to replace it with.

…cord `health_generation` and `invalidate_capacity_cache` have no callers anywhere in the tree, tests included. Both existed for the migration: one let a long-running verification pass tell whether a chunk had stopped being servable underneath it, and the other re-measured free space right after the legacy environment was removed, a step change that no longer happens. This is the release that removes dead migration code, so they go with it. The decision record also claimed this release deliberately preserves an oversized-chunk sidecar and a rollout stamp. Neither exists: the sidecar was removed along with the unreachable branch it served, and the persisted rollout window is not in this release. A record that lists safeguards which are not there is worse than one that lists none, because the next reader plans around them.

…e release The original plan had this release delete the old chunk store and restore the close-group storage penalty together. Separating them is the point. The upgrade monitor picks the newest eligible release rather than the next one, so a node that was offline while the migration ran arrives here having never migrated, holding a legacy store this build cannot read. This release keeps that store rather than deleting it, which is right for its data, but the node cannot serve those chunks and its close group will notice. Restoring the accusation in the same release would slash that node for a state it had no chance to leave, in the release that put it there. So the penalty stays held off here and is restored by the release after this one, once the fleet has been observed clean for long enough to include the nodes that were away. That is a one-line change to a build constant. The cost is that one migration-era constant outlives the release that was meant to remove them all. That is the trade: no release may break the fleet, and that outranks no migration code surviving. The test that named the old value now names this one and says why.

…ecord promising a penalty `flush_namespace` and `stored_len` have no caller anywhere in the tree, and `note_health_changed` has none now that the counter it stamped is gone with the two stores it reconciled. Removing the store's second half left them behind; they go with it. The decision record and the startup comment both still said this release restores the close-group unheld-chunk penalty. It does not, and the reason is the whole of the decision above them: a node that was away while the migration ran arrives here holding a store this build cannot read, and restoring the accusation in the release that stranded it would slash it for a state it had no chance to leave. The release after this one restores it. Both now say so.

The decision record said in three places that this release restores the close-group unheld-chunk penalty, while the shipped constant stays suspended and a section further down explains why. A record that contradicts the code it describes is worse than one that says nothing: the next reader plans around the half they read first. The switch's own comment said the same, describing itself as kept 'after the flip' in a release that does not flip it.

…one directory The release before this one put each node's answer on the wire so a fleet could be seen to have finished moving off the old chunk store, and this release is published on the strength of that count. The cleanup then arrived carrying its own copy of the same judgement: the same constants, the same exact-name matcher, the same "does it carry the mark, is it empty" classification, written out a second time. Two readings of one directory can drift, and either direction is a fault. One lets a node delete a directory it is still reporting as unfinished. The other leaves it reporting `files` while it goes on paying for the disk for ever. Neither shows up in a test that only ever asks one of them. So there is one classifier, and it is the one the signal already uses. `legacy_artifacts` now enumerates with `legacy_directories` and decides with `classify`, and keeps only the prose that tells an operator which of the reasons applied — prose that cannot decide anything, so a stale reading of it costs a warning that names the wrong reason rather than a directory deleted that should not have been. Enumerating through the signal's version also answers a case the cleanup's own silently got wrong: a root that cannot be listed hides every tombstone under it. The signal calls that `unknown` rather than `files`; the cleanup returned quietly and removed nothing, which was the right action with no way for anyone to know it had happened. Also stops the deletion taking a thread per directory. The names this release accepts are the live directory, the unnumbered tombstone and sixty-four numbered ones, so a root that has been through enough restore cycles can present sixty-six at once, and sixty-six concurrent recursive deletions land on the disk that is also serving chunks, at the moment the node is starting. Nothing waits on them, so they are done in turn on one thread. Three tests for three properties, none of which had one before: that a directory is removed exactly when the signal calls it harmless and kept exactly when it does not, asserted in both directions over a staged root; that all sixty-six are removed by one start; and that a root that cannot be listed has nothing removed from it.

Folding the two stores into one moved several names into `chunk_store`, and re-exporting them from `storage` published some of them more widely than before. `CapacityVerdict` was `pub(crate)` on the old store and is again. `StoreLayout` was public through `file_store`, and is now crate-private: it is the on-disk shape of a directory only this crate opens, and it is one of several exports this release withdraws rather than the only one — the full list is in this removal's decision record and in the pull request, because a downstream crate importing any of them stops compiling. `legacy_artifacts` and `migration_signal` are this crate's own business too. The cleanup is called once, from the node builder. The signal was `pub(crate)` in the release that added it, and the copy this branch was built on had widened it to `pub` along with three of its types, for a caller that no longer exists. Publishing either would put a migration this release exists to finish into the API other crates compile against. `LEGACY_ENV_DIR` keeps its one caller, a test, which now names the module it comes from.

Three things the record stated less exactly than the code behaves, each of which would have let a reader draw a stronger conclusion than the release supports. **Kept is not available.** The table said a node in those rows "starts, serves its file store, keeps its old one", which reads as though keeping the directory keeps its contents in play. There is no LMDB reader in this build. A node that keeps a directory keeps its bytes on disk and can serve none of them, and for a node that skipped the previous release altogether that is everything it holds — it serves only what it refetches from here, exactly as a new node would. The table now says what each state serves as well as whether it starts. **The mark is believed rather than verified.** It is the assumption every deletion here rests on, and it was left to be inferred. This build cannot confirm it independently, because it cannot read the environment. What the mark records is stated precisely in a later commit, which corrects the description this one gives of it. **Rollback rolls the format back, not the operation.** The previous release reads the same layout and can still finish a legacy directory this one kept, but `build_upgrade_monitor` is unconditional and `UpgradeConfig` has no field that disables it, so a node put back on the previous binary is dragged forward within the hour. That is why the fleet gate is a gate. The remedy is an upgrade-subsystem change and is deliberately not in the release that also deletes a store. Also records the deletion running on one thread rather than one per directory, that the cleanup and the fleet signal are now one classifier and how that is tested, and that the path-check race cuts both ways rather than only towards deleting.

Three things the default build had no opinion about. `cargo clippy -- -D warnings` on the library alone rejects `pub(crate)` inside a module that is already crate-private, and `chunk_store` is exactly that off `test-utils`. It passes under `--all-features` because the feature makes the module public, so the whole-workspace clippy CI runs never saw it. `sweep_marker_temps` and the signal's `LEGACY_ENV_DIR` say `pub`, which is the same visibility through a crate-private module and the spelling both builds accept. `--no-default-features` compiles `warn!` to nothing, so `why_it_is_kept` — whose only caller is inside one — is dead code there, and the no-logging job builds with `-D warnings`. Marked the way the signal's own token helpers already are, rather than with a `#[cfg]` that would take it out of the build the tests run in. And the enumeration reads as a `let ... else`, which is what it always was.

That release added exactly two, and this one does something different with each. Leaving either to be noticed in a diff is how a deleted test looks like an oversight and a surviving one looks like luck. The migration-continuity test proved an upgrade picks a half-finished copy up where it left off rather than restarting the clock the waves are measured from. It drives `copy_batch`, `migration_phase`, `legacy_only_keys` and `migration_state`, every one of which this release deletes, so it goes with its subject: there is no migration left for an upgrade to continue, and a test of one cannot be rewritten against a release that has none. The commitment-state test is the opposite case. It is the reason the rotation has no emptiness branch, it touches nothing this release removes, and it still passes here — and it has to go on passing, because the branch it rules out is exactly the one an earlier draft of this release was going to put back.

…ith a test This was argued both ways across the three releases and one of the earlier answers — preserve such values to a sidecar rather than destroy them — was wrong. Settling it here stops a later reader reopening it from that sidecar's remains. A chunk is at most 4 MB. `MAX_CHUNK_SIZE` has been that since ant-protocol's first commit, and every released path by which data enters a node from the network enforces it before anything is stored: the protocol handler on a paid store, and replication on both receive and fetch. No value over the ceiling has ever entered this network as a chunk, so one found in a legacy environment is not data with a copy elsewhere and not data anyone is waiting for. It is not a chunk. The one way such a value could reach a disk was ours. The bridge's public `put` wrote to the legacy environment first — `LmdbStorage::put` has no ceiling, deliberately, because the store it wrote to was being abandoned and its verdict was not allowed to refuse chunks the file store had room for — and only then offered the same bytes to the file store, which refused them for size. The bridge recorded the key legacy-only so the copier would retry, and the copier's size arm deleted it. A local caller of that one method is the whole of the exposure. This release removes the bridge and the hole with it. One store, whose `put` refuses over the ceiling before it writes, so there is no partial state and no key recorded anywhere; no `LmdbStorage::put` left to take an unbounded value at all. Nothing is preserved and no sidecar is built: there is no valid chunk there to be the only copy of, and the cost of building for that case was already measured once, when adversarial review found two real defects inside a sidecar that existed only to serve a case that cannot arise. The test addresses the value to its own bytes so the refusal is the size arm rather than the content-address arm, and is mutation-checked: with the size branch deleted it fails. Also records where the fleet gate actually stands, which is nowhere. The signal has under a day of data, the staged migrations have not started — so a node reporting `files` today is saying it has nothing to move, not that it has finished moving it, and the tally cannot tell those apart — and the community and NTFS hosts the gate exists for are not reporting to us. No count taken before the staged migrations run is progress towards it.

…three comments inverting the penalty An adversarial review round on the exact diff returned do-not-ship, and it was right about the reason. **The mark does not mean "its contents were copied out".** This record said so in four places and the module header said it in a fifth, and it is false for a state the previous release produces on purpose. Retirement cleared a directory on two grounds, and only one of them is a local copy: every chunk the node KEPT was copied into the file store and re-hashed there, and every chunk it SHED was proven held by its close group — all but one answering a possession challenge, after the reduced commitment had been delivered — and was then deliberately not copied. The pre-retirement verification pass skips exactly those keys, and retirement then permits them and writes the mark over them. So a marked directory can legitimately hold bytes that are in no file store on this node, and this release deletes them. That behaviour is correct — the previous release was about to do it, and there is a kill point between the mark and the delete precisely so the next start finishes it — but the safety argument for those bytes is the close group's proofs, not a local copy, and saying otherwise misdescribes what authorises every deletion here. It also made the rollback paragraph wrong: for shed keys the remedy was never local, because rolling back would have deleted them too. Replication refetching them from the peers that proved they hold them is the remedy, and that is now what it says. The tests concealed the difference by never staging a file store at all, so they proved that a mark triggers deletion and left a reader free to assume the bytes had been copied first. One now stages the shedding node's state directly — marked directory, no file store — and pins that the deletion does not depend on a local copy, so nobody later adds a check that would strand every shedding node's directory for ever. **Three comments had the penalty backwards.** The constant is named SUSPEND and ships `true`, and this release keeps the penalty held off. But its own doc comment asked "whether this build penalises", the override's comment called the one-way guard inert when in this release it is active and is what refuses an operator's `=0`, and the fetch-fault enum said the lane "is penalised again". An operator reading any of them would expect a live switch that the code rejects. Also narrows two claims to what the code delivers — the cleanup never vetoes a start, which is not the same as every node starting — and names what the new tests do NOT prove: the sixty-six-leftover test does not observe thread count, the mark-last ordering has its failure half asserted on Unix only, and the entry-level unreadable case is not staged.

CI builds documentation with `--all-features` and `-D warnings`, and rustdoc refuses an intra-doc link from a public item to a private one. `sweep_marker_temps` became `pub` two commits ago — the same visibility through a crate-private module, spelled the way both clippy invocations accept — and that turned its doc comment's link to `write_file_atomic` into an error. Named in code font instead of linked; it is the same sentence. Worth recording why this got through: the local check ran `cargo doc --no-deps` without `--all-features`, and the feature is what makes the module public and the item's documentation public with it. That is the second gate in this branch where the default-feature build and the all-features build disagree, in opposite directions — the lib-only clippy run wants `pub` here, the all-features doc run then wants the link gone. Both are now run before pushing.

…no links The test collects one directory per verdict the classifier can return, and the link that supplies one of the two `Holding` shapes only exists on Unix. So the vector it pushes onto is mutated only there, and off Unix `mut` is unused — which the Windows test job builds with `-D warnings` and rejects. Marked `allow(unused_mut)` for exactly the builds that have no link to add, rather than dropping the link shape or making the whole test Unix-only. It is one of the two shapes that prove a link is never followed and never unlinked, and losing it off Unix would leave that platform asserting less than the record claims. Found by CI rather than locally, because every local run was on a Unix host. A cfg probe over this file — turning each `cfg(unix)` into a cfg that is never true and compiling — reproduces what the Windows runner builds, and now passes.

…esolved twice Independent review found a real privilege escalation in the cleanup, and the reasoning this record used to wave it away was wrong. The deleter checked `symlink_metadata(dir)` and then re-opened the same path with `read_dir`, which follows a link. Anything that can replace that directory between the two — a local actor with write access to the node's data directory — can point the name at a target elsewhere on the disk and have this process empty the target instead. The accepted-risk paragraph covered deleting things INSIDE the data directory, on the ground that whoever wins the race is already there. It said nothing about the variant that reaches outside, and that is the one that matters: the node reaches far more of the filesystem than the actor does, and on many installations runs as root. So on Unix the directory is opened once, `O_NOFOLLOW | O_DIRECTORY`, and every unlink is made against that handle with `unlinkat`; subdirectories are opened the same way from the same handle and emptied the same way. A link cannot produce a handle, so no unlink can be redirected through one. The pattern is already here: `open_regular` refuses a link and a FIFO on the handle rather than on the path, for the same class of reason. The directory itself still goes by path, which is safe on its own terms — `rmdir` refuses a symlink rather than following it. Off Unix there is no `unlinkat`, so that build keeps the path-based deleter and the exposure the previous release's deleter also had, stated in the record rather than implied. `rustix` for the safe `openat`/`unlinkat` wrappers, unix targets only. The alternative was hand-written `unsafe` around the same calls in a code path whose whole job is deleting things. It adds nothing to the build: `tempfile`, already a direct dependency, pulls in this exact crate and version with `fs` on, and `Cargo.lock` gains one line, the dependency edge. **The existing symlink test was passing for the wrong reason.** Its target was unmarked, so the gates refused it whatever the deleter did — a link-following deleter passed it too. The target is now marked, so it satisfies every gate and anything that reaches it deletes it. A second test pins the primitive that removes the window, since the interleaving itself cannot be staged without instrumenting the deleter: a link never yields a handle. Both fail with `O_NOFOLLOW` dropped, which is how they were checked. Three smaller things from the same review. The claim that no over-ceiling value has ever existed was overstated: none ever entered the network as a chunk, but the bridge's ceiling-free `LmdbStorage::put` could put one on a disk. The conclusion is unchanged and now the wording is too. On a case-folding filesystem a tombstone stored as `CHUNKS.MDB.RETIRED` is one file to the filesystem and another string to the matcher, so it is kept rather than removed — the safe direction, now written down. And `config/production.toml` no longer ships `[upgrade] enabled = false`, a line serde has always ignored, which told operators they could switch off the very mechanism this release's rollback story turns on.

…er never contains A third adversarial review round on the handle-based deleter written for the last one. Two of its findings are the kind that only show up when somebody attacks the fix rather than the thing it fixed. **The mark's type test was wrong, and wrong in the direction that deletes data.** It asked `st_mode & S_IFREG == S_IFREG`. `S_IFLNK` and `S_IFSOCK` each contain every bit of `S_IFREG`, so a symlink or a socket wearing the mark's name passed it — and with `SYMLINK_NOFOLLOW` on the stat, a symlink is exactly what gets examined. The classifier can call an empty leftover harmless and, before the worker opens it, a restore or a local actor can fill it with a store nothing migrated and drop a link called `RETIRED` beside the contents; the re-ask that exists to catch precisely that would have said yes. The path version this replaced used `is_file()`, which masks correctly, so this was introduced by the conversion. Masked with `S_IFMT` now, with a test for each type. **`O_NOFOLLOW` does not refuse a mount point.** It declines a trailing symlink and nothing else, and `openat` walks into a mount without complaint; a bind mount can also build a cycle. So a subdirectory inside a leftover is now refused rather than entered. A retired chunk environment is flat — two files and a mark — so a directory in one is already something this build does not understand, and descending into it risks emptying a filesystem that has nothing to do with this node. It also leaves no recursion to bound and no descriptor chain to exhaust. Three more from the same round. The directory was still opened twice, because the mark's unlink reopened the path after the emptying dropped the handle; that was a second window and the mark now goes through the same handle as everything else. A listing is not a snapshot, so an entry created while the unlink loop runs is not in it and is still there at the end — the handle is asked once more immediately before the mark goes, because removing it over something left behind produces exactly the unmarked non-empty directory the ordering exists to prevent. And the whole listing was read into memory unbounded, so a corrupt or hostile directory could take the node down through the cleanup meant to return it some disk; more entries than any leftover has is now a refusal. Also corrects what the last round left inconsistent rather than wrong: the record and five code sites still described a marked directory as one whose contents were copied here, which is the premise the round before this one disproved; two more penalty accessors documented themselves as reporting whether the lane penalises when their value is the suspension; a comment introduced two commits ago said "not `all_keys()`" directly above the call to `all_keys()`; the oversized-value test now snapshots the whole store tree rather than one path; and the note on the API boundary no longer claims a narrowing that `test-utils` does not honour.

Main took ADR-0015 for direct browser clients over WebRTC Direct while this branch was open, and the governance check fails a pull request that adds a number its base already uses. Open pull requests hold 0017 to 0019, and 0020 and 0021 are left for the two older open pull requests whose numbers also collide, taken in pull-request order. This record takes 0022. Only this record's own citations move: its title, the crash-safety test that names it, and a new line in the index. Every ADR-0015 reference main already has means the browser record and stays as it is.

… again The storage migration held off one accusation for three releases: "you did not hold a chunk you were supposed to hold". A node short of disk could not avoid failing those checks while it moved its chunks out of a store that never returned space, and it could not stop its peers charging it for that, so the auditors stopped first. This build has no LMDB chunk store left to move out of, so the reason is gone and the penalty is restored on every lane that made the accusation: the responsible-chunk audit (every failure reason, timeouts included, as before the suspension), the fresh-replication possession check, the prune audit, a sole-source replica hint whose sender then denies holding the key, and a fetch from a verified source that answers NotFound. (A fetch answered with Error was charged throughout; it now shares the NotFound handler again.) A node that never finished migrating arrives here with chunks it can no longer read. It now loses trust on those lanes, and with it its routing slots, to peers that hold their chunks. That is the intended outcome. Removed with the switch, because nothing else needed them: - RELEASE_SUSPEND_CLOSE_GROUP_STORAGE_PENALTY, the process-wide switch, its setter and getter, and the startup policy that applied it; - the ANT_SUSPEND_UNHELD_CHUNK_PENALTY environment override, now ignored; - penalise_unheld_close_group_chunk, the helper the lanes went through; - the SingletonHintFault and FetchFault splits, which existed only so two sub-cases could be charged differently; - the penalty_suspended field on the fresh-offer refusal warning. AuditType::as_str is gated on the logging feature again: its only non-macro caller was the helper. BREAKING CHANGE: ant_node::replication::config no longer exports the penalty switch, its functions or its constants, and ANT_SUSPEND_UNHELD_CHUNK_PENALTY has no effect.

…ftover cleanup The previous releases put each node's migration state in its user agent (node/<version> migration/<legacy|files|unknown>) and every fifteen minutes logged a tally of what its peers announced, one INFO line per peer that was not reporting finished. That existed to tell when this release, which removes the LMDB reader, could ship. This release is that decision, so the reporter, the per-peer tally and the token go: - the user agent is now node/<version>, still under the node/ prefix that saorsa-core admits to the DHT on, with a test pinning both; - the migration_signal module is deleted; - the classifier for an old chunk store's leftovers (exact names, the RETIRED mark, links never followed) moves into legacy_artifacts, its only remaining user, unchanged except for visibility and comments. The deletion defences already there (handle-based unlinks, subdirectories refused) are untouched: the cleanup still removes only a marked or empty leftover and still keeps and names anything else. Peers on earlier releases read the shorter user agent as a node that does not report, which only affects their own logging. BREAKING CHANGE: nodes no longer announce a migration/<state> token in their user agent and no longer log migration_event=signal or peer_state lines.

…ration read The migration would not let a node give chunks up until its close group had answered a neighbour sync carrying its reduced commitment root, so the responder state recorded which peers had received the current root and neighbour sync fed it on every answer. The migration is gone and nothing reads that record any more. Removed: - ResponderCommitmentState::note_commitment_delivered, current_delivered_peer_count and current_delivered_peers, the two fields behind them and their test; - the delivery notes in neighbour sync, which snapshots the commitment for gossip exactly as it did before the migration; - ReplicationEngine::sync_state, audit_challenge_coordinator and config, which existed only to hand the migration its possession challenges. BREAKING CHANGE: the ResponderCommitmentState and ReplicationEngine methods listed above are removed.

…hey are now The one-file-per-chunk store came out of the migration release with its comments still arguing in terms of what that release needed: a legacy store beside it, a copier draining a legacy-only set, a pre-retirement pass, and durability that mattered because a copy authorised deleting the only other one. None of that exists in this build, so every such comment now gives the reason that still holds, which is mostly that a chunk reported as stored is what lets a client drop its own copy. Where the migration was the only thing covering a gap, the gap is now described as it is: off Unix a newly created shard directory cannot be flushed, so a power loss soon after can lose the chunks just published into it, and this node gets them back only if a peer offers them while it is still responsible for them. Also corrected: the leftover cleanup's comments said a directory carrying the RETIRED mark holds nothing, or that its chunks are in the file store. A marked directory can hold the chunks the node shed, which retirement proved its close group holds and deliberately did not copy; the comments now say that, and the warning for a kept unmarked store says its chunks may not have been copied rather than that they never were. forget_if_absent loses a comment about a counter bump the closure no longer makes. No behaviour changes apart from the wording of four operator-facing messages.

…R-0022 ADR-0022 said this release keeps the unheld-chunk penalty suspended, keeps the ANT_SUSPEND_UNHELD_CHUNK_PENALTY override working, and is published once the migration signal shows the fleet has finished. The decision owner changed that on 2026-10-01: the release is held back two more weeks instead, the penalty is restored here with the switch and override removed, and the signal goes because nothing is left to gate on it. The record now states that decision and its cost: a node that arrives still holding a store it cannot read is charged on every accusing lane and loses routing slots to peers that can serve its chunks, while its store stays on disk untouched and the close group's replicas are unaffected. The fleet-gate section says how the gate was answered, the validation section says what pins the restored penalty, and the consequences no longer describe a fast lever that no longer exists. Statements that a node "reports" its migration state are removed or made historical.

…er's temporaries The migration release wrote its marker file in the node root through a temporary-then-rename, and a crash between the two left a small temporary there that nothing else would ever remove. So opening the chunk store also swept the root for files named like those temporaries. This build neither writes nor reads that marker, and the root holds a node's identity and whatever its operator put there. Deleting files from it by the shape of their name is not the chunk store's business any more, so the sweep, its name check and its test go; markers and temporaries already on disk are left where they are. The scan still sweeps interrupted writes under chunks/, which is where the layout marker's temporary lives. The test is replaced by one asserting that opening the store leaves every file in the node root alone, marker-shaped temporaries included.

dirvine

Review: APPROVE

Reviewed 41a9c82d20f6cd2c4cdd3e553c2d005f3c24165a against merge-base d41a353544f11b333e6e0ab42d9de7723ef5f772 in WithAutonomi/ant-node.

Six completed independent review seats, including an OpenRouter z-ai/glm-5.2 review, found no introduced blockers. I checked the findings against the source and ran the tests below. No blocking dissent remains.

Scope

Compared the old FileStore and facade with the new ChunkStore, not just the same-path diff. Checked per-key locking, cancelled writes, capacity reservations, corruption rechecks and repair.
Checked legacy cleanup: exact tombstone names, regular-file retirement marks, bounded enumeration, mark-last deletion, Unix handle-relative unlinking and refusal to recurse into subdirectories. Unmarked non-empty stores remain untouched; cleanup does not veto startup.
Checked restoration of all five penalty lanes, including audit timeouts; removal of suspension controls; old-config loading; startup wiring and CI changes.

Verification on this head

Full library suite with --locked --lib --features test-utils: 1,165 passed.
Crash-safety integration suite: 2 passed, 1 ignored (the helper test).
Storage-scale integration suite: 2 passed.
Storage library tests without default features, with test-utils: 92 passed.
Formatting, diff whitespace and strict ADR governance checks passed.
All checks returned by the final GitHub status query passed, including Linux/macOS/Windows and ext4/XFS/btrfs storage jobs.
Local all-target/all-feature Clippy with Rust 1.96.0 reports six errors. I reproduced the same six errors on the base branch: three duration-unit findings in pointer replication tests and three early-drop findings in chunk-store tests. These are not introduced by this PR; the GitHub Clippy job is green.

Release boundary and remaining limitations

This is code approval, not release or migration sign-off. ADR-0022 records the two-week release hold from 2026-10-01. The release owner must confirm the migration window and release version before shipping. This build cannot read an unmarked, non-empty legacy LMDB chunk store; it preserves it but restores penalties for missing chunks.

Non-blocking limits remain explicit: the non-Unix cleanup path retains its documented path-race limitation; process-kill tests do not prove power-loss durability; the deleted node-shutdown/detached-write integration test has no replacement here. I did not run a live fleet migration or validate Windows cleanup locally. These limits must not be read as tested guarantees.

No merge or release action taken.

grumbach force-pushed the storage/lmdb-removal-release-3 branch from ea50837 to fc7a9eb Compare September 10, 2026 03:43

grumbach changed the title ~~Remove the LMDB chunk store and restore the close-group penalty~~ refactor(storage)!: remove the LMDB chunk store, and never refuse a start over what it left behind Sep 10, 2026

dirvine requested changes Sep 10, 2026

View reviewed changes

dirvine approved these changes Sep 10, 2026

View reviewed changes

jacderida mentioned this pull request Sep 14, 2026

fix: reopen the bridge for chunks a committed node never agreed to shed #226

Merged

2 of 7 tasks

grumbach mentioned this pull request Sep 15, 2026

fix(storage): stop counting a peer that disconnects mid-tally as unreported #228

Merged

2 of 7 tasks

grumbach mentioned this pull request Sep 18, 2026

feat(node): serve authenticated browser clients over WebRTC Direct #220

Merged

2 of 7 tasks

grumbach force-pushed the storage/lmdb-removal-release-3 branch from cd5e91d to 982e396 Compare September 30, 2026 07:24

grumbach added 20 commits October 8, 2026 13:26

docs: fix a doubled word left by the rename

c740662

grumbach added 14 commits October 8, 2026 14:19

grumbach force-pushed the storage/lmdb-removal-release-3 branch from 982e396 to 41a9c82 Compare October 8, 2026 05:27

dirvine approved these changes Oct 8, 2026

View reviewed changes

Navigation Menu

Repository navigation

refactor(storage)!: remove the LMDB chunk store, and never refuse a start over what it left behind - #218

refactor(storage)!: remove the LMDB chunk store, and never refuse a start over what it left behind#218
grumbach wants to merge 34 commits into
WithAutonomi:mainfrom
grumbach:storage/lmdb-removal-release-3

grumbach commented Aug 28, 2026 •

edited

Loading

Uh oh!

dirvine left a comment

Uh oh!

dirvine left a comment

Uh oh!

jacderida commented Sep 15, 2026

Uh oh!

dirvine left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

grumbach commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Linear issue

Risk tier

Compatibility

Semver impact

Test evidence

New dependency

ADR

Mitigation / rollback

Uh oh!

dirvine left a comment

Choose a reason for hiding this comment

Review — LMDB chunk store removal (final migration release)

MAJOR — path-based deletion in delete_mark_last enables symlink-swap escalation outside the data dir (src/storage/legacy_artifacts.rs:223–265)

Release gate — do not publish until the fleet gate in ADR-0015 is actually met

Minor / follow-ups (none blocking)

Verdict

Uh oh!

dirvine left a comment

Choose a reason for hiding this comment

Re-review — fix verified, approving

Uh oh!

jacderida commented Sep 15, 2026

T3 testnet evidence — V2-1213, DEV-01 run 583 (2026-09-11/12)

Uh oh!

dirvine left a comment

Choose a reason for hiding this comment

Review: APPROVE

Scope

Verification on this head

Release boundary and remaining limitations

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

grumbach commented Aug 28, 2026 •

edited

Loading

MAJOR — path-based deletion in `delete_mark_last` enables symlink-swap escalation outside the data dir (`src/storage/legacy_artifacts.rs:223–265`)