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.

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

Closed
wants to merge 2 commits into from

Conversation

MrishoLukamba
Copy link
Contributor

closes #347

@seunlanlege
Copy link
Member

Ok stopped reviewing because this pr doesn't seem ready yet. Also, try to revert your format changes to the unrelated files with

git checkout main -- ./path/to/file

evm/lib/forge-std Outdated Show resolved Hide resolved
@MrishoLukamba
Copy link
Contributor Author

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

@MrishoLukamba
Copy link
Contributor Author

@Wizdave97 done

evm/lib/forge-std Outdated Show resolved Hide resolved
evm/lib/forge-std Outdated Show resolved Hide resolved
@seunlanlege seunlanlege changed the title feat(benchmarks) mock runtime config pallet-token-gateway benchmarks Dec 23, 2024
parachain/runtimes/gargantua/Cargo.toml Outdated Show resolved Hide resolved
evm/lib/forge-std Outdated Show resolved Hide resolved
evm/abi/src/generated/host_manager.rs Outdated Show resolved Hide resolved
evm/abi/src/generated/ping_module.rs Outdated Show resolved Hide resolved

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)?;
Copy link
Member

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(
Copy link
Member

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

@seunlanlege
Copy link
Member

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 }
Copy link
Member

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.

@seunlanlege
Copy link
Member

superceded by #364

@seunlanlege seunlanlege closed this Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Benchmark pallet-token-gateway
3 participants