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

chore: add codes for validation rules #241

Merged
merged 1 commit into from
Nov 9, 2023
Merged
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
4 changes: 4 additions & 0 deletions crates/contracts/src/tracer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ pub const JS_TRACER: &str = r#"
return x.opcode;
}).join(' ');
// only store the last EXTCODE* opcode per address - could even be a boolean for our current use-case
// [OP-051] - may call EXTCODESIZE ISZERO
if (last3opcodesString.match(/^(\w+) EXTCODESIZE ISZERO$/) == null) {
this.currentLevel.extCodeAccessInfo[addrHex] = opcode;
// this.debug.push(`potentially illegal EXTCODESIZE without ISZERO for ${addrHex}`)
Expand All @@ -233,12 +234,14 @@ pub const JS_TRACER: &str = r#"
}
}
// not using 'isPrecompiled' to only allow the ones defined by the ERC-4337 as stateless precompiles
// [OP-062] - only allowed the core 9 precompiles
const isAllowedPrecompiled = function(address) {
const addrHex = toHex(address);
const addressInt = parseInt(addrHex);
// this.debug.push(`isPrecompiled address=${addrHex} addressInt=${addressInt}`)
return addressInt > 0 && addressInt < 10;
};
// [OP-041] - access to an address without a deployed code is forbidden for EXTCODE* and *CALL opcodes
if (opcode.match(/^(EXT.*|CALL|CALLCODE|DELEGATECALL|STATICCALL)$/) != null) {
const idx = opcode.startsWith('EXT') ? 0 : 1;
const addr = toAddress(log.stack.peek(idx).toString(16));
Expand All @@ -251,6 +254,7 @@ pub const JS_TRACER: &str = r#"
};
}
}
// [OP-012] - GAS opcode is allowed, but only if followed immediately by *CALL instructions
if (this.lastOp === 'GAS' && !opcode.includes('CALL')) {
// count "GAS" opcode only if not followed by "CALL"
this.countSlot(this.currentLevel.opcodes, 'GAS');
Expand Down
6 changes: 4 additions & 2 deletions crates/uopool/src/validate/sanity/entities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl Entities {
)?))
}

/// [SREP-020]: A BANNED address is not allowed into the mempool.
/// [SREP-020] - a BANNED address is not allowed into the mempool.
fn check_banned(
&self,
entity: &str,
Expand All @@ -55,7 +55,7 @@ impl Entities {
Ok(())
}

/// [SREP-030]: A THROTTLED address is limited to THROTTLED_ENTITY_MEMPOOL_COUNT entries in the mempool.
/// [SREP-030] - THROTTLED address is limited to THROTTLED_ENTITY_MEMPOOL_COUNT entries in the mempool
fn check_throttled<M: Middleware, P, R, E>(
&self,
entity: &str,
Expand Down Expand Up @@ -106,6 +106,8 @@ where
) -> Result<(), SanityCheckError> {
let (sender, factory, paymaster) = uo.get_entities();

// [SREP-040] - an OK staked entity is unlimited by the reputation rule

// sender
let status = self.get_status(&sender, helper)?;
self.check_banned(SENDER, &sender, &status)?;
Expand Down
27 changes: 14 additions & 13 deletions crates/uopool/src/validate/sanity/unstaked_entities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,10 @@ where
) -> Result<(), SanityCheckError> {
let (sender, factory, paymaster) = uo.get_entities();

// [SREP-010] - the "canonical mempool" defines a staked entity if it has MIN_STAKE_VALUE and unstake delay of MIN_UNSTAKE_DELAY

// sender
// [STO-040]
// [STO-040] - UserOperation may not use an entity address (factory/paymaster/aggregator) that is used as an "account" in another UserOperation in the mempool
if helper.mempool.get_number_by_entity(&sender) > 0 {
return Err(SanityCheckError::EntityVerification {
entity: SENDER.to_string(),
Expand All @@ -116,26 +118,25 @@ where
});
}

// [UREP-010] - UserOperation with unstaked sender are only allowed up to SAME_SENDER_MEMPOOL_COUNT times in the mempool
let sender_stake = self.get_stake(&sender, helper).await?;
if helper
.reputation
.verify_stake(SENDER, Some(sender_stake))
.is_err()
&& helper.mempool.get_number_by_sender(&uo.sender) >= SAME_SENDER_MEMPOOL_COUNT
{
// [UREP-010]
if helper.mempool.get_number_by_sender(&uo.sender) >= SAME_SENDER_MEMPOOL_COUNT {
return Err(ReputationError::UnstakedEntityVerification {
entity: SENDER.to_string(),
address: uo.sender,
message: "has too many user operations in the mempool".into(),
}
.into());
return Err(ReputationError::UnstakedEntityVerification {
entity: SENDER.to_string(),
address: uo.sender,
message: "has too many user operations in the mempool".into(),
}
.into());
}

// factory
if let Some(factory) = factory {
// [STO-040]
// [STO-040] - UserOperation may not use an entity address (factory/paymaster/aggregator) that is used as an "account" in another UserOperation in the mempool
if helper.mempool.get_number_by_sender(&factory) > 0 {
return Err(SanityCheckError::EntityVerification {
entity: FACTORY.to_string(),
Expand All @@ -152,7 +153,7 @@ where
.verify_stake(FACTORY, Some(factory_stake))
.is_err()
{
// [UREP-020]
// [UREP-020] - for other entities
let entity = self.get_entity(&factory, helper)?;
let uos_allowed = Self::calculate_allowed_user_operations(entity);
if helper.mempool.get_number_by_entity(&factory) as u64 >= uos_allowed {
Expand All @@ -168,7 +169,7 @@ where

// paymaster
if let Some(paymaster) = paymaster {
// [STO-040]
// [STO-040] - UserOperation may not use an entity address (factory/paymaster/aggregator) that is used as an "account" in another UserOperation in the mempool
if helper.mempool.get_number_by_sender(&paymaster) > 0 {
return Err(SanityCheckError::EntityVerification {
entity: PAYMASTER.to_string(),
Expand All @@ -185,7 +186,7 @@ where
.verify_stake(PAYMASTER, Some(paymaster_stake))
.is_err()
{
// [UREP-020]
// [UREP-020] - for other entities
let entity = self.get_entity(&paymaster, helper)?;
let uos_allowed = Self::calculate_allowed_user_operations(entity);
if helper.mempool.get_number_by_entity(&paymaster) as u64 >= uos_allowed {
Expand Down
6 changes: 5 additions & 1 deletion crates/uopool/src/validate/simulation_trace/call_stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,16 +122,20 @@ where
self.parse_call_stack(helper.js_trace, &mut calls)?;

for call in calls.iter() {
// [OP-052] - may call depositTo(sender) with any value from either the sender or factory
// [OP-053] - may call the fallback function from the sender with any value
if call.to.unwrap_or_default() == helper.entry_point.address()
&& call.from.unwrap_or_default() != helper.entry_point.address()
&& (call.method.is_some()
&& call.method.clone().unwrap_or_default() != *"depositTo")
{
// [OP-054] - any other access to the EntryPoint is forbidden
return Err(SimulationCheckError::CallStack {
message: format!("Illegal call into entry point during validation {call:?}"),
});
}

// [OP-061] - CALL with value is forbidden. The only exception is a call to the EntryPoint described above
if call.to.unwrap_or_default() != helper.entry_point.address()
&& !call.value.unwrap_or_default().is_zero()
{
Expand All @@ -156,7 +160,7 @@ where
})?;
let context = validate_paymaster_return.context;

// [EREP-050]
// [EREP-050] - an unstaked paymaster may not return a context
// This will be removed in the future
if !context.is_empty()
&& helper
Expand Down
2 changes: 2 additions & 0 deletions crates/uopool/src/validate/simulation_trace/code_hashes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ where
uo: &UserOperation,
helper: &mut SimulationTraceHelper<M, P, R, E>,
) -> Result<(), SimulationCheckError> {
// [COD-010] - between the first and the second validations, the EXTCODEHASH value of any visited address, entity or referenced library, may not be changed

let addrs = helper
.js_trace
.calls_from_entry_point
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ where
.cloned();

if let Some(l) = level {
// [OP-041] - access to an address without a deployed code is forbidden for EXTCODE* and *CALL opcodes
for (addr, size) in call_info.contract_size.iter() {
if *addr != uo.sender
if *addr != uo.sender // [OP-042] - exception: access to "sender" address is allowed
&& size.contract_size <= 2
&& size.opcode != CREATE2_OPCODE.to_string()
{
Expand Down
1 change: 1 addition & 0 deletions crates/uopool/src/validate/simulation_trace/gas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ where
_uo: &UserOperation,
helper: &mut SimulationTraceHelper<M, P, R, E>,
) -> Result<(), SimulationCheckError> {
// [OP-020] - revert on "out of gas" is forbidden as it can "leak" the gas limit or the current call stack depth
for call_info in helper.js_trace.calls_from_entry_point.iter() {
if call_info.oog.unwrap_or(false) {
return Err(SimulationCheckError::OutOfGas {});
Expand Down
2 changes: 2 additions & 0 deletions crates/uopool/src/validate/simulation_trace/opcodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ where
.cloned();

if let Some(l) = level {
// [OP-011] - block opcodes
for op in call_info.opcodes.keys() {
if FORBIDDEN_OPCODES.contains(op) {
return Err(SimulationCheckError::Opcode {
Expand All @@ -47,6 +48,7 @@ where
}
}

// [OP-031] - CREATE2 is allowed exactly once in the deployment phase and must deploy code for the "sender" address
if let Some(c) = call_info.opcodes.get(&*CREATE2_OPCODE) {
if LEVEL_TO_ENTITY[l] == FACTORY && *c == 1 {
continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ where
let stake_info_l = stake_info[l];

for (addr, acc) in &call_info.access {
// [STO-010] - Access to the "account" storage is always allowed
if *addr == uo.sender || *addr == helper.entry_point.address() {
continue;
}
Expand All @@ -148,15 +149,17 @@ where
.concat()
{
if self.associated_with_slot(&uo.sender, &slot, &slots)? {
// [STO-021], [STO-022] - Access to associated storage of the account in an external (non-entity contract) is allowed if either The account already exists or There is an initCode and the factory contract is staked
if !(uo.init_code.is_empty()
|| uo.sender == stake_info_l.address
&& stake_info[FACTORY_LEVEL].is_staked())
{
slot_staked = slot.clone();
}
} else if *addr == stake_info_l.address
|| self.associated_with_slot(&stake_info_l.address, &slot, &slots)?
} else if *addr == stake_info_l.address // [STO-031] - access the entity's own storage (if entity staked)
|| self.associated_with_slot(&stake_info_l.address, &slot, &slots)? // [STO-032] - read/write Access to storage slots that is associated with the entity, in any non-entity contract (if entity staked)
|| !acc.writes.contains_key(&slot)
// [STO-033] - read-only access to any storage in non-entity contract (if entity staked)
{
slot_staked = slot.clone();
} else {
Expand Down
Loading