-
Notifications
You must be signed in to change notification settings - Fork 51
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
pallet-token-gateway
benchmarks
#351
base: main
Are you sure you want to change the base?
Conversation
Ok stopped reviewing because this pr doesn't seem ready yet. Also, try to revert your format changes to the unrelated files with
|
everything is working fine, except 2 benches, update and teleport, due to they require setting user balance on asset and native, which turns out to be kinda annoying on setting the balances, as genesis setting doesnt work and manuall setting too. But am on it |
Ok(()) | ||
} | ||
|
||
impl_benchmark_test_suite!(Pallet, crate::mock::new_test_ex(), crate::mock::Test); |
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.
Let's just have the benchmarks so we don't need this mock implementation, it's too much code, we already have unit tests for the pallet.
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.
So just generate the weight and delete the whole mock code?
1e1431f
to
dbd3668
Compare
@Wizdave97 done |
dbd3668
to
8a7868b
Compare
TokenGateway: pallet_token_gateway = 64, | ||
HyperBridge: pallet_hyperbridge = 65, |
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.
Lets revert, hyperbridge doesn't use pallet-token-gateway. The benchmarks are for downstream crates
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.
done
pallet-token-gateway
benchmarks
evm/lib/forge-std
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.
You need to revert your changes to this submodule otherwise we can't merge this pr
parachain/runtimes/gargantua/src/weights/pallet_token_gateway.rs
Outdated
Show resolved
Hide resolved
bfe77be
to
eb99c1c
Compare
|
||
let acc = <<T as frame_system::Config>::Lookup as StaticLookup>::unlookup(account.clone()); | ||
|
||
pallet_balances::Pallet::<T>::force_set_balance(RawOrigin::Root.into(), acc, ed * 100u128)?; |
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.
Instead of pulling in the pallet-balances
crate, just use <<T as pallet_ismp::Config>::Currency as frame_support::fungible::Mutate>::set_balance
to set the account balance.
)?; | ||
|
||
// set asset balance | ||
pallet_assets::Pallet::<T>::create( |
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.
Use the fungibles::Mutate
trait to also do this instead of pulling in pallet-assets
Please fix the tests |
@@ -34,11 +35,16 @@ alloy-sol-types = { workspace = true } | |||
token-gateway-primitives = { workspace = true } | |||
pallet-hyperbridge = { workspace = true } | |||
|
|||
[dev-dependencies] | |||
pallet-balances = { workspace = 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.
You can't move these pallets to dev-dependencies
if you are still calling them in the benchmarks, use the trait methods as suggested and remove them completely.
closes #347