Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Hybrid Router clippy and tests #1291

Merged
merged 4 commits into from
Mar 29, 2024
Merged

Fix Hybrid Router clippy and tests #1291

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2b1fef5
glue everything together
Chralt98 Mar 26, 2024
bada5cd
add hybrid router to configuration
Chralt98 Mar 27, 2024
993d21f
fix tests
Chralt98 Mar 28, 2024
243a0e2
fix conditional tests
Chralt98 Mar 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions primitives/src/constants.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ pub const GLOBAL_DISPUTES_PALLET_ID: PalletId = PalletId(*b"zge/gldp");
/// Lock identifier, mainly used for the locks on the accounts.
pub const GLOBAL_DISPUTES_LOCK_ID: [u8; 8] = *b"zge/gdlk";

// Hybrid Router
/// Pallet identifier, mainly used for named balance reserves.
pub const HYBRID_ROUTER_PALLET_ID: PalletId = PalletId(*b"zge/hybr");

// Liqudity Mining
/// Pallet identifier, mainly used for named balance reserves.
pub const LM_PALLET_ID: PalletId = PalletId(*b"zge/lymg");
Expand Down
6 changes: 6 additions & 0 deletions primitives/src/constants/mock.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ parameter_types! {
pub const VotingOutcomeFee: Balance = 100 * CENT;
}

// Hybrid Router parameters
parameter_types! {
pub const HybridRouterPalletId: PalletId = PalletId(*b"zge/hybr");
pub const MaxOrders: u32 = 100;
}

// Liquidity Mining parameters
parameter_types! {
pub const LiquidityMiningPalletId: PalletId = PalletId(*b"zge/lymg");
Expand Down
6 changes: 5 additions & 1 deletion runtime/battery-station/src/parameters.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,11 @@ parameter_types! {
/// The maximum number of public proposals that can exist at any time.
pub const MaxProposals: u32 = 100;

// Hybrid Router parameters
pub const HybridRouterPalletId: PalletId = HYBRID_ROUTER_PALLET_ID;
/// Maximum number of orders that can be placed in a single trade transaction.
pub const MaxOrders: u32 = 100;

// Identity
/// The amount held on deposit for a registered identity
pub const BasicDeposit: Balance = deposit(1, 258);
Expand Down Expand Up @@ -326,7 +331,6 @@ parameter_types! {

// Orderbook parameters
pub const OrderbookPalletId: PalletId = ORDERBOOK_PALLET_ID;
pub const MaxOrders: u32 = 100;

// Parimutuel parameters
pub const MinBetSize: Balance = 100 * ExistentialDeposit::get();
Expand Down
6 changes: 6 additions & 0 deletions runtime/common/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ macro_rules! decl_common_types {
AuthorizedPalletId::get(),
CourtPalletId::get(),
GlobalDisputesPalletId::get(),
HybridRouterPalletId::get(),
LiquidityMiningPalletId::get(),
OrderbookPalletId::get(),
ParimutuelPalletId::get(),
Expand Down Expand Up @@ -1282,11 +1283,16 @@ macro_rules! impl_config_traits {

impl zrml_hybrid_router::Config for Runtime {
type AssetManager = AssetManager;
#[cfg(feature = "runtime-benchmarks")]
type AmmPoolDeployer = NeoSwaps;
#[cfg(feature = "runtime-benchmarks")]
type CompleteSetOperations = PredictionMarkets;
type MarketCommons = MarketCommons;
type Amm = NeoSwaps;
type OrderBook = Orderbook;
type MaxOrders = MaxOrders;
type RuntimeEvent = RuntimeEvent;
type PalletId = HybridRouterPalletId;
type WeightInfo = zrml_hybrid_router::weights::WeightInfo<Runtime>;
}
};
Expand Down
6 changes: 5 additions & 1 deletion runtime/zeitgeist/src/parameters.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,11 @@ parameter_types! {
/// The maximum number of public proposals that can exist at any time.
pub const MaxProposals: u32 = 100;

// Hybrid Router parameters
pub const HybridRouterPalletId: PalletId = HYBRID_ROUTER_PALLET_ID;
/// Maximum number of orders that can be placed in a single trade transaction.
pub const MaxOrders: u32 = 100;

// Identity
/// The amount held on deposit for a registered identity
pub const BasicDeposit: Balance = deposit(1, 258);
Expand Down Expand Up @@ -326,7 +331,6 @@ parameter_types! {

// Orderbook parameters
pub const OrderbookPalletId: PalletId = ORDERBOOK_PALLET_ID;
pub const MaxOrders: u32 = 100;

// Parimutuel parameters
pub const MinBetSize: Balance = 100 * ExistentialDeposit::get();
Expand Down
76 changes: 56 additions & 20 deletions zrml/hybrid-router/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,36 +9,70 @@ sp-runtime = { workspace = true }
zeitgeist-primitives = { workspace = true }
zrml-market-commons = { workspace = true }


orml-asset-registry = { workspace = true, optional = true }
orml-currencies = { workspace = true, optional = true }
orml-tokens = { workspace = true, optional = true }
pallet-balances = { workspace = true, optional = true }
pallet-randomness-collective-flip = { workspace = true, optional = true }
pallet-timestamp = { workspace = true, optional = true }
pallet-treasury = { workspace = true, optional = true }
pallet-xcm = { workspace = true, optional = true }
serde = { workspace = true, optional = true }
sp-io = { workspace = true, optional = true }
xcm = { workspace = true, optional = true }
xcm-builder = { workspace = true, optional = true }
zrml-authorized = { workspace = true, optional = true }
zrml-court = { workspace = true, optional = true }
zrml-global-disputes = { workspace = true, optional = true }
zrml-liquidity-mining = { workspace = true, optional = true }
zrml-neo-swaps = { workspace = true, optional = true }
zrml-orderbook = { workspace = true, optional = true }
zrml-prediction-markets = { workspace = true, optional = true }
zrml-simple-disputes = { workspace = true, optional = true }
zrml-swaps = { workspace = true, optional = true }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We definitely don't need all of these dependencies. Please reduce this to a minimal set and remove/mark everything else as dev dependency.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your zrml-neo-swaps pallet uses the setup with feature mock too. The reason why I did this was because of compiling the crate zrml-hybrid-router with all features. I got this in the mock:

#[cfg(feature = "parachain")]
zrml_prediction_markets::impl_mock_registry! {
    MockRegistry,
    CurrencyId,
    Balance,
    zeitgeist_primitives::types::CustomMetadata
}

impl_mock_registry uses feature mock and parachain together here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You meant using the dev dependencies, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, my bad, you're right! Marking them as optional is actually correct. Still though: At the very least you don't need zrml-swaps in the dependencies.


[dev-dependencies]
env_logger = { workspace = true }
orml-currencies = { workspace = true, features = ["default"] }
orml-tokens = { workspace = true, features = ["default"] }
pallet-balances = { workspace = true, features = ["default"] }
pallet-randomness-collective-flip = { workspace = true, features = ["default"] }
pallet-timestamp = { workspace = true, features = ["default"] }
pallet-treasury = { workspace = true, features = ["default"] }
sp-io = { workspace = true, features = ["default"] }
zeitgeist-primitives = { workspace = true, features = ["mock", "default"] }
zrml-authorized = { workspace = true, features = ["default"] }
zrml-court = { workspace = true, features = ["default"] }
zrml-global-disputes = { workspace = true, features = ["default"] }
zrml-liquidity-mining = { workspace = true, features = ["default"] }
zrml-market-commons = { workspace = true, features = ["default"] }
zrml-neo-swaps = { workspace = true, features = ["default"] }
zrml-orderbook = { workspace = true, features = ["default"] }
zrml-prediction-markets = { workspace = true, features = ["default"] }
zrml-rikiddo = { workspace = true, features = ["default"] }
zrml-simple-disputes = { workspace = true, features = ["default"] }
zrml-swaps = { workspace = true, features = ["default"] }

test-case = { workspace = true }
zrml-hybrid-router = { workspace = true, features = ["mock"] }

[features]
default = ["std"]
mock = [
"orml-asset-registry/default",
"orml-currencies/default",
"orml-tokens/default",
"pallet-balances/default",
"pallet-randomness-collective-flip/default",
"pallet-timestamp/default",
"pallet-treasury/default",
"pallet-xcm/default",
"serde/default",
"sp-io/default",
"xcm/default",
"zeitgeist-primitives/mock",
"zrml-market-commons/default",
"zrml-neo-swaps/default",
"zrml-orderbook/default",
"zrml-prediction-markets/default",
"zrml-prediction-markets/mock",
"zrml-simple-disputes/default",
"zrml-swaps/default",
"zrml-authorized/default",
"zrml-court/default",
"zrml-global-disputes/default",
"zrml-liquidity-mining/default",
]
parachain = [
"zrml-prediction-markets/parachain",
]
runtime-benchmarks = [
"frame-benchmarking/runtime-benchmarks",
"frame-support/runtime-benchmarks",
"frame-system/runtime-benchmarks",
"xcm-builder/runtime-benchmarks",
"pallet-xcm/runtime-benchmarks",
]
std = [
"frame-benchmarking?/std",
Expand All @@ -47,6 +81,8 @@ std = [
"orml-traits/std",
"parity-scale-codec/std",
"sp-runtime/std",
"xcm-builder/std",
"pallet-xcm/std",
"zeitgeist-primitives/std",
"zrml-market-commons/std",
]
Expand Down
2 changes: 0 additions & 2 deletions zrml/hybrid-router/src/benchmarking.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,6 @@ use zeitgeist_primitives::{
};
use zrml_market_commons::MarketCommonsPalletApi;

pub const MIN_SPOT_PRICE: u128 = CENT / 2;

// Same behavior as `assert_ok!`, except that it wraps the call inside a transaction layer. Required
// when calling into functions marked `require_transactional` to avoid a `Transactional(NoLayer)`
// error.
Expand Down
36 changes: 26 additions & 10 deletions zrml/hybrid-router/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,20 +41,22 @@ mod pallet {
use core::marker::PhantomData;
use frame_support::{
ensure,
pallet_prelude::{Decode, DispatchError, Encode, TypeInfo},
pallet_prelude::DispatchError,
require_transactional,
traits::{IsType, StorageVersion},
BoundedVec, RuntimeDebug,
PalletId,
};
use frame_system::{
ensure_signed,
pallet_prelude::{BlockNumberFor, OriginFor},
};
use orml_traits::MultiCurrency;
use sp_runtime::{
traits::{CheckedSub, Get, SaturatedConversion, Zero},
ArithmeticError, DispatchResult,
traits::{Get, Zero},
DispatchResult,
};
#[cfg(feature = "runtime-benchmarks")]
use zeitgeist_primitives::traits::{CompleteSetOperationsApi, DeployPoolApi};
use zeitgeist_primitives::{
math::{
checked_ops_res::CheckedSubRes,
Expand All @@ -71,6 +73,20 @@ mod pallet {
/// The API to handle different asset classes.
type AssetManager: MultiCurrency<Self::AccountId, CurrencyId = AssetOf<Self>>;

#[cfg(feature = "runtime-benchmarks")]
type AmmPoolDeployer: DeployPoolApi<
AccountId = AccountIdOf<Self>,
Balance = BalanceOf<Self>,
MarketId = MarketIdOf<Self>,
>;

#[cfg(feature = "runtime-benchmarks")]
type CompleteSetOperations: CompleteSetOperationsApi<
AccountId = AccountIdOf<Self>,
Balance = BalanceOf<Self>,
MarketId = MarketIdOf<Self>,
>;

/// The identifier of individual markets.
type MarketCommons: MarketCommonsPalletApi<
AccountId = Self::AccountId,
Expand Down Expand Up @@ -100,6 +116,8 @@ mod pallet {
OrderId = OrderId,
>;

type PalletId: Get<PalletId>;

/// The event type for this pallet.
type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;

Expand All @@ -109,7 +127,6 @@ mod pallet {

/// The current storage version.
const STORAGE_VERSION: StorageVersion = StorageVersion::new(0);
const LOG_TARGET: &str = "runtime::zrml-hybrid-router";

pub(crate) type AssetOf<T> = Asset<MarketIdOf<T>>;
pub(crate) type AccountIdOf<T> = <T as frame_system::Config>::AccountId;
Expand All @@ -121,7 +138,6 @@ mod pallet {
pub(crate) type MomentOf<T> = <<T as Config>::MarketCommons as MarketCommonsPalletApi>::Moment;
pub(crate) type MarketOf<T> =
Market<AccountIdOf<T>, BalanceOf<T>, BlockNumberFor<T>, MomentOf<T>, Asset<MarketIdOf<T>>>;
pub(crate) type OrdersOf<T> = BoundedVec<OrderId, <T as Config>::MaxOrders>;

#[pallet::pallet]
#[pallet::storage_version(STORAGE_VERSION)]
Expand Down Expand Up @@ -195,7 +211,7 @@ mod pallet {
///
/// Complexity: `O(n)`
#[pallet::call_index(0)]
#[pallet::weight(T::WeightInfo::buy())]
#[pallet::weight(T::WeightInfo::buy(*asset_count as u32, orders.len() as u32))]
#[frame_support::transactional]
pub fn buy(
origin: OriginFor<T>,
Expand Down Expand Up @@ -250,7 +266,7 @@ mod pallet {
///
/// Complexity: `O(n)`
#[pallet::call_index(1)]
#[pallet::weight(T::WeightInfo::buy())]
#[pallet::weight(T::WeightInfo::sell(*asset_count as u32, orders.len() as u32))]
#[frame_support::transactional]
pub fn sell(
origin: OriginFor<T>,
Expand Down Expand Up @@ -286,9 +302,9 @@ mod pallet {
{
/// Returns a vector of assets corresponding to the given market ID and market type.
/// For scalar outcomes, the returned vector is [LONG, SHORT].
/// For categorical outcomes,
/// For categorical outcomes,
/// the vector starts with the lowest and ends with the highest categorical outcome.
///
///
/// # Arguments
///
/// * `market_id` - The ID of the market.
Expand Down
Loading
Loading