Skip to content

Conversation

pgherveou
Copy link
Contributor

@pgherveou pgherveou commented Sep 23, 2025

Refactor REVM implementation in pallet-revive

This PR removes technical debt and eliminates tight coupling to specific REVM versions, facilitating integration with other projects (e.g., Foundry). After this refactoring, we primarily depend only on REVM's Bytecode struct.

Background

Most of REVM's generic type system and trait abstractions are unused or had to be ignored to prevent bugs. Interactions with the host in pallet-revive are handled through the Ext trait, making REVM's other abstractions unnecessary or potentially harmful.

Unused REVM abstractions included:

  • Host trait: Unused in the pallet, we relied on the DummyHost default mocked implementation
  • Gas field: Unused, the pallet uses its own gas accounting system
  • Methods from InputsTr: Unused most methods had panic implementations since they couldn't be relied upon
  • Spec: Unused: We only maintain the latest fork for each runtime

Changes

This refactor introduces:

  • Interpreter: Simplified struct containing only the fields actually needed
  • Stack: Simplified implementation using sp_core::U256 instead of alloy_core::primitives::U256 for better integration with the rest of the pallet
  • Memory: Simplified implementation providing only the methods actually needed
  • Instructions:
    • New instructions don't rely on macros and have a simplified signature: Fn(&mut interpreter) -> ControlFlow<Halt>
    • Removed function pointer table lookup for instructions in favor of match statements,
    • Unified gas charging: legacy u64 gas charging is now wrapped in EVMGas that implements Token<T> to provide the associated weight
    • Removed the InterpreterAction, this simplify the interpreter loop to:
loop {
	let opcode = interpreter.bytecode.opcode();
	interpreter.bytecode.relative_jump(1);
	exec_instruction(interpreter, opcode)?;
}
  • Error handling
    • Add a Halt(HaltReason) error variant to the Error<T> enum, most error in Error<T> should probably redefined as Halt(HaltReason) variant to unify error handling between PVM & EVM in a future PR

@pgherveou pgherveou marked this pull request as ready for review September 25, 2025 08:55
table: &revm::interpreter::InstructionTable<EVMInterpreter<'a, E>, DummyHost>,
) -> InterpreterResult {
let host = &mut DummyHost {};
fn run_plain<E: Ext>(interpreter: &mut Interpreter<E>) -> ControlFlow<Halt, Infallible> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note I will re-introduce the tracing version in a follow up PR that will add opcode tracing and let you compare opcode / stack and memory executed on revive vs revm eth interpreter

Copy link
Contributor

Choose a reason for hiding this comment

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

Missing license statement

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