Skip to content
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

7702 changes for devnet 5 #7781

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

7702 changes for devnet 5 #7781

wants to merge 9 commits into from

Conversation

ak88
Copy link
Contributor

@ak88 ak88 commented Nov 21, 2024

DO NOT MERGE!

Mekong does not include devnet 5 changes, so we cannot merge to master for now.

All EXTCODE ops now only reads the first two bytes of the delegation header, instead of reading from delegation target.

Changes described here:
https://github.com/ethereum/EIPs/pull/8969/files

@ak88 ak88 marked this pull request as draft November 21, 2024 12:27
@ak88 ak88 marked this pull request as ready for review November 22, 2024 13:13
Copy link
Contributor

@MarekM25 MarekM25 left a comment

Choose a reason for hiding this comment

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

Looks good, merging it will break mekong, so let's discuss next week how do we handle devnet-5 changes

@@ -793,6 +779,53 @@ public void Execute_SetNormalDelegationAndThenSetDelegationWithZeroAddress_Accou
Assert.That(_stateProvider.HasCode(authority.Address), Is.False);
}

[TestCase(true)]
[TestCase(false)]
public void Execute_EXTCODESIZEOnDelegatedThatTriggersOptimization_ReturnsZeroIfDelegated(bool isDelegated)
Copy link
Contributor

Choose a reason for hiding this comment

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

should test name be ReturnsTwoIfDelegated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test is about testing a specific optimization path in EXTCODESIZE, which will return a one or zero. Its used for contracts to determine if an address is an EOA or not.

Copy link
Contributor

Choose a reason for hiding this comment

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

The EIP mentioned that a few more instructions are affected: CALL, CALLCODE, STATICCALL, DELEGATECALL

so a quick question, do we need any modifications in those opcodes?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am almost 100% sure that this is okay though :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The change only affected the "code reading" ops, but the executing ones (CALL, CALLCODE...) remains the same as before. So they are still affected by 7702 itself, but not by this new change.

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.

4 participants