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

Update EIP-7702: updates for devnet4 #8929

Merged
merged 4 commits into from
Oct 9, 2024

Conversation

lightclient
Copy link
Member

placeholder for further devnet changes.

outstanding clarifications to make (from my notes):

  • authorization should be invalid is nonce == 2^64 - 1
  • This case (add tx-level delegated account to warm)
  • Precompiles will not be run once delegated to (treated as empty accounts)
  • EXTCODEHASH puts 0 on stack if the 7702-delegated account it is delegates to does not exist in trie (as opposed to empty hash keccak256("") of accounts which exists in trie, but have no code, i.e. EOAs (non-delegated to))
  • delegations are not reverted in case transaction fails
  • if delegation target account does not exist, nonce is checked to be 0
  • is the price correct at 2500? is there incentive to use as alternative contract deployment?
  • limit chain id to uint64 and y_parity to uint8

@github-actions github-actions bot added c-update Modifies an existing proposal s-review This EIP is in Review t-core labels Oct 3, 2024
@eth-bot
Copy link
Collaborator

eth-bot commented Oct 3, 2024

✅ All reviewers have approved.

@eth-bot eth-bot changed the title 7702: restrict field sizes Update EIP-7702: restrict field sizes Oct 3, 2024
@jochem-brouwer
Copy link
Member

jochem-brouwer commented Oct 3, 2024

Did you see #8905 and #8906? These address part of your notes :)

EDIT: sorry, just saw you already replied to those PRs :)

EIPS/eip-7702.md Outdated Show resolved Hide resolved
@lightclient lightclient marked this pull request as ready for review October 9, 2024 14:35
@eth-bot eth-bot enabled auto-merge (squash) October 9, 2024 14:36
Copy link
Collaborator

@eth-bot eth-bot left a comment

Choose a reason for hiding this comment

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

All Reviewers Have Approved; Performing Automatic Merge...

Copy link
Collaborator

@eth-bot eth-bot left a comment

Choose a reason for hiding this comment

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

All Reviewers Have Approved; Performing Automatic Merge...

@eth-bot eth-bot merged commit 8628167 into ethereum:master Oct 9, 2024
11 checks passed
@@ -73,7 +73,8 @@ At the start of executing the transaction, after incrementing the sender's nonce
5. Verify the code of `authority` is either empty or already delegated.
6. Verify the nonce of `authority` is equal to `nonce`. In case `authority` does not exist in the trie, verify that `nonce` is equal to `0`.
7. Add `PER_EMPTY_ACCOUNT_COST - PER_AUTH_BASE_COST` gas to the global refund counter if `authority` exists in the trie.
8. Set the code of `authority` to be `0xef0100 || address`. This is a delegation designation.
8. Set the code of `authority` to be `0xef0100 || address`. This is a delegation designation.
* As a special case, if `address` is `0x0000000000000000000000000000000000000000` do not write the designation. Clear the accounts code and reset the account's code hash to `0x0000000000000000000000000000000000000000000000000000000000000000`.
Copy link
Member

Choose a reason for hiding this comment

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

Why is the account code hash set to these zeros? It should be set to keccak256("")?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah you're right. Let me update it.

Copy link
Member

@jochem-brouwer jochem-brouwer left a comment

Choose a reason for hiding this comment

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

I got some minor further questions / clarification comments :)


The `PER_AUTH_BASE_COST` is the cost to process the authorization tuple and set the delegation destination. We are able to compute a fair cost for this operation by reviewing its impact on the system:

* ferry 101 bytes of calldata = `101 * 16 = 1616`
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I see where the 101 comes from (flat bytes of the authority tuple) but this is not super clear. 16 is calldata gas cost for nonzero bytes.

The `PER_AUTH_BASE_COST` is the cost to process the authorization tuple and set the delegation destination. We are able to compute a fair cost for this operation by reviewing its impact on the system:

* ferry 101 bytes of calldata = `101 * 16 = 1616`
* recovering the `authority` address = `3000`
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a note that this is ecrecover precompile cost?


* ferry 101 bytes of calldata = `101 * 16 = 1616`
* recovering the `authority` address = `3000`
* reading the nonce and code of `authority` = `5000`
Copy link
Member

Choose a reason for hiding this comment

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

Where does the 5000 cost come from? (If I would check cold extcodecopy I pay 2600 gas, and then nonce can be directly read since we have to read the account RLP (which contains the nonce))


For most intents and purposes, an account delegated to `0x0` is indistinguishable from a true EOA. However, one particular unfortunate case is unavoidable. Even if a user has a zeroed out delegation designation, most operations that interact with that account will encounter an additional `COLD_ACCOUNT_READ_COST` upon the first touch.

This is not ideal and may be a significant enough concern to impact the overall adoption of the EIP. For these reasons, we have opted to include a mechanism which allow users to restore their EOA to its original pureness.
Copy link
Member

Choose a reason for hiding this comment

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

This is a super great addition 😄 👍

@ethereum ethereum deleted a comment from GIgako19929 Oct 9, 2024
@ethereum ethereum deleted a comment from GIgako19929 Oct 9, 2024
@ethereum ethereum deleted a comment from GIgako19929 Oct 9, 2024
@lightclient lightclient changed the title Update EIP-7702: restrict field sizes Update EIP-7702: updates for devnet4 Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-update Modifies an existing proposal s-review This EIP is in Review t-core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants