refactor: remove unused put_raw, put_overwrite, get_raw storage APIs#35
refactor: remove unused put_raw, put_overwrite, get_raw storage APIs#35
Conversation
These methods bypass content-address verification (BLAKE3) and are not called by any handler, protocol, or external code. They were speculative API for future non-chunk data types that don't exist yet. Removing them eliminates an unnecessary attack surface — having bypass methods in the storage layer risks accidental wiring that would allow storing data without proper content-address validation. If non-content-addressed storage is needed later, it should be designed with proper authorization from the start.
Greptile SummaryThis PR removes three
Confidence Score: 5/5
|
| Filename | Overview |
|---|---|
| src/storage/lmdb.rs | Removes put_raw, put_overwrite, and get_raw (bypassed BLAKE3 verification) along with their 4 tests; no callers remain in the codebase, and the verified put/get API is untouched. |
Sequence Diagram
sequenceDiagram
participant Caller
participant LmdbStorage
participant BLAKE3
participant LMDB
Note over LmdbStorage: Retained verified API only
Caller->>LmdbStorage: put(address, content)
LmdbStorage->>BLAKE3: compute_address(content)
BLAKE3-->>LmdbStorage: computed_address
alt address mismatch
LmdbStorage-->>Caller: Err(ContentAddressMismatch)
else address matches
LmdbStorage->>LMDB: write_txn → put(key, value) → commit
LMDB-->>LmdbStorage: Ok
LmdbStorage-->>Caller: Ok(was_new)
end
Caller->>LmdbStorage: get(address)
LmdbStorage->>LMDB: read_txn → get(key)
LMDB-->>LmdbStorage: Option(bytes)
alt found & verify_on_read
LmdbStorage->>BLAKE3: compute_address(bytes)
BLAKE3-->>LmdbStorage: computed_address
alt verification fails
LmdbStorage-->>Caller: Err(VerificationFailed)
else ok
LmdbStorage-->>Caller: Ok(Some(bytes))
end
else not found
LmdbStorage-->>Caller: Ok(None)
end
Last reviewed commit: "refactor: remove unu..."
There was a problem hiding this comment.
Pull request overview
Removes the raw/overwrite LMDB storage APIs that bypass BLAKE3 content-address verification, along with their dedicated tests, to keep LmdbStorage strictly content-addressed.
Changes:
- Removed
put_raw(),put_overwrite(), andget_raw()fromLmdbStorage. - Removed tests that exclusively covered these raw/overwrite APIs.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -247,141 +247,6 @@ impl LmdbStorage { | |||
| Ok(true) | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Removing put_raw/put_overwrite/get_raw changes the public API surface of LmdbStorage (which is re-exported from src/storage/mod.rs). If this crate follows semver for its library users, this should be treated as a breaking change (e.g., bump major version and/or document in changelog/release notes), or consider keeping stubs/deprecations for one release if downstream users might rely on these methods.
| /// Deprecated: use [`put`] instead. | |
| #[deprecated(note = "use `put` instead")] | |
| pub async fn put_raw(&self, address: &XorName, content: &[u8]) -> Result<bool> { | |
| self.put(address, content).await | |
| } | |
| /// Deprecated: use [`put`] instead and handle existing values explicitly. | |
| /// | |
| /// This method retains the public API surface for backward compatibility. | |
| #[deprecated(note = "use `put` instead and handle existing values explicitly")] | |
| pub async fn put_overwrite(&self, address: &XorName, content: &[u8]) -> Result<()> { | |
| // Delegate to `put`; ignore whether the chunk already existed. | |
| let _ = self.put(address, content).await?; | |
| Ok(()) | |
| } | |
| /// Deprecated: use [`get`] instead. | |
| #[deprecated(note = "use `get` instead")] | |
| pub async fn get_raw(&self, address: &XorName) -> Result<Option<Vec<u8>>> { | |
| self.get(address).await | |
| } |
Add unit and e2e tests covering the remaining Section 18 scenarios: Unit tests (32 new): - Quorum: #4 fail→abandoned, #16 timeout→inconclusive, #27 single-round dual-evidence, #28 dynamic threshold undersized, #33 batched per-key, #34 partial response unresolved, #42 quorum-derived paid-list auth - Admission: #5 unauthorized peer, #7 out-of-range rejected - Config: #18 invalid config rejected, #26 dynamic paid threshold - Scheduling: #8 dedup safety, #8 replica/paid collapse - Neighbor sync: #35 round-robin cooldown skip, #36 cycle completion, #38 snapshot stability mid-join, #39 unreachable removal + slot fill, #40 cooldown peer removed, #41 cycle termination guarantee, consecutive rounds, cycle preserves sync times - Pruning: #50 hysteresis prevents premature delete, #51 timestamp reset on heal, #52 paid/record timestamps independent, #23 entry removal - Audit: #19/#53 partial failure mixed responsibility, #54 all pass, #55 empty failure discard, #56 repair opportunity filter, response count validation, digest uses full record bytes - Types: #13 bootstrap drain, repair opportunity edge cases, terminal state variants - Bootstrap claims: #46 first-seen recorded, #49 cleared on normal E2e tests (4 new): - #2 fresh offer with empty PoP rejected - #5/#37 neighbor sync request returns response - #11 audit challenge multi-key (present + absent) - Fetch not-found for non-existent key Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add unit and e2e tests covering the remaining Section 18 scenarios: Unit tests (32 new): - Quorum: #4 fail→abandoned, #16 timeout→inconclusive, #27 single-round dual-evidence, #28 dynamic threshold undersized, #33 batched per-key, #34 partial response unresolved, #42 quorum-derived paid-list auth - Admission: #5 unauthorized peer, #7 out-of-range rejected - Config: #18 invalid config rejected, #26 dynamic paid threshold - Scheduling: #8 dedup safety, #8 replica/paid collapse - Neighbor sync: #35 round-robin cooldown skip, #36 cycle completion, #38 snapshot stability mid-join, #39 unreachable removal + slot fill, #40 cooldown peer removed, #41 cycle termination guarantee, consecutive rounds, cycle preserves sync times - Pruning: #50 hysteresis prevents premature delete, #51 timestamp reset on heal, #52 paid/record timestamps independent, #23 entry removal - Audit: #19/#53 partial failure mixed responsibility, #54 all pass, #55 empty failure discard, #56 repair opportunity filter, response count validation, digest uses full record bytes - Types: #13 bootstrap drain, repair opportunity edge cases, terminal state variants - Bootstrap claims: #46 first-seen recorded, #49 cleared on normal E2e tests (4 new): - #2 fresh offer with empty PoP rejected - #5/#37 neighbor sync request returns response - #11 audit challenge multi-key (present + absent) - Fetch not-found for non-existent key Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Removes put_raw(), put_overwrite(), and get_raw() from LmdbStorage. These bypass BLAKE3 content-address verification and have no callers outside their own tests. -270 lines, 4 tests removed.