Skip to content

Conversation

@ashutoshvarma
Copy link
Contributor

Closes #1474

Pull Request Summary
Add two new extrinsic for adding/removing single invulnerable

Note: Added new benchmarks in v1 style, otherwise diff would become big if all benches are changed to v2

Check list

  • added or updated unit tests
  • updated Astar official documentation
  • added OnRuntimeUpgrade hook for precompile revert code registration
  • added benchmarks & weights for any modified runtime logics.

@ashutoshvarma ashutoshvarma added the runtime This PR/Issue is related to the topic “runtime”. label Jul 17, 2025
@ashutoshvarma
Copy link
Contributor Author

/bench astar-dev,shibuya-dev,shiden-dev pallet_collator_selection

@github-actions
Copy link

Benchmarks job is scheduled at https://github.com/AstarNetwork/Astar/actions/runs/16337375139.
Please wait for a while.
Branch: feat/add-remove-invulnerable
SHA: 46b545a


let origin = T::UpdateOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
<CollatorSelection<T>>::set_invulnerables(origin.clone(), initial_invulnerables.clone())?;
let to_remove = initial_invulnerables.last().unwrap().clone();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This gives us the most time spent iterating, but eliminates memory copying.

Can you try re-running the benchmarks while removing the first element, to see if the weight becomes worse?

Copy link
Contributor Author

@ashutoshvarma ashutoshvarma Jul 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Locally, it weights didn't change much, few runs slightly more, few runs slightly less. So within standard error.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks!

I guess it would be become more prevalent if we had huge vectors.

@github-actions
Copy link

Benchmarks have been finished.
You can download artifacts if exists https://github.com/AstarNetwork/Astar/actions/runs/16337375139.

Copy link
Contributor

@PierreOssun PierreOssun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ! Nit suggestion

@github-actions
Copy link

Code Coverage

Package Line Rate Branch Rate Health
pallets/collective-proxy/src 94% 0%
primitives/src 54% 0%
pallets/dapp-staking/rpc/runtime-api/src 0% 0%
precompiles/assets-erc20/src 77% 0%
pallets/xc-asset-config/src 62% 0%
chain-extensions/types/assets/src 0% 0%
pallets/static-price-provider/src 91% 0%
chain-extensions/pallet-assets/src 54% 0%
pallets/price-aggregator/src 75% 0%
precompiles/dapp-staking/src/test 0% 0%
pallets/inflation/src 84% 0%
pallets/collator-selection/src 89% 0%
pallets/dynamic-evm-base-fee/src 82% 0%
precompiles/unified-accounts/src 100% 0%
precompiles/dapp-staking/src 89% 0%
precompiles/dispatch-lockdrop/src 83% 0%
chain-extensions/types/unified-accounts/src 0% 0%
pallets/ethereum-checked/src 76% 0%
pallets/astar-xcm-benchmarks/src/fungible 100% 0%
pallets/dapp-staking/src/test 0% 0%
primitives/src/xcm 64% 0%
precompiles/xcm/src 69% 0%
chain-extensions/unified-accounts/src 0% 0%
pallets/democracy-mbm/src 30% 0%
pallets/astar-xcm-benchmarks/src/generic 100% 0%
pallets/vesting-mbm/src 87% 0%
pallets/astar-xcm-benchmarks/src 86% 0%
precompiles/substrate-ecdsa/src 67% 0%
pallets/dapp-staking/src 80% 0%
precompiles/sr25519/src 56% 0%
pallets/unified-accounts/src 79% 0%
pallets/dapp-staking/src/benchmarking 95% 0%
Summary 74% (3789 / 5116) 0% (0 / 0)

Minimum allowed line rate is 50%

Copy link
Contributor

@Dinonard Dinonard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@ipapandinas ipapandinas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, LGTM!

@ashutoshvarma ashutoshvarma merged commit ac3ece5 into master Jul 17, 2025
8 checks passed
@ashutoshvarma ashutoshvarma deleted the feat/add-remove-invulnerable branch July 17, 2025 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

runtime This PR/Issue is related to the topic “runtime”.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improved collator selection whitelisting

5 participants