Skip to content

propagate trace parse error in extract_sload_slots#4515

Open
ashleychandy wants to merge 1 commit into
cowprotocol:mainfrom
ashleychandy:sload-trace-parse-error
Open

propagate trace parse error in extract_sload_slots#4515
ashleychandy wants to merge 1 commit into
cowprotocol:mainfrom
ashleychandy:sload-trace-parse-error

Conversation

@ashleychandy

Copy link
Copy Markdown
Contributor

Description

extract_sload_slots previously called unwrap_or_default() on the geth
trace parse, silently treating non-default frame variants as empty traces.
This caused parse failures to surface as DetectionError::NotFound rather
than a distinct simulation error, making root cause analysis harder.

Changes

  • Change extract_sload_slots return type to Result<Vec<(Address, B256)>, SimulationError>
  • Replace unwrap_or_default() with explicit try_into_default_frame() error propagation
  • Propagate error at call sites in approval and balance detectors via ?
  • Fix structured logging in balance: move token to a tracing field

@ashleychandy ashleychandy requested a review from a team as a code owner June 11, 2026 15:07

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors extract_sload_slots to return a Result instead of a vector, replacing the previous unwrap_or_default fallback with explicit error propagation and warning logs when trace parsing fails. Callers in approval/mod.rs and balance/mod.rs have been updated to propagate these errors. No critical issues found.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

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.

1 participant