Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 146 additions & 0 deletions crates/ev-revm/src/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ mod tests {
const ADMIN_PROXY_RUNTIME: [u8; 42] = alloy_primitives::hex!(
"36600060003760006000366000600073000000000000000000000000000000000000f1005af1600080f3"
);
const OVERSIZED_RUNTIME_SIZE: usize = 32 * 1024;

fn empty_state() -> State<CacheDB<EmptyDB>> {
State::builder()
Expand All @@ -279,6 +280,39 @@ mod tests {
.build()
}

fn oversized_initcode(runtime_size: usize) -> Bytes {
const INIT_PREFIX_LEN: usize = 14;

assert!(
runtime_size <= u16::MAX as usize,
"runtime size must fit in PUSH2"
);

let size_hi = ((runtime_size >> 8) & 0xff) as u8;
let size_lo = (runtime_size & 0xff) as u8;
let mut initcode = Vec::with_capacity(INIT_PREFIX_LEN + runtime_size);

initcode.extend_from_slice(&[
0x61,
size_hi,
size_lo, // PUSH2 size
0x60,
INIT_PREFIX_LEN as u8, // PUSH1 offset
0x60,
0x00, // PUSH1 0
0x39, // CODECOPY
0x61,
size_hi,
size_lo, // PUSH2 size
0x60,
0x00, // PUSH1 0
0xf3, // RETURN
]);
initcode.resize(INIT_PREFIX_LEN + runtime_size, 0u8);

Bytes::from(initcode)
}

#[test]
fn factory_applies_base_fee_redirect() {
let sink = address!("0x00000000000000000000000000000000000000fe");
Expand Down Expand Up @@ -563,4 +597,116 @@ mod tests {
.expect("mint precompile should mint after activation");
assert_eq!(mintee_account.info.balance, amount);
}

#[test]
fn contract_size_limit_rejects_oversized_code() {
let caller = address!("0x0000000000000000000000000000000000000abc");
let initcode = oversized_initcode(OVERSIZED_RUNTIME_SIZE);

let mut state = empty_state();
state.insert_account(
caller,
AccountInfo {
balance: U256::from(10_000_000_000u64),
nonce: 0,
code_hash: KECCAK_EMPTY,
code: None,
},
);

let mut evm_env: alloy_evm::EvmEnv<SpecId> = EvmEnv::default();
evm_env.cfg_env.chain_id = 1;
evm_env.cfg_env.spec = SpecId::CANCUN;
evm_env.block_env.number = U256::from(1);
evm_env.block_env.basefee = 1;
evm_env.block_env.gas_limit = 30_000_000;

let mut evm = EvEvmFactory::new(alloy_evm::eth::EthEvmFactory::default(), None, None, None)
.create_evm(state, evm_env);

let tx = crate::factory::TxEnv {
caller,
kind: TxKind::Create,
gas_limit: 15_000_000,
gas_price: 1,
value: U256::ZERO,
data: initcode,
..Default::default()
};

let result_and_state = evm
.transact_raw(tx)
.expect("oversized create executes to a halt");

let ExecutionResult::Halt { reason, .. } = result_and_state.result else {
panic!("expected oversized code to halt");
};
assert!(
matches!(reason, HaltReason::CreateContractSizeLimit),
"expected create code size limit halt"
);
}

#[test]
fn contract_size_limit_allows_oversized_code_when_configured() {
let caller = address!("0x0000000000000000000000000000000000000def");
let initcode = oversized_initcode(OVERSIZED_RUNTIME_SIZE);

let mut state = empty_state();
state.insert_account(
caller,
AccountInfo {
balance: U256::from(10_000_000_000u64),
nonce: 0,
code_hash: KECCAK_EMPTY,
code: None,
},
);

let mut evm_env: alloy_evm::EvmEnv<SpecId> = EvmEnv::default();
evm_env.cfg_env.chain_id = 1;
evm_env.cfg_env.spec = SpecId::CANCUN;
evm_env.block_env.number = U256::from(1);
evm_env.block_env.basefee = 1;
evm_env.block_env.gas_limit = 30_000_000;

let mut evm = EvEvmFactory::new(
alloy_evm::eth::EthEvmFactory::default(),
None,
None,
Some(ContractSizeLimitSettings::new(64 * 1024, 0)),
)
.create_evm(state, evm_env);

let tx = crate::factory::TxEnv {
caller,
kind: TxKind::Create,
gas_limit: 15_000_000,
gas_price: 1,
value: U256::ZERO,
data: initcode,
..Default::default()
};

let result_and_state = evm.transact_raw(tx).expect("oversized create executes");

let ExecutionResult::Success { .. } = result_and_state.result else {
panic!("expected oversized code to deploy with custom limit");
};

let created_address = result_and_state
.result
.created_address()
.expect("created address available");
let state: EvmState = result_and_state.state;
let created_account = state
.get(&created_address)
.expect("created contract must be in state");
let code = created_account.info.code.as_ref().expect("code stored");
assert_eq!(
code.len(),
OVERSIZED_RUNTIME_SIZE,
"contract runtime size should match initcode payload"
);
}
Comment on lines +601 to +711
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

There's significant code duplication between contract_size_limit_rejects_oversized_code and contract_size_limit_allows_oversized_code_when_configured. The setup for state, caller account, and evm_env is nearly identical.

To improve readability and maintainability, consider extracting this common setup into a helper function. This function could take the ContractSizeLimitSettings as a parameter and return the configured EVM instance and other necessary test data.

}
34 changes: 31 additions & 3 deletions crates/node/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,13 @@ impl EvolvePayloadBuilderConfig {

/// Validates the configuration
pub const fn validate(&self) -> Result<(), ConfigError> {
Ok(())
match (
self.contract_size_limit,
self.contract_size_limit_activation_height,
) {
(Some(0), _) | (None, Some(_)) => Err(ConfigError::InvalidConfig),
_ => Ok(()),
}
}

/// Returns the configured base-fee redirect sink and activation height (defaulting to 0).
Expand Down Expand Up @@ -320,8 +326,8 @@ mod tests {
}

#[test]
fn test_validate_always_ok() {
// Test that validate always returns Ok for now
fn test_validate_accepts_defaults() {
// Test that validate accepts default configurations
let config = EvolvePayloadBuilderConfig::new();
assert!(config.validate().is_ok());

Expand Down Expand Up @@ -475,4 +481,26 @@ mod tests {
DEFAULT_CONTRACT_SIZE_LIMIT
);
}

#[test]
fn test_contract_size_limit_zero_invalid() {
let config = EvolvePayloadBuilderConfig {
contract_size_limit: Some(0),
contract_size_limit_activation_height: Some(0),
..EvolvePayloadBuilderConfig::new()
};

assert!(matches!(config.validate(), Err(ConfigError::InvalidConfig)));
}

#[test]
fn test_contract_size_limit_activation_without_limit_invalid() {
let config = EvolvePayloadBuilderConfig {
contract_size_limit: None,
contract_size_limit_activation_height: Some(10),
..EvolvePayloadBuilderConfig::new()
};

assert!(matches!(config.validate(), Err(ConfigError::InvalidConfig)));
}
}
31 changes: 31 additions & 0 deletions crates/tests/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,37 @@ pub fn create_test_chain_spec_with_mint_admin(mint_admin: Address) -> Arc<ChainS
create_test_chain_spec_with_extras(None, Some(mint_admin))
}

/// Creates a reusable chain specification with a custom contract size limit.
pub fn create_test_chain_spec_with_contract_size_limit(
contract_size_limit: usize,
activation_height: Option<u64>,
) -> Arc<ChainSpec> {
let mut genesis: Genesis =
serde_json::from_str(include_str!("../assets/genesis.json")).expect("valid genesis");

let mut extras = serde_json::Map::new();
extras.insert("contractSizeLimit".to_string(), json!(contract_size_limit));
if let Some(height) = activation_height {
extras.insert(
"contractSizeLimitActivationHeight".to_string(),
json!(height),
);
}

genesis
.config
.extra_fields
.insert("evolve".to_string(), serde_json::Value::Object(extras));

Arc::new(
ChainSpecBuilder::default()
.chain(reth_chainspec::Chain::from_id(TEST_CHAIN_ID))
.genesis(genesis)
.cancun_activated()
.build(),
)
}
Comment on lines +59 to +88
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The logic for creating a ChainSpec (loading genesis, building with ChainSpecBuilder) is duplicated between this new function create_test_chain_spec_with_contract_size_limit and the existing create_test_chain_spec_with_extras.

To reduce duplication, you could refactor create_test_chain_spec_with_extras to be more generic, allowing it to handle different "extra" configurations, and have the public helper functions call it. This would centralize the ChainSpec creation logic.


fn create_test_chain_spec_with_extras(
base_fee_sink: Option<Address>,
mint_admin: Option<Address>,
Expand Down
Loading