Fixed BlockHandler with Call filter (#3087)

* Fixed BlockHanlder with Call filter

The `EthereumBlockFilter.extend` was not properly extending the base structure with the extension struct.

The new code reduces drasticly the cloning behavior that was happening way too much in the previous implementation.

Fixes #2314

* Added more test for coverage

Co-authored-by: Matthieu Vachon <[email protected]>

Author: maoueh

chain/ethereum/src/adapter.rs

@@ -692,24 +692,25 @@ impl EthereumBlockFilter {
         } = other;
 
         self.trigger_every_block = self.trigger_every_block || trigger_every_block;
-        self.contract_addresses = self.contract_addresses.iter().cloned().fold(
-            HashSet::new(),
-            |mut addresses, (start_block, address)| {
-                match contract_addresses
-                    .iter()
-                    .cloned()
-                    .find(|(_, other_address)| &address == other_address)
-                {
-                    Some((other_start_block, address)) => {
-                        addresses.insert((cmp::min(other_start_block, start_block), address));
-                    }
-                    None => {
-                        addresses.insert((start_block, address));
+
+        for other in contract_addresses {
+            let (other_start_block, other_address) = other;
+
+            match self.find_contract_address(&other.1) {
+                Some((current_start_block, current_address)) => {
+                    if other_start_block < current_start_block {
+                        self.contract_addresses
+                            .remove(&(current_start_block, current_address));
+                        self.contract_addresses
+                            .insert((other_start_block, other_address));
                     }
                 }
-                addresses
-            },
-        );
+                None => {
+                    self.contract_addresses
+                        .insert((other_start_block, other_address));
+                }
+            }
+        }
     }
 
     fn requires_traces(&self) -> bool {
@@ -725,6 +726,13 @@ impl EthereumBlockFilter {
 
         self.contract_addresses.is_empty()
     }
+
+    fn find_contract_address(&self, candidate: &Address) -> Option<(i32, Address)> {
+        self.contract_addresses
+            .iter()
+            .find(|(_, current_address)| candidate == current_address)
+            .cloned()
+    }
 }
 
 pub enum ProviderStatus {
@@ -1188,8 +1196,6 @@ mod tests {
 
     #[test]
     fn matching_ethereum_call_filter() {
-        let address = |id: u64| Address::from_low_u64_be(id);
-        let bytes = |value: Vec<u8>| Bytes::from(value);
         let call = |to: Address, input: Vec<u8>| EthereumCall {
             to,
             input: bytes(input),
@@ -1287,6 +1293,125 @@ mod tests {
         );
     }
 
+    #[test]
+    fn extending_ethereum_block_filter_no_found() {
+        let mut base = EthereumBlockFilter {
+            contract_addresses: HashSet::new(),
+            trigger_every_block: false,
+        };
+
+        let extension = EthereumBlockFilter {
+            contract_addresses: HashSet::from_iter(vec![(10, address(1))]),
+            trigger_every_block: false,
+        };
+
+        base.extend(extension);
+
+        assert_eq!(
+            HashSet::from_iter(vec![(10, address(1))]),
+            base.contract_addresses,
+        );
+    }
+
+    #[test]
+    fn extending_ethereum_block_filter_conflict_picks_lowest_block_from_ext() {
+        let mut base = EthereumBlockFilter {
+            contract_addresses: HashSet::from_iter(vec![(10, address(1))]),
+            trigger_every_block: false,
+        };
+
+        let extension = EthereumBlockFilter {
+            contract_addresses: HashSet::from_iter(vec![(2, address(1))]),
+            trigger_every_block: false,
+        };
+
+        base.extend(extension);
+
+        assert_eq!(
+            HashSet::from_iter(vec![(2, address(1))]),
+            base.contract_addresses,
+        );
+    }
+
+    #[test]
+    fn extending_ethereum_block_filter_conflict_picks_lowest_block_from_base() {
+        let mut base = EthereumBlockFilter {
+            contract_addresses: HashSet::from_iter(vec![(2, address(1))]),
+            trigger_every_block: false,
+        };
+
+        let extension = EthereumBlockFilter {
+            contract_addresses: HashSet::from_iter(vec![(10, address(1))]),
+            trigger_every_block: false,
+        };
+
+        base.extend(extension);
+
+        assert_eq!(
+            HashSet::from_iter(vec![(2, address(1))]),
+            base.contract_addresses,
+        );
+    }
+
+    #[test]
+    fn extending_ethereum_block_filter_every_block_in_ext() {
+        let mut base = EthereumBlockFilter {
+            contract_addresses: HashSet::default(),
+            trigger_every_block: false,
+        };
+
+        let extension = EthereumBlockFilter {
+            contract_addresses: HashSet::default(),
+            trigger_every_block: true,
+        };
+
+        base.extend(extension);
+
+        assert_eq!(true, base.trigger_every_block);
+    }
+
+    #[test]
+    fn extending_ethereum_block_filter_every_block_in_base_and_merge_contract_addresses() {
+        let mut base = EthereumBlockFilter {
+            contract_addresses: HashSet::from_iter(vec![(10, address(2))]),
+            trigger_every_block: true,
+        };
+
+        let extension = EthereumBlockFilter {
+            contract_addresses: HashSet::from_iter(vec![]),
+            trigger_every_block: false,
+        };
+
+        base.extend(extension);
+
+        assert_eq!(true, base.trigger_every_block);
+        assert_eq!(
+            HashSet::from_iter(vec![(10, address(2))]),
+            base.contract_addresses,
+        );
+    }
+
+    #[test]
+    fn extending_ethereum_block_filter_every_block_in_ext_and_merge_contract_addresses() {
+        let mut base = EthereumBlockFilter {
+            contract_addresses: HashSet::from_iter(vec![(10, address(2))]),
+            trigger_every_block: false,
+        };
+
+        let extension = EthereumBlockFilter {
+            contract_addresses: HashSet::from_iter(vec![(10, address(1))]),
+            trigger_every_block: true,
+        };
+
+        base.extend(extension);
+
+        assert_eq!(true, base.trigger_every_block);
+        assert_eq!(
+            HashSet::from_iter(vec![(10, address(2)), (10, address(1))]),
+            base.contract_addresses,
+        );
+    }
+
     #[test]
     fn extending_ethereum_call_filter() {
         let mut base = EthereumCallFilter {
@@ -1333,6 +1458,14 @@ mod tests {
             Some(&(1, HashSet::from_iter(vec![[1u8; 4]])))
         );
     }
+
+    fn address(id: u64) -> Address {
+        Address::from_low_u64_be(id)
+    }
+
+    fn bytes(value: Vec<u8>) -> Bytes {
+        Bytes::from(value)
+    }
 }
 
 // Tests `eth_get_logs_filters` in instances where all events are filtered on by all contracts.

chain/ethereum/src/ethereum_adapter.rs

@@ -1987,3 +1987,129 @@ async fn get_transaction_receipts_for_transaction_hashes(
 
     Ok(receipts_by_hash)
 }
+
+#[cfg(test)]
+mod tests {
+
+    use crate::trigger::{EthereumBlockTriggerType, EthereumTrigger};
+
+    use super::{parse_block_triggers, EthereumBlock, EthereumBlockFilter, EthereumBlockWithCalls};
+    use graph::blockchain::BlockPtr;
+    use graph::prelude::ethabi::ethereum_types::U64;
+    use graph::prelude::web3::types::{Address, Block, Bytes, H256};
+    use graph::prelude::EthereumCall;
+    use std::collections::HashSet;
+    use std::iter::FromIterator;
+    use std::sync::Arc;
+
+    #[test]
+    fn parse_block_triggers_every_block() {
+        let block = EthereumBlockWithCalls {
+            ethereum_block: EthereumBlock {
+                block: Arc::new(Block {
+                    hash: Some(hash(2)),
+                    number: Some(U64::from(2)),
+                    ..Default::default()
+                }),
+                ..Default::default()
+            },
+            calls: Some(vec![EthereumCall {
+                to: address(4),
+                input: bytes(vec![1; 36]),
+                ..Default::default()
+            }]),
+        };
+
+        assert_eq!(
+            vec![EthereumTrigger::Block(
+                BlockPtr::from((hash(2), 2)),
+                EthereumBlockTriggerType::Every
+            )],
+            parse_block_triggers(
+                &EthereumBlockFilter {
+                    contract_addresses: HashSet::from_iter(vec![(10, address(1))]),
+                    trigger_every_block: true,
+                },
+                &block
+            ),
+            "every block should generate a trigger even when address don't match"
+        );
+    }
+
+    #[test]
+    fn parse_block_triggers_specific_call_not_found() {
+        let block = EthereumBlockWithCalls {
+            ethereum_block: EthereumBlock {
+                block: Arc::new(Block {
+                    hash: Some(hash(2)),
+                    number: Some(U64::from(2)),
+                    ..Default::default()
+                }),
+                ..Default::default()
+            },
+            calls: Some(vec![EthereumCall {
+                to: address(4),
+                input: bytes(vec![1; 36]),
+                ..Default::default()
+            }]),
+        };
+
+        assert_eq!(
+            Vec::<EthereumTrigger>::new(),
+            parse_block_triggers(
+                &EthereumBlockFilter {
+                    contract_addresses: HashSet::from_iter(vec![(1, address(1))]),
+                    trigger_every_block: false,
+                },
+                &block
+            ),
+            "block filter specifies address 1 but block does not contain any call to it"
+        );
+    }
+
+    #[test]
+    fn parse_block_triggers_specific_call_found() {
+        let block = EthereumBlockWithCalls {
+            ethereum_block: EthereumBlock {
+                block: Arc::new(Block {
+                    hash: Some(hash(2)),
+                    number: Some(U64::from(2)),
+                    ..Default::default()
+                }),
+                ..Default::default()
+            },
+            calls: Some(vec![EthereumCall {
+                to: address(4),
+                input: bytes(vec![1; 36]),
+                ..Default::default()
+            }]),
+        };
+
+        assert_eq!(
+            vec![EthereumTrigger::Block(
+                BlockPtr::from((hash(2), 2)),
+                EthereumBlockTriggerType::WithCallTo(address(4))
+            )],
+            parse_block_triggers(
+                &EthereumBlockFilter {
+                    contract_addresses: HashSet::from_iter(vec![(1, address(4))]),
+                    trigger_every_block: false,
+                },
+                &block
+            ),
+            "block filter specifies address 4 and block has call to it"
+        );
+    }
+
+    fn address(id: u64) -> Address {
+        Address::from_low_u64_be(id)
+    }
+
+    fn hash(id: u8) -> H256 {
+        H256::from([id; 32])
+    }
+
+    fn bytes(value: Vec<u8>) -> Bytes {
+        Bytes::from(value)
+    }
+}