fix(sdk): properly handle deletion -> creation -> deletion sequence (#213)

* fix(sdk): properly handle deletion -> creation -> deletion sequence

This commit resolves a bug that occurred when an attribute underwent a deletion, followed by creation, and then another deletion. The system would incorrectly ignore the final deletion because the attribute was mistakenly marked as newly created during the process.

* docs: fix typo

Co-authored-by: Louise Poole <louisecarmenpoole@gmail.com>

---------

Co-authored-by: zizou <111426680+flopell@users.noreply.github.com>
Co-authored-by: Louise Poole <louisecarmenpoole@gmail.com>

substreams/Cargo.lock

@@ -1682,21 +1682,6 @@ dependencies = [
  "substreams-ethereum",
 ]
 
-[[package]]
-name = "tycho-substreams"
-version = "0.2.1"
-dependencies = [
- "ethabi 18.0.0",
- "hex",
- "itertools 0.12.1",
- "num-bigint",
- "prost 0.11.9",
- "serde",
- "serde_json",
- "substreams",
- "substreams-ethereum",
-]
-
 [[package]]
 name = "tycho-substreams"
 version = "0.2.1"
@@ -1729,6 +1714,22 @@ dependencies = [
  "substreams-ethereum",
 ]
 
+[[package]]
+name = "tycho-substreams"
+version = "0.2.2"
+dependencies = [
+ "ethabi 18.0.0",
+ "hex",
+ "itertools 0.12.1",
+ "num-bigint",
+ "prost 0.11.9",
+ "rstest",
+ "serde",
+ "serde_json",
+ "substreams",
+ "substreams-ethereum",
+]
+
 [[package]]
 name = "typenum"
 version = "1.17.0"

substreams/crates/tycho-substreams/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "tycho-substreams"
-version = "0.2.1"
+version = "0.2.2"
 edition = "2021"
 description = "Tycho substreams development kit, contains tycho-indexer block changes model and helper functions for common indexing tasks."
 repository = "https://github.com/propeller-heads/tycho-protocol-sdk/tree/main/substreams/crates/tycho-substreams"
@@ -21,3 +21,6 @@ ethabi = "18.0.0"
 num-bigint = "0.4.4"
 serde = "1.0.204"
 serde_json = "1.0.120"
+
+[dev-dependencies]
+rstest = "0.24.0"

substreams/crates/tycho-substreams/src/lib.rs

@@ -4,6 +4,7 @@ pub mod balances;
 pub mod block_storage;
 pub mod contract;
 pub mod entrypoint;
+#[cfg(test)]
 mod mock_store;
 pub mod models;
 pub mod pb;

substreams/crates/tycho-substreams/src/models.rs

@@ -427,26 +427,35 @@ impl InterimEntityChanges {
     }
 
     pub fn set_attribute(&mut self, attr: &Attribute) {
-        // Add any attribute creation to the map
+        // If the attribute is created in this transaction, add it to the set of created
-        if attr.change == i32::from(ChangeType::Creation) {
+        // attributes.
+        // Note: If it's already present in `self.attributes` it means this is not a real
+        // creation (it can be a deletion -> creation sequence for example), in that case we don't
+        // want to mark it as created.
+        if attr.change == i32::from(ChangeType::Creation) &&
+            !self.attributes.contains_key(&attr.name)
+        {
             self.created_attributes
                 .insert(attr.name.clone());
         }
 
+        // If a freshly created attribute is deleted, remove the creation and don't emit the
+        // deletion.
         if attr.change == i32::from(ChangeType::Deletion) &&
             self.created_attributes
                 .contains(&attr.name)
         {
-            // If a freshly created attribute is deleted, remove the creation.
             self.attributes.remove(&attr.name);
-        } else {
+            return;
+        }
+
+        // Otherwise, add the attribute to the map.
         self.attributes
             .entry(attr.name.clone())
             .and_modify(|existing| *existing = attr.clone())
             .or_insert(attr.clone());
     }
 }
-}
 
 impl From<InterimEntityChanges> for Option<EntityChanges> {
     fn from(value: InterimEntityChanges) -> Self {
@@ -606,12 +615,130 @@ impl TransactionChanges {
 
 #[cfg(test)]
 mod test {
+    use rstest::rstest;
     use substreams_ethereum::pb::eth::v2::StorageChange;
 
-    use crate::models::{Attribute, ChangeType, EntityChanges};
+    use crate::models::{Attribute, ChangeType, EntityChanges, Transaction, TransactionChanges};
 
     use super::{InterimContractChange, TransactionChangesBuilder};
 
+    fn create_attribute_change(value: u8, change_type: ChangeType) -> EntityChanges {
+        EntityChanges {
+            component_id: "component".to_string(),
+            attributes: vec![Attribute {
+                name: "attribute".to_string(),
+                value: vec![value],
+                change: change_type.into(),
+            }],
+        }
+    }
+
+    fn create_transaction_changes(
+        attribute_value: u8,
+        change_type: ChangeType,
+    ) -> Option<TransactionChanges> {
+        Some(TransactionChanges {
+            tx: Some(Transaction {
+                hash: [].to_vec(),
+                from: [].to_vec(),
+                to: [].to_vec(),
+                index: 0,
+            }),
+            contract_changes: Default::default(),
+            component_changes: Default::default(),
+            balance_changes: Default::default(),
+            entity_changes: vec![EntityChanges {
+                component_id: "component".to_string(),
+                attributes: vec![Attribute {
+                    name: "attribute".to_string(),
+                    value: vec![attribute_value],
+                    change: change_type.into(),
+                }],
+            }],
+            entrypoints: Default::default(),
+            entrypoint_params: Default::default(),
+        })
+    }
+
+    #[rstest]
+    #[case::deletion_creation_deletion(
+        vec![
+            (0, ChangeType::Deletion),
+            (1, ChangeType::Creation),
+            (0, ChangeType::Deletion),
+        ],
+        create_transaction_changes(0, ChangeType::Deletion)
+    )]
+    #[case::creation_deletion_creation(
+        vec![
+            (1, ChangeType::Creation),
+            (0, ChangeType::Deletion),
+            (2, ChangeType::Creation),
+        ],
+        create_transaction_changes(2, ChangeType::Creation)
+    )]
+    #[case::creation_deletion(
+        vec![
+            (1, ChangeType::Creation),
+            (0, ChangeType::Deletion),
+        ],
+        None
+    )]
+    #[case::deletion_creation_deletion_creation(
+        vec![
+            (0, ChangeType::Deletion),
+            (1, ChangeType::Creation),
+            (2, ChangeType::Deletion),
+            (3, ChangeType::Creation),
+        ],
+        create_transaction_changes(3, ChangeType::Creation)
+    )]
+    #[case::creation_deletion_creation_deletion(
+        vec![
+            (1, ChangeType::Creation),
+            (0, ChangeType::Deletion),
+            (2, ChangeType::Creation),
+            (3, ChangeType::Deletion),
+        ],
+        None
+    )]
+    #[case::creation_update(
+        vec![
+            (1, ChangeType::Creation),
+            (2, ChangeType::Update),
+        ],
+        create_transaction_changes(2, ChangeType::Update)
+    )]
+    #[case::creation_update_deletion(
+        vec![
+            (1, ChangeType::Creation),
+            (2, ChangeType::Update),
+            (3, ChangeType::Deletion),
+        ],
+        None
+    )]
+    #[case::deletion_update(
+        vec![
+            (0, ChangeType::Deletion),
+            (1, ChangeType::Update),
+        ],
+        create_transaction_changes(1, ChangeType::Update)
+    )]
+    fn test_attribute_sequences(
+        #[case] changes: Vec<(u8, ChangeType)>,
+        #[case] expected: Option<TransactionChanges>,
+    ) {
+        let mut builder = TransactionChangesBuilder::new(&Transaction::default());
+
+        for (value, change_type) in changes {
+            let change = create_attribute_change(value, change_type);
+            builder.add_entity_change(&change);
+        }
+
+        let tx_changes = builder.build();
+        assert_eq!(tx_changes, expected);
+    }
+
     #[test]
     fn test_transaction_changes_builder_ignored_contract_changes() {
         let mut builder = TransactionChangesBuilder::new(&super::Transaction::default());