Skip to content

Commit

Permalink
refactor: group optimism invalid txn errors (#1604)
Browse files Browse the repository at this point in the history 
* refactor: group optimism invalid txn errors

* Update crates/primitives/src/result.rs
  • Loading branch information
@hack3r-0m
hack3r-0m authored Jul 11, 2024
1 parent 6e082a8 commit 9e18cb1
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 52 deletions.
102 changes: 58 additions & 44 deletions crates/primitives/src/result.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,45 @@ impl<DBError> From<InvalidHeader> for EVMError<DBError> {
}
}

/// Transaction validation error for Optimism.
#[cfg(feature = "optimism")]
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub enum OptimismInvalidTransaction {
/// System transactions are not supported post-regolith hardfork.
///
/// Before the Regolith hardfork, there was a special field in the `Deposit` transaction
/// type that differentiated between `system` and `user` deposit transactions. This field
/// was deprecated in the Regolith hardfork, and this error is thrown if a `Deposit` transaction
/// is found with this field set to `true` after the hardfork activation.
///
/// In addition, this error is internal, and bubbles up into a [HaltReason::FailedDeposit] error
/// in the `revm` handler for the consumer to easily handle. This is due to a state transition
/// rule on OP Stack chains where, if for any reason a deposit transaction fails, the transaction
/// must still be included in the block, the sender nonce is bumped, the `mint` value persists, and
/// special gas accounting rules are applied. Normally on L1, [EVMError::Transaction] errors
/// are cause for non-inclusion, so a special [HaltReason] variant was introduced to handle this
/// case for failed deposit transactions.
#[cfg(feature = "optimism")]
DepositSystemTxPostRegolith,
/// Deposit transaction haults bubble up to the global main return handler, wiping state and
/// only increasing the nonce + persisting the mint value.
///
/// This is a catch-all error for any deposit transaction that is results in a [HaltReason] error
/// post-regolith hardfork. This allows for a consumer to easily handle special cases where
/// a deposit transaction fails during validation, but must still be included in the block.
///
/// In addition, this error is internal, and bubbles up into a [HaltReason::FailedDeposit] error
/// in the `revm` handler for the consumer to easily handle. This is due to a state transition
/// rule on OP Stack chains where, if for any reason a deposit transaction fails, the transaction
/// must still be included in the block, the sender nonce is bumped, the `mint` value persists, and
/// special gas accounting rules are applied. Normally on L1, [EVMError::Transaction] errors
/// are cause for non-inclusion, so a special [HaltReason] variant was introduced to handle this
/// case for failed deposit transactions.
#[cfg(feature = "optimism")]
HaltedDepositPostRegolith,
}

/// Transaction validation error.
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
Expand Down Expand Up @@ -272,43 +311,30 @@ pub enum InvalidTransaction {
AuthorizationListNotSupported,
/// EIP-7702 transaction has invalid fields set.
AuthorizationListInvalidFields,
/// System transactions are not supported post-regolith hardfork.
///
/// Before the Regolith hardfork, there was a special field in the `Deposit` transaction
/// type that differentiated between `system` and `user` deposit transactions. This field
/// was deprecated in the Regolith hardfork, and this error is thrown if a `Deposit` transaction
/// is found with this field set to `true` after the hardfork activation.
///
/// In addition, this error is internal, and bubbles up into a [HaltReason::FailedDeposit] error
/// in the `revm` handler for the consumer to easily handle. This is due to a state transition
/// rule on OP Stack chains where, if for any reason a deposit transaction fails, the transaction
/// must still be included in the block, the sender nonce is bumped, the `mint` value persists, and
/// special gas accounting rules are applied. Normally on L1, [EVMError::Transaction] errors
/// are cause for non-inclusion, so a special [HaltReason] variant was introduced to handle this
/// case for failed deposit transactions.
#[cfg(feature = "optimism")]
DepositSystemTxPostRegolith,
/// Deposit transaction haults bubble up to the global main return handler, wiping state and
/// only increasing the nonce + persisting the mint value.
///
/// This is a catch-all error for any deposit transaction that is results in a [HaltReason] error
/// post-regolith hardfork. This allows for a consumer to easily handle special cases where
/// a deposit transaction fails during validation, but must still be included in the block.
///
/// In addition, this error is internal, and bubbles up into a [HaltReason::FailedDeposit] error
/// in the `revm` handler for the consumer to easily handle. This is due to a state transition
/// rule on OP Stack chains where, if for any reason a deposit transaction fails, the transaction
/// must still be included in the block, the sender nonce is bumped, the `mint` value persists, and
/// special gas accounting rules are applied. Normally on L1, [EVMError::Transaction] errors
/// are cause for non-inclusion, so a special [HaltReason] variant was introduced to handle this
/// case for failed deposit transactions.
/// Optimism-specific transaction validation error.
#[cfg(feature = "optimism")]
HaltedDepositPostRegolith,
OptimismError(OptimismInvalidTransaction),
}

#[cfg(feature = "std")]
impl std::error::Error for InvalidTransaction {}

#[cfg(feature = "optimism")]
impl fmt::Display for OptimismInvalidTransaction {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::DepositSystemTxPostRegolith => write!(
f,
"deposit system transactions post regolith hardfork are not supported"
),
Self::HaltedDepositPostRegolith => write!(
f,
"deposit transaction halted post-regolith; error will be bubbled up to main return handler"
),
}
}
}

impl fmt::Display for InvalidTransaction {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Expand Down Expand Up @@ -368,19 +394,7 @@ impl fmt::Display for InvalidTransaction {
write!(f, "authorization list tx has invalid fields")
}
#[cfg(feature = "optimism")]
Self::DepositSystemTxPostRegolith => {
write!(
f,
"deposit system transactions post regolith hardfork are not supported"
)
}
#[cfg(feature = "optimism")]
Self::HaltedDepositPostRegolith => {
write!(
f,
"deposit transaction halted post-regolith; error will be bubbled up to main return handler"
)
}
Self::OptimismError(op_error) => op_error.fmt(f),
}
}
}
Expand Down
20 changes: 12 additions & 8 deletions crates/revm/src/optimism/handler_register.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ use crate::{
optimism,
primitives::{
db::Database, spec_to_generic, Account, EVMError, Env, ExecutionResult, HaltReason,
HashMap, InvalidTransaction, ResultAndState, Spec, SpecId, SpecId::REGOLITH, U256,
HashMap, InvalidTransaction, OptimismInvalidTransaction, ResultAndState, Spec, SpecId,
SpecId::REGOLITH, U256,
},
Context, ContextPrecompiles, FrameResult,
};
Expand Down Expand Up @@ -51,7 +52,10 @@ pub fn validate_env<SPEC: Spec, DB: Database>(env: &Env) -> Result<(), EVMError<
// Do not allow for a system transaction to be processed if Regolith is enabled.
let tx = &env.tx.optimism;
if tx.is_system_transaction.unwrap_or(false) && SPEC::enabled(SpecId::REGOLITH) {
return Err(InvalidTransaction::DepositSystemTxPostRegolith.into());
return Err(InvalidTransaction::OptimismError(
OptimismInvalidTransaction::DepositSystemTxPostRegolith,
)
.into());
}

env.validate_tx::<SPEC>()?;
Expand Down Expand Up @@ -298,9 +302,9 @@ pub fn output<SPEC: Spec, EXT, DB: Database>(
// and the caller nonce will be incremented there.
let is_deposit = context.evm.inner.env.tx.optimism.source_hash.is_some();
if is_deposit && SPEC::enabled(REGOLITH) {
return Err(EVMError::Transaction(
InvalidTransaction::HaltedDepositPostRegolith,
));
return Err(EVMError::Transaction(InvalidTransaction::OptimismError(
OptimismInvalidTransaction::HaltedDepositPostRegolith,
)));
}
}
Ok(result)
Expand Down Expand Up @@ -611,9 +615,9 @@ mod tests {
env.tx.optimism.is_system_transaction = Some(true);
assert_eq!(
validate_env::<RegolithSpec, EmptyDB>(&env),
Err(EVMError::Transaction(
InvalidTransaction::DepositSystemTxPostRegolith
))
Err(EVMError::Transaction(InvalidTransaction::OptimismError(
OptimismInvalidTransaction::DepositSystemTxPostRegolith
)))
);

// Pre-regolith system transactions should be allowed.
Expand Down

0 comments on commit 9e18cb1

Please sign in to comment.