EIP-7997 - #2499
Conversation
1d68532 to
45c5e72
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| return; | ||
| } | ||
|
|
||
| if (MONAD_UNLIKELY(!state.account_exists(FACTORY_ADDRESS))) { |
There was a problem hiding this comment.
[P1] The deployment guard should be !state.account_has_code_or_nonce(FACTORY_ADDRESS), not !state.account_exists(FACTORY_ADDRESS). account_exists returns true for any touched account (including one that only holds a balance), so if a user sends value to 0x4e59b44847b379578588920cA78FbF26c0B4956C before the MONAD_NEXT/Amsterdam activation, this check skips the factory deployment entirely and the fork silently ships without EIP-7997 code at the predeploy address. The base commit ("Add and use code-or-nonce primitive") introduced account_has_code_or_nonce specifically for this idiom — block_hash_history.cpp:57-58 uses it, and State::create_contract already preserves any existing balance per YP §7.
| if (MONAD_UNLIKELY(!state.account_exists(FACTORY_ADDRESS))) { | |
| if (MONAD_UNLIKELY(!state.account_has_code_or_nonce(FACTORY_ADDRESS))) { |
| // You should have received a copy of the GNU General Public License | ||
| // along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
|
|
||
| #include <category/core/config.hpp> |
There was a problem hiding this comment.
[P2] Missing #pragma once. CLAUDE.md's style section requires #pragma once on all headers, and the sibling block_hash_history.hpp:16 follows it. Without a guard, any TU that transitively includes this header more than once compiles slower and is one added definition away from an ODR breakage.
| #include <category/core/config.hpp> | |
| #pragma once | |
| #include <category/core/config.hpp> |
| // along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
|
|
||
| #include <category/core/config.hpp> | ||
| #include <category/execution/ethereum/state3/state.hpp> |
There was a problem hiding this comment.
[P3] Prefer forward-declaring State here — the sibling block_hash_history.hpp:27 uses class State; instead of pulling in the full state3/state.hpp transitive graph, which keeps compile time down and cuts unnecessary rebuild churn when state.hpp changes.
There was a problem hiding this comment.
Reviewed the EIP-7997 factory deployment. Runtime bytecode matches Arachnid's proxy verbatim and the trait wiring for MONAD_NEXT → MONAD_ETH_AMSTERDAM looks right, but the deployment guard uses the wrong existence predicate:
- [P1]
deterministic_factory_contract.cpp:46— uses!state.account_exists(...); must use!state.account_has_code_or_nonce(...)so a pre-fork balance transfer to0x4e59...956Cdoesn't silently skip the deploy at activation. This is exactly the pattern the base commit ("Add and use code-or-nonce primitive") set up, andblock_hash_history.cpp:57-58already uses it. - [P2]
deterministic_factory_contract.hpp:16— missing#pragma once(CLAUDE.md style rule). - [P3]
deterministic_factory_contract.hpp:17— forward-declareStateinstead of pulling in the fullstate3/state.hpp, matchingblock_hash_history.hpp:27.
No unit test alongside the new source (compare block_hash_history_test.cpp). Consider a matching test that verifies the deploy is a no-op on second entry and — with the fix above — that it still deploys when the address holds a pre-existing balance.
Verdict: NEEDS CHANGES
🤖 Generated with Claude Code
45c5e72 to
68957ae
Compare
68957ae to
6dec7f1
Compare
There was a problem hiding this comment.
Pull request overview
Adds initial plumbing for EIP-7997 by introducing a new trait feature flag and deploying the deterministic factory system contract during block header execution.
Changes:
- Extend
Traitswitheip_7997_active()and implement activation in bothEvmTraitsandMonadTraits. - Deploy the deterministic factory contract (EIP-7997) during
execute_block_header. - Wire the new implementation into the execution library build.
Verdict: NEEDS CHANGES
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| category/vm/evm/traits.hpp | Adds eip_7997_active() to the Traits concept and both trait families. |
| category/execution/ethereum/execute_block_header.cpp | Calls deterministic factory deployment as part of block-header execution. |
| category/execution/ethereum/deterministic_factory_contract.hpp | Declares the deployment helper for the EIP-7997 system contract. |
| category/execution/ethereum/deterministic_factory_contract.cpp | Implements deterministic factory deployment logic and instantiates across traits. |
| category/execution/CMakeLists.txt | Adds the new deterministic factory sources to the execution library. |
🤖 Generated with Claude Code
Suppressed comments (1)
category/execution/ethereum/deterministic_factory_contract.cpp:46
- Using
account_exists()here can skip deployment if the address was previously “created” by a balance transfer (balance-only account), leaving the system contract without code/nonce. The existing system-contract deploy path (deploy_block_hash_history_contract) usesaccount_has_code_or_nonce()for this reason.
if (MONAD_UNLIKELY(!state.account_exists(FACTORY_ADDRESS))) {
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| #include <category/core/config.hpp> | ||
| #include <category/execution/ethereum/state3/state.hpp> | ||
| #include <category/vm/evm/traits.hpp> |
| #include <category/core/address.hpp> | ||
| #include <category/core/byte_string.hpp> | ||
| #include <category/core/bytes.hpp> | ||
| #include <category/core/hex.hpp> | ||
| #include <category/execution/ethereum/deterministic_factory_contract.hpp> | ||
| #include <category/vm/evm/explicit_traits.hpp> |
| // EIP-7997 | ||
| template <Traits traits> | ||
| void deploy_deterministic_factory_contract(State &state) | ||
| { | ||
| if constexpr (!traits::eip_7997_active()) { | ||
| return; | ||
| } | ||
|
|
||
| if (MONAD_UNLIKELY(!state.account_exists(FACTORY_ADDRESS))) { | ||
| state.create_contract(FACTORY_ADDRESS); | ||
| state.set_code(FACTORY_ADDRESS, FACTORY_CODE); | ||
| state.set_nonce(FACTORY_ADDRESS, 1); | ||
| } |
No description provided.