From a29ff9ffd8b37091439d40e8a3ddfe9b15ce469e Mon Sep 17 00:00:00 2001
From: dancoombs <dan.coombs@alchemy.com>
Date: Sat, 22 Jul 2023 12:43:02 -0400
Subject: [PATCH] fix(rpc): return signature error before sim violations

---
 src/builder/bundle_proposer.rs | 19 ++++++++++---------
 src/common/simulation.rs       |  9 ++++++---
 src/rpc/eth/mod.rs             |  6 ++----
 3 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/src/builder/bundle_proposer.rs b/src/builder/bundle_proposer.rs
index 2d010a763..5317af922 100644
--- a/src/builder/bundle_proposer.rs
+++ b/src/builder/bundle_proposer.rs
@@ -207,10 +207,9 @@ where
             Ok(success) => Ok((
                 op.op,
                 Some(success).filter(|success| {
-                    !success.signature_failed
-                        && success
-                            .valid_time_range
-                            .contains(Timestamp::now(), TIME_RANGE_BUFFER)
+                    success
+                        .valid_time_range
+                        .contains(Timestamp::now(), TIME_RANGE_BUFFER)
                 }),
             )),
             Err(error) => match error {
@@ -671,7 +670,10 @@ mod tests {
     use crate::common::{
         grpc::mocks::{self, MockOpPool},
         protos::op_pool::GetOpsResponse,
-        simulation::{AggregatorSimOut, MockSimulator, SimulationError, SimulationSuccess},
+        simulation::{
+            AggregatorSimOut, MockSimulator, SimulationError, SimulationSuccess,
+            SimulationViolation,
+        },
         types::{MockEntryPointLike, MockProviderLike, ValidTimeRange},
     };
 
@@ -738,10 +740,9 @@ mod tests {
         let bundle = simple_make_bundle(vec![MockOp {
             op: op.clone(),
             simulation_result: Box::new(|| {
-                Ok(SimulationSuccess {
-                    signature_failed: true,
-                    ..Default::default()
-                })
+                Err(SimulationError::Violations(vec![
+                    SimulationViolation::InvalidSignature,
+                ]))
             }),
         }])
         .await;
diff --git a/src/common/simulation.rs b/src/common/simulation.rs
index ca1157019..2d37e0a90 100644
--- a/src/common/simulation.rs
+++ b/src/common/simulation.rs
@@ -40,7 +40,6 @@ pub struct SimulationSuccess {
     pub mempools: Vec<H256>,
     pub block_hash: H256,
     pub pre_op_gas: U256,
-    pub signature_failed: bool,
     pub valid_time_range: ValidTimeRange,
     pub aggregator: Option<AggregatorSimOut>,
     pub code_hash: H256,
@@ -228,6 +227,10 @@ where
 
         let mut violations = vec![];
 
+        if entry_point_out.return_info.sig_failed {
+            violations.push(SimulationViolation::InvalidSignature);
+        }
+
         let sender_address = entity_infos.sender_address();
 
         for (index, phase) in tracer_out.phases.iter().enumerate().take(3) {
@@ -454,7 +457,6 @@ where
         let account_is_staked = is_staked(sender_info, self.sim_settings);
         let ValidationReturnInfo {
             pre_op_gas,
-            sig_failed,
             valid_after,
             valid_until,
             ..
@@ -463,7 +465,6 @@ where
             mempools,
             block_hash,
             pre_op_gas,
-            signature_failed: sig_failed,
             valid_time_range: ValidTimeRange::new(valid_after, valid_until),
             aggregator,
             code_hash,
@@ -479,6 +480,8 @@ where
 pub enum SimulationViolation {
     // Make sure to maintain the order here based on the importance
     // of the violation for converting to an JRPC error
+    #[display("invalid signature")]
+    InvalidSignature,
     #[display("reverted while simulating {0} validation: {1}")]
     UnintendedRevertWithMessage(EntityType, String, Option<Address>),
     #[display("{0.kind} uses banned opcode: {2} in contract {1:?}")]
diff --git a/src/rpc/eth/mod.rs b/src/rpc/eth/mod.rs
index 4803e1716..572140a99 100644
--- a/src/rpc/eth/mod.rs
+++ b/src/rpc/eth/mod.rs
@@ -385,7 +385,6 @@ where
             valid_time_range,
             code_hash,
             aggregator,
-            signature_failed,
             entities_needing_stake,
             block_hash,
             account_is_staked,
@@ -395,9 +394,7 @@ where
             .await
             .map_err(EthRpcError::from)?;
 
-        if signature_failed {
-            Err(EthRpcError::SignatureCheckFailed)?
-        } else if let Some(agg) = &aggregator {
+        if let Some(agg) = &aggregator {
             // TODO(danc): all aggregators are currently unsupported
             Err(EthRpcError::UnsupportedAggregator(
                 UnsupportedAggregatorData {
@@ -633,6 +630,7 @@ impl From<SimulationError> for EthRpcError {
         };
 
         match violation {
+            SimulationViolation::InvalidSignature => Self::SignatureCheckFailed,
             SimulationViolation::UnintendedRevertWithMessage(
                 EntityType::Paymaster,
                 reason,