-
Notifications
You must be signed in to change notification settings - Fork 45
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.
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
Feature/stable pool mgx 1307 #838
Conversation
create_pool bench with tests
pallets/market/src/benchmarking.rs
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any ideas here?
i just did a copypaste of the whole bench and changed xyk -> sswap
we should get the max of the two for the extrinsic
#[pallet::hooks] | ||
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> { | ||
fn integrity_test() { | ||
assert!(true, "template",); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove
Self::deposit_event(Event::AssetsSwapped { who: sender.clone(), swaps }); | ||
|
||
// total swaps inc | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
todo total swaps storage
pallets/market/src/mock.rs
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currently for the purposes of running benchmark tests
might move the tests form runtime-integration herer
pallets/market/src/weights.rs
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
edit after merge, CI run, copypaste bench values
@@ -999,15 +795,16 @@ pub mod config { | |||
ExistenceRequirement::KeepAlive, | |||
) { | |||
Ok(imbalance) => Ok(Some(LiquidityInfoEnum::Imbalance((T1::get(), imbalance)))), | |||
// TODO make sure atleast 1 planck KSM is charged | |||
Err(_) if fee_2.is_zero() => Err(InvalidTransaction::Payment.into()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it can happen that a fee would be zero in the second asset
fail rather then withdraw 1 token
} | ||
} | ||
|
||
fn format_u128_with_leading_zeros(num: TokenId, width: usize) -> Vec<u8> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replacement for the thingy we have XYK to get the symbols...a bit more readable version
@@ -1285,6 +1091,81 @@ pub mod config { | |||
fn get_asset_l1_id(asset_id: T::AssetId) -> Option<L1Asset> { | |||
orml_asset_registry::IdToL1Asset::<T>::get(asset_id) | |||
} | |||
|
|||
fn create_pool_asset( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the asset mutation trait is in XYK pallet, didn't want to touch XYK so added a fn to this trait
CallType::MultiSell { .. } | | ||
CallType::MultiBuy { .. } | | ||
CallType::AtomicBuy { .. } | | ||
CallType::AtomicSell { .. } | | ||
CallType::CompoundRewards | | ||
CallType::ProvideLiquidityWithConversion => true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
xyk calls are removed, so all of the stuff needs to go...
@@ -15,7 +15,7 @@ else | |||
DOCKER_LABEL=${GIT_REV}-dirty | |||
fi | |||
|
|||
DOCKER_IMAGE_TAG=${@:-mangatasolutions/rollup-node:dev} | |||
DOCKER_IMAGE_TAG=${@:-mangatasolutions/rollup-node:local} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the local scripts and docs provide build instr with local
tag not dev
else branch did not check whether it matches the other asset
@@ -0,0 +1 @@ | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ups?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is a feature so all the tests pass :D
No description provided.