Fix EIP2929 #1124

jochem-brouwer · 2021-02-22T00:30:48Z

The problem with the current implementation of EIP2929 is that the warm storage slots / addresses are only tracked per internal message, not on the entire transaction as the EIP implies. This PR moves the warm storage tracker from the Interpreter to the StateManager. This is done, because we also have to bookkeep if an internal message reverts. If that is the case, then extra added warm slots in these reverted internal messages, are now considered cold for the remainder of the transaction, instead of warm.

Since we explicitly revert/commit/checkpoint on StateManager, this is the logical place for this cache.

I also added an extra rule here, which I don't recall having removed, which also adds any created contract to the warm address list.

However, there are some problems here: (which is why this PR is WIP)

(1) We also use this checkpoint/commit/revert mechanism when running a block. Currently, this means warm storages are tracked during the entire block, which we don't want. Fix? Maybe we can explicitly clear the slots at the end of a transaction, and at the start of a transaction we also require an empty starting point?
(2) There's also a problem with runCall/runCode: this does never checkpoint state manager, and thus the current cache is the empty list. The current logic actually expects that there is a Map/Set available. Fix: we can probably initialize the default value as the empty Map, this should work fine. If we combine this with the fix above this should be OK. This is also the reason why the API tests file for EIP2929, since this uses runCode.

(I have written this down to also sit down and figure out what my current problems are, but came along with some thoughts for fixes along the way, I think this be fine. We also use this "end of tx" logic for the touched accounts.)

Current tests added as API tests:

Load a cold storage slot. Now call the current contract, reload the storage slot (it is now warm). (I should add the correct gas number here, but verified that this works)

API tests to add:

Call the current contract. Load the storage slot. Now revert this internal transaction. In the main message, re-load the storage slot. It should charge cold SLOAD cost twice.
Query an external account. Call current contract, query external account again (it is now warm).
Call current contract. Query external account. Revert internal transaction. Query external account (charges cold costs twice).

These do of course not cover everything, but give a rough idea that the implementation is correct.

vm: update StateManager interface

vm: remove storage maps from interpreter

vm: add some extra EIP2929 tests (WIP)

jochem-brouwer · 2021-02-22T00:31:53Z

packages/vm/lib/evm/evm.ts

@@ -272,6 +273,10 @@ export default class EVM {
      }
    }

+    if (this._vm._common.eips().includes(2929)) {
+      this._state.addWarmAddress(message.to.buf)


Note: this is in the wrong place, we should do this before the checkpoint in the new message (in case that the creation fails, then the address should /still/ be in the warm list)

There will likely be a reason why you nevertheless put this here along with giving a comment, but it is not obvious too me (too difficult for some reasons?). Could you clarify?

Yeah sorry this is a bit weird. I realized somewhere around pushing that this might be wrong, checked it, and realized that it is indeed wrong. It is more of a reminder comment that I should fix it :)

codecov · 2021-02-22T00:32:23Z

Codecov Report

Merging #1124 (d67e8bd) into master (fa84b93) will decrease coverage by 1.80%.
The diff coverage is 93.33%.

Flag	Coverage Δ
block	`?`
blockchain	`?`
client	`?`
common	`?`
devp2p	`?`
ethash	`?`
tx	`?`
vm	`83.21% <93.33%> (+0.20%)`	⬆️

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

jochem-brouwer · 2021-02-22T00:33:04Z

packages/vm/tests/api/EIPs/eip-2929.spec.ts

+    // call to address 0xFFFF..FF
+    //const callFF = '6000808080806000195AF1'
+    //await runCodeTest(callFF + '60003415601C57600080808080305AF15B00', 1, t)
+    //await runCodeTest('3415601557' + callFF + '600080FD5B600080808080305A60005400', 1, t)


These tests are WIP tests as described in message with the PR.

jochem-brouwer · 2021-02-22T00:34:19Z

Small note, for #1048, the VM side of logic is now extremely easy, we can first checkpoint, and then just dump in the entire transaction list in the state manager (we mark them as warm).

holgerd77 · 2021-02-22T08:54:37Z

@jochem-brouwer great analysis and integration, thanks so much Jochem! 😄 👍

Yeah, I think for (1) it should be ok to just clear after a tx? 🤔 Maybe just a generic public method StateManager.clearWarmAccounts() which is then called in VM.runTx() after tx execution?

Short side note: I would have a slight preference for warmed instead of warm for all the method names (so e.g. -> addWarmedAccount(), no strong opinions here though, just to note.

(2) Would think that the suggested solution should work on first thought. Will give additional comments here if I get some additional insights here.

holgerd77

Really nice and clean process of doing this transition here! 👍 Have left some comments.

holgerd77 · 2021-02-22T09:30:20Z

packages/vm/lib/state/stateManager.ts

+  isWarmAddress(address: Buffer): boolean {
+    for (let i = this._accessedStorage.length; i >= 0; i--) {
+      const currentMap = this._accessedStorage[this._accessedStorage.length - 1]
+      if (currentMap.has(address.toString('hex'))) {


If I read the code correctly there seems to be no reason for this reverse order traversing (?) and this could then be simplified to for (const currentMap of this._accessedStorage).

I did this in reverse order since this is an optimization. Imagine that we are rather deep in our call stack, say 10 levels deep. If we now access a storage slot on the current address (assuming that this is the first time we are at this address) then we actually mark the slot as warm in the end of the array. If we traverse the array in normal order, we first have to check the 9 other maps before figuring out that the 10th map (at current address) has marked it as warm. (It is a small optimization)

holgerd77 · 2021-02-22T09:30:42Z

packages/vm/lib/state/stateManager.ts

+    const storageKey = slot.toString('hex')
+
+    for (let i = this._accessedStorage.length; i >= 0; i--) {
+      const currentMap = this._accessedStorage[this._accessedStorage.length - 1]


Same here, loop can be simplified.

holgerd77 · 2021-02-22T09:32:20Z

packages/vm/lib/evm/evm.ts

@@ -272,6 +273,10 @@ export default class EVM {
      }
    }

+    if (this._vm._common.eips().includes(2929)) {
+      this._state.addWarmAddress(message.to.buf)


There will likely be a reason why you nevertheless put this here along with giving a comment, but it is not obvious too me (too difficult for some reasons?). Could you clarify?

holgerd77 · 2021-02-22T09:38:13Z

packages/vm/lib/runTx.ts

+    for (let addressInt = 1; addressInt <= numPrecompiles; addressInt++) {
+      // Check if precompile exists on the current Common of the Block
+      if (getPrecompile(new Address(Buffer.from(addressInt)), block._common)) {
+        state.addWarmAddress(new Address(Buffer.from(addressInt)))


This is not so beautiful to re-create this address space for the precompiles, we already have the precompiles const in precompiles/index.ts with all the addresses available, and this is also already exported.

Can you please use this here for address creation? Then there is also no need to do this extra numPrecompiles export.

Should have left a comment here. This is for forward compatibility. Geths implementation only marks current active precompiles to be warm. If we do not implement something like I created here (I agree it is not beautiful currently) then we get a consensus bug, for instance if we call a BLS precompile. Geth would mark it as cold, while we mark it as warm address. (GetPrecompile checks if precompile is available at current HF)

Ah ok, thanks. Can you then either:

a) add an additional getPrecompiles(common: Comon) function to precompiles/index.ts just looping over precompiles and return the active ones analog to getPrecompile()? or

b) minimal solution: at least do the numPrecompiles count dynamically by doing a precompiles.length + 1 in precompiles/index.ts, so that we do not introduce a relatively error prone new static constant there

This whole logic really doesn't belong there and we should at some point move to Common (not now though likely).

I will go for option (a). #727 is related here

holgerd77 · 2021-02-22T09:45:34Z

packages/vm/lib/state/interface.ts

+  addWarmAddress(address: Buffer): void
+  isWarmAddress(address: Buffer): boolean
+  addWarmStorage(address: Buffer, slot: Buffer): void
+  isWarmStorage(address: Buffer, slot: Buffer): boolean


These methods unfortunately can't be added here yet to be mandatory, since this would be a breaking change.

Just sitting here realizing that this gets - again - a bit trickier from the compatibility angle. Suggestion: we add these here as optional and then make it very clear in the code docs /release notes, that - if people want to run EIP2929 (and likely also 2930) functionality AND have their custom state manager implementation - they need to implement these methods.

Ug. Likely also always (?) a good advice for people to do their custom implementations by inheriting from the default state manager. Then this has a higher chance that a custom state manager will continue to work on such kind of updates.

Yep you are right. I only fear that if I remove this, then TypeScript is going to complain about possible non existent methods...

Yeah, that's what we have to live with and what the ! operator is for. Unfortunately no way around that we just can't do such an interface change along the road without a major VM release. 😕

Sadly we cannot use the ! on methods. (I tried). Now we have to resort to explicit casting or casting the state manager as any =(

holgerd77 · 2021-02-22T09:50:38Z

packages/vm/lib/evm/opcodes/EIP2929.ts

  const warmRead = runState._common.param('gasPrices', 'warmstorageread')
  const coldSload = runState._common.param('gasPrices', 'coldsload')

  // @ts-ignore Set Object is possibly 'undefined'
-  if (runState.accessedStorage.has(address) && runState.accessedStorage.get(address).has(keyStr)) {
+  if (runState.stateManager.isWarmStorage(address, key)) {


I think the @ts-ignore line from above can be removed.

holgerd77 · 2021-02-22T09:57:21Z

packages/vm/tests/api/EIPs/eip-2929.spec.ts

+    const result = await vm.runTx({ tx })
+
+    st.ok(result.gasUsed.toNumber() == expectedGasUsed)
+  }


That's a cool test helper function here. 😄

vm: make code backwards compatible vm: fix EIP2929 on CREATE* operations

vm: active precompiles method fixes

jochem-brouwer · 2021-02-22T15:11:22Z

I think I resolved all issues above. Am pretty confident in the implementation currently.

Just some small remarks:

(1) Before we join YoloV3, it makes sense to at least get all Berlin tests passing. The basics are now covered by some API tests, but I'm certain that I've missed some edge cases here.
(2) I had to convert the original EIP2929 tests to use runTx instead of runCode. This made me realize that runCode and runCall are not very consistent with other parts of the package. For instance, runCode doesn't do any checkpointing, and therefore the entire EIP2929 logic does not work. It makes sense to either deprecate both methods, or to internally wrap these things to use runTx internally (I think we should do the latter). With slightly fancy tricks this should not be too hard to do.

jochem-brouwer · 2021-02-22T15:41:09Z

PR run fails but should be easy fix (will do ASAP):

2021-02-22T15:10:23.7910151Z # runTx
2021-02-22T15:10:23.7973406Z # should fail to run without signature
2021-02-22T15:10:23.7974659Z # should fail without sufficient funds
2021-02-22T15:10:23.8023909Z not ok 486 should fail with appropriate error
2021-02-22T15:10:23.8025394Z   ---
2021-02-22T15:10:23.8026181Z     operator: ok
2021-02-22T15:10:23.8027047Z     expected: true
2021-02-22T15:10:23.8028772Z     actual:   false
2021-02-22T15:10:23.8030461Z     at: <anonymous> (/home/runner/work/ethereumjs-monorepo/ethereumjs-monorepo/packages/vm/tests/api/runTx.spec.ts:36:10)
2021-02-22T15:10:23.8031905Z     stack: |-
2021-02-22T15:10:23.8032958Z       Error: should fail with appropriate error
2021-02-22T15:10:23.8095393Z           at Test.assert [as _assert] (/home/runner/work/ethereumjs-monorepo/ethereumjs-monorepo/node_modules/tape/lib/test.js:228:54)
2021-02-22T15:10:23.8098744Z           at Test.bound [as _assert] (/home/runner/work/ethereumjs-monorepo/ethereumjs-monorepo/node_modules/tape/lib/test.js:80:32)
2021-02-22T15:10:23.8100543Z           at Test.assert (/home/runner/work/ethereumjs-monorepo/ethereumjs-monorepo/node_modules/tape/lib/test.js:347:10)
2021-02-22T15:10:23.8102057Z           at Test.bound [as ok] (/home/runner/work/ethereumjs-monorepo/ethereumjs-monorepo/node_modules/tape/lib/test.js:80:32)
2021-02-22T15:10:23.8103540Z           at /home/runner/work/ethereumjs-monorepo/ethereumjs-monorepo/packages/vm/tests/api/runTx.spec.ts:36:10
2021-02-22T15:10:23.8104409Z           at runMicrotasks (<anonymous>)
2021-02-22T15:10:23.8105187Z           at processTicksAndRejections (internal/process/task_queues.js:97:5)
2021-02-22T15:10:23.8106043Z           at runNextTicks (internal/process/task_queues.js:66:3)
2021-02-22T15:10:23.8106740Z           at processImmediate (internal/timers.js:434:9)
2021-02-22T15:10:23.8107230Z   ...
2021-02-22T15:10:23.8107723Z not ok 487 error should include "enough funds"
2021-02-22T15:10:23.8108322Z   ---
2021-02-22T15:10:23.8108671Z     operator: ok
2021-02-22T15:10:23.8109076Z     expected: true
2021-02-22T15:10:23.8109464Z     actual:   false
2021-02-22T15:10:23.8110600Z     at: <anonymous> (/home/runner/work/ethereumjs-monorepo/ethereumjs-monorepo/packages/vm/tests/api/runTx.spec.ts:44:10)
2021-02-22T15:10:23.8111534Z     stack: |-
2021-02-22T15:10:23.8112012Z       Error: error should include "enough funds"
2021-02-22T15:10:23.8113256Z           at Test.assert [as _assert] (/home/runner/work/ethereumjs-monorepo/ethereumjs-monorepo/node_modules/tape/lib/test.js:228:54)
2021-02-22T15:10:23.8114822Z           at Test.bound [as _assert] (/home/runner/work/ethereumjs-monorepo/ethereumjs-monorepo/node_modules/tape/lib/test.js:80:32)
2021-02-22T15:10:23.8116356Z           at Test.assert (/home/runner/work/ethereumjs-monorepo/ethereumjs-monorepo/node_modules/tape/lib/test.js:347:10)
2021-02-22T15:10:23.8117867Z           at Test.bound [as ok] (/home/runner/work/ethereumjs-monorepo/ethereumjs-monorepo/node_modules/tape/lib/test.js:80:32)
2021-02-22T15:10:23.8119363Z           at /home/runner/work/ethereumjs-monorepo/ethereumjs-monorepo/packages/vm/tests/api/runTx.spec.ts:44:10
2021-02-22T15:10:23.8120252Z           at runMicrotasks (<anonymous>)
2021-02-22T15:10:23.8121034Z           at processTicksAndRejections (internal/process/task_queues.js:97:5)
2021-02-22T15:10:23.8121899Z           at runNextTicks (internal/process/task_queues.js:66:3)
2021-02-22T15:10:23.8122962Z           at processImmediate (internal/timers.js:434:9)
2021-02-22T15:10:23.8123449Z   ...

holgerd77 · 2021-02-22T16:59:33Z

packages/vm/lib/evm/opcodes/EIP2929.ts

@@ -44,11 +46,12 @@ export function accessStorageEIP2929(runState: RunState, key: Buffer, isSstore:
  const baseFee = !isSstore ? runState._common.param('gasPrices', 'sload') : 0
  const address = runState.eei.getAddress().buf

-  const slotIsCold = !runState.stateManager.isWarmStorage(address, key)
+  const slotIsCold = !(<EIP2929StateManager>runState.stateManager).isWarmedStorage(address, key)


Nice and thought-through solution, cool! 👍

holgerd77 · 2021-02-22T17:04:09Z

packages/vm/tests/api/EIPs/eip-2929.spec.ts

-    //await runCodeTest('3415601557' + callFF + '600080FD5B600080808080305A60005400', 1, t)
+    const callFF = '6000808080806000195AF1'
+    // call address 0xFF..FF, now call same contract again, call 0xFF..FF again (it is now warm)
+    await runCodeTest(callFF + '60003415601B57600080808080305AF15B00', 23909, t)


Out of interest: how did you generate these bytcode sequences? 🤔

One of my (I guess) somewhat esoteric hobbies is writing raw EVM assembly. I do this from time to time, and getting more "fluent" in raw EVM 😄 It is very cool to figure out how you can create a minimal implementation which does what it should do. The minimal proxy contract, for instance, is also very interesting. As a side note: I hope that at some point we get a new gas golf contest. They allow raw assembly too 😄

Lol, I absolutely need more hobbies I can brag about

Maybe you can give a course at some PiT 😁 (far from fluent assembly writer)

Oh yeah - that sounds fun 😄 , I can definitely prepare something at some PiT 👍

holgerd77 · 2021-02-22T17:21:06Z

@jochem-brouwer Have updated this branch since I needed the latest changes from #1125 for some experimentation, hope that's ok, don't forget to pull the branch changes (and if you didn't and have already done some substantial amount of work: just force-push, maybe announce here before though).

holgerd77 · 2021-02-23T16:00:16Z

Currently running the following test DEBUG=vm:*:gas ts-node ./tests/tester --blockchain --file='static_callCreate3' --fork=Berlin. There the warmstorageread gas usage is actually getting negative (?):

Can this be right?

I generally have to say that I am not understanding the role of this basefee yet passed to e.g. accessAddressEIP2929() and than used in code parts like:

} else if (baseFee !== undefined && baseFee > 0) {
    console.log(runState._common.param('gasPrices', 'warmstorageread'))
    console.log(baseFee)
    runState.eei.useGas(
      new BN(runState._common.param('gasPrices', 'warmstorageread') - baseFee),
      'EIP-2929 -> warmstorageread'
    )
  }

Why is this subtracted at all for useGas()? The EIP is just stating:

Otherwise, charge WARM_STORAGE_READ_COST gas.

🤔

holgerd77 · 2021-02-23T16:05:43Z

Update: ah, just seeing from the screenshot that this is double-applied and first added and then subtracted again, likely for reasons of implementation simplification, but actually a bit for the cost of readability. Hmm. And this might have side effects to due to these steps happening in two separate PiTs in the control flow.

holgerd77 · 2021-02-23T16:17:28Z

The test from above actually also has such a slightly different gasUsage:

Actual: 137237
Expected: 137217

So just a difference of 20 (this trick with the coinbase account is actually gold. 😁 )

jochem-brouwer · 2021-02-23T16:27:10Z

Hi @holgerd77, I also had to do a double take here when I saw that it is negative, but this is correct. In total, for a CALL to a warm account, we charge the WARM gas (100 gas). Internally, what we do is we take the warm gas and the base call gas value (700), and subtract these. So we get -600. Later on (or earlier, but doesnt really matter) we charge the 700 gas, so net we charge 100 gas, which is correct according to the EIP.

jochem-brouwer · 2021-02-23T16:28:33Z

I am currently setting up retesteth and geth to get a trace of this transaction, but geth seems to have changed how we should start it in "retesteth" mode, and I cannot find any docs how I should run it 😢

holgerd77 · 2021-02-23T16:42:41Z

I think if we do this ping-pong "first subtract the base fee, then add it again" thing we might operate on the wrong values for things like maxCallGas. I think we should likely do this properly and simply exclude the respective CALL* and other opcodes from the basefee directly in runStep() in the interpreter.

jochem-brouwer · 2021-02-23T16:45:08Z

I'm not really sure if I like adding these exceptions to the base fee. You are right that it /might/ mess up maxCallGas, but since this operation is done before calling maxCallGas it is correct. I think this approach is elegant.

jochem-brouwer · 2021-02-23T16:46:13Z

(Or well, I am assuming it is correct - might also not be the case of course since this could be the culprit 😅 )

holgerd77 · 2021-02-23T16:48:42Z

I have the impression that is no exception or something but just the direct implementation of the EIP (there is just no basefee for these opcodes, right? Correct me if I am wrong).

This otherwise doesn't play well with this specification from the EIP:

Note: For call-variants, the 100/2600 cost is applied immediately (exactly like how 700 was charged before this EIP), i.e: before calculating the 63/64ths available for entering the call.

And for example for the CREATE opcode the code looks like this:

// '0xf0' range - closures
  // 0xf0: CREATE
  [
    0xf0,
    async function (runState: RunState) {
      if (runState.eei.isStatic()) {
        trap(ERROR.STATIC_STATE_CHANGE)
      }

      const [value, offset, length] = runState.stack.popN(3)

      subMemUsage(runState, offset, length)
      let gasLimit = new BN(runState.eei.getGasLeft())
      gasLimit = maxCallGas(gasLimit, runState.eei.getGasLeft(), runState)

      let data = Buffer.alloc(0)
      if (!length.isZero()) {
        data = runState.memory.read(offset.toNumber(), length.toNumber())
      }

      accessAddressEIP2929(runState, runState.eei.getAddress())
      const ret = await runState.eei.create(gasLimit, value, data)
      runState.stack.push(ret)
    },
  ]

This seems to be wrong? 🤔 Or is it just enough to switch the order here?

jochem-brouwer · 2021-02-23T16:55:03Z

This indeed looks wrong. Weird that tests do not pick this up (it seems that it is not picked up).

jochem-brouwer · 2021-02-23T17:13:21Z

I think if we do this ping-pong "first subtract the base fee, then add it again" thing we might operate on the wrong values for things like maxCallGas. I think we should likely do this properly and simply exclude the respective CALL* and other opcodes from the basefee directly in runStep() in the interpreter.

What we could maybe do here is not explicitly add these exceptions in code, but just mark these methods to use 0 gas in Common, and handle the base fee in the functions themselves?

holgerd77 · 2021-02-23T17:49:55Z

I think if we do this ping-pong "first subtract the base fee, then add it again" thing we might operate on the wrong values for things like maxCallGas. I think we should likely do this properly and simply exclude the respective CALL* and other opcodes from the basefee directly in runStep() in the interpreter.

What we could maybe do here is not explicitly add these exceptions in code, but just mark these methods to use 0 gas in Common, and handle the base fee in the functions themselves?

👍 That sounds like a good way to handle it.

jochem-brouwer · 2021-02-23T19:04:54Z

Yup - found the bug with help of the trace! Thanks to @winsvega

It has indeed to do with the base gas! At some point, if we have less than 700 gas (base call fee) then we go OOG when we deduct the fee. However, we actually charge 100 gas, so we should not go out of gas.

I will fix this with the method described above (base gas to 0).

In retrospect, I don't know why I haven't realized this earlier..!

vm: fix early OOG errors on 2929

jochem-brouwer · 2021-02-23T20:44:22Z

Berlin tests pass, have just added MuirGlacier to CI to ensure that it still works. Ready for review!

Thanks for all the help on this one, @holgerd77! 😄

holgerd77 · 2021-02-23T20:59:02Z

@jochem-brouwer yeah, thanks for going so straightly through this. Again - as I wrote in the other thread - really take some moment if you need to rebalance your time, these kind of in-between tasks just do large re-shuffles one just can't really plan in. 🙂

holgerd77

So great, all checks pass, congrats! 😄

LGTM

Feel free to merge once this fits for you.

holgerd77 · 2021-02-23T21:01:04Z

packages/vm/lib/evm/opcodes/EIP2929.ts

        'EIP-2929 -> coldaccountaccess'
      )
    }
    // Warm: (selfdestruct beneficiary address reads are not charged when warm)
-  } else if (baseFee !== undefined && baseFee > 0) {
+  } else if (chargeGas && !isSelfdestruct) {


That's a lot better than these implicit assumptions from before! 👍

holgerd77 · 2021-02-23T21:03:37Z

packages/vm/lib/evm/opcodes/functions.ts

@@ -940,6 +940,8 @@ export const handlers: Map<number, OpHandler> = new Map([

      const [value, offset, length] = runState.stack.popN(3)

+      accessAddressEIP2929(runState, runState.eei.getAddress(), false)
+


Just as some post-review question: is the order with subMemUsage() relevant here? Since in all the other cases this is the other way around?

Good question, and also a bit sloppy of me. But I just explicitly checked; the memory gas cost is deterministic and does not depend on the current gas available, so the order does not matter. I don't really want to trigger a new CI run, so I will merge here.

jochem-brouwer · 2021-02-23T21:10:10Z

Thanks for the quick review @holgerd77 😄

jochem-brouwer added 5 commits February 22, 2021 00:06

vm: stateManager: add EIP2929 access tracker

bdf73e7

vm: add origin/to/precompiles to warm addresses

acd608c

vm: update StateManager interface

vm: update EIP2929 to use StateManager

212f4cd

vm: fix build errors

2eeddab

vm: remove storage maps from interpreter

vm: fix EIP2929 StateManager bug

3d9dcbc

vm: add some extra EIP2929 tests (WIP)

jochem-brouwer added PR state: WIP prio: P2 very important package: vm labels Feb 22, 2021

jochem-brouwer commented Feb 22, 2021

View reviewed changes

holgerd77 reviewed Feb 22, 2021

View reviewed changes

jochem-brouwer added 3 commits February 22, 2021 14:55

vm: clean EIP2929 code

dfadddf

vm: make code backwards compatible vm: fix EIP2929 on CREATE* operations

vm: make EIP2929 use runTx

b40432b

vm: active precompiles method fixes

vm: add EIP2929 tests

488d392

jochem-brouwer added PR state: needs review and removed PR state: WIP labels Feb 22, 2021

jochem-brouwer marked this pull request as ready for review February 22, 2021 15:08

jochem-brouwer requested review from cgewecke, holgerd77 and ryanio February 22, 2021 15:11

holgerd77 reviewed Feb 22, 2021

View reviewed changes

Merge branch 'master' into fix-eip2929

99c0253

common: set EIP2929-related op gas costs to 0

231b071

vm: fix early OOG errors on 2929

jochem-brouwer marked this pull request as ready for review February 23, 2021 20:39

jochem-brouwer added PR state: needs review and removed PR state: WIP labels Feb 23, 2021

jochem-brouwer mentioned this pull request Feb 23, 2021

Implement EIP2718 and EIP2930 #1048

Merged

4 tasks

jochem-brouwer requested a review from holgerd77 February 23, 2021 20:47

CI: add MuirGlacier

d67e8bd

jochem-brouwer force-pushed the fix-eip2929 branch from 31aba34 to d67e8bd Compare February 23, 2021 20:50

holgerd77 approved these changes Feb 23, 2021

View reviewed changes

jochem-brouwer merged commit 93c1110 into master Feb 23, 2021

jochem-brouwer deleted the fix-eip2929 branch February 23, 2021 21:10

holgerd77 added the package: common label Feb 23, 2021

holgerd77 mentioned this pull request Feb 23, 2021

Breaking Releases - Meta Issue (VM v6, others) (old) #1024

Closed

50 tasks

holgerd77 mentioned this pull request Feb 16, 2022

v6 Breaking Release Planning #1717

Closed

51 tasks

		@@ -940,6 +940,8 @@ export const handlers: Map<number, OpHandler> = new Map([

		const [value, offset, length] = runState.stack.popN(3)

		accessAddressEIP2929(runState, runState.eei.getAddress(), false)

Fix EIP2929 #1124

Fix EIP2929 #1124

Conversation

jochem-brouwer commented Feb 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Feb 22, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

jochem-brouwer commented Feb 22, 2021

holgerd77 commented Feb 22, 2021

holgerd77 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

holgerd77 Feb 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jochem-brouwer commented Feb 22, 2021

jochem-brouwer commented Feb 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

holgerd77 commented Feb 22, 2021

holgerd77 commented Feb 23, 2021

holgerd77 commented Feb 23, 2021

holgerd77 commented Feb 23, 2021

jochem-brouwer commented Feb 23, 2021

jochem-brouwer commented Feb 23, 2021

holgerd77 commented Feb 23, 2021

jochem-brouwer commented Feb 23, 2021

jochem-brouwer commented Feb 23, 2021

holgerd77 commented Feb 23, 2021

jochem-brouwer commented Feb 23, 2021

jochem-brouwer commented Feb 23, 2021

holgerd77 commented Feb 23, 2021

jochem-brouwer commented Feb 23, 2021 • edited Loading

jochem-brouwer commented Feb 23, 2021 • edited Loading

holgerd77 commented Feb 23, 2021

holgerd77 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jochem-brouwer commented Feb 23, 2021

jochem-brouwer commented Feb 22, 2021 •

edited

Loading

codecov bot commented Feb 22, 2021 •

edited

Loading

holgerd77 Feb 22, 2021 •

edited

Loading

jochem-brouwer commented Feb 22, 2021 •

edited

Loading

jochem-brouwer commented Feb 23, 2021 •

edited

Loading

jochem-brouwer commented Feb 23, 2021 •

edited

Loading