test: add Rex4 system contract deployment and idempotence tests#187
test: add Rex4 system contract deployment and idempotence tests#187viktorcrypt wants to merge 1 commit intomegaeth-labs:mainfrom
Conversation
| } | ||
|
|
There was a problem hiding this comment.
rex4_chain_spec() only activates MegaHardfork::Rex4 without enabling prerequisite hardforks (Rex, Rex1, Rex2, Rex3). In production, all prior hardforks would be active. If apply_pre_execution_changes() deploys earlier system contracts (Oracle, HP Timestamp, Keyless Deploy) conditionally on their hardfork activation, this test won't exercise those paths, potentially masking ordering dependencies.
Consider using MegaHardforkConfig::default().with_all_activated() or chaining all prerequisite hardforks to better reflect production conditions.
| let cache_acc = db.load_cache_account(address).expect("should load cache account"); | ||
| assert!( | ||
| cache_acc.account_info().is_none(), | ||
| "{name} should not be deployed for this spec boundary" | ||
| ); | ||
| } |
There was a problem hiding this comment.
assert_contract_not_deployed checks cache_acc.account_info().is_none(), which asserts the entire account doesn't exist. This is correct for a fresh MemoryDatabase, but a more precise check would verify the code_hash is KECCAK_EMPTY (no code deployed) rather than asserting the account doesn't exist at all. If the executor ever touches the account for a non-code reason, this assertion would break even though the contract isn't actually deployed.
| let db_ref = executor.evm_mut().db_mut(); | ||
| assert_contract_not_deployed(db_ref, ACCESS_CONTROL_ADDRESS, "MegaAccessControl"); | ||
| assert_contract_not_deployed(db_ref, LIMIT_CONTROL_ADDRESS, "MegaLimitControl"); | ||
| } |
There was a problem hiding this comment.
Significant setup duplication across all three test functions (~20 lines of identical boilerplate each). The existing test suite typically extracts setup into helper functions. Consider a helper like fn make_executor(spec: MegaSpecId, hardforks: MegaHardforkConfig) -> MegaBlockExecutor<...> to reduce repetition.
|
I addressed the requested changes and force-pushed the updated branch. The test now uses production-like hardfork activation, checks Unfortunately GitHub seems to be in a bad state for this PR after yesterday’s repository outage: my original fork is now shown as forked from I’ve created a fresh fork from |
Summary
Adds deployment wiring tests for the two Rex4 system contracts:
MegaAccessControl(0x...0004) andMegaLimitControl(0x...0005).What is tested
addresses after
apply_pre_execution_changes()on a Rex4 blockapply_pre_execution_changes()twice does not panic or corrupt state
Why
The interception behavior of these contracts was already covered,
but the hardfork-gated deployment wiring in
block/executor.rshadno test coverage. These tests close that gap for the Rex4 unstable frontier.