Skip to content

Conversation

@mattsse
Copy link
Member

@mattsse mattsse commented Nov 5, 2025

this still needs some work

with recent revm change we can no longer access &mut Account and need workarounds for this

ideally helpers on EvmInternals

todo

figure out which ones and upstream to alloy-evm and make nicer

Copy link
Member Author

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

these lgtm now

Comment on lines +93 to +96
// Transfer the value between accounts
internals
.transfer(from_address, to_address, value)
.map_err(|e| PrecompileError::Other(format!("Failed to perform transfer: {e:?}")))?;
Copy link
Member Author

Choose a reason for hiding this comment

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

checks out

Copy link
Contributor

Choose a reason for hiding this comment

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

The inner journal impl makes the same balance checks as above but the errors are not bubbled up, this may be a nice follow-up.
https://github.com/bluealloy/revm/blob/a1fdb9d9e98f9dd14b7577edbad49c139ab53b16/crates/context/src/journal/inner.rs#L315-L320

Copy link
Member

@zerosnacks zerosnacks left a comment

Choose a reason for hiding this comment

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

lgtm

@zerosnacks zerosnacks marked this pull request as ready for review November 5, 2025 12:36
@zerosnacks zerosnacks added this pull request to the merge queue Nov 5, 2025
Merged via the queue into master with commit cc24b6b Nov 5, 2025
15 checks passed
@zerosnacks zerosnacks deleted the matt/bump-revm31 branch November 5, 2025 12:48
@github-project-automation github-project-automation bot moved this to Done in Foundry Nov 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants