From ef034a22d65d86d02c336017255a14f591ee2f77 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Fri, 3 Jul 2020 19:28:46 +0800 Subject: [PATCH] Make AssetChanged trait clearer sematically (#130) * Make the methods in AssetChanged trait clearer * Nits * . --- xpallets/assets/src/lib.rs | 22 +++++----- xpallets/assets/src/traits.rs | 68 +++++++++++++++++++++---------- xpallets/assets/src/trigger.rs | 73 ++++++++++------------------------ 3 files changed, 78 insertions(+), 85 deletions(-) diff --git a/xpallets/assets/src/lib.rs b/xpallets/assets/src/lib.rs index 7554fba110e98..e3735b19e1b16 100644 --- a/xpallets/assets/src/lib.rs +++ b/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 Module { // check let new = current.checked_add(&value).ok_or(Error::::Overflow)?; - AssetTriggerEventAfter::::on_issue_before(id, who); + AssetChangedTrigger::::on_issue_pre(id, who); // set to storage let imbalance = Self::make_type_balance_be(who, id, type_, new); @@ -612,7 +612,7 @@ impl Module { PositiveImbalance::::new(Zero::zero(), *id, type_) }; - AssetTriggerEventAfter::::on_issue(id, who, value)?; + AssetChangedTrigger::::on_issue_post(id, who, value)?; Ok(positive) } @@ -631,7 +631,7 @@ impl Module { .checked_sub(&value) .ok_or(Error::::InsufficientBalance)?; - AssetTriggerEventAfter::::on_destroy_before(id, who); + AssetChangedTrigger::::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 Module { NegativeImbalance::::new(Zero::zero(), *id, type_) }; - AssetTriggerEventAfter::::on_destroy(id, who, value)?; + AssetChangedTrigger::::on_destroy_post(id, who, value)?; Ok(negative) } @@ -683,10 +683,8 @@ impl Module { // same account, same type, return directly // same account also do trigger if do_trigger { - AssetTriggerEventAfter::::on_move_before( - id, from, from_type, to, to_type, value, - ); - AssetTriggerEventAfter::::on_move(id, from, from_type, to, to_type, value)?; + AssetChangedTrigger::::on_move_pre(id, from, from_type, to, to_type, value); + AssetChangedTrigger::::on_move_post(id, from, from_type, to, to_type, value)?; } return Ok(( SignedImbalance::Positive(PositiveImbalance::::zero()), @@ -701,14 +699,14 @@ impl Module { } if do_trigger { - AssetTriggerEventAfter::::on_move_before(id, from, from_type, to, to_type, value); + AssetChangedTrigger::::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::::on_move(id, from, from_type, to, to_type, value)?; + AssetChangedTrigger::::on_move_post(id, from, from_type, to, to_type, value)?; } Ok((from_imbalance, to_imbalance)) } @@ -735,7 +733,7 @@ impl Module { let _imbalance = Self::make_type_balance_be(who, id, type_, val); - AssetTriggerEventAfter::::on_set_balance(id, who, type_, val)?; + AssetChangedTrigger::::on_set_balance(id, who, type_, val)?; } Ok(()) } diff --git a/xpallets/assets/src/traits.rs b/xpallets/assets/src/traits.rs index 039ad80cc18d8..34cb46d64983d 100644 --- a/xpallets/assets/src/traits.rs +++ b/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 { - 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 { } 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(()) + } } diff --git a/xpallets/assets/src/trigger.rs b/xpallets/assets/src/trigger.rs index 839b909b9b8b1..88264c4f9a935 100644 --- a/xpallets/assets/src/trigger.rs +++ b/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 OnAssetChanged 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 OnAssetChanged for () {} -impl OnAssetRegisterOrRevoke for () { - fn on_register(_: &AssetId, _: bool) -> DispatchResult { - Ok(()) - } - fn on_revoke(_: &AssetId) -> DispatchResult { - Ok(()) - } -} +impl OnAssetRegisterOrRevoke for () {} impl OnAssetRegisterOrRevoke for (A, B) { fn on_register(id: &AssetId, is_psedu_intention: bool) -> DispatchResult { @@ -70,10 +35,10 @@ impl OnAssetRegisterOrRe } } -pub struct AssetTriggerEventAfter(::sp_std::marker::PhantomData); +pub struct AssetChangedTrigger(::sp_std::marker::PhantomData); -impl AssetTriggerEventAfter { - pub fn on_move_before( +impl AssetChangedTrigger { + pub fn on_move_pre( id: &AssetId, from: &T::AccountId, from_type: AssetType, @@ -81,9 +46,10 @@ impl AssetTriggerEventAfter { 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 AssetTriggerEventAfter { 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::::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::::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,