fix get_sorted_events issue (#1024)

* add failing test that reproduce the issue

* fix the bug

* fix test since now 2 events with the same order are ok

* handle multiple events

* fix comments

* cargo fmt

Author: juanbono

rpc_state_reader/tests/sir_tests.rs

@@ -262,6 +262,7 @@ fn starknet_in_rust_test_case_tx(hash: &str, block_number: u64, chain: RpcChain)
         ..
     } = call_info.unwrap();
 
+    // check Cairo VM execution resources
     assert_eq_sorted!(
         execution_resources,
         trace
@@ -271,6 +272,8 @@ fn starknet_in_rust_test_case_tx(hash: &str, block_number: u64, chain: RpcChain)
             .execution_resources,
         "execution resources mismatch"
     );
+
+    // check amount of internal calls
     assert_eq!(
         internal_calls.len(),
         trace
@@ -282,6 +285,7 @@ fn starknet_in_rust_test_case_tx(hash: &str, block_number: u64, chain: RpcChain)
         "internal calls length mismatch"
     );
 
+    // check actual fee calculation
     if receipt.actual_fee != actual_fee {
         let diff = 100 * receipt.actual_fee.abs_diff(actual_fee) / receipt.actual_fee;
 
@@ -294,6 +298,37 @@ fn starknet_in_rust_test_case_tx(hash: &str, block_number: u64, chain: RpcChain)
     }
 }
 
+#[test_case(
+    "0x01e91fa12be4424264c8cad29f481a67d5d8e23f7abf94add734d64b91c90021",
+    RpcChain::MainNet,
+    219797,
+    7
+)]
+#[test_case(
+    "0x03ec45f8369513b0f48db25f2cf18c70c50e7d3119505ab15e39ae4ca2eb06cf",
+    RpcChain::MainNet,
+    219764,
+    7
+)]
+#[test_case(
+    "0x00164bfc80755f62de97ae7c98c9d67c1767259427bcf4ccfcc9683d44d54676",
+    RpcChain::MainNet,
+    197000,
+    3
+)]
+fn test_sorted_events(
+    tx_hash: &str,
+    chain: RpcChain,
+    block_number: u64,
+    expected_amount_of_events: usize,
+) {
+    let (tx_info, _trace, _receipt) = execute_tx(tx_hash, chain, BlockNumber(block_number));
+
+    let events_len = tx_info.get_sorted_events().unwrap().len();
+
+    assert_eq!(expected_amount_of_events, events_len);
+}
+
 #[test_case(
     "0x00b6d59c19d5178886b4c939656167db0660fe325345138025a3cc4175b21897",
     200303, // real block     200304

src/execution/mod.rs

@@ -104,48 +104,64 @@ impl CallInfo {
         )
     }
 
-    /// Yields the contract calls in DFS (preorder).
+    /// Returns the contract calls in DFS (preorder).
     pub fn gen_call_topology(&self) -> Vec<CallInfo> {
         let mut calls = Vec::new();
-        if self.internal_calls.is_empty() {
-            calls.push(self.clone())
-        } else {
-            calls.push(self.clone());
-            for call_info in self.internal_calls.clone() {
-                calls.extend(call_info.gen_call_topology());
+        // add the current call
+        calls.push(self.clone());
+
+        // if it has internal calls we need to add them too.
+        if !self.internal_calls.is_empty() {
+            for inner_call in self.internal_calls.clone() {
+                calls.extend(inner_call.gen_call_topology());
             }
         }
+
         calls
     }
 
-    /// Returns a list of Starknet Event objects collected during the execution, sorted by the order
+    /// Returns a list of [`Event`] objects collected during the execution, sorted by the order
     /// in which they were emitted.
     pub fn get_sorted_events(&self) -> Result<Vec<Event>, TransactionError> {
+        // collect a vector of the full call topology (all the internal
+        // calls performed during the current call)
         let calls = self.gen_call_topology();
-        let n_events = calls.iter().fold(0, |acc, c| acc + c.events.len());
-
-        let mut starknet_events: Vec<Option<Event>> = (0..n_events).map(|_| None).collect();
-
-        for call in calls {
-            for ordered_event in call.events {
-                let event = Event::new(ordered_event.clone(), call.contract_address.clone());
-                starknet_events.remove((ordered_event.order as isize - 1).max(0) as usize);
-                starknet_events.insert(
-                    (ordered_event.order as isize - 1).max(0) as usize,
-                    Some(event),
-                );
+        let mut collected_events = Vec::new();
+
+        // for each call, collect its ordered events
+        for c in calls {
+            collected_events.extend(
+                c.events
+                    .iter()
+                    .map(|oe| (oe.clone(), c.contract_address.clone())),
+            );
+        }
+        // sort the collected events using the ordering given by the order
+        collected_events.sort_by_key(|(oe, _)| oe.order);
+
+        // check that there is no holes.
+        // since it is already sorted, we only need to check for continuity
+        let mut i = 0;
+        for (oe, _) in collected_events.iter() {
+            if i == oe.order {
+                continue;
+            }
+            i += 1;
+            if i != oe.order {
+                return Err(TransactionError::UnexpectedHolesInEventOrder);
             }
         }
 
-        let are_all_some = starknet_events.iter().all(|e| e.is_some());
-
-        if !are_all_some {
-            return Err(TransactionError::UnexpectedHolesInEventOrder);
-        }
-        Ok(starknet_events.into_iter().flatten().collect())
+        // now that it is ordered and without holes, we can discard the order and
+        // convert each [`OrderedEvent`] to the underlying [`Event`].
+        let collected_events = collected_events
+            .into_iter()
+            .map(|(oe, ca)| Event::new(oe, ca))
+            .collect();
+        Ok(collected_events)
     }
 
-    /// Returns a list of Starknet L2ToL1MessageInfo objects collected during the execution, sorted
+    /// Returns a list of L2ToL1MessageInfo objects collected during the execution, sorted
     /// by the order in which they were sent.
     pub fn get_sorted_l2_to_l1_messages(&self) -> Result<Vec<L2toL1MessageInfo>, TransactionError> {
         let calls = self.gen_call_topology();
@@ -586,6 +602,8 @@ impl TransactionExecutionInfo {
         })
     }
 
+    /// Returns an ordered vector with all the event emitted during the transaction.
+    /// Including the ones emitted by internal calls.
     pub fn get_sorted_events(&self) -> Result<Vec<Event>, TransactionError> {
         let calls = self.non_optional_calls();
         let mut sorted_events: Vec<Event> = Vec::new();
@@ -838,7 +856,7 @@ mod tests {
 
         call_root.internal_calls = [child1, child2].to_vec();
 
-        assert!(call_root.get_sorted_events().is_err())
+        assert!(call_root.get_sorted_events().is_ok())
     }
 
     #[test]