Skip to content

Commit

Permalink
chore(refactor): Propagate fatal error (#1116)
Browse files Browse the repository at this point in the history 
* chore: clippy and removal of +nightly in ci

* use std

* Use stable rust for clippy

* fmt and stable rust

* rm nightly items

* temp

* chore(refactor): Propagate fatal error

* rm must_use as Return enforce it

* cleanup
  • Loading branch information
@rakita
rakita committed Feb 21, 2024
1 parent d60fc87 commit 2d4fb06
Show file tree
Hide file tree
Showing 11 changed files with 300 additions and 233 deletions.
4 changes: 2 additions & 2 deletions crates/interpreter/src/interpreter.rs
View file
Expand Up @@ -179,7 +179,7 @@ impl Interpreter {
self.gas.erase_cost(create_outcome.gas().remaining());
}
InstructionResult::FatalExternalError => {
self.instruction_result = InstructionResult::FatalExternalError;
panic!("Fatal external error in insert_create_outcome");
}
_ => {
push!(self, U256::ZERO);
Expand Down Expand Up @@ -237,7 +237,7 @@ impl Interpreter {
push!(self, U256::ZERO);
}
InstructionResult::FatalExternalError => {
self.instruction_result = InstructionResult::FatalExternalError;
panic!("Fatal external error in insert_call_outcome");
}
_ => {
push!(self, U256::ZERO);
Expand Down
156 changes: 64 additions & 92 deletions crates/revm/src/context.rs
View file
Expand Up @@ -94,7 +94,7 @@ pub struct EvmContext<DB: Database> {
/// Database to load data from.
pub db: DB,
/// Error that happened during execution.
pub error: Option<DB::Error>,
pub error: Result<(), EVMError<DB::Error>>,
/// Precompiles that are available for evm.
pub precompiles: Precompiles,
/// Used as temporary value holder to store L1 block info.
Expand Down Expand Up @@ -125,7 +125,7 @@ impl<DB: Database> EvmContext<DB> {
env: self.env,
journaled_state: self.journaled_state,
db,
error: None,
error: Ok(()),
precompiles: self.precompiles,
#[cfg(feature = "optimism")]
l1_block_info: self.l1_block_info,
Expand All @@ -137,7 +137,7 @@ impl<DB: Database> EvmContext<DB> {
env: Box::default(),
journaled_state: JournaledState::new(SpecId::LATEST, HashSet::new()),
db,
error: None,
error: Ok(()),
precompiles: Precompiles::default(),
#[cfg(feature = "optimism")]
l1_block_info: None,
Expand All @@ -150,7 +150,7 @@ impl<DB: Database> EvmContext<DB> {
env,
journaled_state: JournaledState::new(SpecId::LATEST, HashSet::new()),
db,
error: None,
error: Ok(()),
precompiles: Precompiles::default(),
#[cfg(feature = "optimism")]
l1_block_info: None,
Expand All @@ -177,8 +177,7 @@ impl<DB: Database> EvmContext<DB> {
pub fn load_access_list(&mut self) -> Result<(), EVMError<DB::Error>> {
for (address, slots) in self.env.tx.access_list.iter() {
self.journaled_state
.initial_account_load(*address, slots, &mut self.db)
.map_err(EVMError::Database)?;
.initial_account_load(*address, slots, &mut self.db)?;
}
Ok(())
}
Expand All @@ -191,82 +190,64 @@ impl<DB: Database> EvmContext<DB> {

/// Fetch block hash from database.
#[inline]
#[must_use]
pub fn block_hash(&mut self, number: U256) -> Option<B256> {
self.db
.block_hash(number)
.map_err(|e| self.error = Some(e))
.ok()
pub fn block_hash(&mut self, number: U256) -> Result<B256, EVMError<DB::Error>> {
self.db.block_hash(number).map_err(EVMError::Database)
}

/// Load account and return flags (is_cold, exists)
#[inline]
#[must_use]
pub fn load_account(&mut self, address: Address) -> Option<(bool, bool)> {
pub fn load_account(&mut self, address: Address) -> Result<(bool, bool), EVMError<DB::Error>> {
self.journaled_state
.load_account_exist(address, &mut self.db)
.map_err(|e| self.error = Some(e))
.ok()
}

/// Return account balance and is_cold flag.
#[inline]
#[must_use]
pub fn balance(&mut self, address: Address) -> Option<(U256, bool)> {
pub fn balance(&mut self, address: Address) -> Result<(U256, bool), EVMError<DB::Error>> {
self.journaled_state
.load_account(address, &mut self.db)
.map_err(|e| self.error = Some(e))
.ok()
.map(|(acc, is_cold)| (acc.info.balance, is_cold))
}

/// Return account code and if address is cold loaded.
#[inline]
#[must_use]
pub fn code(&mut self, address: Address) -> Option<(Bytecode, bool)> {
let (acc, is_cold) = self
.journaled_state
pub fn code(&mut self, address: Address) -> Result<(Bytecode, bool), EVMError<DB::Error>> {
self.journaled_state
.load_code(address, &mut self.db)
.map_err(|e| self.error = Some(e))
.ok()?;
Some((acc.info.code.clone().unwrap(), is_cold))
.map(|(a, is_cold)| (a.info.code.clone().unwrap(), is_cold))
}

/// Get code hash of address.
#[inline]
#[must_use]
pub fn code_hash(&mut self, address: Address) -> Option<(B256, bool)> {
let (acc, is_cold) = self
.journaled_state
.load_code(address, &mut self.db)
.map_err(|e| self.error = Some(e))
.ok()?;
pub fn code_hash(&mut self, address: Address) -> Result<(B256, bool), EVMError<DB::Error>> {
let (acc, is_cold) = self.journaled_state.load_code(address, &mut self.db)?;
if acc.is_empty() {
return Some((B256::ZERO, is_cold));
return Ok((B256::ZERO, is_cold));
}

Some((acc.info.code_hash, is_cold))
Ok((acc.info.code_hash, is_cold))
}

/// Load storage slot, if storage is not present inside the account then it will be loaded from database.
#[inline]
#[must_use]
pub fn sload(&mut self, address: Address, index: U256) -> Option<(U256, bool)> {
pub fn sload(
&mut self,
address: Address,
index: U256,
) -> Result<(U256, bool), EVMError<DB::Error>> {
// account is always warm. reference on that statement https://eips.ethereum.org/EIPS/eip-2929 see `Note 2:`
self.journaled_state
.sload(address, index, &mut self.db)
.map_err(|e| self.error = Some(e))
.ok()
self.journaled_state.sload(address, index, &mut self.db)
}

/// Storage change of storage slot, before storing `sload` will be called for that slot.
#[inline]
#[must_use]
pub fn sstore(&mut self, address: Address, index: U256, value: U256) -> Option<SStoreResult> {
pub fn sstore(
&mut self,
address: Address,
index: U256,
value: U256,
) -> Result<SStoreResult, EVMError<DB::Error>> {
self.journaled_state
.sstore(address, index, value, &mut self.db)
.map_err(|e| self.error = Some(e))
.ok()
}

/// Returns transient storage value.
Expand All @@ -283,20 +264,23 @@ impl<DB: Database> EvmContext<DB> {

/// Make create frame.
#[inline]
#[must_use]
pub fn make_create_frame(&mut self, spec_id: SpecId, inputs: &CreateInputs) -> FrameOrResult {
pub fn make_create_frame(
&mut self,
spec_id: SpecId,
inputs: &CreateInputs,
) -> Result<FrameOrResult, EVMError<DB::Error>> {
// Prepare crate.
let gas = Gas::new(inputs.gas_limit);

let return_error = |e| {
FrameOrResult::new_create_result(
Ok(FrameOrResult::new_create_result(
InterpreterResult {
result: e,
gas,
output: Bytes::new(),
},
None,
)
))
};

// Check depth
Expand All @@ -305,9 +289,7 @@ impl<DB: Database> EvmContext<DB> {
}

// Fetch balance of caller.
let Some((caller_balance, _)) = self.balance(inputs.caller) else {
return return_error(InstructionResult::FatalExternalError);
};
let (caller_balance, _) = self.balance(inputs.caller)?;

// Check if caller has enough balance to send to the created contract.
if caller_balance < inputs.value {
Expand All @@ -333,14 +315,8 @@ impl<DB: Database> EvmContext<DB> {
};

// Load account so it needs to be marked as warm for access list.
if self
.journaled_state
.load_account(created_address, &mut self.db)
.map_err(|e| self.error = Some(e))
.is_err()
{
return return_error(InstructionResult::FatalExternalError);
}
self.journaled_state
.load_account(created_address, &mut self.db)?;

// create account, transfer funds and make the journal checkpoint.
let checkpoint = match self.journaled_state.create_account_checkpoint(
Expand All @@ -366,45 +342,40 @@ impl<DB: Database> EvmContext<DB> {
inputs.value,
));

FrameOrResult::new_create_frame(
Ok(FrameOrResult::new_create_frame(
created_address,
checkpoint,
Interpreter::new(contract, gas.limit(), false),
)
))
}

/// Make call frame
#[inline]
#[must_use]
pub fn make_call_frame(&mut self, inputs: &CallInputs) -> FrameOrResult {
pub fn make_call_frame(
&mut self,
inputs: &CallInputs,
) -> Result<FrameOrResult, EVMError<DB::Error>> {
let gas = Gas::new(inputs.gas_limit);

let return_result = |instruction_result: InstructionResult| {
FrameOrResult::new_call_result(
Ok(FrameOrResult::new_call_result(
InterpreterResult {
result: instruction_result,
gas,
output: Bytes::new(),
},
inputs.return_memory_offset.clone(),
)
))
};

// Check depth
if self.journaled_state.depth() > CALL_STACK_LIMIT {
return return_result(InstructionResult::CallTooDeep);
}

let account = match self
let (account, _) = self
.journaled_state
.load_code(inputs.contract, &mut self.db)
{
Ok((account, _)) => account,
Err(e) => {
self.error = Some(e);
return return_result(InstructionResult::FatalExternalError);
}
};
.load_code(inputs.contract, &mut self.db)?;
let code_hash = account.info.code_hash();
let bytecode = account.info.code.clone().unwrap_or_default();

Expand All @@ -413,21 +384,19 @@ impl<DB: Database> EvmContext<DB> {

// Touch address. For "EIP-158 State Clear", this will erase empty accounts.
if inputs.transfer.value == U256::ZERO {
if self.load_account(inputs.context.address).is_none() {
return return_result(InstructionResult::FatalExternalError);
};
self.load_account(inputs.context.address)?;
self.journaled_state.touch(&inputs.context.address);
}

// Transfer value from caller to called account
if let Err(e) = self.journaled_state.transfer(
if let Some(result) = self.journaled_state.transfer(
&inputs.transfer.source,
&inputs.transfer.target,
inputs.transfer.value,
&mut self.db,
) {
)? {
self.journaled_state.checkpoint_revert(checkpoint);
return return_result(e);
return return_result(result);
}

if let Some(precompile) = self.precompiles.get(&inputs.contract) {
Expand All @@ -437,7 +406,10 @@ impl<DB: Database> EvmContext<DB> {
} else {
self.journaled_state.checkpoint_revert(checkpoint);
}
FrameOrResult::new_call_result(result, inputs.return_memory_offset.clone())
Ok(FrameOrResult::new_call_result(
result,
inputs.return_memory_offset.clone(),
))
} else if !bytecode.is_empty() {
let contract = Box::new(Contract::new_with_context(
inputs.input.clone(),
Expand All @@ -446,11 +418,11 @@ impl<DB: Database> EvmContext<DB> {
&inputs.context,
));
// Create interpreter and executes call and push new CallStackFrame.
FrameOrResult::new_call_frame(
Ok(FrameOrResult::new_call_frame(
inputs.return_memory_offset.clone(),
checkpoint,
Interpreter::new(contract, gas.limit(), inputs.is_static),
)
))
} else {
self.journaled_state.checkpoint_commit();
return_result(InstructionResult::Stop)
Expand Down Expand Up @@ -650,7 +622,7 @@ pub(crate) mod test_utils {
env,
journaled_state: JournaledState::new(SpecId::CANCUN, HashSet::new()),
db,
error: None,
error: Ok(()),
precompiles: Precompiles::default(),
#[cfg(feature = "optimism")]
l1_block_info: None,
Expand All @@ -663,7 +635,7 @@ pub(crate) mod test_utils {
env,
journaled_state: JournaledState::new(SpecId::CANCUN, HashSet::new()),
db,
error: None,
error: Ok(()),
precompiles: Precompiles::default(),
#[cfg(feature = "optimism")]
l1_block_info: None,
Expand All @@ -690,7 +662,7 @@ mod tests {
let contract = address!("dead10000000000000000000000000000001dead");
let call_inputs = test_utils::create_mock_call_inputs(contract);
let res = evm_context.make_call_frame(&call_inputs);
let FrameOrResult::Result(err) = res else {
let Ok(FrameOrResult::Result(err)) = res else {
panic!("Expected FrameOrResult::Result");
};
assert_eq!(
Expand All @@ -711,7 +683,7 @@ mod tests {
let mut call_inputs = test_utils::create_mock_call_inputs(contract);
call_inputs.transfer.value = U256::from(1);
let res = evm_context.make_call_frame(&call_inputs);
let FrameOrResult::Result(result) = res else {
let Ok(FrameOrResult::Result(result)) = res else {
panic!("Expected FrameOrResult::Result");
};
assert_eq!(
Expand All @@ -732,7 +704,7 @@ mod tests {
let contract = address!("dead10000000000000000000000000000001dead");
let call_inputs = test_utils::create_mock_call_inputs(contract);
let res = evm_context.make_call_frame(&call_inputs);
let FrameOrResult::Result(result) = res else {
let Ok(FrameOrResult::Result(result)) = res else {
panic!("Expected FrameOrResult::Result");
};
assert_eq!(result.interpreter_result().result, InstructionResult::Stop);
Expand All @@ -757,7 +729,7 @@ mod tests {
let mut evm_context = create_cache_db_evm_context_with_balance(Box::new(env), cdb, bal);
let call_inputs = test_utils::create_mock_call_inputs(contract);
let res = evm_context.make_call_frame(&call_inputs);
let FrameOrResult::Frame(Frame::Call(call_frame)) = res else {
let Ok(FrameOrResult::Frame(Frame::Call(call_frame))) = res else {
panic!("Expected FrameOrResult::Frame(Frame::Call(..))");
};
assert_eq!(call_frame.return_memory_range, 0..0,);
Expand Down

0 comments on commit 2d4fb06

Please sign in to comment.