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

Remove Gov V1 collectives #2643

Merged
merged 27 commits into from
Feb 21, 2024
Merged

Remove Gov V1 collectives #2643

merged 27 commits into from
Feb 21, 2024

Conversation

noandrea
Copy link
Collaborator

@noandrea noandrea commented Feb 13, 2024

What does it do?

Adds a migration to remove Council and TechCommitte collective

What important points reviewers should know?

It contains a storage migration. The members of the collective are currently 5 Moonbeam/Moonriver/Moonbase

⚠️ Breaking Changes ⚠️

  1. The following origins are now either Root or GenerealAdmin (the council has been removed)
    • AssetsForceOrigin
    • LocalAssetModifierOrigin
    • IdentityForceOrigin
    • IdentityRegistrarOrigin
  2. ForeignAssetModifierOrigin requires Root, GenerealAdmin or OpenTechCommittee(5,9)
  3. Fast Tracking a proposal is not possible anymore
  4. Maintenance mode privileges are migrated to the OpenTechCollective
  5. Precompiles at the following address will revert
    • 2062
    • 2063
  6. Council and TechCommittee collective do not exists anymore

@librelois librelois mentioned this pull request Feb 15, 2024
24 tasks
@noandrea noandrea added B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes breaking Needs to be mentioned in breaking changes labels Feb 18, 2024
@noandrea noandrea added D3-trivial PR contains trivial changes in a runtime directory that do not require an audit D5-nicetohaveaudit⚠️ PR contains trivial changes to logic that should be properly reviewed. and removed D3-trivial PR contains trivial changes in a runtime directory that do not require an audit labels Feb 19, 2024
@RomarQ
Copy link
Contributor

RomarQ commented Feb 19, 2024

Is it intended that we only remove pallet_collective instances in the moonbase runtime?

Copy link
Contributor

github-actions bot commented Feb 19, 2024

Coverage Report

@@                    Coverage Diff                     @@
##           master   noandrea-remove-gov-v1      +/-   ##
==========================================================
- Coverage   80.48%                   74.12%   -6.36%     
- Files         292                      223      -69     
- Lines       93129                    71505   -21624     
==========================================================
- Hits        74952                    52997   -21955     
+ Misses      18177                    18508     +331     
Files Changed Coverage
/client/rpc/finality/src/lib.rs 0.00% (-100.00%) 🔽
/client/rpc/manual-xcm/src/lib.rs 0.00% (-84.62%) 🔽
/client/rpc/txpool/src/lib.rs 0.00% (-65.38%) 🔽
/client/rpc-core/txpool/src/lib.rs 0.00% (-100.00%) 🔽
/client/rpc-core/txpool/src/types/content.rs 0.00% (-70.37%) 🔽
/client/rpc-core/txpool/src/types/inspect.rs 0.00% (-88.89%) 🔽
/client/rpc-core/txpool/src/types/mod.rs 0.00% (-100.00%) 🔽
/client/vrf/src/lib.rs 0.00% (-96.00%) 🔽
/node/cli-opt/src/account_key.rs 31.15% (-1.64%) 🔽
/node/cli-opt/src/lib.rs 0.00% (-48.00%) 🔽
/pallets/asset-manager/src/lib.rs 76.26% (-7.64%) 🔽
/pallets/asset-manager/src/mock.rs 97.90% (+4.09%) 🔼
/pallets/asset-manager/src/tests.rs 99.79% (-0.21%) 🔽
/pallets/erc20-xcm-bridge/src/erc20_matcher.rs 92.77% (-4.88%) 🔽
/pallets/erc20-xcm-bridge/src/lib.rs 8.90% (-3.26%) 🔽
/pallets/erc20-xcm-bridge/src/mock.rs 0.96% (-3.55%) 🔽
/pallets/erc20-xcm-bridge/src/xcm_holding_ext.rs 81.20% (-13.67%) 🔽
/pallets/ethereum-xcm/src/lib.rs 82.09% (-2.69%) 🔽
/pallets/ethereum-xcm/src/mock.rs 32.29% (-6.88%) 🔽
/pallets/ethereum-xcm/src/tests/v1/eip1559.rs 99.41% (-0.01%) 🔽
/pallets/ethereum-xcm/src/tests/v1/eip2930.rs 99.43% (-0.01%) 🔽
/pallets/moonbeam-lazy-migrations/src/lib.rs 90.00% (-1.67%) 🔽
/pallets/moonbeam-lazy-migrations/src/mock.rs 4.87% (-13.88%) 🔽
/pallets/moonbeam-orbiters/src/lib.rs 69.80% (-4.39%) 🔽
/pallets/moonbeam-orbiters/src/mock.rs 99.19% (+1.98%) 🔼
/pallets/moonbeam-orbiters/src/types.rs 65.26% (+0.61%) 🔼
/pallets/moonbeam-xcm-benchmarks/src/weights/fungible.rs 0.00% (-24.24%) 🔽
/pallets/moonbeam-xcm-benchmarks/src/weights/generic.rs 0.00% (-2.96%) 🔽
/pallets/moonbeam-xcm-benchmarks/src/weights/mod.rs 0.00% (-13.11%) 🔽
/pallets/parachain-staking/src/auto_compound.rs 95.64% (-0.03%) 🔽
/pallets/parachain-staking/src/delegation_requests.rs 91.92% (-0.02%) 🔽
/pallets/parachain-staking/src/inflation.rs 85.98% (-1.52%) 🔽
/pallets/parachain-staking/src/lib.rs 90.24% (-0.38%) 🔽
/pallets/parachain-staking/src/mock.rs 99.28% (-0.14%) 🔽
/pallets/parachain-staking/src/set.rs 40.00% (-0.74%) 🔽
/pallets/parachain-staking/src/tests.rs 91.57% (-0.02%) 🔽
/pallets/parachain-staking/src/types.rs 77.35% (-0.56%) 🔽
/pallets/proxy-genesis-companion/src/lib.rs 57.14% (-24.68%) 🔽
/pallets/proxy-genesis-companion/src/mock.rs 7.98% (-19.70%) 🔽
/pallets/xcm-transactor/src/encode.rs 90.32% (-9.68%) 🔽
/pallets/xcm-transactor/src/lib.rs 85.64% (-2.84%) 🔽
/pallets/xcm-transactor/src/mock.rs 95.04% (+2.96%) 🔼
/precompiles/assets-erc20/src/mock.rs 9.95% (-58.90%) 🔽
/precompiles/author-mapping/src/mock.rs 5.71% (-12.57%) 🔽
/precompiles/balances-erc20/src/mock.rs 99.60% (+2.27%) 🔼
/precompiles/batch/src/mock.rs 99.68% (+0.80%) 🔼
/precompiles/call-permit/src/lib.rs 97.83% (-0.01%) 🔽
/precompiles/call-permit/src/mock.rs 6.00% (-17.27%) 🔽
/precompiles/collective/src/mock.rs 97.19% (+2.93%) 🔼
/precompiles/conviction-voting/src/lib.rs 93.41% (+0.01%) 🔼
/precompiles/conviction-voting/src/mock.rs 97.05% (+3.72%) 🔼
/precompiles/crowdloan-rewards/src/mock.rs 99.17% (+1.10%) 🔼
/precompiles/crowdloan-rewards/src/tests.rs 98.47% (-0.01%) 🔽
/precompiles/gmp/src/mock.rs 5.62% (-2.73%) 🔽
/precompiles/identity/src/mock.rs 99.02% (+1.93%) 🔼
/precompiles/pallet-democracy/src/mock.rs 99.02% (+1.06%) 🔼
/precompiles/pallet-democracy/src/tests.rs 89.29% (-0.04%) 🔽
/precompiles/parachain-staking/src/mock.rs 98.85% (+1.04%) 🔼
/precompiles/precompile-registry/src/mock.rs 6.16% (-18.36%) 🔽
/precompiles/preimage/src/mock.rs 99.35% (+1.72%) 🔼
/precompiles/proxy/src/mock.rs 97.86% (+3.72%) 🔼
/precompiles/proxy/src/tests.rs 96.79% (-0.02%) 🔽
/precompiles/randomness/src/mock.rs 98.38% (+2.90%) 🔼
/precompiles/referenda/src/mock.rs 98.52% (+1.00%) 🔼
/precompiles/relay-encoder/src/mock.rs 5.25% (-3.69%) 🔽
/precompiles/utils/src/precompile_set.rs 61.06% (-16.21%) 🔽
/precompiles/utils/src/solidity/codec/xcm.rs 67.95% (-0.14%) 🔽
/precompiles/utils/src/testing/modifier.rs 87.67% (-0.64%) 🔽
/precompiles/utils/tests-external/lib.rs 33.22% (-15.93%) 🔽
/precompiles/xcm-transactor/src/mock.rs 95.51% (+2.50%) 🔼
/precompiles/xcm-utils/src/lib.rs 88.52% (-0.82%) 🔽
/precompiles/xcm-utils/src/mock.rs 96.84% (+1.17%) 🔼
/precompiles/xtokens/src/mock.rs 16.25% (-7.47%) 🔽
/precompiles/xtokens/src/tests.rs 99.52% (-0.01%) 🔽
/primitives/account/src/lib.rs 64.23% (+0.52%) 🔼
/primitives/ext/src/lib.rs 44.83% (-3.45%) 🔽
/primitives/rpc/debug/src/lib.rs 0.00% (-84.85%) 🔽
/primitives/rpc/txpool/src/lib.rs 0.00% (-83.33%) 🔽
/primitives/xcm/src/asset_id_conversions.rs 0.00% (-66.67%) 🔽
/primitives/xcm/src/ethereum_xcm.rs 93.07% (-1.80%) 🔽
/primitives/xcm/src/fee_handlers.rs 95.57% (-2.85%) 🔽
/primitives/xcm/src/origin_conversion.rs 0.00% (-74.07%) 🔽
/runtime/common/src/migrations.rs 0.00% (-71.35%) 🔽
/runtime/common/src/weights/pallet_assets.rs 0.00% (-9.04%) 🔽
/runtime/common/src/weights/pallet_author_mapping.rs 0.00% (-80.00%) 🔽
/runtime/common/src/weights/pallet_balances.rs 0.00% (-12.05%) 🔽
/runtime/common/src/weights/pallet_collective.rs 0.00% (-11.25%) 🔽
/runtime/common/src/weights/pallet_crowdloan_rewards.rs 0.00% (-66.18%) 🔽
/runtime/common/src/weights/pallet_moonbeam_orbiters.rs 0.00% (-9.26%) 🔽
/runtime/common/src/weights/pallet_parachain_staking.rs 0.00% (-19.83%) 🔽
/runtime/common/src/weights/pallet_utility.rs 0.00% (-44.00%) 🔽
/runtime/common/src/weights/pallet_xcm_transactor.rs 0.00% (-11.90%) 🔽
/runtime/evm_tracer/src/lib.rs 0.00% (-65.96%) 🔽

Coverage generated Wed Feb 21 10:04:02 UTC 2024

@noandrea noandrea added the D1-runtime-migration PR introduces code that might require downstream chains to run a runtime upgrade. label Feb 19, 2024
@noandrea
Copy link
Collaborator Author

it is intended to remove the Council and Tech Committee collectives, not the whole pallet_collective

@noandrea noandrea marked this pull request as ready for review February 20, 2024 13:52
Copy link
Contributor

@RomarQ RomarQ left a comment

Choose a reason for hiding this comment

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

Apart from the pipeline failures, the changes look good to me.

Is there a reason for not removing the tests instead of just disabling them?

I am asking because every test (even if skipped/disabled) slows down the test run.

@@ -69,13 +68,8 @@ parameter_types! {
}

/// We allow root and Chain council to execute privileged asset operations.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// We allow root and Chain council to execute privileged asset operations.
/// We allow Root and General Admin to execute privileged asset operations.

runtime/moonbeam/src/governance/democracy.rs Show resolved Hide resolved
test/helpers/voting.ts Show resolved Hide resolved
runtime/moonbase/src/asset_config.rs Outdated Show resolved Hide resolved
runtime/moonbeam/src/asset_config.rs Outdated Show resolved Hide resolved
librelois and others added 9 commits February 20, 2024 15:46
@noandrea noandrea merged commit 53dddf3 into master Feb 21, 2024
29 checks passed
@noandrea noandrea deleted the noandrea-remove-gov-v1 branch February 21, 2024 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes breaking Needs to be mentioned in breaking changes D1-runtime-migration PR introduces code that might require downstream chains to run a runtime upgrade. D5-nicetohaveaudit⚠️ PR contains trivial changes to logic that should be properly reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants