Skip to content

Conversation

cgewecke
Copy link
Member

@cgewecke cgewecke commented Apr 27, 2021

This PR is a diff vs. ethereumjs v4. Most recent commit here is from March 6, 2021.

As things stand, to upgrade their js-vm to V5 & Berlin, Optimism would have to maintain and publish from a fork of the ethereumjs-monorepo.

This entails significant maintenance overhead.

ethereum-js could make itself more configurable and reduce complexity for OVM if

  • ethereumjs VM

    • adds init initialization hook, initialization options
    • makes EVM a VM option in some form
    • makes logging configurable?
  • Optimism could extend the StateManager EVM and EEI classes

class OptimisticStateManager extends DefaultStateManager

class OVM extends EVM
  class OEEI extends EEI // OVM instantiates OEEI

A new VM might look like

import VM from "ethereumjs/vm";
import { OptimisticStateManager, OVM } from "@eth-optimism/something";

const extension: {
    init: () => // this is invoked in addition to standard init?
    evm: OVM // this would always be passed as an option to `runCode` and `runCall` and instantiated there. 
    something: ...,  // stuff that gets attached to the vm object and can be accessed via `.extension`
    ...    
}

const vm = new VM({
  ...,
  stateManager: new OptimisticStateManager(...),
  extension
});

)
}

message = message.toOvmMessage(this._vm, this._block || new Block())
Copy link
Member Author

Choose a reason for hiding this comment

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

Could toOvmMessage be a static helper in evm.ts which took the original message as an arg?

This would mean there was no need to extend the message class.

await state.revert()
throw e
}
}
Copy link
Member Author

@cgewecke cgewecke Apr 27, 2021

Choose a reason for hiding this comment

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

Hmmm....not sure what to do here with runTx.

const message = new Message({
caller: tx.getSenderAddress(),
gasLimit: gasLimit,
gasLimit: gasLimit.mul(new BN(5)), // TODO: Find a cleaner way to do this, it works for now though.
Copy link
Member Author

Choose a reason for hiding this comment

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

What is this?

to: tx.to.toString('hex') !== '' ? tx.to : undefined,
value: tx.value,
data: tx.data,
data: tx.serialize(),
Copy link
Member Author

Choose a reason for hiding this comment

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

👀

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.

6 participants