From 2b5f0579a55d628d0af59657a798d6ed78e1d0f2 Mon Sep 17 00:00:00 2001 From: clearloop Date: Thu, 10 Jun 2021 21:13:49 +0800 Subject: [PATCH 1/7] feat(committee): throw error if adding existed assets --- pallets/asset-index/src/lib.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pallets/asset-index/src/lib.rs b/pallets/asset-index/src/lib.rs index aae50e7d30..9dc8acf027 100644 --- a/pallets/asset-index/src/lib.rs +++ b/pallets/asset-index/src/lib.rs @@ -158,6 +158,8 @@ pub mod pallet { pub enum Error { /// Thrown if adding units to an asset holding causes its numerical type to overflow AssetUnitsOverflow, + /// Thrown if adding an existed asset + AssetExisted, /// Thrown if no index could be found for an asset identifier. UnsupportedAsset, /// Thrown if calculating the volume of units of an asset with it's price overflows. @@ -522,6 +524,12 @@ pub mod pallet { units: &T::Balance, availability: &AssetAvailability, ) -> DispatchResult { + if T::PriceFeed::get_price(*asset_id).is_err() { + // init price feed if not exists + } else { + return Err(>::AssetExisted.into()); + } + Holdings::::try_mutate(asset_id, |value| -> Result<_, Error> { let index_asset_data = value.get_or_insert_with(|| { IndexAssetData::::new(T::Balance::zero(), availability.clone()) From 25acf11518242a880bf7ad0e8c9c1f161a49197e Mon Sep 17 00:00:00 2001 From: clearloop Date: Fri, 11 Jun 2021 14:49:07 +0800 Subject: [PATCH 2/7] feat(price-feed): add ensure_price_feed to prevent empty price pair --- pallets/asset-index/src/lib.rs | 12 +++++------ pallets/price-feed/src/lib.rs | 34 ++++++++++++++++++++++++++++++++ pallets/price-feed/src/traits.rs | 6 +++++- 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/pallets/asset-index/src/lib.rs b/pallets/asset-index/src/lib.rs index 9dc8acf027..bc8a187567 100644 --- a/pallets/asset-index/src/lib.rs +++ b/pallets/asset-index/src/lib.rs @@ -42,7 +42,7 @@ pub mod pallet { use xcm::opaque::v0::MultiLocation; use pallet_asset_depository::MultiAssetDepository; - use pallet_price_feed::{AssetPricePair, PriceFeed}; + use pallet_price_feed::{AssetPricePair, Price, PriceFeed}; use pallet_remote_asset_manager::RemoteAssetManager; use crate::traits::WithdrawalFee; @@ -192,6 +192,10 @@ pub mod pallet { ) -> DispatchResultWithPostInfo { T::AdminOrigin::ensure_origin(origin.clone())?; let caller = ensure_signed(origin)?; + + // Store intial price pair if not exists + T::PriceFeed::ensure_price(asset_id, Price::from_inner((value / units).into()))?; + >::add_asset( &asset_id, &units, @@ -524,12 +528,6 @@ pub mod pallet { units: &T::Balance, availability: &AssetAvailability, ) -> DispatchResult { - if T::PriceFeed::get_price(*asset_id).is_err() { - // init price feed if not exists - } else { - return Err(>::AssetExisted.into()); - } - Holdings::::try_mutate(asset_id, |value| -> Result<_, Error> { let index_asset_data = value.get_or_insert_with(|| { IndexAssetData::::new(T::Balance::zero(), availability.clone()) diff --git a/pallets/price-feed/src/lib.rs b/pallets/price-feed/src/lib.rs index 21afa7290d..ae5bd240cd 100644 --- a/pallets/price-feed/src/lib.rs +++ b/pallets/price-feed/src/lib.rs @@ -76,6 +76,16 @@ pub mod pallet { pub type AssetFeeds = StorageMap<_, Blake2_128Concat, T::AssetId, FeedIdFor, OptionQuery>; + #[pallet::storage] + /// (AssetId) -> AssetPricePair + /// + /// This storage stores the initial price pair for quote assets based on `SelfAssetId` + /// + /// * insert: adding a new asset with no price pair been set yet + /// * remove: an asset has its price pair, remove automatically + pub type InitialPricePair = + StorageMap<_, Blake2_128Concat, T::AssetId, AssetPricePair, OptionQuery>; + #[pallet::genesis_config] pub struct GenesisConfig where @@ -272,6 +282,30 @@ pub mod pallet { Ok(AssetPricePair { base, quote, price }) } + + /// Ensure quote asset has price pair based on `Self::AssetId` + /// + /// * Set initial price pair if feed not exists + /// * Remove initial price pair if feed exists + fn ensure_price( + quote: T::AssetId, + price: Price, + ) -> Result, DispatchError> { + Ok( + if let Ok(pair) = Self::get_price_pair(T::SelfAssetId::get(), quote.clone()) { + >::remove(quote); + pair + } else { + let pair = AssetPricePair { + base: T::SelfAssetId::get(), + quote: quote.clone(), + price, + }; + >::insert(quote, pair.clone()); + pair + }, + ) + } } /// Trait for the asset-index pallet extrinsic weights. diff --git a/pallets/price-feed/src/traits.rs b/pallets/price-feed/src/traits.rs index d710a862de..df1f016e95 100644 --- a/pallets/price-feed/src/traits.rs +++ b/pallets/price-feed/src/traits.rs @@ -1,7 +1,7 @@ // Copyright 2021 ChainSafe Systems // SPDX-License-Identifier: LGPL-3.0-only -use crate::types::AssetPricePair; +use crate::types::{AssetPricePair, Price}; use frame_support::dispatch::DispatchError; /// An interface to access price data @@ -14,4 +14,8 @@ pub trait PriceFeed { base: AssetId, quote: AssetId, ) -> Result, DispatchError>; + + /// Set initial price pair if feed not exists + fn ensure_price(quote: AssetId, units: Price) + -> Result, DispatchError>; } From bc6a3748915091976df244754d967ab34b56075b Mon Sep 17 00:00:00 2001 From: clearloop Date: Fri, 11 Jun 2021 15:50:02 +0800 Subject: [PATCH 3/7] feat(asset-index): update the tests of asset-index --- pallets/asset-index/src/mock.rs | 14 ++++++++------ pallets/asset-index/src/tests.rs | 26 ++++++++------------------ 2 files changed, 16 insertions(+), 24 deletions(-) diff --git a/pallets/asset-index/src/mock.rs b/pallets/asset-index/src/mock.rs index 85c39bc51f..e08f196bc9 100644 --- a/pallets/asset-index/src/mock.rs +++ b/pallets/asset-index/src/mock.rs @@ -178,11 +178,7 @@ pub const ASSET_B_PRICE_MULTIPLIER: Balance = 3; pub struct MockPriceFeed; impl PriceFeed for MockPriceFeed { fn get_price(quote: AssetId) -> Result, DispatchError> { - if quote == UNKNOWN_ASSET_ID { - Err(pallet_asset_index::Error::::UnsupportedAsset.into()) - } else { - Self::get_price_pair(PINT_ASSET_ID, quote) - } + Self::get_price_pair(PINT_ASSET_ID, quote) } fn get_price_pair( @@ -190,7 +186,8 @@ impl PriceFeed for MockPriceFeed { quote: AssetId, ) -> Result, DispatchError> { let price = match quote { - ASSET_A_ID => { + // includes unknown asset id since we don't need to mock initial price pair here + ASSET_A_ID | UNKNOWN_ASSET_ID => { Price::checked_from_rational(600, 600 / ASSET_A_PRICE_MULTIPLIER).unwrap() } ASSET_B_ID => { @@ -200,6 +197,11 @@ impl PriceFeed for MockPriceFeed { }; Ok(AssetPricePair { base, quote, price }) } + + fn ensure_price(_: AssetId, _: Price) -> Result, DispatchError> { + // pass all unknown asset id + Self::get_price(UNKNOWN_ASSET_ID) + } } // Build genesis storage according to the mock runtime. diff --git a/pallets/asset-index/src/tests.rs b/pallets/asset-index/src/tests.rs index 643d18ba2e..54187e855b 100644 --- a/pallets/asset-index/src/tests.rs +++ b/pallets/asset-index/src/tests.rs @@ -148,13 +148,6 @@ fn deposit_works_with_user_balance() { fn deposit_fails_for_unknown_assets() { let initial_balances: Vec<(AccountId, Balance)> = vec![(ADMIN_ACCOUNT_ID, 0)]; new_test_ext(initial_balances).execute_with(|| { - assert_ok!(AssetIndex::add_asset( - Origin::signed(ADMIN_ACCOUNT_ID), - ASSET_A_ID, - 100, - AssetAvailability::Liquid(MultiLocation::Null), - 5 - )); assert_noop!( AssetIndex::deposit(Origin::signed(ASHLEY), UNKNOWN_ASSET_ID, 1_000), pallet::Error::::UnsupportedAsset @@ -163,7 +156,7 @@ fn deposit_fails_for_unknown_assets() { } #[test] -fn deposit_fails_for_when_price_feed_unavailable() { +fn deposit_ok_for_when_price_feed_unavailable() { let initial_balances: Vec<(AccountId, Balance)> = vec![(ADMIN_ACCOUNT_ID, 0)]; new_test_ext(initial_balances).execute_with(|| { assert_ok!(AssetIndex::add_asset( @@ -173,10 +166,12 @@ fn deposit_fails_for_when_price_feed_unavailable() { AssetAvailability::Liquid(MultiLocation::Null), 5 )); - assert_noop!( - AssetIndex::deposit(Origin::signed(ASHLEY), UNKNOWN_ASSET_ID, 1_000), - pallet::Error::::UnsupportedAsset - ); + assert_ok!(AssetDepository::deposit(&UNKNOWN_ASSET_ID, &ASHLEY, 1_000)); + assert_ok!(AssetIndex::deposit( + Origin::signed(ASHLEY), + UNKNOWN_ASSET_ID, + 1 + ),); }) } @@ -191,16 +186,11 @@ fn deposit_fails_on_overflowing() { AssetAvailability::Liquid(MultiLocation::Null), 5 )); - assert_ok!(AssetDepository::deposit(&ASSET_A_ID, &ASHLEY, Balance::MAX)); + assert_noop!( AssetIndex::deposit(Origin::signed(ASHLEY), ASSET_A_ID, Balance::MAX), pallet::Error::::AssetVolumeOverflow ); - assert_ok!(AssetIndex::deposit( - Origin::signed(ASHLEY), - ASSET_A_ID, - 1_000 - )); }) } From 25cd6bd151d01fa09927cccb456d5e2d64dcbaae Mon Sep 17 00:00:00 2001 From: clearloop Date: Fri, 11 Jun 2021 16:32:17 +0800 Subject: [PATCH 4/7] feat(asset-index): fix typos --- pallets/asset-index/src/lib.rs | 2 -- pallets/asset-index/src/mock.rs | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/pallets/asset-index/src/lib.rs b/pallets/asset-index/src/lib.rs index bc8a187567..0100699726 100644 --- a/pallets/asset-index/src/lib.rs +++ b/pallets/asset-index/src/lib.rs @@ -158,8 +158,6 @@ pub mod pallet { pub enum Error { /// Thrown if adding units to an asset holding causes its numerical type to overflow AssetUnitsOverflow, - /// Thrown if adding an existed asset - AssetExisted, /// Thrown if no index could be found for an asset identifier. UnsupportedAsset, /// Thrown if calculating the volume of units of an asset with it's price overflows. diff --git a/pallets/asset-index/src/mock.rs b/pallets/asset-index/src/mock.rs index e08f196bc9..cd94cf650c 100644 --- a/pallets/asset-index/src/mock.rs +++ b/pallets/asset-index/src/mock.rs @@ -199,7 +199,7 @@ impl PriceFeed for MockPriceFeed { } fn ensure_price(_: AssetId, _: Price) -> Result, DispatchError> { - // pass all unknown asset id + // pass all unknown asset ids Self::get_price(UNKNOWN_ASSET_ID) } } From 23129a1b248aeec050e976a163c67f10a0d4cf0c Mon Sep 17 00:00:00 2001 From: clearloop Date: Fri, 11 Jun 2021 16:33:09 +0800 Subject: [PATCH 5/7] feat(pallet-index): fix typos --- pallets/price-feed/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pallets/price-feed/src/lib.rs b/pallets/price-feed/src/lib.rs index ae5bd240cd..f5ae39c835 100644 --- a/pallets/price-feed/src/lib.rs +++ b/pallets/price-feed/src/lib.rs @@ -83,7 +83,7 @@ pub mod pallet { /// /// * insert: adding a new asset with no price pair been set yet /// * remove: an asset has its price pair, remove automatically - pub type InitialPricePair = + pub type InitialPricePairs = StorageMap<_, Blake2_128Concat, T::AssetId, AssetPricePair, OptionQuery>; #[pallet::genesis_config] @@ -293,7 +293,7 @@ pub mod pallet { ) -> Result, DispatchError> { Ok( if let Ok(pair) = Self::get_price_pair(T::SelfAssetId::get(), quote.clone()) { - >::remove(quote); + >::remove(quote); pair } else { let pair = AssetPricePair { @@ -301,7 +301,7 @@ pub mod pallet { quote: quote.clone(), price, }; - >::insert(quote, pair.clone()); + >::insert(quote, pair.clone()); pair }, ) From 2ebb65e1cc55131d2f0966df902129569fa32140 Mon Sep 17 00:00:00 2001 From: clearloop Date: Tue, 15 Jun 2021 19:16:06 +0800 Subject: [PATCH 6/7] feat(price-feed): update docs and remove removing intial pair --- pallets/asset-index/src/lib.rs | 5 ++++- pallets/price-feed/src/lib.rs | 9 ++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/pallets/asset-index/src/lib.rs b/pallets/asset-index/src/lib.rs index 0100699726..0c436535fa 100644 --- a/pallets/asset-index/src/lib.rs +++ b/pallets/asset-index/src/lib.rs @@ -192,7 +192,10 @@ pub mod pallet { let caller = ensure_signed(origin)?; // Store intial price pair if not exists - T::PriceFeed::ensure_price(asset_id, Price::from_inner((value / units).into()))?; + T::PriceFeed::ensure_price( + asset_id, + Price::from_inner(value.saturating_mul(units).into()), + )?; >::add_asset( &asset_id, diff --git a/pallets/price-feed/src/lib.rs b/pallets/price-feed/src/lib.rs index f5ae39c835..0531c24385 100644 --- a/pallets/price-feed/src/lib.rs +++ b/pallets/price-feed/src/lib.rs @@ -82,7 +82,6 @@ pub mod pallet { /// This storage stores the initial price pair for quote assets based on `SelfAssetId` /// /// * insert: adding a new asset with no price pair been set yet - /// * remove: an asset has its price pair, remove automatically pub type InitialPricePairs = StorageMap<_, Blake2_128Concat, T::AssetId, AssetPricePair, OptionQuery>; @@ -283,19 +282,15 @@ pub mod pallet { Ok(AssetPricePair { base, quote, price }) } - /// Ensure quote asset has price pair based on `Self::AssetId` + /// Ensure quote asset has an initial price pair /// /// * Set initial price pair if feed not exists - /// * Remove initial price pair if feed exists fn ensure_price( quote: T::AssetId, price: Price, ) -> Result, DispatchError> { Ok( - if let Ok(pair) = Self::get_price_pair(T::SelfAssetId::get(), quote.clone()) { - >::remove(quote); - pair - } else { + if Self::get_price_pair(T::SelfAssetId::get(), quote.clone()).is_none() { let pair = AssetPricePair { base: T::SelfAssetId::get(), quote: quote.clone(), From 52a1661a5de8da8e6ff7c8783d10edcf06f86a89 Mon Sep 17 00:00:00 2001 From: clearloop Date: Tue, 15 Jun 2021 19:41:24 +0800 Subject: [PATCH 7/7] chore(price-feed): fix typos --- pallets/price-feed/src/lib.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/pallets/price-feed/src/lib.rs b/pallets/price-feed/src/lib.rs index 0531c24385..eb88157468 100644 --- a/pallets/price-feed/src/lib.rs +++ b/pallets/price-feed/src/lib.rs @@ -289,17 +289,17 @@ pub mod pallet { quote: T::AssetId, price: Price, ) -> Result, DispatchError> { - Ok( - if Self::get_price_pair(T::SelfAssetId::get(), quote.clone()).is_none() { - let pair = AssetPricePair { - base: T::SelfAssetId::get(), - quote: quote.clone(), - price, - }; - >::insert(quote, pair.clone()); - pair - }, - ) + let pair = AssetPricePair { + base: T::SelfAssetId::get(), + quote: quote.clone(), + price, + }; + + if Self::get_price_pair(T::SelfAssetId::get(), quote.clone()).is_err() { + >::insert(quote, pair.clone()); + } + + Ok(pair) } }