Improve AssetInDestruction ordering & add test

zrml/asset-router/src/tests/custom_types.rs

@@ -53,3 +53,34 @@ fn asset_in_destruction_indestructible_state_works() {
     aid.transit_state();
     assert_eq!(*aid.state(), DestructionState::Indestructible);
 }
+
+#[test]
+fn asset_in_destruction_ordering_works() {
+    // Order by destruction state first.
+    let asset_1 = Aid::new(0);
+    let mut asset_2 = asset_1;
+    assert_eq!(asset_2.transit_state(), Some(&DestructionState::Approvals));
+    let mut asset_3 = asset_2;
+    assert_eq!(asset_3.transit_state(), Some(&DestructionState::Finalization));
+    let mut asset_4 = asset_3;
+    assert_eq!(asset_4.transit_state(), Some(&DestructionState::Destroyed));
+    let mut asset_5 = asset_1;
+    asset_5.transit_indestructible();
+
+    let mut asset_vec = vec![asset_5, asset_4, asset_3, asset_2, asset_1];
+    let mut expected = vec![asset_1, asset_2, asset_3, asset_4, asset_5];
+    asset_vec.sort();
+    assert_eq!(asset_vec, expected);
+
+    // On equal destruction state, order by asset id.
+    let mut asset_dif_id_1 = Aid::new(1);
+    asset_dif_id_1.transit_state();
+    let mut asset_dif_id_2 = Aid::new(2);
+    asset_dif_id_2.transit_state();
+
+    asset_vec.push(asset_dif_id_1);
+    asset_vec.push(asset_dif_id_2);
+    asset_vec.sort();
+    expected = vec![asset_1, asset_dif_id_2, asset_dif_id_1, asset_2, asset_3, asset_4, asset_5];
+    assert_eq!(asset_vec, expected);
+}

zrml/asset-router/src/tests/managed_destroy.rs

@@ -43,7 +43,9 @@ fn adds_assets_properly() {
 
         assert_ok!(AssetRouter::create(CUSTOM_ASSET, ALICE, true, CUSTOM_ASSET_MIN_BALANCE));
         assert_ok!(AssetRouter::managed_destroy(CUSTOM_ASSET, None));
-        assert_eq!(crate::DestroyAssets::<Runtime>::get(), vec![campaign_asset, custom_asset]);
+        let mut expected = vec![campaign_asset, custom_asset];
+        expected.sort();
+        assert_eq!(crate::DestroyAssets::<Runtime>::get(), expected);
 
         crate::IndestructibleAssets::<Runtime>::put(BoundedVec::truncate_from(vec![
             CAMPAIGN_ASSET,
@@ -90,7 +92,9 @@ fn adds_multi_assets_properly() {
             AssetRouter::managed_destroy_multi(assets.clone()),
             Error::<Runtime>::DestructionInProgress
         );
-        assert_eq!(crate::DestroyAssets::<Runtime>::get(), vec![campaign_asset, custom_asset]);
+        let mut expected = vec![campaign_asset, custom_asset];
+        expected.sort();
+        assert_eq!(crate::DestroyAssets::<Runtime>::get(), expected);
 
         crate::IndestructibleAssets::<Runtime>::put(BoundedVec::truncate_from(vec![
             CAMPAIGN_ASSET,

zrml/asset-router/src/types.rs

@@ -23,33 +23,23 @@ use scale_info::TypeInfo;
 pub(crate) type DestroyAssetsT<T> =
     BoundedVec<AssetInDestruction<<T as Config>::AssetType>, ConstU32<8192>>;
 
-#[derive(Clone, Copy, Debug, Eq, PartialEq, Decode, Encode, MaxEncodedLen, TypeInfo)]
+#[derive(
+    Clone, Copy, Debug, Eq, PartialEq, PartialOrd, Ord, Decode, Encode, MaxEncodedLen, TypeInfo,
+)]
 pub(crate) enum DestructionState {
     Accounts,
     Approvals,
-    Destroyed,
     Finalization,
+    Destroyed,
     Indestructible,
 }
 
-#[derive(Clone, Copy, Encode, Debug, Decode, MaxEncodedLen, TypeInfo)]
+#[derive(Clone, Copy, Encode, Eq, Debug, Decode, MaxEncodedLen, PartialEq, TypeInfo)]
 pub(crate) struct AssetInDestruction<A> {
     asset: A,
     state: DestructionState,
 }
 
-// Ordering hack for binary search of assets in destruction.
-impl<A> PartialEq for AssetInDestruction<A>
-where
-    A: Eq + Ord + PartialEq + PartialOrd,
-{
-    fn eq(&self, other: &Self) -> bool {
-        self.asset == other.asset
-    }
-}
-
-impl<A> Eq for AssetInDestruction<A> where A: Eq + Ord + PartialEq + PartialOrd {}
-
 impl<A> PartialOrd for AssetInDestruction<A>
 where
     A: Eq + Ord + PartialEq + PartialOrd,
@@ -59,12 +49,25 @@ where
     }
 }
 
+// Ordering for binary search of assets in destruction.
+// Prioritize asset state first, then asset.
 impl<A> Ord for AssetInDestruction<A>
 where
     A: Eq + Ord + PartialEq + PartialOrd,
 {
     fn cmp(&self, other: &Self) -> Ordering {
-        self.asset.cmp(&other.asset)
+        match self.state.cmp(&other.state) {
+            Ordering::Equal => {
+                // Since asset destruction will always pop from the vector, sorting has to be reverse.
+                match self.asset.cmp(&other.asset) {
+                    Ordering::Equal => Ordering::Equal,
+                    Ordering::Less => Ordering::Greater,
+                    Ordering::Greater => Ordering::Less,
+                }
+            },
+            Ordering::Less => Ordering::Less,
+            Ordering::Greater => Ordering::Greater
+        }
     }
 }