Skip to content

fix(l1): return error instead of panic in TrieWrapper::put_batch#6887

Open
reflecttypefor wants to merge 1 commit into
lambdaclass:mainfrom
reflecttypefor:main
Open

fix(l1): return error instead of panic in TrieWrapper::put_batch#6887
reflecttypefor wants to merge 1 commit into
lambdaclass:mainfrom
reflecttypefor:main

Conversation

@reflecttypefor

Copy link
Copy Markdown

Motivation

TrieWrapper::put_batch currently calls unimplemented!(), which panics at runtime if ever invoked. Since TrieWrapper is a read-only view over the trie layer cache, this method should never be called — but a panic is still an unnecessarily harsh failure mode. Returning a recoverable error is more defensive and consistent with other read-only TrieDB implementations in the codebase.

Description

Replace unimplemented!("This function should not be called") with Err(TrieError::DbError(anyhow::anyhow!("TrieWrapper is read-only; put_batch should not be called"))), following the same pattern used by BackendTrieDBLocked::put_batch in crates/storage/trie.rs.

This is a one-line behavioral change with no impact on the happy path — the function was never meant to succeed.

Checklist

  • Updated STORE_SCHEMA_VERSION (crates/storage/lib.rs) if the PR includes breaking changes to the Store requiring a re-sync.

Closes #issue_number

Signed-off-by: reflecttypefor <reflecttypefor@outlook.com>
@reflecttypefor reflecttypefor requested a review from a team as a code owner June 18, 2026 03:40
@greptile-apps

greptile-apps Bot commented Jun 18, 2026

Copy link
Copy Markdown

Greptile Summary

Converts a runtime panic in TrieWrapper::put_batch into a recoverable error, consistent with how BackendTrieDBLocked::put_batch handles the same read-only constraint in crates/storage/trie.rs.

  • Replaces unimplemented!("This function should not be called") with Err(TrieError::DbError(anyhow::anyhow!(...))), making any accidental invocation gracefully propagate rather than abort the process.
  • Removes the stale // TODO: Get rid of this. comment and adds a descriptive comment explaining why writes are disallowed.

Confidence Score: 5/5

Safe to merge — the change only affects the panic-vs-error failure mode of a method that is never meant to be called on the happy path.

The fix is a single-site replacement of a panic with a recoverable error, matching an identical pattern already present in the same codebase. There are no callers that depend on the panic behaviour, and the happy path is completely unaffected.

No files require special attention.

Important Files Changed

Filename Overview
crates/storage/layering.rs Single-line behavioral change: replaces unimplemented!() panic in TrieWrapper::put_batch with a recoverable Err(TrieError::DbError(...)), matching the pattern already used by BackendTrieDBLocked::put_batch.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Caller invokes put_batch on TrieWrapper] --> B{Before this PR}
    A --> C{After this PR}
    B --> D["unimplemented!() → process panic 💥"]
    C --> E["Err(TrieError::DbError) → caller receives error ✅"]
    E --> F[Caller can handle / propagate gracefully]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Caller invokes put_batch on TrieWrapper] --> B{Before this PR}
    A --> C{After this PR}
    B --> D["unimplemented!() → process panic 💥"]
    C --> E["Err(TrieError::DbError) → caller receives error ✅"]
    E --> F[Caller can handle / propagate gracefully]
Loading

Reviews (1): Last reviewed commit: "fix(l1): return error instead of panic i..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants