test: fix formely ignored sync handlers tests

eqlabs · Aug 10, 2023 · 5b64082 · 5b64082
1 parent d62bcc5
commit 5b64082
Show file tree

Hide file tree

Showing 5 changed files with 77 additions and 18 deletions.
diff --git a/crates/gateway-types/src/reply.rs b/crates/gateway-types/src/reply.rs
@@ -290,6 +290,10 @@ pub mod transaction {
 
     impl<T> Dummy<T> for Receipt {
         fn dummy_with_rng<R: rand::Rng + ?Sized>(_: &T, rng: &mut R) -> Self {
+            let execution_status = Faker.fake_with_rng(rng);
+            let revert_error =
+                (execution_status == ExecutionStatus::Reverted).then(|| Faker.fake_with_rng(rng));
+
             // Those fields that were missing in very old receipts are always present
             Self {
                 actual_fee: Some(Faker.fake_with_rng(rng)),
@@ -299,8 +303,8 @@ pub mod transaction {
                 l2_to_l1_messages: Faker.fake_with_rng(rng),
                 transaction_hash: Faker.fake_with_rng(rng),
                 transaction_index: Faker.fake_with_rng(rng),
-                execution_status: Faker.fake_with_rng(rng),
-                revert_error: Faker.fake_with_rng(rng),
+                execution_status,
+                revert_error,
             }
         }
     }

diff --git a/crates/p2p_proto/proto/common.proto b/crates/p2p_proto/proto/common.proto
@@ -118,6 +118,10 @@ message CommonTransactionReceiptProperties {
   // Optional
   MessageToL2 consumed_message = 6;
   ExecutionResources execution_resources = 7;
+  // FIXME: the following 2 fields are not yet in the spec
+  ExecutionStatus execution_status = 8;
+  // Empty if execution_status == SUCCEEDED
+  string revert_error = 9;
 }
 
 message ExecutionResources {
@@ -128,13 +132,22 @@ message ExecutionResources {
     uint64 output_builtin = 4;
     uint64 pedersen_builtin = 5;
     uint64 range_check_builtin = 6;
+    // FIXME: the following 3 fields are not yet in the spec
+    uint64 keccak_builtin = 7;
+    uint64 poseidon_builtin = 8;
+    uint64 segment_arena_builtin = 9;
   }
 
   BuiltinInstanceCounter builtin_instance_counter = 1;
   uint64 n_steps = 2;
   uint64 n_memory_holes = 3;
 }
 
+enum ExecutionStatus {
+  SUCCEEDED = 0;
+  REVERTED = 1;
+}
+
 message InvokeTransactionReceipt {
   CommonTransactionReceiptProperties common = 1;
 }

diff --git a/crates/p2p_proto/src/common.rs b/crates/p2p_proto/src/common.rs
@@ -218,6 +218,9 @@ pub mod execution_resources {
         pub output_builtin: u64,
         pub pedersen_builtin: u64,
         pub range_check_builtin: u64,
+        pub keccak_builtin: u64,
+        pub poseidon_builtin: u64,
+        pub segment_arena_builtin: u64,
     }
 }
 
@@ -232,6 +235,36 @@ pub struct CommonTransactionReceiptProperties {
     #[optional]
     pub consumed_message: Option<MessageToL2>,
     pub execution_resources: ExecutionResources,
+    pub execution_status: ExecutionStatus,
+    pub revert_error: String,
+}
+
+#[derive(Debug, Clone, PartialEq, Eq, Dummy)]
+pub enum ExecutionStatus {
+    Succeeded,
+    Reverted,
+}
+
+impl TryFromProtobuf<i32> for ExecutionStatus {
+    fn try_from_protobuf(input: i32, field_name: &'static str) -> Result<Self, std::io::Error> {
+        match input {
+            0 => Ok(ExecutionStatus::Succeeded),
+            1 => Ok(ExecutionStatus::Reverted),
+            _ => Err(std::io::Error::new(
+                std::io::ErrorKind::InvalidData,
+                format!("Failed to parse {field_name}: missing txn field"),
+            )),
+        }
+    }
+}
+
+impl ToProtobuf<proto::common::ExecutionStatus> for ExecutionStatus {
+    fn to_protobuf(self) -> proto::common::ExecutionStatus {
+        match self {
+            ExecutionStatus::Succeeded => proto::common::ExecutionStatus::Succeeded,
+            ExecutionStatus::Reverted => proto::common::ExecutionStatus::Reverted,
+        }
+    }
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, ToProtobuf, TryFromProtobuf, Dummy)]

diff --git a/crates/pathfinder/src/p2p_network/client.rs b/crates/pathfinder/src/p2p_network/client.rs
@@ -287,8 +287,8 @@ pub mod conv {
             use super::gw;
             use p2p_proto::common::{
                 DeclareTransactionReceipt, DeployAccountTransactionReceipt,
-                DeployTransactionReceipt, InvokeTransactionReceipt, L1HandlerTransactionReceipt,
-                Receipt,
+                DeployTransactionReceipt, ExecutionStatus, InvokeTransactionReceipt,
+                L1HandlerTransactionReceipt, Receipt,
             };
             use pathfinder_common::{
                 event::Event, ContractAddress, EntryPoint, EthereumAddress, EventData, EventKey,
@@ -331,10 +331,9 @@ pub mod conv {
                                         output_builtin: b.output_builtin,
                                         pedersen_builtin: b.pedersen_builtin,
                                         range_check_builtin: b.range_check_builtin,
-                                        // FIXME once p2p has these builtins.
-                                        keccak_builtin: Default::default(),
-                                        poseidon_builtin: Default::default(),
-                                        segment_arena_builtin: Default::default(),
+                                        keccak_builtin: b.keccak_builtin,
+                                        poseidon_builtin: b.poseidon_builtin,
+                                        segment_arena_builtin: b.segment_arena_builtin,
                                     }
                                 },
                                 n_steps: common.execution_resources.n_steps,
@@ -384,9 +383,12 @@ pub mod conv {
                                 common.transaction_index.into(),
                             )
                             .expect("u32::MAX is always smaller than i64::MAX"),
-                            // FIXME: once p2p supports reverted
-                            execution_status: Default::default(),
-                            revert_error: Default::default(),
+                            execution_status: match common.execution_status {
+                                ExecutionStatus::Succeeded => gw::ExecutionStatus::Succeeded,
+                                ExecutionStatus::Reverted => gw::ExecutionStatus::Reverted,
+                            },
+                            revert_error: (common.execution_status == ExecutionStatus::Reverted)
+                                .then_some(common.revert_error),
                         })
                     }
                 }

diff --git a/crates/pathfinder/src/p2p_network/sync_handlers.rs b/crates/pathfinder/src/p2p_network/sync_handlers.rs
@@ -252,8 +252,9 @@ mod conv {
             execution_resources::BuiltinInstanceCounter, invoke_transaction::EntryPoint,
             CommonTransactionReceiptProperties, DeclareTransaction, DeclareTransactionReceipt,
             DeployAccountTransaction, DeployAccountTransactionReceipt, DeployTransaction,
-            DeployTransactionReceipt, Event, ExecutionResources, InvokeTransaction,
-            InvokeTransactionReceipt, MessageToL1, MessageToL2, Receipt, Transaction,
+            DeployTransactionReceipt, Event, ExecutionResources, ExecutionStatus,
+            InvokeTransaction, InvokeTransactionReceipt, MessageToL1, MessageToL2, Receipt,
+            Transaction,
         };
         use pathfinder_common::{Fee, L1ToL2MessageNonce, TransactionNonce};
         use stark_hash::Felt;
@@ -304,11 +305,22 @@ mod conv {
                             output_builtin: b.output_builtin,
                             pedersen_builtin: b.pedersen_builtin,
                             range_check_builtin: b.range_check_builtin,
+                            keccak_builtin: b.keccak_builtin,
+                            poseidon_builtin: b.poseidon_builtin,
+                            segment_arena_builtin: b.segment_arena_builtin,
                         },
                         n_steps: x.n_steps,
                         n_memory_holes: x.n_memory_holes,
                     }
                 },
+                execution_status: match gw_r.execution_status {
+                    gw::ExecutionStatus::Succeeded => ExecutionStatus::Succeeded,
+                    gw::ExecutionStatus::Reverted => ExecutionStatus::Reverted,
+                },
+                revert_error: match gw_r.execution_status {
+                    gw::ExecutionStatus::Succeeded => Default::default(),
+                    gw::ExecutionStatus::Reverted => gw_r.revert_error.unwrap_or_default(),
+                },
             };
 
             let version = Felt::from_be_slice(gw_t.version().0.as_bytes())
@@ -854,9 +866,7 @@ mod tests {
             }
 
             proptest! {
-                // FIXME: unignore once reverted is supported
                 #[test]
-                #[ignore]
                 fn forward((start, count, seed, num_blocks) in super::strategy::forward()) {
                     let (storage, from_db) = storage_with_seed(seed, num_blocks);
 
@@ -890,12 +900,9 @@ mod tests {
                     prop_assert_eq!(from_p2p, from_db)
                 }
 
-                // FIXME: unignore once reverted is supported
                 #[test]
-                #[ignore]
                 fn backward((start, count, seed, num_blocks) in super::strategy::backward()) {
                     let (storage, from_db) = storage_with_seed(seed, num_blocks);
-
                     let start_hash = match from_db.get(usize::try_from(start).unwrap()).map(|x| x.0.hash) {
                         Some(h) => h,
                         None => {