Conversation
… gone The client PUT handler asks ChunkStore::holds_verified whether the node already holds the offered chunk, and answers AlreadyExists on a true before it checks capacity, closeness or payment. In holds_verified's file-store match, the arm after "bytes identical" was `Ok(_)`, which also caught `Ok(None)`: the index still named the chunk but its file was gone. The read drops that index entry, and the arm then called FileStore::repair with the offered bytes. On Unix the repair publishes through a temporary file and a rename, so it recreated the file and the index entry and returned true. A PUT carrying no payment for such a chunk was therefore written to disk and acknowledged without a payment or closeness check. Off Unix the repair opens the file without creating it, so there it failed and left the key marked wrong. `Ok(None)` now has its own arm that writes nothing. If the legacy LMDB environment still holds exactly the offered bytes, the node does hold the chunk: the answer is true and the key goes back into the legacy-only set, as `get` already does when it falls back to LMDB. Otherwise the answer is false and the offer takes the ordinary path, which checks capacity, closeness and payment before storing. Tests cover a store with no legacy environment, a copied chunk still held in LMDB, a copied chunk whose LMDB bytes are wrong, and the handler refusing an unpaid PUT for such a chunk.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A client PUT is checked against what the node already holds before any payment is verified:
handle_put_innercallsChunkStore::holds_verified, and atrueis answeredAlreadyExistsstraight away, ahead of the capacity check, the closeness gate andverify_payment.holds_verifiedcould create the chunk it was asked about. In its file-store match, the arm after "bytes identical" wasOk(_), which also caughtOk(None): the index still named the chunk, but its file was gone. The read drops that index entry, and the arm then calledFileStore::repairwith the offered bytes. On Unix the repair publishes through a temporary and a rename, so it recreates the file, puts the index entry back, andholds_verifiedreturnstrue. The result: a PUT carrying no payment for a chunk whose file had vanished was stored and answeredAlreadyExists, with no payment check, no closeness check, and no replication fan-out. Off Unix the repair opens the existing file withoutcreate, so there it fails, returnsfalse, and leaves the key marked wrong.It needs a file that has vanished while the index still names it: a file removed outside the node, or one caught between its unlink and its index removal by a corruption cleanup, or by a delete whose caller was cancelled. A remote client cannot produce that state, the offered bytes are always hash-checked, and clients pay before they PUT, so no client loses money. It is still a pre-payment write of a chunk the node did not hold, and it contradicts what
holds_verifiedis documented to answer.The fix
An
Ok(None)arm of its own. It writes nothing:true, and the key goes back into the legacy-only set, the same thinggetalready does when it falls back to LMDB for a missing file.false, and the offer goes through the ordinary path, which checks capacity, closeness and payment beforeputpublishes it.The
Ok(Some(_))repair arm for a file that is present with the wrong bytes is unchanged, as is the legacy-only branch above it.Left as it is: when LMDB holds different bytes under the key and a paid PUT then fails in the file store,
putfiles the key as legacy-only, because LMDB reports an existing key as a duplicate without comparing bytes. That isput's existing behaviour for any such key, the client getsStorageFailed, and the copier drops the key when it finds the legacy bytes wrong. Closing it would add an LMDB read to every duplicate dual write.Linear issue
Closes V2-1259
Risk tier
Compatibility
Semver impact
Test evidence
Local, on
31fcbaeplus this branch (macOS, APFS):cargo test --lib: 1089 passed, 0 failed.cargo fmt --all --check,cargo clippy --all-features --all-targets -- -D clippy::panic -D clippy::unwrap_used -D clippy::expect_used,RUSTDOCFLAGS=--deny=warnings cargo doc --no-depsandRUSTFLAGS="-D warnings" cargo build --release --no-default-features: all clean.a_chunk_whose_file_has_gone_is_not_written_back_by_the_checkholds_verifiedmust answerfalse, write nothing, and leaveputable to publish it as newa_chunk_whose_file_has_gone_is_still_held_while_the_legacy_environment_has_ittrue, no file written, key back in the legacy-only seta_chunk_whose_file_has_gone_is_not_held_on_wrong_legacy_bytesfalse, no file written, key not requeuedtest_put_for_a_chunk_whose_file_has_gone_is_not_already_existsAlreadyExists, and nothing is writtenMutation-checked: with the old
Ok(_)arm restored, all four fail (the handler test getsAlreadyExists). WithOk(None) => falseand no legacy lookup, only the legacy-held test fails. With a legacy lookup that accepts any bytes, only the wrong-bytes test fails. The existinga_damaged_chunk_is_repaired_from_the_copy_being_offeredanda_legacy_only_chunk_with_wrong_bytes_is_replaced_by_the_offerpass either way. The file-vanished tests discriminate on Unix, where the old repair recreated the file; CI's Linux and macOS jobs run them.Not covered: a dev testnet run. The state needs a file removed underneath a running node.
The same arm is on
rc-2026.9.2(src/storage/chunk_store.rs,holds_verified).New dependency
none
ADR
https://github.com/WithAutonomi/ant-node/blob/31fcbae2688a1d81008ef1b57fe09eda4b13817b/docs/adr/ADR-0014-file-based-chunk-store-and-lmdb-retirement.md
Mitigation / rollback
Revert the commit. The only behaviour that changes back is the answer for a chunk whose file is missing while the index still names it.