additional safety for multi request logic

Signed-off-by: Uncle Stinky <uncle.stinky@ghostchain.io>

pallets/slow-clap/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "ghost-slow-clap"
-version = "0.4.13"
+version = "0.4.14"
 description = "Applause protocol for the EVM bridge"
 license.workspace = true
 authors.workspace = true

pallets/slow-clap/src/lib.rs

@@ -166,6 +166,9 @@ enum OffchainErr<NetworkId> {
     EmptyResponses,
     NotValidator,
     DifferentEvmResponseTypes,
+    MissingBlockNumber(u32, u32),
+    ContradictoryTransactionLogs(u32, u32),
+    ContradictoryBlockMedian(u64, u64, u64),
     UnparsableRequestBody(Vec<u8>),
     NoEndpointAvailable(NetworkId),
     StorageRetrievalError(NetworkId),
@@ -204,9 +207,21 @@ impl<NetworkId: core::fmt::Debug> core::fmt::Debug for OffchainErr<NetworkId> {
                 fmt,
                 "Different endpoints returned conflicting response types."
             ),
+            OffchainErr::MissingBlockNumber(ref index, ref length) => write!(
+                fmt,
+                "Could not get block response at index {index} where total length is {length}.",
+            ),
+            OffchainErr::ContradictoryBlockMedian(ref mid, ref prev, ref distance) => write!(
+                fmt,
+                "Contradictory block median: values are {prev} {mid} while max distance is {distance}.",
+            ),
+            OffchainErr::ContradictoryTransactionLogs(ref count, ref number) => write!(
+                fmt,
+                "Contradictory tx logs: {number} event sequences from {count} endpoints.",
+            ),
             OffchainErr::UnparsableRequestBody(ref bytes) => write!(
                 fmt,
-                "Could not get valid utf8 request body from bytes: {:?}",
+                "Could not get valid utf8 request body from bytes: {:?}.",
                 bytes
             ),
             OffchainErr::NoEndpointAvailable(ref network_id) => write!(
@@ -883,18 +898,17 @@ impl<T: Config> Pallet<T> {
 
                 let pending_requests =
                     Self::prepare_pending_evm_requests(&rpc_endpoints, &request_body)?;
-                let parsed_evm_responses =
+                let parsed_evm_responses = Self::fetch_multiple_evm_from_remote(pending_requests)
-                    Self::fetch_multiple_evm_from_remote(pending_requests)
                     .iter()
                     .filter_map(|response_bytes| {
-                            let parsed_evm_response =
+                        let parsed_evm_response = Self::parse_evm_response(response_bytes).ok()?;
-                                Self::parse_evm_response(response_bytes).ok()?;
                         Some(parsed_evm_response)
                     })
                     .collect::<Vec<EvmResponseType>>();
 
                 Self::check_evm_responses_correctness(&parsed_evm_responses)?;
-                let parsed_evm_response = Self::get_balanced_evm_response(&parsed_evm_responses)?;
+                let parsed_evm_response =
+                    Self::get_balanced_evm_response(&parsed_evm_responses, max_block_distance)?;
 
                 let new_block_range = match parsed_evm_response {
                     EvmResponseType::BlockNumber(new_evm_block) if from_block.le(&to_block) => {
@@ -992,9 +1006,7 @@ impl<T: Config> Pallet<T> {
                 .deadline(deadline)
                 .send()
             {
-                Ok(pending) => {
+                Ok(pending) => pending_requests.push(pending),
-                    pending_requests.push(pending)
-                }
                 Err(_) => {
                     log::info!(
                         target: LOG_TARGET,
@@ -1021,6 +1033,7 @@ impl<T: Config> Pallet<T> {
 
     fn get_balanced_evm_response(
         parsed_evm_responses: &Vec<EvmResponseType>,
+        max_block_distance: u64,
     ) -> OffchainResult<T, EvmResponseType> {
         let first_evm_response = parsed_evm_responses
             .first()
@@ -1035,27 +1048,68 @@ impl<T: Config> Pallet<T> {
                         EvmResponseType::BlockNumber(block) => Some((index as u32, *block)),
                         EvmResponseType::TransactionLogs(_) => None,
                     })
-                    .collect();
+                    .collect::<Vec<_>>();
+
+                let block_numbers_len = block_numbers.len() as u32;
                 let median_value = Self::calculate_median_value(&mut block_numbers);
+
+                // there is no intention to make it resistent to *ANY* type of issues around RPC
+                // ednpoint. here we are trying to protect against `parsed_evm_responses.len() ==
+                // 2` while one of it is malicious in order not to fall in the block backoff later
+                let mid_idx =
+                    block_numbers.partition_point(|&block_meta| block_meta.1 < median_value);
+                let mid_block = block_numbers.get(mid_idx).map(|(_, block)| *block).ok_or(
+                    OffchainErr::MissingBlockNumber(mid_idx as u32, block_numbers_len),
+                )?;
+                let prev_block = if block_numbers_len % 2 == 0 {
+                    let prev_idx = mid_idx.saturating_sub(1);
+                    let prev_block = block_numbers.get(prev_idx).map(|(_, block)| *block).ok_or(
+                        OffchainErr::MissingBlockNumber(prev_idx as u32, block_numbers_len),
+                    )?;
+                    prev_block
+                } else {
+                    mid_block
+                };
+
+                if mid_block.abs_diff(prev_block) > max_block_distance {
+                    return Err(OffchainErr::ContradictoryBlockMedian(
+                        mid_block,
+                        prev_block,
+                        max_block_distance,
+                    ));
+                }
+
                 EvmResponseType::BlockNumber(median_value)
             }
             EvmResponseType::TransactionLogs(_) => {
-                let mut btree_map = BTreeMap::new();
+                let mut count_btree_map = BTreeMap::new();
                 parsed_evm_responses.iter().for_each(|response| {
                     if let EvmResponseType::TransactionLogs(logs) = response {
                         let mut inner_logs = logs.clone();
                         inner_logs.sort_by_key(|l| l.block_number);
-                        *btree_map.entry(inner_logs).or_insert(0) += 1;
+                        *count_btree_map.entry(inner_logs).or_insert(0) += 1;
                     }
                 });
 
-                let best_logs = btree_map
+                let (best_logs_ref, max_count) = count_btree_map
-                    .into_iter()
+                    .iter()
                     .max_by_key(|&(_, count)| count)
-                    .map(|(v, _)| v.clone())
+                    .map(|(logs, count)| (logs, count))
                     .ok_or(OffchainErr::EmptyResponses)?;
 
-                EvmResponseType::TransactionLogs(best_logs)
+                let best_logs_count = count_btree_map
+                    .values()
+                    .filter(|&&count| count == *max_count)
+                    .count();
+
+                if best_logs_count > 1 {
+                    return Err(OffchainErr::ContradictoryTransactionLogs(
+                        *max_count,
+                        best_logs_count as u32,
+                    ));
+                }
+
+                EvmResponseType::TransactionLogs(best_logs_ref.clone())
             }
         };
 
@@ -1079,9 +1133,7 @@ impl<T: Config> Pallet<T> {
         Ok(())
     }
 
-    fn fetch_multiple_evm_from_remote(
+    fn fetch_multiple_evm_from_remote(pending_requests: Vec<PendingRequest>) -> Vec<Vec<u8>> {
-        pending_requests: Vec<PendingRequest>,
-    ) -> Vec<Vec<u8>> {
         let mut requests_failed = 0;
         let mut responses_failed = 0;
         let mut responses_non_200 = 0;

pallets/slow-clap/src/tests.rs

@@ -1565,9 +1565,10 @@ fn should_check_responses_correctly() {
 #[test]
 fn should_get_balanced_responses_correctly() {
     new_test_ext().execute_with(|| {
+        let max_distance = 420;
         let empty_responses = vec![];
         assert_err!(
-            SlowClap::get_balanced_evm_response(&empty_responses),
+            SlowClap::get_balanced_evm_response(&empty_responses, max_distance),
             OffchainErr::EmptyResponses,
         );
 
@@ -1577,10 +1578,20 @@ fn should_get_balanced_responses_correctly() {
             EvmResponseType::BlockNumber(422),
             EvmResponseType::BlockNumber(1337),
         ];
-        let median_block = SlowClap::get_balanced_evm_response(&correct_block_responses)
+        let median_block =
+            SlowClap::get_balanced_evm_response(&correct_block_responses, max_distance)
                 .expect("median block should be extractable; qed");
         assert_eq!(median_block, EvmResponseType::BlockNumber(421));
 
+        let contradictory_block_responses = vec![
+            EvmResponseType::BlockNumber(69),
+            EvmResponseType::BlockNumber(1337),
+        ];
+        assert_err!(
+            SlowClap::get_balanced_evm_response(&contradictory_block_responses, max_distance),
+            OffchainErr::ContradictoryBlockMedian(1337, 69, max_distance),
+        );
+
         let first_correct_log = Log {
             transaction_hash: Some(H256::random()),
             block_number: Some(69),
@@ -1641,7 +1652,7 @@ fn should_get_balanced_responses_correctly() {
                 second_wrong_transaction_log,
             ]),
         ];
-        let best_logs = SlowClap::get_balanced_evm_response(&correct_log_responses)
+        let best_logs = SlowClap::get_balanced_evm_response(&correct_log_responses, 420)
             .expect("best logs should be extractable; qed");
         assert_eq!(
             best_logs,