Implement EIP5656 MCOPY #2808
Implement EIP5656 MCOPY #2808
Conversation
Codecov Report
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. |
| // MCOPY | ||
| const [dst, src, length] = runState.stack.popN(3) | ||
| const data = runState.memory.read(Number(src), Number(length), true) | ||
| runState.memory.write(Number(dst), Number(length), data) |
There was a problem hiding this comment.
It seems like write will throw in the case of writing beyond memory length, but the EIP semantics expects this case automatically handled with extra gas accounted for: If length > 0 and (src + length or dst + length) is beyond the current memory length, the memory is extended with respective gas cost applied.
Also see the write function: if (offset + size > this._store.length) throw new Error('Value exceeds memory capacity')
There was a problem hiding this comment.
An example test case wasn't provided either. Maybe we could add one for it as well.
There was a problem hiding this comment.
Thanks for this comment! Note that in write, we also call extend in memory. I am rather sure (and I think I once introduced this) is that this explicit error check is added as a precaution. However, I doubt this can ever happen.
( see memory.ts line 55: this.extend(offset, size), this extends memory prior to writing)
(The unit tests absolutely do not cover all the cases. I have introduced them for completeness since these are in the EIP. However, EIP authors have written tests: ethereum/tests#1229 which we pass)
|
We pass these as well: ethereum/tests#1234 |
https://eips.ethereum.org/EIPS/eip-5656
TODOS:
To test against
ethereum/tests: ensureethereum/testsis on latest version of develop (socd ./packages/ethereum-tests/ && git checkout develop && git pull).Then, edit the cancun common to include EIP 5656. Now run the tests in vm:
npm run test:blockchain -- --fork=Cancun --dir=../EIPTests/BlockchainTests/StateTests/stEIP5656-MCOPY