-
Notifications
You must be signed in to change notification settings - Fork 17
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
Completely remove cumulus (dev-)dependencies #44
Conversation
This PR title looked alarming when it came across my inbox 😆 |
Although tests work fine in this case, benchmarking is run against the moonbase runtime and this fetches the relay block height from |
So even if we do that, this associated type has only the I see a couple of solutions:
|
Sounds reasonable to me, how much less efficient is it? |
The key idea here is that a pallet may be used in multiple different runtimes, and the costs to use the pallet may differ between those runtimes. I think rather than hacking at the pallet level to get accurate benchmarks for the one runtime that we care most about (at the expense of anyone else who uses this pallet) we should instead run the benchmarks against each runtime that actually uses the pallet. @notlesh weren't you having similar ideas about pallets that Moonbeam uses from Substrate? So my main point is that this problem is not specific to crowdloan rewards, so it should not have a crowdloan-rewards-specific solution. |
I like that idea, but in that case we need to write runtime-dependent benchmark code. The benchmarking code that exists here aims to be generic, something that we might want to forget about |
Superseded by #61 due to merge conflicts |
This PR is a followup to #43. It removes all dependencies (even dev-dependencies) on crates from cumulus.
Tests now use the parachain's own block number rather than the mocked relay number. This means that the
InitVestingBlock
was 1 rather than 2 as it was before. This caused most of the block indices to be off-by-one in tests that tested for specific vested amounts.