From d8311a181a600a09d1105529a18e2c04a9cbe669 Mon Sep 17 00:00:00 2001
From: Xiliang Chen <xlchen1291@gmail.com>
Date: Tue, 30 Jun 2020 10:56:17 +1200
Subject: [PATCH] Refactor (#220)
* refactor auction
* refactor gradually-update
* oracle cleanup
---
auction/src/lib.rs | 74 +++++++++++++++--------------
gradually-update/src/lib.rs | 76 +++++++++++++++---------------
oracle/src/default_combine_data.rs | 14 +++---
oracle/src/lib.rs | 29 ++++++++----
oracle/src/timestamped_value.rs | 12 -----
5 files changed, 102 insertions(+), 103 deletions(-)
delete mode 100644 oracle/src/timestamped_value.rs
diff --git a/auction/src/lib.rs b/auction/src/lib.rs
index 6cd3a7a..ef08e4d 100644
--- a/auction/src/lib.rs
+++ b/auction/src/lib.rs
@@ -70,40 +70,44 @@ decl_module! {
pub fn bid(origin, id: T::AuctionId, #[compact] value: T::Balance) {
let from = ensure_signed(origin)?;
- let mut auction = <Auctions<T>>::get(id).ok_or(Error::<T>::AuctionNotExist)?;
+ <Auctions<T>>::try_mutate_exists(id, |auction| -> DispatchResult {
+ let mut auction = auction.as_mut().ok_or(Error::<T>::AuctionNotExist)?;
+
+ let block_number = <frame_system::Module<T>>::block_number();
+
+ // make sure auction is started
+ ensure!(block_number >= auction.start, Error::<T>::AuctionNotStarted);
+
+ if let Some(ref current_bid) = auction.bid {
+ ensure!(value > current_bid.1, Error::<T>::InvalidBidPrice);
+ } else {
+ ensure!(!value.is_zero(), Error::<T>::InvalidBidPrice);
+ }
+ let bid_result = T::Handler::on_new_bid(
+ block_number,
+ id,
+ (from.clone(), value),
+ auction.bid.clone(),
+ );
+
+ ensure!(bid_result.accept_bid, Error::<T>::BidNotAccepted);
+ match bid_result.auction_end_change {
+ Change::NewValue(new_end) => {
+ if let Some(old_end_block) = auction.end {
+ <AuctionEndTime<T>>::remove(&old_end_block, id);
+ }
+ if let Some(new_end_block) = new_end {
+ <AuctionEndTime<T>>::insert(&new_end_block, id, ());
+ }
+ auction.end = new_end;
+ },
+ Change::NoChange => {},
+ }
+ auction.bid = Some((from.clone(), value));
+
+ Ok(())
+ })?;
- let block_number = <frame_system::Module<T>>::block_number();
-
- // make sure auction is started
- ensure!(block_number >= auction.start, Error::<T>::AuctionNotStarted);
-
- if let Some(ref current_bid) = auction.bid {
- ensure!(value > current_bid.1, Error::<T>::InvalidBidPrice);
- } else {
- ensure!(!value.is_zero(), Error::<T>::InvalidBidPrice);
- }
- let bid_result = T::Handler::on_new_bid(
- block_number,
- id,
- (from.clone(), value),
- auction.bid.clone(),
- );
-
- ensure!(bid_result.accept_bid, Error::<T>::BidNotAccepted);
- match bid_result.auction_end_change {
- Change::NewValue(new_end) => {
- if let Some(old_end_block) = auction.end {
- <AuctionEndTime<T>>::remove(&old_end_block, id);
- }
- if let Some(new_end_block) = new_end {
- <AuctionEndTime<T>>::insert(&new_end_block, id, ());
- }
- auction.end = new_end;
- },
- Change::NoChange => {},
- }
- auction.bid = Some((from.clone(), value));
- <Auctions<T>>::insert(id, auction);
Self::deposit_event(RawEvent::Bid(id, from, value));
}
@@ -127,7 +131,7 @@ impl<T: Trait> Module<T> {
fn _on_finalize(now: T::BlockNumber) {
for (auction_id, _) in <AuctionEndTime<T>>::drain_prefix(&now) {
if let Some(auction) = <Auctions<T>>::take(&auction_id) {
- T::Handler::on_auction_ended(auction_id, auction.bid.clone());
+ T::Handler::on_auction_ended(auction_id, auction.bid);
}
}
}
@@ -171,7 +175,7 @@ impl<T: Trait> Auction<T::AccountId, T::BlockNumber> for Module<T> {
fn remove_auction(id: Self::AuctionId) {
if let Some(auction) = <Auctions<T>>::take(&id) {
if let Some(end_block) = auction.end {
- <AuctionEndTime<T>>::remove(&end_block, id);
+ <AuctionEndTime<T>>::remove(end_block, id);
}
}
}
diff --git a/gradually-update/src/lib.rs b/gradually-update/src/lib.rs
index 6290afd..7553d3c 100644
--- a/gradually-update/src/lib.rs
+++ b/gradually-update/src/lib.rs
@@ -9,7 +9,7 @@ use frame_support::{
};
use frame_system::{self as system, ensure_root};
-use sp_runtime::{traits::SaturatedConversion, RuntimeDebug};
+use sp_runtime::{traits::SaturatedConversion, DispatchResult, RuntimeDebug};
use sp_std::prelude::Vec;
mod mock;
@@ -83,11 +83,13 @@ decl_module! {
ensure!(current_value.len() == update.target_value.len(), Error::<T>::InvalidTargetValue);
}
- let mut gradually_updates = GraduallyUpdates::get();
- ensure!(!gradually_updates.contains(&update), Error::<T>::GraduallyUpdateHasExisted);
+ GraduallyUpdates::try_mutate(|gradually_updates| -> DispatchResult {
+ ensure!(!gradually_updates.contains(&update), Error::<T>::GraduallyUpdateHasExisted);
- gradually_updates.push(update.clone());
- GraduallyUpdates::put(gradually_updates);
+ gradually_updates.push(update.clone());
+
+ Ok(())
+ })?;
Self::deposit_event(RawEvent::GraduallyUpdate(update.key, update.per_block, update.target_value));
}
@@ -97,13 +99,14 @@ decl_module! {
pub fn cancel_gradually_update(origin, key: StorageKey) {
T::DispatchOrigin::try_origin(origin).map(|_| ()).or_else(ensure_root)?;
- let gradually_updates: Vec<GraduallyUpdate> = GraduallyUpdates::get()
- .into_iter()
- .filter(|item| item.key != key)
- .collect();
+ GraduallyUpdates::try_mutate(|gradually_updates| -> DispatchResult {
+ let old_len = gradually_updates.len();
+ gradually_updates.retain(|item| item.key != key);
- ensure!(GraduallyUpdates::decode_len().unwrap_or_default() - gradually_updates.len() == 1, Error::<T>::CancelGradullyUpdateNotExisted);
- GraduallyUpdates::put(gradually_updates);
+ ensure!(gradually_updates.len() != old_len, Error::<T>::CancelGradullyUpdateNotExisted);
+
+ Ok(())
+ })?;
Self::deposit_event(RawEvent::CancelGraduallyUpdate(key));
}
@@ -121,44 +124,41 @@ impl<T: Trait> Module<T> {
return;
}
- let gradually_updates = GraduallyUpdates::get();
+ let mut gradually_updates = GraduallyUpdates::get();
let initial_count = gradually_updates.len();
- let gradually_updates = gradually_updates
- .into_iter()
- .filter(|update| {
- let mut keep = true;
- let current_value = storage::unhashed::get::<StorageValue>(&update.key).unwrap_or_default();
- let current_value_u128 = u128::from_le_bytes(Self::convert_vec_to_u8(¤t_value));
+ gradually_updates.retain(|update| {
+ let mut keep = true;
+ let current_value = storage::unhashed::get::<StorageValue>(&update.key).unwrap_or_default();
+ let current_value_u128 = u128::from_le_bytes(Self::convert_vec_to_u8(¤t_value));
- let frequency_u128: u128 = T::UpdateFrequency::get().saturated_into();
+ let frequency_u128: u128 = T::UpdateFrequency::get().saturated_into();
- let step = u128::from_le_bytes(Self::convert_vec_to_u8(&update.per_block));
- let step_u128 = step.checked_mul(frequency_u128).unwrap();
+ let step = u128::from_le_bytes(Self::convert_vec_to_u8(&update.per_block));
+ let step_u128 = step.checked_mul(frequency_u128).unwrap();
- let target_u128 = u128::from_le_bytes(Self::convert_vec_to_u8(&update.target_value));
+ let target_u128 = u128::from_le_bytes(Self::convert_vec_to_u8(&update.target_value));
- let new_value_u128 = if current_value_u128 > target_u128 {
- (current_value_u128.checked_sub(step_u128).unwrap()).max(target_u128)
- } else {
- (current_value_u128.checked_add(step_u128).unwrap()).min(target_u128)
- };
+ let new_value_u128 = if current_value_u128 > target_u128 {
+ (current_value_u128.checked_sub(step_u128).unwrap()).max(target_u128)
+ } else {
+ (current_value_u128.checked_add(step_u128).unwrap()).min(target_u128)
+ };
- // current_value equal target_value, remove gradually_update
- if new_value_u128 == target_u128 {
- keep = false;
- }
+ // current_value equal target_value, remove gradually_update
+ if new_value_u128 == target_u128 {
+ keep = false;
+ }
- let mut value = new_value_u128.encode();
- value.truncate(update.target_value.len());
+ let mut value = new_value_u128.encode();
+ value.truncate(update.target_value.len());
- storage::unhashed::put(&update.key, &value);
+ storage::unhashed::put(&update.key, &value);
- Self::deposit_event(RawEvent::GraduallyUpdateBlockNumber(now, update.key.clone(), value));
+ Self::deposit_event(RawEvent::GraduallyUpdateBlockNumber(now, update.key.clone(), value));
- keep
- })
- .collect::<Vec<_>>();
+ keep
+ });
// gradually_update has finished. Remove it from GraduallyUpdates.
if gradually_updates.len() < initial_count {
diff --git a/oracle/src/default_combine_data.rs b/oracle/src/default_combine_data.rs
index cdcd9ac..3a0f727 100644
--- a/oracle/src/default_combine_data.rs
+++ b/oracle/src/default_combine_data.rs
@@ -17,25 +17,23 @@ where
{
fn combine_data(
_key: &T::OracleKey,
- values: Vec<TimestampedValueOf<T>>,
+ mut values: Vec<TimestampedValueOf<T>>,
prev_value: Option<TimestampedValueOf<T>>,
) -> Option<TimestampedValueOf<T>> {
let expires_in = ExpiresIn::get();
let now = T::Time::now();
- let mut valid_values = values
- .into_iter()
- .filter(|x| x.timestamp + expires_in > now)
- .collect::<Vec<TimestampedValueOf<T>>>();
- let count = valid_values.len() as u32;
+ values.retain(|x| x.timestamp + expires_in > now);
+
+ let count = values.len() as u32;
let minimum_count = MinimumCount::get();
if count < minimum_count {
return prev_value;
}
- valid_values.sort_by(|a, b| a.value.cmp(&b.value));
+ values.sort_by(|a, b| a.value.cmp(&b.value));
let median_index = count / 2;
- Some(valid_values[median_index as usize].clone())
+ Some(values[median_index as usize].clone())
}
}
diff --git a/oracle/src/lib.rs b/oracle/src/lib.rs
index e2a0896..8f9b65c 100644
--- a/oracle/src/lib.rs
+++ b/oracle/src/lib.rs
@@ -5,9 +5,8 @@
mod default_combine_data;
mod mock;
mod tests;
-mod timestamped_value;
-use codec::Encode;
+use codec::{Decode, Encode};
pub use default_combine_data::DefaultCombineData;
use frame_support::{
decl_error, decl_event, decl_module, decl_storage, ensure,
@@ -15,12 +14,14 @@ use frame_support::{
weights::{DispatchClass, Pays},
IterableStorageMap, Parameter,
};
+#[cfg(feature = "std")]
+use serde::{Deserialize, Serialize};
use sp_runtime::{
traits::Member,
transaction_validity::{
InvalidTransaction, TransactionPriority, TransactionSource, TransactionValidity, ValidTransaction,
},
- DispatchResult,
+ DispatchResult, RuntimeDebug,
};
use sp_std::{convert::TryInto, prelude::*, vec};
// FIXME: `pallet/frame-` prefix should be used for all pallet modules, but currently `frame_system`
@@ -29,7 +30,6 @@ use sp_std::{convert::TryInto, prelude::*, vec};
use frame_system::{self as system, ensure_none, ensure_root, ensure_signed};
pub use orml_traits::{CombineData, DataProvider, DataProviderExtended, OnNewData, OnRedundantCall};
use orml_utilities::OrderedSet;
-pub use timestamped_value::TimestampedValue;
use sp_application_crypto::{KeyTypeId, RuntimeAppPublic};
pub const ORACLE: KeyTypeId = KeyTypeId(*b"orac");
@@ -53,6 +53,13 @@ pub type AuthorityId = app_sr25519::Public;
type MomentOf<T> = <<T as Trait>::Time as Time>::Moment;
pub type TimestampedValueOf<T> = TimestampedValue<<T as Trait>::OracleValue, MomentOf<T>>;
+#[derive(Encode, Decode, RuntimeDebug, Eq, PartialEq, Clone, Copy)]
+#[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
+pub struct TimestampedValue<Value, Moment> {
+ pub value: Value,
+ pub timestamp: Moment,
+}
+
pub trait Trait: frame_system::Trait {
type Event: From<Event<Self>> + Into<<Self as frame_system::Trait>::Event>;
type OnNewData: OnNewData<Self::AccountId, Self::OracleKey, Self::OracleValue>;
@@ -126,7 +133,7 @@ decl_module! {
) {
ensure_none(origin.clone()).or_else(|_| ensure_root(origin))?;
let who = Self::members().0[index as usize].clone();
- Self::_feed_values(who, values)?;
+ Self::do_feed_values(who, values);
}
#[weight = 10_000_000]
@@ -192,7 +199,10 @@ impl<T: Trait> Module<T> {
pub fn get_all_values() -> Vec<(T::OracleKey, Option<TimestampedValueOf<T>>)> {
<Values<T>>::iter()
.map(|(key, _)| key)
- .map(|key| (key.clone(), Self::get_no_op(&key)))
+ .map(|key| {
+ let v = Self::get_no_op(&key);
+ (key, v)
+ })
.collect()
}
@@ -201,7 +211,7 @@ impl<T: Trait> Module<T> {
T::CombineData::combine_data(key, values, Self::values(key))
}
- fn _feed_values(who: T::AccountId, values: Vec<(T::OracleKey, T::OracleValue)>) -> DispatchResult {
+ fn do_feed_values(who: T::AccountId, values: Vec<(T::OracleKey, T::OracleValue)>) {
let now = T::Time::now();
for (key, value) in &values {
@@ -216,8 +226,6 @@ impl<T: Trait> Module<T> {
}
Self::deposit_event(RawEvent::NewFeedData(who, values));
-
- Ok(())
}
}
@@ -258,7 +266,8 @@ impl<T: Trait> DataProvider<T::OracleKey, T::OracleValue> for Module<T> {
impl<T: Trait> DataProviderExtended<T::OracleKey, T::OracleValue, T::AccountId> for Module<T> {
fn feed_value(who: T::AccountId, key: T::OracleKey, value: T::OracleValue) -> DispatchResult {
- Self::_feed_values(who, vec![(key, value)])
+ Self::do_feed_values(who, vec![(key, value)]);
+ Ok(())
}
}
diff --git a/oracle/src/timestamped_value.rs b/oracle/src/timestamped_value.rs
deleted file mode 100644
index 26b80d7..0000000
--- a/oracle/src/timestamped_value.rs
+++ /dev/null
@@ -1,12 +0,0 @@
-use codec::{Decode, Encode};
-use sp_runtime::RuntimeDebug;
-
-#[cfg(feature = "std")]
-use serde::{Deserialize, Serialize};
-
-#[derive(Encode, Decode, RuntimeDebug, Eq, PartialEq, Clone, Copy)]
-#[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
-pub struct TimestampedValue<Value, Moment> {
- pub value: Value,
- pub timestamp: Moment,
-}
--
GitLab