Skip to content

Conversation

@blishko
Copy link
Collaborator

@blishko blishko commented Oct 20, 2025

Description

This is a follow-up for #903, but this time for SLOAD opcode.
We separate the handling of symbolic and concrete case.
Symbolic case does not case about gas and concrete case can be simpler because it can focus on handling only concrete values.
Moreover, we can specialize accessStorageForGas for concrete case, because now it is used only in concrete mode.

Checklist

  • tested locally
  • added automated tests
  • updated the docs
  • updated the changelog

@blishko blishko requested a review from msooseth October 20, 2025 09:30
Copy link
Collaborator

@msooseth msooseth left a comment

Choose a reason for hiding this comment

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

LGTM, although I'd prefer the small change I suggested :)

@blishko blishko force-pushed the execution-refactor branch 2 times, most recently from 0ed9b4e to e37dd34 Compare October 21, 2025 12:37
@blishko blishko requested a review from msooseth October 21, 2025 12:47
@blishko
Copy link
Collaborator Author

blishko commented Oct 21, 2025

I had to keep the old behaviour in case the current contract is external.
It means it was fetched by RPC and we might not have its storage represented accurately.
When loading a slot, we need to check if we don't need to fetch it with another RPC call.

Copy link
Collaborator

@msooseth msooseth left a comment

Choose a reason for hiding this comment

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

Nice! Maybe you could compact it a bit as per my comment?

This function is needed only in concrete mode and we can simplify it
when we know the slot is a concrete value.
@blishko blishko force-pushed the execution-refactor branch from e37dd34 to fa1a99a Compare October 21, 2025 13:54
@blishko blishko merged commit b67859f into main Oct 21, 2025
7 checks passed
@blishko blishko deleted the execution-refactor branch October 21, 2025 14:24
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.

3 participants