fix(rocksdb): prioritize slot ordering over version in comparison logic [MTG-1415] (#446)

* fix(rocksdb): prioritize slot ordering over version in comparison logic

Change the comparison order in macro to check
slot_updated values first before checking update_version. This ensures account-based
assets are properly indexed by slot-first ordering, which better represents the
correct temporal sequence of asset updates.

Closes MTG-1415

* chore: fmt

* test: Fix tests after reprioritising the ordering

Author: StanChe

rocks-db/src/columns/asset.rs

@@ -3607,7 +3607,12 @@ mod tests {
         }
         assert!(asset.other_known_owners().is_none());
         let asset_mapped = AssetCompleteDetails::from(asset);
-        assert_eq!(asset_mapped.owner.unwrap().owner.value.unwrap(), new_owner);
+        // Now that slot is prioritized over version, we expect the existing owner to be kept
+        // because the new owner has a smaller slot (1) despite higher write version
+        assert_eq!(
+            asset_mapped.owner.as_ref().unwrap().owner.value.unwrap(),
+            Pubkey::from_str(EXISTING_OWNER).unwrap()
+        );
     }
 
     #[test]
@@ -3650,13 +3655,9 @@ mod tests {
         }
         assert!(asset.other_known_owners().is_none());
         let asset_mapped = AssetCompleteDetails::from(asset);
-        assert_eq!(
-            asset_mapped.owner.as_ref().unwrap().owner.value.unwrap(),
-            Pubkey::from_str(EXISTING_OWNER).unwrap()
-        );
-        // This is ther case, when the is current owner was not ever set and now it's updated by some old update.
-        // This update shouldn't have happened as usually the vesioning of owner, delegate and is_current_owner is done together.
-        // For the case of empty data after the migration this is acceptable imho.
+        // With slot prioritized over version, the new owner should be used because it has a higher slot
+        // even though it has a smaller write version
+        assert_eq!(asset_mapped.owner.as_ref().unwrap().owner.value.unwrap(), new_owner);
         assert!(asset_mapped.owner.unwrap().is_current_owner.value);
     }
 

rocks-db/src/mappers.rs

@@ -228,13 +228,19 @@ macro_rules! impl_partial_ord_for_updated {
     ($name:ident) => {
         impl PartialOrd for fb::$name<'_> {
             fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-                Some(match self.update_version().partial_cmp(&other.update_version()) {
-                    Some(std::cmp::Ordering::Equal) => {
-                        self.slot_updated().cmp(&other.slot_updated())
-                    },
-                    Some(ord) => ord,
-                    None => self.slot_updated().cmp(&other.slot_updated()),
-                })
+                // Compare slot_updated first
+                let slot_order = self.slot_updated().cmp(&other.slot_updated());
+
+                // If slots are equal, then check update_version
+                if slot_order == Ordering::Equal {
+                    match self.update_version().partial_cmp(&other.update_version()) {
+                        Some(ord) => Some(ord),
+                        None => Some(Ordering::Equal), // If versions can't be compared, consider them equal since slots are equal
+                    }
+                } else {
+                    // If slots are different, use that ordering
+                    Some(slot_order)
+                }
             }
         }
     };