Skip to content

chore: integrate context precompile evm example#14430

Merged
klkvr merged 8 commits intoklkvr/new-revmfrom
rakita/new-revm
Feb 17, 2025
Merged

chore: integrate context precompile evm example#14430
klkvr merged 8 commits intoklkvr/new-revmfrom
rakita/new-revm

Conversation

@rakita
Copy link
Collaborator

@rakita rakita commented Feb 11, 2025

Based on #14115

Updates examples for latest revm

Example is slightly changed to cache all precompiles calls as it feels like a better example.

@rakita rakita requested review from gakonst, klkvr and mattsse and removed request for gakonst February 11, 2025 21:18
Copy link
Collaborator

@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.

some notes on api

Comment on lines +66 to +73
Copy link
Collaborator

@mattsse mattsse Feb 11, 2025

Choose a reason for hiding this comment

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

UX wise it would be a lot simpler if we didn't have to deal with inspectors on this level, then we could also get rid of create_evm_with_inspector function here entirely.
not a blocker, but it illustrates why dealing with NooPInspector on this level feels strange

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This feels possible and look like a proper way to do things, but needs to be cleared in Revm.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Have added a issue here: bluealloy/revm#2069 it shouldn't take a lot of time and if it is quick fix i will do it tomorrow

Comment on lines 93 to 100
Copy link
Collaborator

Choose a reason for hiding this comment

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

like this should just be self.create_evm(db, input).with_insp(inspector)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hm maybe it is even possible now, let me check

Copy link
Collaborator Author

@rakita rakita Feb 11, 2025

Choose a reason for hiding this comment

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

Yep, it is possible, have changed the code

Copy link
Collaborator

Choose a reason for hiding this comment

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

naming nit PrecompileErrors -> PrecompileError

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@rakita
Copy link
Collaborator Author

rakita commented Feb 11, 2025

hm op still needs to be integrated among others.

@klkvr klkvr force-pushed the rakita/new-revm branch 2 times, most recently from 05c48cf to 4c476a0 Compare February 12, 2025 18:01
@klkvr klkvr force-pushed the rakita/new-revm branch 13 times, most recently from 664b80c to 2f5ba39 Compare February 13, 2025 07:32
@emhane emhane added A-execution Related to the Execution and EVM C-example Examples labels Feb 16, 2025
@klkvr klkvr force-pushed the rakita/new-revm branch 2 times, most recently from 0137ee7 to 05241e3 Compare February 17, 2025 15:56
@klkvr klkvr requested a review from DaniPopes as a February 17, 2025 15:56
Copy link
Member

Choose a reason for hiding this comment

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

should use std

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah no need for once_cell here

@klkvr klkvr merged commit d8b41ec into klkvr/new-revm Feb 17, 2025
37 checks passed
@klkvr klkvr deleted the rakita/new-revm branch February 17, 2025 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-execution Related to the Execution and EVM C-example Examples

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants