Make AssetChanged trait clearer sematically (paritytech#130)

* Make the methods in AssetChanged trait clearer

* Nits

* .

xpallets/assets/src/lib.rs

@@ -30,7 +30,7 @@ use frame_system::{self as system, ensure_root, ensure_signed};
 use chainx_primitives::{AssetId, Desc, Memo, Token};
 use xpallet_support::{debug, ensure_with_errorlog, info};
 
-use self::trigger::AssetTriggerEventAfter;
+use self::trigger::AssetChangedTrigger;
 
 pub use self::traits::{ChainT, OnAssetChanged, OnAssetRegisterOrRevoke, TokenJackpotAccountIdFor};
 pub use self::types::{
@@ -601,7 +601,7 @@ impl<T: Trait> Module<T> {
         // check
         let new = current.checked_add(&value).ok_or(Error::<T>::Overflow)?;
 
-        AssetTriggerEventAfter::<T>::on_issue_before(id, who);
+        AssetChangedTrigger::<T>::on_issue_pre(id, who);
 
         // set to storage
         let imbalance = Self::make_type_balance_be(who, id, type_, new);
@@ -612,7 +612,7 @@ impl<T: Trait> Module<T> {
             PositiveImbalance::<T>::new(Zero::zero(), *id, type_)
         };
 
-        AssetTriggerEventAfter::<T>::on_issue(id, who, value)?;
+        AssetChangedTrigger::<T>::on_issue_post(id, who, value)?;
         Ok(positive)
     }
 
@@ -631,7 +631,7 @@ impl<T: Trait> Module<T> {
             .checked_sub(&value)
             .ok_or(Error::<T>::InsufficientBalance)?;
 
-        AssetTriggerEventAfter::<T>::on_destroy_before(id, who);
+        AssetChangedTrigger::<T>::on_destroy_pre(id, who);
 
         let imbalance = Self::make_type_balance_be(who, id, type_, new);
         let negative = if let SignedImbalance::Negative(n) = imbalance {
@@ -641,7 +641,7 @@ impl<T: Trait> Module<T> {
             NegativeImbalance::<T>::new(Zero::zero(), *id, type_)
         };
 
-        AssetTriggerEventAfter::<T>::on_destroy(id, who, value)?;
+        AssetChangedTrigger::<T>::on_destroy_post(id, who, value)?;
         Ok(negative)
     }
 
@@ -683,10 +683,8 @@ impl<T: Trait> Module<T> {
             // same account, same type, return directly
             // same account also do trigger
             if do_trigger {
-                AssetTriggerEventAfter::<T>::on_move_before(
-                    id, from, from_type, to, to_type, value,
-                );
-                AssetTriggerEventAfter::<T>::on_move(id, from, from_type, to, to_type, value)?;
+                AssetChangedTrigger::<T>::on_move_pre(id, from, from_type, to, to_type, value);
+                AssetChangedTrigger::<T>::on_move_post(id, from, from_type, to, to_type, value)?;
             }
             return Ok((
                 SignedImbalance::Positive(PositiveImbalance::<T>::zero()),
@@ -701,14 +699,14 @@ impl<T: Trait> Module<T> {
         }
 
         if do_trigger {
-            AssetTriggerEventAfter::<T>::on_move_before(id, from, from_type, to, to_type, value);
+            AssetChangedTrigger::<T>::on_move_pre(id, from, from_type, to, to_type, value);
         }
 
         let from_imbalance = Self::make_type_balance_be(from, id, from_type, new_from_balance);
         let to_imbalance = Self::make_type_balance_be(to, id, to_type, new_to_balance);
 
         if do_trigger {
-            AssetTriggerEventAfter::<T>::on_move(id, from, from_type, to, to_type, value)?;
+            AssetChangedTrigger::<T>::on_move_post(id, from, from_type, to, to_type, value)?;
         }
         Ok((from_imbalance, to_imbalance))
     }
@@ -735,7 +733,7 @@ impl<T: Trait> Module<T> {
 
             let _imbalance = Self::make_type_balance_be(who, id, type_, val);
 
-            AssetTriggerEventAfter::<T>::on_set_balance(id, who, type_, val)?;
+            AssetChangedTrigger::<T>::on_set_balance(id, who, type_, val)?;
         }
         Ok(())
     }

xpallets/assets/src/traits.rs

@@ -23,27 +23,47 @@ pub trait ChainT {
     }
 }
 
+/// Hooks for doing stuff when the assets are minted/moved/destroyed.
 pub trait OnAssetChanged<AccountId, Balance> {
-    fn on_move_before(
-        id: &AssetId,
-        from: &AccountId,
-        from_type: AssetType,
-        to: &AccountId,
-        to_type: AssetType,
-        value: Balance,
-    );
-    fn on_move(
-        id: &AssetId,
-        from: &AccountId,
-        from_type: AssetType,
-        to: &AccountId,
-        to_type: AssetType,
-        value: Balance,
-    ) -> result::Result<(), AssetErr>;
-    fn on_issue_before(id: &AssetId, who: &AccountId);
-    fn on_issue(id: &AssetId, who: &AccountId, value: Balance) -> DispatchResult;
-    fn on_destroy_before(id: &AssetId, who: &AccountId);
-    fn on_destroy(id: &AssetId, who: &AccountId, value: Balance) -> DispatchResult;
+    /// Triggered before issuing the fresh assets.
+    fn on_issue_pre(_id: &AssetId, _who: &AccountId) {}
+
+    /// Triggered after issuing the fresh assets.
+    fn on_issue_post(_id: &AssetId, _who: &AccountId, _value: Balance) -> DispatchResult {
+        Ok(())
+    }
+
+    /// Triggered before moving the assets.
+    fn on_move_pre(
+        _id: &AssetId,
+        _from: &AccountId,
+        _from_type: AssetType,
+        _to: &AccountId,
+        _to_type: AssetType,
+        _value: Balance,
+    ) {
+    }
+
+    /// Triggered after moving the assets.
+    fn on_move_post(
+        _id: &AssetId,
+        _from: &AccountId,
+        _from_type: AssetType,
+        _to: &AccountId,
+        _to_type: AssetType,
+        _value: Balance,
+    ) -> result::Result<(), AssetErr> {
+        Ok(())
+    }
+
+    /// Triggered before destroying the assets.
+    fn on_destroy_pre(_id: &AssetId, _who: &AccountId) {}
+
+    /// Triggered after the assets has been destroyed.
+    fn on_destroy_post(_id: &AssetId, _who: &AccountId, _value: Balance) -> DispatchResult {
+        Ok(())
+    }
+
     fn on_set_balance(
         _id: &AssetId,
         _who: &AccountId,
@@ -55,6 +75,10 @@ pub trait OnAssetChanged<AccountId, Balance> {
 }
 
 pub trait OnAssetRegisterOrRevoke {
-    fn on_register(_: &AssetId, _: bool) -> DispatchResult;
-    fn on_revoke(_: &AssetId) -> DispatchResult;
+    fn on_register(_: &AssetId, _: bool) -> DispatchResult {
+        Ok(())
+    }
+    fn on_revoke(_: &AssetId) -> DispatchResult {
+        Ok(())
+    }
 }

xpallets/assets/src/trigger.rs

@@ -7,44 +7,9 @@ use crate::traits::{OnAssetChanged, OnAssetRegisterOrRevoke};
 use crate::types::{AssetErr, AssetType};
 use crate::{Module, RawEvent, Trait};
 
-impl<AccountId, Balance> OnAssetChanged<AccountId, Balance> for () {
-    fn on_move_before(
-        _id: &AssetId,
-        _from: &AccountId,
-        _from_type: AssetType,
-        _to: &AccountId,
-        _to_type: AssetType,
-        _value: Balance,
-    ) {
-    }
-    fn on_move(
-        _id: &AssetId,
-        _from: &AccountId,
-        _from_type: AssetType,
-        _to: &AccountId,
-        _to_type: AssetType,
-        _value: Balance,
-    ) -> result::Result<(), AssetErr> {
-        Ok(())
-    }
-    fn on_issue_before(_: &AssetId, _: &AccountId) {}
-    fn on_issue(_: &AssetId, _: &AccountId, _: Balance) -> DispatchResult {
-        Ok(())
-    }
-    fn on_destroy_before(_: &AssetId, _: &AccountId) {}
-    fn on_destroy(_: &AssetId, _: &AccountId, _: Balance) -> DispatchResult {
-        Ok(())
-    }
-}
+impl<AccountId, Balance> OnAssetChanged<AccountId, Balance> for () {}
 
-impl OnAssetRegisterOrRevoke for () {
-    fn on_register(_: &AssetId, _: bool) -> DispatchResult {
-        Ok(())
-    }
-    fn on_revoke(_: &AssetId) -> DispatchResult {
-        Ok(())
-    }
-}
+impl OnAssetRegisterOrRevoke for () {}
 
 impl<A: OnAssetRegisterOrRevoke, B: OnAssetRegisterOrRevoke> OnAssetRegisterOrRevoke for (A, B) {
     fn on_register(id: &AssetId, is_psedu_intention: bool) -> DispatchResult {
@@ -70,20 +35,21 @@ impl<A: OnAssetRegisterOrRevoke, B: OnAssetRegisterOrRevoke> OnAssetRegisterOrRe
     }
 }
 
-pub struct AssetTriggerEventAfter<T: Trait>(::sp_std::marker::PhantomData<T>);
+pub struct AssetChangedTrigger<T: Trait>(::sp_std::marker::PhantomData<T>);
 
-impl<T: Trait> AssetTriggerEventAfter<T> {
-    pub fn on_move_before(
+impl<T: Trait> AssetChangedTrigger<T> {
+    pub fn on_move_pre(
         id: &AssetId,
         from: &T::AccountId,
         from_type: AssetType,
         to: &T::AccountId,
         to_type: AssetType,
         value: T::Balance,
     ) {
-        T::OnAssetChanged::on_move_before(id, from, from_type, to, to_type, value);
+        T::OnAssetChanged::on_move_pre(id, from, from_type, to, to_type, value);
     }
-    pub fn on_move(
+
+    pub fn on_move_post(
         id: &AssetId,
         from: &T::AccountId,
         from_type: AssetType,
@@ -99,25 +65,30 @@ impl<T: Trait> AssetTriggerEventAfter<T> {
             to_type,
             value,
         ));
-        T::OnAssetChanged::on_move(id, from, from_type, to, to_type, value)?;
+        T::OnAssetChanged::on_move_post(id, from, from_type, to, to_type, value)?;
         Ok(())
     }
-    pub fn on_issue_before(id: &AssetId, who: &T::AccountId) {
-        T::OnAssetChanged::on_issue_before(id, who);
+
+    pub fn on_issue_pre(id: &AssetId, who: &T::AccountId) {
+        T::OnAssetChanged::on_issue_pre(id, who);
     }
-    pub fn on_issue(id: &AssetId, who: &T::AccountId, value: T::Balance) -> DispatchResult {
+
+    pub fn on_issue_post(id: &AssetId, who: &T::AccountId, value: T::Balance) -> DispatchResult {
         Module::<T>::deposit_event(RawEvent::Issue(id.clone(), who.clone(), value));
-        T::OnAssetChanged::on_issue(id, who, value)?;
+        T::OnAssetChanged::on_issue_post(id, who, value)?;
         Ok(())
     }
-    pub fn on_destroy_before(id: &AssetId, who: &T::AccountId) {
-        T::OnAssetChanged::on_destroy_before(id, who);
+
+    pub fn on_destroy_pre(id: &AssetId, who: &T::AccountId) {
+        T::OnAssetChanged::on_destroy_pre(id, who);
     }
-    pub fn on_destroy(id: &AssetId, who: &T::AccountId, value: T::Balance) -> DispatchResult {
+
+    pub fn on_destroy_post(id: &AssetId, who: &T::AccountId, value: T::Balance) -> DispatchResult {
         Module::<T>::deposit_event(RawEvent::Destory(id.clone(), who.clone(), value));
-        T::OnAssetChanged::on_destroy(id, who, value)?;
+        T::OnAssetChanged::on_destroy_post(id, who, value)?;
         Ok(())
     }
+
     pub fn on_set_balance(
         id: &AssetId,
         who: &T::AccountId,