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

Implements eip-7708 #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Implements eip-7708 #1

wants to merge 1 commit into from

Conversation

mralj
Copy link
Collaborator

@mralj mralj commented Sep 22, 2024

Implements: https://eips.ethereum.org/EIPS/eip-7708

Open questions:

Missing definitions

  1. Magic was not defined in EIP, I have set a placeholder for Magic as the hash of 0000...000000000 as proposed here
  2. Address is also not defined in EIP, I have set placeholder 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE as proposed here
  3. Why do we not create LOGs when CREATE is called?

"Stuff" I'm unsure about

From EIP:

The LOG of a value-transferring transaction should be placed before any logs created by EVM execution. The other two LOGs are placed when the value transfer executes.

Q1
"value-transferring transaction" - is this just EOA to EOA transfer? (eg. Bob directly transferred 1 ETH to Alice)

Q2
This one is about adding logs in the proper place.
I think I placed SELF_DESTRUCT appropriately. But I'm unsure about handling CALL and "value-transferring tx"

This is my understanding of transaction execution flow (is this OK?):

Tx Execution Diagram

My understanding is that we potentially have 3 options for placing logs:

  1. In TransitionDb: Not OK, because we cannot handle SELF_DESTRUCT here (since we don't know contract balance) and doesn't satisfy EIP requirement "...The other two LOGs are placed when the value transfer executes."
  2. In Transfer (core/evm.go): Not ok because SELF_DESTRUCT doesn't call this function - but I'm unsure why. Can we refactor both opSelfdestruct and opSelfdestruct6780, especially since opSelfdestruct6780 does the same steps as Transfer and have logs created in Transfer?
  3. The placement I chose, as far as I can tell, satisfies both EIP requirements (even though this seems a bit odd, since unlike EIP specification logs are created at the same place for CALL and "value-transferring tx" at the same place, and SELF_DESTRUCT is handled specially)

Q3
EIP activation - how to choose when EIP will be activated, especially given this is code aimed at rollups.

Q4
What are the usual test practices? I found no log-specific tests (apart from marshaling/unmarshalling). Is mocking StateDB and the op-codes the best way to go?

@mralj mralj marked this pull request as ready for review September 22, 2024 15:09
@mralj mralj self-assigned this Sep 23, 2024
mralj pushed a commit that referenced this pull request Oct 1, 2024
Merge op-geth changes to v1.14.7 branch
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.

1 participant