Skip to content

Commit

Permalink
apply review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@Chralt98
Chralt98 committed Sep 28, 2023
1 parent 9d0a1e3 commit b7d3e08
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 74 deletions.
28 changes: 14 additions & 14 deletions zrml/orderbook-v1/src/benchmarks.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@
#![allow(clippy::type_complexity)]

use super::*;
use crate::{utils::market_mock, Pallet as OrderBook};
use crate::utils::market_mock;
#[cfg(test)]
use crate::Pallet as OrderBook;
use frame_benchmarking::{account, benchmarks, whitelisted_caller};
use frame_support::dispatch::UnfilteredDispatchable;
use frame_system::RawOrigin;
Expand Down Expand Up @@ -63,11 +65,11 @@ fn order_common_parameters<T: Config>(

// Creates an order of type `order_type`. `seed` specifies the account seed,
// None will return a whitelisted account
// Returns `account`, `asset` and `order_hash`
// Returns `account`, `asset`, `order_id`
fn place_order<T: Config>(
order_type: OrderSide,
seed: Option<u32>,
) -> Result<(T::AccountId, MarketIdOf<T>, T::Hash), &'static str> {
) -> Result<(T::AccountId, MarketIdOf<T>, OrderId), &'static str> {
let (acc, outcome_asset, outcome_asset_amount, base_asset_amount, market_id) =
order_common_parameters::<T>(seed)?;

Expand All @@ -81,29 +83,27 @@ fn place_order<T: Config>(
}
.dispatch_bypass_filter(RawOrigin::Signed(acc.clone()).into())?;

let order_hash = OrderBook::<T>::order_hash(&acc, order_id);

Ok((acc, market_id, order_hash))
Ok((acc, market_id, order_id))
}

benchmarks! {
remove_order_ask {
let (caller, _, order_hash) = place_order::<T>(OrderSide::Ask, None)?;
}: remove_order(RawOrigin::Signed(caller), order_hash)
let (caller, _, order_id) = place_order::<T>(OrderSide::Ask, None)?;
}: remove_order(RawOrigin::Signed(caller), order_id)

remove_order_bid {
let (caller, _, order_hash) = place_order::<T>(OrderSide::Bid, None)?;
}: remove_order(RawOrigin::Signed(caller), order_hash)
let (caller, _, order_id) = place_order::<T>(OrderSide::Bid, None)?;
}: remove_order(RawOrigin::Signed(caller), order_id)

fill_order_ask {
let caller = generate_funded_account::<T>(None)?;
let (_, _, order_hash) = place_order::<T>(OrderSide::Ask, Some(0))?;
}: fill_order(RawOrigin::Signed(caller), order_hash, None)
let (_, _, order_id) = place_order::<T>(OrderSide::Ask, Some(0))?;
}: fill_order(RawOrigin::Signed(caller), order_id, None)

fill_order_bid {
let caller = generate_funded_account::<T>(None)?;
let (_, _, order_hash) = place_order::<T>(OrderSide::Bid, Some(0))?;
}: fill_order(RawOrigin::Signed(caller), order_hash, None)
let (_, _, order_id) = place_order::<T>(OrderSide::Bid, Some(0))?;
}: fill_order(RawOrigin::Signed(caller), order_id, None)

place_order_ask {
let (caller, outcome_asset, outcome_asset_amount, base_asset_amount, market_id) = order_common_parameters::<T>(None)?;
Expand Down
43 changes: 15 additions & 28 deletions zrml/orderbook-v1/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,8 @@ use frame_support::{
use frame_system::{ensure_signed, pallet_prelude::OriginFor};
use orml_traits::{BalanceStatus, MultiCurrency, NamedMultiReservableCurrency};
pub use pallet::*;
use parity_scale_codec::Encode;
use sp_runtime::{
traits::{CheckedSub, Get, Hash, Zero},
traits::{CheckedSub, Get, Zero},
ArithmeticError, Perquintill, SaturatedConversion, Saturating,
};
use zeitgeist_primitives::{
Expand Down Expand Up @@ -85,7 +84,6 @@ mod pallet {
pub(crate) type MarketIdOf<T> =
<<T as Config>::MarketCommons as MarketCommonsPalletApi>::MarketId;
pub(crate) type AccountIdOf<T> = <T as frame_system::Config>::AccountId;
pub(crate) type HashOf<T> = <T as frame_system::Config>::Hash;
pub(crate) type OrderOf<T> = Order<AccountIdOf<T>, BalanceOf<T>, MarketIdOf<T>>;
pub(crate) type MomentOf<T> = <<T as Config>::MarketCommons as MarketCommonsPalletApi>::Moment;
pub(crate) type MarketCommonsBalanceOf<T> =
Expand All @@ -108,7 +106,7 @@ mod pallet {
pub type NextOrderId<T> = StorageValue<_, OrderId, ValueQuery>;

#[pallet::storage]
pub type Orders<T: Config> = StorageMap<_, Twox64Concat, T::Hash, OrderOf<T>, OptionQuery>;
pub type Orders<T: Config> = StorageMap<_, Twox64Concat, OrderId, OrderOf<T>, OptionQuery>;

#[pallet::event]
#[pallet::generate_deposit(fn deposit_event)]
Expand All @@ -117,20 +115,19 @@ mod pallet {
T: Config,
{
OrderFilled {
order_hash: HashOf<T>,
order_id: OrderId,
maker: AccountIdOf<T>,
taker: AccountIdOf<T>,
filled: BalanceOf<T>,
unfilled_outcome_asset_amount: BalanceOf<T>,
unfilled_base_asset_amount: BalanceOf<T>,
},
OrderPlaced {
order_hash: HashOf<T>,
order_id: OrderId,
order: OrderOf<T>,
},
OrderRemoved {
order_hash: HashOf<T>,
order_id: OrderId,
maker: T::AccountId,
},
}
Expand Down Expand Up @@ -167,13 +164,10 @@ mod pallet {
T::WeightInfo::remove_order_ask().max(T::WeightInfo::remove_order_bid())
)]
#[transactional]
pub fn remove_order(
origin: OriginFor<T>,
order_hash: T::Hash,
) -> DispatchResultWithPostInfo {
pub fn remove_order(origin: OriginFor<T>, order_id: OrderId) -> DispatchResultWithPostInfo {
let sender = ensure_signed(origin)?;

let order_data = <Orders<T>>::get(order_hash).ok_or(Error::<T>::OrderDoesNotExist)?;
let order_data = <Orders<T>>::get(order_id).ok_or(Error::<T>::OrderDoesNotExist)?;

let maker = &order_data.maker;
ensure!(sender == *maker, Error::<T>::NotOrderCreator);
Expand All @@ -197,9 +191,9 @@ mod pallet {
}
}

<Orders<T>>::remove(order_hash);
<Orders<T>>::remove(order_id);

Self::deposit_event(Event::OrderRemoved { order_hash, maker: maker.clone() });
Self::deposit_event(Event::OrderRemoved { order_id, maker: maker.clone() });

match order_data.side {
OrderSide::Bid => Ok(Some(T::WeightInfo::remove_order_bid()).into()),
Expand All @@ -222,13 +216,12 @@ mod pallet {
#[transactional]
pub fn fill_order(
origin: OriginFor<T>,
order_hash: T::Hash,
order_id: OrderId,
maker_partial_fill: Option<BalanceOf<T>>,
) -> DispatchResultWithPostInfo {
let taker = ensure_signed(origin)?;

let mut order_data =
<Orders<T>>::get(order_hash).ok_or(Error::<T>::OrderDoesNotExist)?;
let mut order_data = <Orders<T>>::get(order_id).ok_or(Error::<T>::OrderDoesNotExist)?;
let market = T::MarketCommons::market(&order_data.market_id)?;
ensure!(market.scoring_rule == ScoringRule::Orderbook, Error::<T>::InvalidScoringRule);
ensure!(market.status == MarketStatus::Active, Error::<T>::MarketIsNotActive);
Expand Down Expand Up @@ -340,13 +333,13 @@ mod pallet {
unfilled_outcome_asset_amount.saturating_add(unfilled_base_asset_amount);

if total_unfilled.is_zero() {
<Orders<T>>::remove(order_hash);
<Orders<T>>::remove(order_id);
} else {
<Orders<T>>::insert(order_hash, order_data.clone());
<Orders<T>>::insert(order_id, order_data.clone());
}

Self::deposit_event(Event::OrderFilled {
order_hash,
order_id,
maker,
taker: taker.clone(),
filled: maker_fill,
Expand Down Expand Up @@ -393,10 +386,8 @@ mod pallet {
let order_id = <NextOrderId<T>>::get();
let next_order_id = order_id.checked_add(1).ok_or(ArithmeticError::Overflow)?;

let order_hash = Self::order_hash(&who, order_id);
let order = Order {
market_id,
order_id,
side: side.clone(),
maker: who.clone(),
outcome_asset,
Expand Down Expand Up @@ -424,9 +415,9 @@ mod pallet {
}
}

<Orders<T>>::insert(order_hash, order.clone());
<Orders<T>>::insert(order_id, order.clone());
<NextOrderId<T>>::put(next_order_id);
Self::deposit_event(Event::OrderPlaced { order_hash, order_id, order });
Self::deposit_event(Event::OrderPlaced { order_id, order });

match side {
OrderSide::Bid => Ok(Some(T::WeightInfo::place_order_bid()).into()),
Expand All @@ -442,10 +433,6 @@ mod pallet {
T::PalletId::get().0
}

pub fn order_hash(creator: &T::AccountId, order_id: OrderId) -> T::Hash {
(creator, order_id).using_encoded(T::Hashing::hash)
}

pub fn outcome_assets(
market_id: MarketIdOf<T>,
market: &MarketOf<T>,
Expand Down
Loading

0 comments on commit b7d3e08

Please sign in to comment.