ethereumjs-monorepo icon indicating copy to clipboard operation
ethereumjs-monorepo copied to clipboard

Improved Optimism Bedrock Compatibility

Open holgerd77 opened this issue 2 years ago • 19 comments

This PR aims to improve compatibility with the upcoming Optimism Bedrock release, using the Bedrock Goerli deployment as a testbed.

holgerd77 avatar May 20 '23 08:05 holgerd77

Codecov Report

Merging #2713 (910449b) into master (d1ba362) will increase coverage by 0.83%. The diff coverage is 87.14%.

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
block 90.88% <79.54%> (-0.21%) :arrow_down:
blockchain 90.72% <ø> (ø)
client 86.78% <ø> (ø)
common 97.01% <100.00%> (+0.01%) :arrow_up:
devp2p 89.29% <ø> (?)
ethash ∅ <ø> (∅)
evm 79.99% <100.00%> (+<0.01%) :arrow_up:
rlp ∅ <ø> (?)
statemanager 86.28% <ø> (ø)
trie 89.92% <ø> (ø)
tx 95.74% <ø> (?)
util 81.13% <ø> (ø)
vm 83.01% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

codecov[bot] avatar May 20 '23 08:05 codecov[bot]

Just dropping this link list here from some internal chat discussion so that it won't get lost:

  • https://github.com/ethereum-optimism/optimism/blob/ba65c54a6a18e49381706414490ef44a2ad3ef44/l2geth/params/protocol_params.go
  • https://github.com/ethereum-optimism/optimism/pull/1568
  • https://changelog.optimism.io/2022/02/01/berlin-hardfork.html
  • https://github.com/ethereum-optimism/optimism/blob/1729edd37be21cdfbdd0f46c0bd90952362ad2a6/l2geth/core/vm/eips.go#LL136C13-L136C13

(guess most of this should be retrievable by the now excellently structured official docs though as well)

Will also mention @kchojn here since he might be interested in this work (we had a chat exchange on Discord).

holgerd77 avatar May 23 '23 09:05 holgerd77

Thanks @holgerd77. Let me know if I should open an issue, mainly because these problems are more related to Optimism and not Bedrock in detail. In connection with the reprocessing of the entire Optimism story, using ethereumjs, I encountered several problems that I would like to share (I also applied some temporary solutions):

Before Berlin Hardfork:

EIIP-2929

Berlin Hardfork Block Number: 3950000;

In general, a minimal version of EIP-2929 is needed:

  • https://github.com/ethereum-optimism/optimism/pull/1568
  • https://changelog.optimism.io/2022/02/01/berlin-hardfork.html
  • https://github.com/ethereum-optimism/optimism/blob/ba65c54a6a18e49381706414490ef44a2ad3ef44/l2geth/params/protocol_params.go

for reprocessing purposes, I added my JSON file with eip2929-minimal, this allowed the blocks to be reprocessed correctly.

Btw, before the Berlin hardfork, basically everything was treated as a new storage slot.

L1BLOCK_NUMBER

This opcode is missing for Optimism.

After Berlin Hardfork:

GasRefund

  • In some transactions like this: https://optimistic.etherscan.io/tx/0x9c7acaca35db9b7badcbca7afeb37071855d77dc5c254a04eda39ab20ebe6931 I have to use a temporary solution: //gasRefund = gasRefund < maxRefund ? gasRefund : maxRefund; commented out in the @ethereumjs/vm/dist/runTx.js file

  • In some transactions like this: https://optimistic.etherscan.io/tx/0xd591640f72909619d661f632b132f8ff6485f01fd9d9b0d37b5e24abcd1a5703/advanced gasRefund = (results.execResult.executionGasUsed + txBaseFee) / BigInt(2) helps to solve the problem in the @ethereumjs/vm/dist/runTx.js file

0 address

Optimism has some system transactions that don't quite work like legacy transactions. Example:

  • https://optimistic.etherscan.io/tx/0xf4304cb09b3f58a8e5d20fec5f393c96ccffe0269aaf632cb2be7a8a0f0c91cc

The transaction is a system transaction, but it has v=0x0 and r=0x0 and s=0x0. This is not a valid signature. To fix this, we need to do 2 things:

  • replace 0x0 with undefined in the v, r, and s fields (this is done in my own rpcTransactionToEthereumjsTransaction0x0 function)
  const v = (rpcTransaction as any).v === "0x0" ? undefined : (rpcTransaction as any).v;
const r = (rpcTransaction as any).r === "0x0" ? undefined : (rpcTransaction as any).r;
const s = (rpcTransaction as any).s === "0x0" ? undefined : (rpcTransaction as any).s;
  • transaction should always be signed, so we need to mock isSigned to always return true in baseTransaction.js in @ethereumjs/tx/dist:
    isSigned();
{
  // const { v, r, s } = this;
  // if (v === undefined || r === undefined || s === undefined) {
  //     return false;
  // }
  // else {
  //     return true;
  // }
  return true;
}

Sometimes When does a transaction have a zero address as the sender, it uses 0x36bde71c97b33cc4729cf772ae268934f7ab70b2 (l1TxOrigin).

To fix it, we need:

  • legacyTransaction.js in @ethereumjs/tx/dist:
  getSenderPublicKey();
{
  const isOptimismNetwork = this.common.chainName().toLowerCase().includes("optimistic");

  const msgHash = this.getMessageToVerifySignature();
  const { v, r, s } = this;
  this._validateHighS();
  try {
    return (0, util_1.ecrecover)(msgHash, v, (0, util_1.bigIntToUnpaddedBuffer)(r), (0, util_1.bigIntToUnpaddedBuffer)(s), this.supports(types_1.Capability.EIP155ReplayProtection) ? this.common.chainId() : undefined);
  } catch (e) {
    const msg = this._errorMsg("Invalid Signature");

    // Optimism network zero address handler
    if (isOptimismNetwork) {
      return null;
    }

    throw new Error(msg);
  }
}

In this case, we added a check for the Optimism network and if it is, then we return null (not an error).

  • baseTransaction.js in @ethereumjs/tx/dist:
    getSenderAddress();
{
  const senderPublicKey = this.getSenderPublicKey();
  if (!senderPublicKey) {
    return new util_1.Address(Buffer.alloc(20));
  }
  return new util_1.Address((0, util_1.publicToAddress)(this.getSenderPublicKey()));
}

In this case, we added a check if the senderPublicKey is null, then we return the zero address.

  • runTx.js in @ethereumjs/vm/dist ( before beforeTx event)
  await this._emit("beforeTx", tx);
let caller = tx.getSenderAddress();
if (caller.equals(util_1.Address.zero()) && this._common.chainName().toLowerCase().includes("optimistic")) {
  const senderAddr = "36bde71c97b33cc4729cf772ae268934f7ab70b2";
  caller = new util_1.Address(Buffer.from(senderAddr, "hex"));
}

In this case, we added a check if the caller is the zero address and if it is, then we replace it with the special address (l1TxOrigin).

CALL

gas.js

const warmStorageRead = common.param("gasPrices", "warmstorageread");
const coldAccountAccess = common.param("gasPrices", "coldaccountaccess");
const coldCost = coldAccountAccess - warmStorageRead;

  • tx example: https://optimistic.etherscan.io/tx/0xdff6faec7d59dfcec3e6388ca2578f357a067517ec48ce383762dc3e2141012b

I Had to do some changes in gas calculation in CALL instruction. I needed to add some gas after EIP2929

Summary

Most of these changes are temporary and some kind of hack. But it allowed me to reprocess more than 99.0% of blocks in the whole history of the Optimism network. If you need more examples or more explanation, let me know, I'll be happy to help

kchojn avatar May 23 '23 09:05 kchojn

Hi @kchojn Karol, thanks a lot for all these detailed notes, that so so invaluable 🙏. I also already stumbled (and recognize) various points you made, your experiences here are really are great help to go forward here! 🤩

My current tendency would be to "only" make EthereumJS Bedrock compatible and do not focus too much on the old blocks (and e.g. not add the L1BLOCK_NUMBER opcode). Do you see some great advantage in also try to get these running? The Bedrock update is so much around the corner and from my feeling this will be the start of "the real" [tm] Optimism, with this technology (update) be the foundation of the whole OP stack with other rollups building on this Bedrock fundament.

Doesn't need to be decided ad hoc though, also not against to take the older stuff in. We just also do not have to decide now but can also slowly expand later on if needed.

For the system txs I guess it's likely worth to implement this as a dedicated tx type, not sure if would want to start with this now, but in general. For now I might also just skip them or something, or side-use some hacks.

One question on your technical setup: did you process the Optimism blocks with the EthersStateManager? 🤔 Or some other setup?

And did you use our next-v7-releases code from the develop-v7 branch (now merged into master since two weeks or so?

Anyhow: again, thanks a lot!! 😀 ❤️ 🙏

holgerd77 avatar May 24 '23 07:05 holgerd77

Update: ah, I will just start with implementing the Optimism system tx type today, then we'll have this clean and do not always need to route around this or do some hacky stuff and then we can just simply run the full blocks with e.g. VM.runBlock().

I will also introduce a high-level ChainType property for our underlying Common library with the options to switch between Ethereum mainnet (Ethereum) and Optimism compatible (Optimism) chains. Then we can introduce clean switches on this (e.g. throwing if a system tx is instantiated on ChainType.Ethereum).

holgerd77 avatar May 24 '23 07:05 holgerd77

@holgerd77 Hi, I understand, it is very useful, but I am able to modify the code to make it work, if you want to keep the start optimism from Bedrock (it is reasonable). Dedicated types would probably be very useful, but on the other hand, it would be difficult to maintain for many chains. I am at the stage of PoC Arbitrum and there I have, for example, ~5-6 new transaction types. I use a custom state manager (rpc/db) that is based on EVMStateAccess. Modified because, with this, I capture every storage/state change. I have not used v7, blocks since about 90m I process on official release v6 latest :grinning: Bedrock changes will be from v7 or still in v6? From my perspective I can start testing these changes as Bedrock happens, I don't know what your plans are for the release. Anyway, any changes from your side, are very useful. Thanks!

kchojn avatar May 24 '23 08:05 kchojn

@kchojn Ah, just read your system tx section, I will be able to do this a lot cleaner with a dedicated tx type, your comments are super helpful though, can just go along one by one (so there are for sure (?) no Optimism 1559 system tx, right?).

I have also added the ChainType now in 14b2076. This is for sure not to support all kind of chains, it will e.g. quite for sure not be realistic that we support the Arbitrum stack due to the large change set - especially in the EVM. So this is only for selected switches and will relatively likely be "Optimism only" for quite some time (or also long term), or with eventually 1-2 additions (not sure about Polygon e.g., but maybe they are even in the ChainType.Ethereum realm? Anyhow, but opens the possibility for additional things like that).

Update: Ah, just re-read and realized that you meant "Dedicated Types" for "Transaction Types", yeah, this also needs to remain selective. For Optimism though this very much seems justified to me, this is at the end only one type and this system tx type is very prevalent. And the switches are there anyhow, I think it will rather be a bit cleaner with a type inheriting from I guess LegacyTransaction (will at least try, not 100% sure yet), think this won't be that much more code than inline switches.

I use a custom state manager (rpc/db) that is based on EVMStateAccess. Modified because, with this, I capture every storage/state change.

🤯

That's totally impressive!

So we on our side will work towards having the EthersStateManager ready to also be used towards Optimism, you can there pass in your Optimism-provider backed Ethers instance and then request state on the fly.

Both EthersStateManager/StateManager and generally all the EVM/VM state related stuff is under heavy rework right now for v7 (so this indicates our breaking release round for all EthereumJS libraries), but in the finishing stages, #2702 from @jochem-brouwer is finalizing the EVM/VM state refactoring, likely to be merged in the current or next week.

Bedrock changes will be from v7 or still in v6? From my perspective I can start testing these changes as Bedrock happens, I don't know what your plans are for the release.

Bedrock changes will be for v7, v6 is in maintenance mode now. We are relatively close, so there will be - minimally - 2 pre-releases during June and then likely the final releases in July this year! 😀

holgerd77 avatar May 24 '23 08:05 holgerd77

@holgerd77 Sounds like a very good change! Yes, sorry, I write very chaotically. For Arbitrum, it's actually heavier, so I have some work ahead of me. 😄 But about six months ago I did a PoC for Polygon using ethereumjs. And it was very pleasant to process blocks, there were probably some system events that I missed that I didn't see in trace, at least I think so. In any case, Polygon seems to me to be very close to what it is. Well, I'm glad that Optimism will be so well supported!

Bedrock changes will be for v7, v6 is in maintenance mode now. We are relatively close, so there will be - minimally - 2 pre-releases during June and then likely the final releases in July this year! grinning

By the final release, I will have time to test your changes for Bedrock. 👀 Still fully focused on the history,

kchojn avatar May 24 '23 09:05 kchojn

Squashed some commits here and rebased and retargeted towards #2702 (EEI/EVM refactor work from Jochem)

holgerd77 avatar Jun 01 '23 07:06 holgerd77

Cherry-picked this on top of master

holgerd77 avatar Jun 02 '23 08:06 holgerd77

Gm, Is this pr suitable for Bedrock testing? I will update the configuration for the mainnet ofc

kchojn avatar Jun 14 '23 11:06 kchojn

Gm, Is this pr suitable for Bedrock testing? I will update the configuration for the mainnet ofc

I think it's worth trying. I know @holgerd77 tested it with a couple of Optimism Bedrock blocks from goerli a while back. We haven't rebased it recently so not sure if anything is missing or needs changes but let us know your results!

acolytec3 avatar Jun 16 '23 02:06 acolytec3

@holgerd77 just started working on it. Is there any quick way to do a rebuild of this branch/pr in my project? I tried to do it manually, but tsc throws me errors every now and then

Edit: nvm, I found a way, thx!

kchojn avatar Jul 06 '23 15:07 kchojn

@holgerd77, Thanks for this PR. I did a lot of testing and it looks like everything is working. However, we would like to support optimism 100% (without skipping specific types). Does implementing all the logic for tx 126 for optimism, involve only transactionFactory or are there other places where I need to add support? I would like to undertake this, due to the fact that our solutions are based mostly on ethereumjs, and it will be a good exercise for upcoming chains (matic)

kchojn avatar Jul 11 '23 10:07 kchojn

@holgerd77, Thanks for this PR. I did a lot of testing and it looks like everything is working. However, we would like to support optimism 100% (without skipping specific types). Does implementing all the logic for tx 126 for optimism, involve only transactionFactory or are there other places where I need to add support? I would like to undertake this, due to the fact that our solutions are based mostly on ethereumjs, and it will be a good exercise for upcoming chains (matic)

Yeah, this work here turned out to get very generic, so the new flag is basically just a universal tool just for these kind of tx-type-unknown situation. 🙂 This will likely be helpful for various situations, but I already thought that this will likely not fit your use case.

It would be totally great if you would want to add broader Optimism support! ❤️ Did a quick consultation on this in our internal chat to confirm. We can promise you continued support on this with a close feedback loop!

So what we will short-term do is just to extract the skipTxTypes functionality from here and release this in separation.

What you can cherry-pick from here - if you want - is likely the deep Common integration (so the specific optimism.json chain file and such) as well as the added test data, that might already help as a start and we wouldn't directly add this chain file to master.

Yes, and then I would think the system tx type is the main thing. And for our "library thinking" we always want to have the EVM/VM included, so it would be good if this gets to a point (again) where an Optimism block can be instantiated and then run in the VM with `VM.runBlock(). Not sure if the VM is doing anything on system txs? (maybe change some state?).

So I guess that would be what we would call "library-compatible". If we can run this with VM.runBlock() and the state would be matching afterwards.

Does this help?

Really curious to see what will come out of this! 🤩

holgerd77 avatar Jul 12 '23 11:07 holgerd77

@holgerd77 small update, I implemented everything and we can reprocess 100% of optimism blocks correctly with ethjs (Amazing!)! Announcement: full post-bedrock op data

As soon as I update ethereumjs to the latest versions, before the new hf on mainnet, I will open a PR here with my changes to Optimism:

  • support for tx 126 - we do not use skiptx option, we process these transactions as well 😎
  • all chainconfig
  • plus other small aspects (very very small changes in the current ejs code)

I guess, it will support Base as well

Edit: I know it took a long time, but I'm working on a lot of things 😵‍💫

kchojn avatar Nov 14 '23 18:11 kchojn

This is amazing! I work at OP labs so let me know if I can help get this over finish line

roninjin10 avatar Nov 21 '23 18:11 roninjin10

@kchojn Yes, can only agree: this is amazing! 🤩 Exciting to hear! ❤️

holgerd77 avatar Nov 23 '23 11:11 holgerd77

@holgerd77 small update, I implemented everything and we can reprocess 100% of optimism blocks correctly with ethjs (Amazing!)! Announcement: full post-bedrock op data

As soon as I update ethereumjs to the latest versions, before the new hf on mainnet, I will open a PR here with my changes to Optimism:

* support for tx 126 - we do not use skiptx option, we process these transactions as well 😎

* all chainconfig

* plus other small aspects (very very small changes in the current ejs code)

I guess, it will support Base as well

Edit: I know it took a long time, but I'm working on a lot of things 😵‍💫

Hey, thanks for the awesome work! Any updates on the PR? I'd be interested to get involved and can start after any updates have been pushed if there are any.

am1r021 avatar Apr 25 '24 01:04 am1r021