-
Notifications
You must be signed in to change notification settings - Fork 759
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
EIP-7702 devnet-3 readiness #3581
Conversation
Note to self: have to take care of the edge case when a precompile is the delegation address |
evm: fix extcodehash/extcodesize for delegated accounts
Failing execution-spec-tests These tests are likely wrong on the side of
|
Ok, have cross-checked all tests! Bugs found are fixed here. |
Could you like the tests which are tested here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall though a couple of things we should clean up before merging
packages/evm/src/evm.ts
Outdated
// EIP-7702 delegation check | ||
if ( | ||
this.common.isActivatedEIP(7702) && | ||
equalsBytes(message.code.slice(0, 3), new Uint8Array([0xef, 0x01, 0x00])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we make a constant out of 0xef0100
and call it DELEGATION_DESIGNATOR
or whatever is the best way to refer to this instead of a magic number?
packages/vm/src/runTx.ts
Outdated
throw new Error(msg) | ||
if (vm.common.isActivatedEIP(7702)) { | ||
const code = await state.getCode(caller) | ||
if (!equalsBytes(code.slice(0, 3), new Uint8Array([0xef, 0x01, 0x00]))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, let's make 0xef0100 a constant and not a magic number.
Does devnet-3 have a URL (yet)? If so, can it be added to the PR description? Thanks! 🙂 |
I thought there was a devnet-3 spec somewhere, but I can't find it 🤔 NVM, found it https://notes.ethereum.org/@ethpandaops/pectra-devnet-3. Added to description. |
@@ -64,6 +64,9 @@ const journalCacheCleanUpLabel = 'Journal/cache cleanup' | |||
const receiptsLabel = 'Receipts' | |||
const entireTxLabel = 'Entire tx' | |||
|
|||
// EIP-7702 flag: if contract code starts with these 3 bytes, it is a 7702-delegated EOA | |||
const DELEGATION_7702_FLAG = new Uint8Array([0xef, 0x01, 0x00]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be in common
or else a constants section? Feels out of place to declare a constant like this in runTx
.
Updates the EIP-7702 implementation to this spec: https://github.com/ethereum/EIPs/blob/52de6be37bfb7c134d0a2280f3d5b3d2433dc572/EIPS/eip-7702.md (devnet-3 spec (?))
NOTE for t8ntool to work for state tests, I have disabled throwing on
requests.ts
if code does not exist, we should carefully review this as a side-effect of this PR. (Will add a blocked label for this one since we should discuss)This makes EthereumJS devnet-3 ready, see: https://notes.ethereum.org/@ethpandaops/pectra-devnet-3