feat(spec-specs): add EIP-7997 deterministic factory deployment#3200
feat(spec-specs): add EIP-7997 deterministic factory deployment#3200nerolation wants to merge 4 commits into
Conversation
|
I think thats actually what we do in geth already |
Does this imply that the fork transition block should contain the account code access in the block level access list and also the execution witness? |
49cba58 to
135af0f
Compare
|
I've now clarified it further in the EIP. It's now saying that even with the contract being accessed at the fork transition, it won't be in the BAL. Only on chains where the contract doesn't exist we put it in the BAL. |
Install the factory runtime code on the fork activation block, detected by the parent header being a pre-amsterdam header, when the code is not already present. The nonce and code changes are recorded in the block access list at the pre-execution index. On chains where the factory already exists this is a no-op, and no other block ever touches the account.
1b254f7 to
8d8858b
Compare
|
Nimbus EL also does it like that (but the witness tracking for stateless testing triggered this issue).
We should clarify in the EIP the behaviour for the witness also (cc @jsign ) (I realize that within execution-specs this is tracked with the same mechanism, but not all clients work like that). |
|
In EEST, the execution witness uses the same state tracker as BALs. From the execution witness POV, at the fork transition block there will be a state lookup to check if the contract exists or not. This implies this state access should materialize into an MPT proof in the execution witness as expected (just as any other state access that happens in the block). If we exclude this state lookup from the execution witness, then a stateless program can't know whether the contract exists, and thus can't decide what to do since that state access isn't part of the data received (i.e. remember is a stateless program that receives state information as input). @nerolation, is there any benefit to having this exclusion rule for BALs? I feel like having an exception for a single lookup only in a fork transition might not be worth it and might even make things more risky (since sounds not natural to ignore a state access). It would force EEST to have this rule at the BAL level, not at the "state tracker" level shared with the execution witness. It is kind of saying that only after some starting point of the block execution state is tracked, instead of just doing it always. Said differently, any state access should go through the state tracker feels like a simpler rule to me (i.e. including any extra state access in fork transitions). |
|
Yeah I can def see this reasoning. Seems like we have to bring this to ACDT on Monday. Right now the EIP clarifies that the lookup wouldn't be covered by the BAL. but I'm happy to update this: |
The factory account appears in the activation block's access list even when the code check finds it already deployed, as an accessed account with no changes.
|
The latest commit removed the early return guard and let's the deployment/check be included independent from the factory existing or not. |
…nsitions Move the activation gate from apply_body to execute_block, where the parent header is available. The filler marks a block whose parent belongs to an earlier fork by passing parentFork in the t8n environment, and the t8n applies the deployment for forks that define it when the parent fork does not. Add transition tests covering the install, the access-only recording, non-canonical code reset, and same-block use.
|
Ref, PR (seems to) implement ethereum/EIPs#11991 |
| parent_short_name = re.sub( | ||
| r"(?<!^)(?=[A-Z])", "_", self.env.parent_fork | ||
| ).lower() | ||
| parent_short_name = re.sub("^b_p_o", "bpo", parent_short_name) |
There was a problem hiding this comment.
Can you leave a comment what this does or what this parent_short_name is? Does this standardize fork (transition?) names?
| | `test_factory_installed_at_transition` | Ensure BAL records the [EIP-7997](https://eips.ethereum.org/EIPS/eip-7997) factory install on the Amsterdam activation block as an irregular state transition at `block_access_index=0`. File: `tests/amsterdam/eip7997_deterministic_factory_predeploy/test_fork_transition.py`. Uses `@pytest.mark.valid_at_transition_to("Amsterdam")`. | Factory removed from genesis via a zeroed pre-alloc override. Blocks at `timestamp=14_999` (pre-fork), `15_000` (empty activation block), `15_001` (CREATE2 deployment through the factory). | Activation block BAL **MUST** include the factory with `nonce_changes=[(0, 1)]` and `code_changes=[(0, runtime code)]`. The next block **MUST** record only the tx-driven `nonce_changes=[(1, 2)]` with explicitly empty `code_changes` (the install must not repeat). Post-state: factory at nonce 2 with canonical code, CREATE2 child deployed. | ✅ Completed | | ||
| | `test_factory_access_recorded_at_transition` | Ensure the activation block records an access-only BAL entry when the factory is already deployed (the mainnet case). File: `tests/amsterdam/eip7997_deterministic_factory_predeploy/test_fork_transition.py`. | Factory pre-existing with canonical code, parametrized `nonce ∈ [1, 5]`. Unrelated Alice→Bob transfers in the activation block and the block after. | Activation block BAL **MUST** include the factory as an accessed account with no changes (`BalAccountExpectation.empty()`). The following block **MUST NOT** include the factory at all. Post-state: factory unchanged. | ✅ Completed | | ||
| | `test_factory_code_reset_at_transition` | Ensure a factory account holding non-canonical code is reset on the activation block, with nonce and balance preservation rules in the BAL. File: `tests/amsterdam/eip7997_deterministic_factory_predeploy/test_fork_transition.py`. | Factory pre-existing with `STOP` code and balance 1000, parametrized: `nonce=0` (set to 1) and `nonce=7` (preserved). Empty activation block. | Activation block BAL **MUST** include `code_changes=[(0, runtime code)]`. `nonce_changes` **MUST** be `[(0, 1)]` for the zero-nonce case and explicitly empty for the nonzero case. Post-state: canonical code, balance preserved. | ✅ Completed | | ||
| | `test_factory_install_merged_with_same_block_use` | Ensure the factory install and a same-block CREATE2 use merge into a single BAL entry across indices. File: `tests/amsterdam/eip7997_deterministic_factory_predeploy/test_fork_transition.py`. | Factory removed from genesis. The activation block itself contains a CREATE2 deployment through the freshly installed factory. | BAL **MUST** contain one factory entry with `code_changes=[(0, runtime code)]` and `nonce_changes=[(0, 1), (1, 2)]` — the install at the pre-execution index and the tx-driven increment merged in index order. | ✅ Completed | |
There was a problem hiding this comment.
There is one more case:
This is for chains which could have deployed the factory the "normal" way but did not, and now it is inserted as irregular state transition. We then replay the create tx https://etherscan.io/tx/0xeddf9e61fb9d8f5111840daef55e5fde0041f5702856532cdbb5a02998033d26
This should be rejected due to https://eips.ethereum.org/EIPS/eip-684 (the tx is valid, the contract deployment is not)
| 1, | ||
| [BalNonceChange(block_access_index=0, post_nonce=1)], | ||
| id="zero_nonce_set_to_one", | ||
| ), |
There was a problem hiding this comment.
This seems to include nonce 0 and nonce 7 as starting nonce, not the implied range [0,7]
7 participants
Reopening #3198 through this pr.
This time the state access is gated by checking the PreviousHeader, and will only happen in the fork transition block.
It feels like the right solution now.
cc @marioevz @MariusVanDerWijden