Skip to content

Commit

Permalink
fix pr comments
Browse files Browse the repository at this point in the history
  • Loading branch information
akildemir committed Nov 10, 2023
1 parent 074464b commit 4a66526
Show file tree
Hide file tree
Showing 22 changed files with 179 additions and 539 deletions.
1 change: 0 additions & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ jobs:
-p serai-primitives \
-p serai-coins-primitives \
-p serai-coins-pallet \
-p serai-dex-primitives \
-p serai-dex-pallet \
-p serai-validator-sets-primitives \
-p serai-validator-sets-pallet \
Expand Down
13 changes: 0 additions & 13 deletions Cargo.lock

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

4 changes: 2 additions & 2 deletions coordinator/src/substrate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,8 @@ async fn handle_batch_and_burns<D: Db, Pro: Processors>(
}
}

for burn in serai.coins().burn_events().await? {
if let CoinsEvent::Burn { from: _, instruction } = burn {
for burn in serai.coins().burn_with_instruction_events().await? {
if let CoinsEvent::BurnWithInstruction { from: _, instruction } = burn {
let network = instruction.balance.coin.network();
network_had_event(&mut burns, &mut batches, network);

Expand Down
1 change: 0 additions & 1 deletion deny.toml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ exceptions = [
{ allow = ["AGPL-3.0"], name = "serai-coordinator" },

{ allow = ["AGPL-3.0"], name = "serai-coins-pallet" },
{ allow = ["AGPL-3.0"], name = "serai-dex-primitives" },
{ allow = ["AGPL-3.0"], name = "serai-dex-pallet" },

{ allow = ["AGPL-3.0"], name = "serai-in-instructions-pallet" },
Expand Down
16 changes: 12 additions & 4 deletions substrate/client/src/serai/coins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ impl<'a> SeraiCoins<'a> {
self.0.events::<Coins, _>(|event| matches!(event, CoinsEvent::Mint { .. })).await
}

pub async fn burn_events(&self) -> Result<Vec<CoinsEvent>, SeraiError> {
self.0.events::<Coins, _>(|event| matches!(event, CoinsEvent::Burn { .. })).await
pub async fn burn_with_instruction_events(&self) -> Result<Vec<CoinsEvent>, SeraiError> {
self.0.events::<Coins, _>(|event| matches!(event, CoinsEvent::BurnWithInstruction { .. })).await
}

pub async fn coin_supply(&self, coin: Coin) -> Result<Amount, SeraiError> {
Expand Down Expand Up @@ -64,7 +64,15 @@ impl<'a> SeraiCoins<'a> {
)
}

pub fn burn(instruction: OutInstructionWithBalance) -> Payload<Composite<()>> {
Payload::new(PALLET, "burn", scale_composite(coins::Call::<Runtime>::burn { instruction }))
pub fn burn(balance: Balance) -> Payload<Composite<()>> {
Payload::new(PALLET, "burn", scale_composite(coins::Call::<Runtime>::burn { balance }))
}

pub fn burn_with_instruction(instruction: OutInstructionWithBalance) -> Payload<Composite<()>> {
Payload::new(
PALLET,
"burn_with_instruction",
scale_composite(coins::Call::<Runtime>::burn_with_instruction { instruction }),
)
}
}
6 changes: 3 additions & 3 deletions substrate/client/tests/burn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ serai_test!(
&serai
.sign(
&PairSigner::new(pair),
&SeraiCoins::burn(instruction.clone()),
&SeraiCoins::burn_with_instruction(instruction.clone()),
0,
BaseExtrinsicParamsBuilder::new(),
)
Expand All @@ -102,8 +102,8 @@ serai_test!(
.await;

let serai = serai.as_of(block).coins();
let events = serai.burn_events().await.unwrap();
assert_eq!(events, vec![CoinsEvent::Burn { from: address.into(), instruction }]);
let events = serai.burn_with_instruction_events().await.unwrap();
assert_eq!(events, vec![CoinsEvent::BurnWithInstruction { from: address.into(), instruction }]);
assert_eq!(serai.coin_supply(coin).await.unwrap(), Amount(0));
assert_eq!(serai.coin_balance(coin, address).await.unwrap(), Amount(0));
})
Expand Down
69 changes: 35 additions & 34 deletions substrate/coins/pallet/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

#[frame_support::pallet]
pub mod pallet {
use sp_std::vec::Vec;
use sp_std::{vec::Vec, any::TypeId};
use sp_core::sr25519::Public;
use sp_runtime::{
traits::{DispatchInfoOf, PostDispatchInfoOf},
Expand All @@ -18,6 +18,8 @@ pub mod pallet {
pub use coins_primitives as primitives;
use primitives::*;

type LiquidityTokensInstance = crate::Instance1;

#[pallet::config]
pub trait Config<I: 'static = ()>: frame_system::Config<AccountId = Public> {
type RuntimeEvent: From<Event<Self, I>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;
Expand All @@ -27,12 +29,12 @@ pub mod pallet {
#[derive(Clone, PartialEq, Eq, Debug, Encode, Decode)]
pub struct GenesisConfig<T: Config<I>, I: 'static = ()> {
pub accounts: Vec<(T::AccountId, Balance)>,
pub ignore: PhantomData<I>, // TODO: just to own I.
pub _ignore: PhantomData<I>,
}

impl<T: Config<I>, I: 'static> Default for GenesisConfig<T, I> {
fn default() -> Self {
GenesisConfig { accounts: Default::default(), ignore: Default::default() }
GenesisConfig { accounts: Default::default(), _ignore: Default::default() }
}
}

Expand All @@ -47,8 +49,8 @@ pub mod pallet {
#[pallet::generate_deposit(fn deposit_event)]
pub enum Event<T: Config<I>, I: 'static = ()> {
Mint { to: Public, balance: Balance },
Burn { from: Public, instruction: OutInstructionWithBalance },
SriBurn { from: Public, amount: Amount },
Burn { from: Public, balance: Balance },
BurnWithInstruction { from: Public, instruction: OutInstructionWithBalance },
Transfer { from: Public, to: Public, balance: Balance },
}

Expand Down Expand Up @@ -95,7 +97,7 @@ pub mod pallet {
// we can unwrap, we are not burning more then what we have
// If this errors, it'll halt the runtime however (due to being called at the start of every
// block), requiring extra care when reviewing
Self::burn_sri(FEE_ACCOUNT.into(), amount).unwrap();
Self::burn_internal(FEE_ACCOUNT.into(), Balance { coin, amount }).unwrap();
Weight::zero() // TODO
}
}
Expand Down Expand Up @@ -153,8 +155,8 @@ pub mod pallet {
Ok(())
}

// Burn `balance` from the specified account.
pub fn burn_internal(from: Public, balance: Balance) -> Result<(), Error<T, I>> {
/// Burn `balance` from the specified account.
fn burn_internal(from: Public, balance: Balance) -> Result<(), Error<T, I>> {
// don't waste time if amount == 0
if balance.amount.0 == 0 {
return Ok(());
Expand All @@ -170,24 +172,6 @@ pub mod pallet {
Ok(())
}

pub fn burn_sri(from: Public, amount: Amount) -> Result<(), Error<T, I>> {
Self::burn_internal(from, Balance { coin: Coin::Serai, amount })?;
Self::deposit_event(Event::SriBurn { from, amount });
Ok(())
}

pub fn burn_non_sri(
from: Public,
instruction: OutInstructionWithBalance,
) -> Result<(), Error<T, I>> {
if instruction.balance.coin == Coin::Serai {
Err(Error::<T, I>::SriBurnNotAllowed)?;
}
Self::burn_internal(from, instruction.balance)?;
Self::deposit_event(Event::Burn { from, instruction });
Ok(())
}

/// Transfer `balance` from `from` to `to`.
pub fn transfer_internal(
from: Public,
Expand All @@ -200,12 +184,6 @@ pub mod pallet {
Self::deposit_event(Event::Transfer { from, to, balance });
Ok(())
}

pub fn minimum_balance(_coin: Coin) -> Amount {
// TODO: use precision here to determine the min amount?
// TODO: this should also match with dex Config MintMinLiquidity type.
Amount(1)
}
}

#[pallet::call]
Expand All @@ -218,11 +196,34 @@ pub mod pallet {
Ok(())
}

/// Burn `balance` from the caller.
#[pallet::call_index(1)]
#[pallet::weight((0, DispatchClass::Normal))] // TODO
pub fn burn(origin: OriginFor<T>, instruction: OutInstructionWithBalance) -> DispatchResult {
pub fn burn(origin: OriginFor<T>, balance: Balance) -> DispatchResult {
let from = ensure_signed(origin)?;
Self::burn_internal(from, balance)?;
Self::deposit_event(Event::Burn { from, balance });
Ok(())
}

/// Burn `balance` with `OutInstructionWithBalance` from the caller.
/// Errors if called for SRI or Instance1 instance of this pallet.
#[pallet::call_index(2)]
#[pallet::weight((0, DispatchClass::Normal))] // TODO
pub fn burn_with_instruction(
origin: OriginFor<T>,
instruction: OutInstructionWithBalance,
) -> DispatchResult {
if instruction.balance.coin == Coin::Serai {
Err(Error::<T, I>::SriBurnNotAllowed)?;
}
if TypeId::of::<I>() == TypeId::of::<LiquidityTokensInstance>() {
Err(Error::<T, I>::SriBurnNotAllowed)?;
}

let from = ensure_signed(origin)?;
Self::burn_non_sri(from, instruction)?;
Self::burn_internal(from, instruction.balance)?;
Self::deposit_event(Event::BurnWithInstruction { from, instruction });
Ok(())
}
}
Expand Down
5 changes: 0 additions & 5 deletions substrate/dex/pallet/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ frame-benchmarking = { git = "https://github.com/serai-dex/substrate", default-f

coins-pallet = { package = "serai-coins-pallet", path = "../../coins/pallet", default-features = false }

dex-primitives = { package = "serai-dex-primitives", path = "../primitives", default-features = false }
serai-primitives = { path = "../../primitives", default-features = false }

[features]
Expand All @@ -46,8 +45,6 @@ std = [

"serai-primitives/std",

"dex-primitives/std",

"frame-system/std",
"frame-support/std",
"frame-benchmarking?/std",
Expand All @@ -60,8 +57,6 @@ runtime-benchmarks = [
"frame-system/runtime-benchmarks",
"frame-support/runtime-benchmarks",
"frame-benchmarking/runtime-benchmarks",

"dex-primitives/runtime-benchmarks",
]
try-runtime = [
"sp-runtime/try-runtime",
Expand Down
42 changes: 17 additions & 25 deletions substrate/dex/pallet/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,24 +53,20 @@ fn create_coin<T: Config>(coin: &Coin) -> (T::AccountId, AccountIdLookupOf<T>) {
}

fn create_coin_and_pool<T: Config>(
coin1: &Coin,
coin2: &Coin,
coin: &Coin,
) -> (PoolCoinId, T::AccountId, AccountIdLookupOf<T>) {
assert_eq!(*coin1, Coin::native());
let (caller, caller_lookup) = create_coin::<T>(coin);
assert_ok!(Dex::<T>::create_pool(*coin));

let (caller, caller_lookup) = create_coin::<T>(coin2);
assert_ok!(Dex::<T>::create_pool(*coin2));

(*coin2, caller, caller_lookup)
(*coin, caller, caller_lookup)
}

benchmarks! {
add_liquidity {
let coin1 = Coin::native();
let coin2 = Coin::Bitcoin;
let (lp_token, caller, _) = create_coin_and_pool::<T>(&coin1, &coin2);
let ed: u64 = Coins::<T>::minimum_balance(coin1).0;
let add_amount: u64 = 1000 + ed;
let (lp_token, caller, _) = create_coin_and_pool::<T>(&coin2);
let add_amount: u64 = 1000;
}: _(
SystemOrigin::Signed(caller),
coin2,
Expand Down Expand Up @@ -106,9 +102,8 @@ benchmarks! {
remove_liquidity {
let coin1 = Coin::native();
let coin2 = Coin::Monero;
let (lp_token, caller, _) = create_coin_and_pool::<T>(&coin1, &coin2);
let ed: u64 = Coins::<T>::minimum_balance(coin1).0;
let add_amount: u64 = 100 * ed;
let (lp_token, caller, _) = create_coin_and_pool::<T>(&coin2);
let add_amount: u64 = 100;
let lp_minted = Dex::<T>::calc_lp_amount_for_zero_supply(
add_amount,
1000u64
Expand Down Expand Up @@ -145,18 +140,16 @@ benchmarks! {
let native = Coin::native();
let coin1 = Coin::Bitcoin;
let coin2 = Coin::Ether;
let (_, caller, _) = create_coin_and_pool::<T>(&native, &coin1);
let (_, caller, _) = create_coin_and_pool::<T>(&coin1);
let (_, _) = create_coin::<T>(&coin2);
let ed: u64 = Coins::<T>::minimum_balance(native).0;
let ed_bump = 2u64;

Dex::<T>::add_liquidity(
SystemOrigin::Signed(caller).into(),
coin1,
200u64,
// TODO: this call otherwise fails with `InsufficientLiquidityMinted`.
// might be again related to their expectance on ed being > 1.
100 * (ed + ed_bump),
// TODO: this call otherwise fails with `InsufficientLiquidityMinted` if we don't multiply
// with 3. Might be again related to their expectance on ed being > 1.
100 * 3,
0u64,
0u64,
caller,
Expand All @@ -171,7 +164,7 @@ benchmarks! {
SystemOrigin::Signed(caller).into(),
coin2,
1000u64,
500 * ed,
500,
0u64,
0u64,
caller,
Expand All @@ -192,15 +185,14 @@ benchmarks! {
let native = Coin::native();
let coin1 = Coin::Bitcoin;
let coin2 = Coin::Ether;
let (_, caller, _) = create_coin_and_pool::<T>(&native, &coin1);
let (_, caller, _) = create_coin_and_pool::<T>(&coin1);
let (_, _) = create_coin::<T>(&coin2);
let ed: u64 = Coins::<T>::minimum_balance(native).0;

Dex::<T>::add_liquidity(
SystemOrigin::Signed(caller).into(),
coin1,
500u64,
1000 * ed,
1000,
0u64,
0u64,
caller,
Expand All @@ -213,7 +205,7 @@ benchmarks! {
SystemOrigin::Signed(caller).into(),
coin2,
1000u64,
500 * ed,
500,
0u64,
0u64,
caller,
Expand All @@ -226,7 +218,7 @@ benchmarks! {
SystemOrigin::Signed(caller),
path.clone(),
100u64,
1000 * ed,
1000,
caller
)
verify {
Expand Down
Loading

0 comments on commit 4a66526

Please sign in to comment.