[FRAME Core] remove unnecessary overrides while using derive_impl for frame_system by Gilt0 · Pull Request #3317 · paritytech/polkadot-sdk

Gilt0 · 2024-02-14T07:58:40Z

Description

This PR removes redundant type definition from test definition config implementations like

#[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig)]
impl frame_system::Config for Test {
    type A = A;
    ...
}

This changes avoid redundancies in the code as the macro derive_impl defines the relevant types. To implement the changes, it was a simple fact of running tests and making sure that the tests would still run while the definition would be removed.

Closes #3237

As a note, here is a brief account of things done from the Issue's description statement

alliance migrate alliance, fast-unstake and bags list to use derive-impl #1636
asset-conversion                                                                                            DONE
asset-rate                                                                                                  DONE
assets                                                                                                      DONE
atomic-swap                                                                                                 DONE
aura                                                                                                        DONE
authority-discovery                                                                                         DONE                                                                     
authorship  migrate babe and authorship to use derive-impl #1790
babe  migrate babe and authorship to use derive-impl #1790
bags-list migrate alliance, fast-unstake and bags list to use derive-impl #1636
balances                                                                                                    DONE
beefy                                                                                                       NOTHING TO DO --- also noted this error without failing tests Feb 13 13:49:08.941 ERROR runtime::timestamp: `pallet_timestamp::UnixTime::now` is called at genesis, invalid value returned: 0
beefy-mmr                                                                                                   NOTHING TO DO
bounties                                                                                                    DONE
child-bounties                                                                                              DONE
collective                                                                                                  DONE
contracts                                                                                                   DONE
conviction-voting                                                                                           DONE
core-fellowship                                                                                             NOTHING TO DO
democracy                                                                                                   DONE
election-provider-multi-phase                                                                               NOTHING TO DO
elections-phragmen                                                                                          DONE
executive                                                                                                   NOTHING TO DO
fast-unstake migrate alliance, fast-unstake and bags list to use derive-impl #1636
glutton                                                                                                     DONE
grandpa                                                                                                     DONE
identity                                                                                                    DONE
im-online                                                                                                   NOTHING TO DO
indices Refactor indices pallet #1789
insecure-randomness-collective-flip                                                                         DONE
lottery                                                                                                     DONE
membership                                                                                                  DONE
merkle-mountain-range                                                                                       NOTHING TO DO
message-queue                                                                                               DONE
multisig add frame_system::DefaultConfig to individual pallet DefaultConfigs substrate#14453
nft-fractionalization                                                                                       DONE
nfts                                                                                                        DONE
nicks Refactor pallet-state-trie-migration to fungible::* traits #1801                                      NOT IN REPO
nis                                                                                                         DONE
node-authorization                                                                                          DONE
nomination-pools                                                                                            NOTHING TO DO -- ONLY impl for Runtime
offences                                                                                                    DELETED EVERYTHING -- IS THAT CORRECT??
preimage                                                                                                    DONE
proxy add frame_system::DefaultConfig to individual pallet DefaultConfigs substrate#14453
ranked-collective                                                                                           NOTHING TO DO
recovery                                                                                                    DONE
referenda                                                                                                   DONE
remark                                                                                                      DONE
root-offences                                                                                               DONE
root-testing                                                                                                NOTHING TO DO
salary                                                                                                      NOTHING TO DO
scheduler                                                                                                   DONE
scored-pool                                                                                                 DONE
session                                                                                                     DONE -- substrate/frame/session/benchmarking/src/mock.rs untouched
society                                                                                                     NOTHING TO DO
staking                                                                                                     DONE
staking-bags-benchmarks                                                                                     NOT IN REPO
state-trie-migration                                                                                        NOTHING TO DO
statement                                                                                                   DONE
sudo                                                                                                        DONE
system                                                                                                      DONE
timestamp                                                                                                   DONE
tips                                                                                                        DONE
transaction-payment                                                                                         NOTHING TO DO
transaction-storage                                                                                         NOTHING TO DO
treasury                                                                                                    DONE
try-runtime                                                                                                 NOTHING TO DO -- no specific mention of 'for Test'
uniques                                                                                                     DONE
utility                                                                                                     DONE
vesting                                                                                                     DONE
whitelist                                                                                                   DONE

ggwpez · 2024-02-14T12:53:51Z

Thanks for the cleanup 😄 Please still put a descriptive title for this merge request.

Gilt0 · 2024-02-14T23:13:04Z

@ggwpez You're welcome. Good opportunity to learn from the ground up substrate. :)
Also I blindly copy pasted the issue title as I thought it was appropriate. Let me know if not.

I am at a stall sadly, there is an error I am not able to fix. I tried reverting changes to the identity pallet (no rational except that the log above the error mentioned it. Unsuccessful.

Any pointers please (see the error below)?

[2024-02-14T22:59:57Z INFO  runtime::frame-support] 🩺 Running "Beefy" try-state checks
[2024-02-14T22:59:57Z INFO  runtime::frame-support] 🩺 Running "Mmr" try-state checks
[2024-02-14T22:59:57Z INFO  runtime::frame-support] 🩺 Running "BeefyMmrLeaf" try-state checks
[2024-02-14T22:59:57Z INFO  runtime::frame-support] 🩺 Running "IdentityMigrator" try-state checks
[2024-02-14T22:59:57Z ERROR runtime] panicked at /builds/parity/mirrors/polkadot-sdk/polkadot/runtime/westend/src/lib.rs:2227:65:
    called `Result::unwrap()` on an `Err` value: Other("Detected errors while executing try_state checks. See logs for more info.")
thread 'main' panicked at cli/main.rs:325:6:
called `Result::unwrap()` on an `Err` value: Input("failed to execute TryRuntime_on_runtime_upgrade: Execution aborted due to trap: wasm trap: wasm `unreachable` instruction executed\nWASM backtrace:\nerror while executing at wasm backtrace:\n    0: 0x73d60 - <unknown>!rust_begin_unwind\n    1: 0x11dbf - <unknown>!core::panicking::panic_fmt::he07f4fcfc0e8e78f\n    2: 0xfdb0 - <unknown>!core::result::unwrap_failed::h601a124147f65f04\n    3: 0x116ace - <unknown>!<westend_runtime::Runtime as frame_try_runtime::runtime_decl_for_try_runtime::TryRuntimeV1<sp_runtime::generic::block::Block<sp_runtime::generic::header::Header<u32,sp_runtime::traits::BlakeTwo256>,sp_runtime::generic::unchecked_extrinsic::UncheckedExtrinsic<sp_runtime::multiaddress::MultiAddress<<<sp_runtime::MultiSignature as sp_runtime::traits::Verify>::Signer as sp_runtime::traits::IdentifyAccount>::AccountId,()>,westend_runtime::RuntimeCall,sp_runtime::MultiSignature,(frame_system::extensions::check_non_zero_sender::CheckNonZeroSender<westend_runtime::Runtime>,frame_system::extensions::check_spec_version::CheckSpecVersion<westend_runtime::Runtime>,frame_system::extensions::check_tx_version::CheckTxVersion<westend_runtime::Runtime>,frame_system::extensions::check_genesis::CheckGenesis<westend_runtime::Runtime>,frame_system::extensions::check_mortality::CheckMortality<westend_runtime::Runtime>,frame_system::extensions::check_nonce::CheckNonce<westend_runtime::Runtime>,frame_system::extensions::check_weight::CheckWeight<westend_runtime::Runtime>,pallet_transaction_payment::ChargeTransactionPayment<westend_runtime::Runtime>)>>>>::on_runtime_upgrade::h3d77c7b947511727\n    4: 0x20ff92 - <unknown>!TryRuntime_on_runtime_upgrade")
stack backtrace:
   0: rust_begin_unwind
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/std/src/panicking.rs:597:5
   1: core::panicking::panic_fmt
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/core/src/panicking.rs:72:14
   2: core::result::unwrap_failed
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/core/src/result.rs:1652:5
   3: tokio::runtime::park::CachedParkThread::block_on
   4: try_runtime::main

gupnik · 2024-02-15T05:02:04Z

@ggwpez You're welcome. Good opportunity to learn from the ground up substrate. :) Also I blindly copy pasted the issue title as I thought it was appropriate. Let me know if not.

I am at a stall sadly, there is an error I am not able to fix. I tried reverting changes to the identity pallet (no rational except that the log above the error mentioned it. Unsuccessful.

Any pointers please (see the error below)?

[2024-02-14T22:59:57Z INFO  runtime::frame-support] 🩺 Running "Beefy" try-state checks
[2024-02-14T22:59:57Z INFO  runtime::frame-support] 🩺 Running "Mmr" try-state checks
[2024-02-14T22:59:57Z INFO  runtime::frame-support] 🩺 Running "BeefyMmrLeaf" try-state checks
[2024-02-14T22:59:57Z INFO  runtime::frame-support] 🩺 Running "IdentityMigrator" try-state checks
[2024-02-14T22:59:57Z ERROR runtime] panicked at /builds/parity/mirrors/polkadot-sdk/polkadot/runtime/westend/src/lib.rs:2227:65:
    called `Result::unwrap()` on an `Err` value: Other("Detected errors while executing try_state checks. See logs for more info.")
thread 'main' panicked at cli/main.rs:325:6:
called `Result::unwrap()` on an `Err` value: Input("failed to execute TryRuntime_on_runtime_upgrade: Execution aborted due to trap: wasm trap: wasm `unreachable` instruction executed\nWASM backtrace:\nerror while executing at wasm backtrace:\n    0: 0x73d60 - <unknown>!rust_begin_unwind\n    1: 0x11dbf - <unknown>!core::panicking::panic_fmt::he07f4fcfc0e8e78f\n    2: 0xfdb0 - <unknown>!core::result::unwrap_failed::h601a124147f65f04\n    3: 0x116ace - <unknown>!<westend_runtime::Runtime as frame_try_runtime::runtime_decl_for_try_runtime::TryRuntimeV1<sp_runtime::generic::block::Block<sp_runtime::generic::header::Header<u32,sp_runtime::traits::BlakeTwo256>,sp_runtime::generic::unchecked_extrinsic::UncheckedExtrinsic<sp_runtime::multiaddress::MultiAddress<<<sp_runtime::MultiSignature as sp_runtime::traits::Verify>::Signer as sp_runtime::traits::IdentifyAccount>::AccountId,()>,westend_runtime::RuntimeCall,sp_runtime::MultiSignature,(frame_system::extensions::check_non_zero_sender::CheckNonZeroSender<westend_runtime::Runtime>,frame_system::extensions::check_spec_version::CheckSpecVersion<westend_runtime::Runtime>,frame_system::extensions::check_tx_version::CheckTxVersion<westend_runtime::Runtime>,frame_system::extensions::check_genesis::CheckGenesis<westend_runtime::Runtime>,frame_system::extensions::check_mortality::CheckMortality<westend_runtime::Runtime>,frame_system::extensions::check_nonce::CheckNonce<westend_runtime::Runtime>,frame_system::extensions::check_weight::CheckWeight<westend_runtime::Runtime>,pallet_transaction_payment::ChargeTransactionPayment<westend_runtime::Runtime>)>>>>::on_runtime_upgrade::h3d77c7b947511727\n    4: 0x20ff92 - <unknown>!TryRuntime_on_runtime_upgrade")
stack backtrace:
   0: rust_begin_unwind
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/std/src/panicking.rs:597:5
   1: core::panicking::panic_fmt
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/core/src/panicking.rs:72:14
   2: core::result::unwrap_failed
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/core/src/result.rs:1652:5
   3: tokio::runtime::park::CachedParkThread::block_on
   4: try_runtime::main

@Gilt0 You can ignore this one for now as its happening on master as well.

Gilt0 · 2024-02-15T06:44:52Z

@gupnik Noted, thank you.

gupnik

Nice. Thank you!

gupnik · 2024-02-17T10:46:35Z

bot fmt

command-bot · 2024-02-17T10:46:40Z

@gupnik https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5244409 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 1-874f74a4-7963-4f46-bf4a-462188d720de to cancel this command or bot cancel to cancel all commands in this pull request.

command-bot · 2024-02-17T10:47:54Z

@gupnik Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5244409 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5244409/artifacts/download.

Gilt0 · 2024-02-18T05:10:39Z

@gupnik You're welcome, if there is something else to do, happy to help!

gupnik · 2024-02-19T11:37:15Z

@gupnik You're welcome, if there is something else to do, happy to help!

@Gilt0 I have one more that's up for grabs in case you are interested :) #1657

Gilt0 · 2024-02-19T11:54:02Z

@gupnik You're welcome, if there is something else to do, happy to help!

@Gilt0 I have one more that's up for grabs in case you are interested :) #1657

Yes please 🤗

* master: (41 commits) Add Coretime to Westend (#3319) removed `pallet::getter` from `pallet-sudo` (#3370) gossip-support: add unittests for update authorities (#3258) [FRAME Core] remove unnecessary overrides while using derive_impl for frame_system (#3317) Update coretime-westend bootnodes (#3380) `im-online` removal cleanup: remove off-chain storage (#2290) Bump the known_good_semver group with 1 update (#3379) Fix documentation dead link (#3372) Make `sp-keystore` `no_std`-compatible and fix the `build-runtimes-polkavm` CI job (#3363) Remove unused `im-online` weights (#3373) Ensure referenda `TracksInfo` is sorted (#3325) rpc server: add rate limiting middleware (#3301) do not block finality for "disabled" disputes (#3358) fix(zombienet): docker `img` version to use in merge queues for bridges (#3337) Various nits and alignments for SP testnets found during bumping `polkadot-fellows` repo (#3359) Add broker pallet to `coretime-westend` (#3272) remove recursion limit (#3348) Update subkey README.md (#3355) Bump the known_good_semver group with 6 updates (#3347) Document LocalKeystore insert method (#3336) ...

… frame_system (paritytech#3317) # Description This PR removes redundant type definition from test definition config implementations like ``` #[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig)] impl frame_system::Config for Test { type A = A; ... } ``` This changes avoid redundancies in the code as the macro `derive_impl` defines the relevant types. To implement the changes, it was a simple fact of running tests and making sure that the tests would still run while the definition would be removed. Closes paritytech#3237 As a note, here is a brief account of things done from the Issue's description statement ``` alliance migrate alliance, fast-unstake and bags list to use derive-impl paritytech#1636 asset-conversion DONE asset-rate DONE assets DONE atomic-swap DONE aura DONE authority-discovery DONE authorship migrate babe and authorship to use derive-impl paritytech#1790 babe migrate babe and authorship to use derive-impl paritytech#1790 bags-list migrate alliance, fast-unstake and bags list to use derive-impl paritytech#1636 balances DONE beefy NOTHING TO DO --- also noted this error without failing tests Feb 13 13:49:08.941 ERROR runtime::timestamp: `pallet_timestamp::UnixTime::now` is called at genesis, invalid value returned: 0 beefy-mmr NOTHING TO DO bounties DONE child-bounties DONE collective DONE contracts DONE conviction-voting DONE core-fellowship NOTHING TO DO democracy DONE election-provider-multi-phase NOTHING TO DO elections-phragmen DONE executive NOTHING TO DO fast-unstake migrate alliance, fast-unstake and bags list to use derive-impl paritytech#1636 glutton DONE grandpa DONE identity DONE im-online NOTHING TO DO indices Refactor indices pallet paritytech#1789 insecure-randomness-collective-flip DONE lottery DONE membership DONE merkle-mountain-range NOTHING TO DO message-queue DONE multisig add frame_system::DefaultConfig to individual pallet DefaultConfigs substrate#14453 nft-fractionalization DONE nfts DONE nicks Refactor pallet-state-trie-migration to fungible::* traits paritytech#1801 NOT IN REPO nis DONE node-authorization DONE nomination-pools NOTHING TO DO -- ONLY impl for Runtime offences DELETED EVERYTHING -- IS THAT CORRECT?? preimage DONE proxy add frame_system::DefaultConfig to individual pallet DefaultConfigs substrate#14453 ranked-collective NOTHING TO DO recovery DONE referenda DONE remark DONE root-offences DONE root-testing NOTHING TO DO salary NOTHING TO DO scheduler DONE scored-pool DONE session DONE -- substrate/frame/session/benchmarking/src/mock.rs untouched society NOTHING TO DO staking DONE staking-bags-benchmarks NOT IN REPO state-trie-migration NOTHING TO DO statement DONE sudo DONE system DONE timestamp DONE tips DONE transaction-payment NOTHING TO DO transaction-storage NOTHING TO DO treasury DONE try-runtime NOTHING TO DO -- no specific mention of 'for Test' uniques DONE utility DONE vesting DONE whitelist DONE ``` --------- Co-authored-by: command-bot <> Co-authored-by: gupnik <nikhilgupta.iitk@gmail.com>

Gilt0 added 30 commits February 13, 2024 12:54

cleaning impl frame_system::Config for Test

85f05a0

cleaning impl frame_system::Config for Test

f09b699

cleaning impl frame_system::Config for Test

e80290b

cleaning impl frame_system::Config for Test

8edc003

cleaning impl frame_system::Config for Test

7f60211

cleaning impl frame_system::Config for Test

dbd68b7

cleaning impl frame_system::Config for Test

8970e67

cleaning impl frame_system::Config for Test

7e1f712

cleaning impl frame_system::Config for Test

1d78113

cleaning impl frame_system::Config for Test

7d8a201

cleaning impl frame_system::Config for Test

ec8df66

cleaning impl frame_system::Config for Test

85a03db

cleaning impl frame_system::Config for Test

b16147e

cleaning impl frame_system::Config for Test

5b257f8

cleaning impl frame_system::Config for Test

776a366

cleaning impl frame_system::Config for Test

2a9e553

cleaning impl frame_system::Config for Test

979a5e8

cleaning impl frame_system::Config for Test

dcc39fe

cleaning impl frame_system::Config for Test

e4d14c4

cleaning impl frame_system::Config for Test

22c1009

cleaning impl frame_system::Config for Test

7998cf5

cleaning impl frame_system::Config for Test

8d79e50

cleaning impl frame_system::Config for Test

9b8fead

cleaning impl frame_system::Config for Test

9990098

cleaning impl frame_system::Config for Test

10a615b

cleaning impl frame_system::Config for Test

86ec57d

cleaning impl frame_system::Config for Test

4fe2797

cleaning impl frame_system::Config for Test

87db743

cleaning impl frame_system::Config for Test

8b50688

cleaning impl frame_system::Config for Test

aab09d6

fixing action test-linux-stable

653c80e

Gilt0 added 2 commits February 15, 2024 07:45

Fixing actions - reverting change

c63b46b

keeping changes as it did not resolve the error

db32489

Gilt0 changed the title ~~Closing issue 3237~~ [FRAME Core] remove unnecessary overrides while using derive_impl for frame_system Feb 14, 2024

gupnik approved these changes Feb 17, 2024

View reviewed changes

".git/.scripts/commands/fmt/fmt.sh"

f25c735

Merge branch 'master' into issue-3237

8ae44c6

franciscoaguirre approved these changes Feb 19, 2024

View reviewed changes

Merge branch 'master' into issue-3237

9798646

gupnik enabled auto-merge February 19, 2024 11:25

gupnik added this pull request to the merge queue Feb 19, 2024

Merged via the queue into paritytech:master with commit b78c72c Feb 19, 2024

Gilt0 deleted the issue-3237 branch February 20, 2024 19:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FRAME Core] remove unnecessary overrides while using derive_impl for frame_system#3317

[FRAME Core] remove unnecessary overrides while using derive_impl for frame_system#3317
gupnik merged 60 commits intoparitytech:masterfrom
Gilt0:issue-3237

Gilt0 commented Feb 14, 2024

Uh oh!

ggwpez commented Feb 14, 2024

Uh oh!

Gilt0 commented Feb 14, 2024 •

edited

Loading

Uh oh!

gupnik commented Feb 15, 2024 •

edited

Loading

Uh oh!

Gilt0 commented Feb 15, 2024

Uh oh!

gupnik left a comment

Uh oh!

gupnik commented Feb 17, 2024

Uh oh!

command-bot bot commented Feb 17, 2024 •

edited

Loading

Uh oh!

command-bot bot commented Feb 17, 2024

Uh oh!

Gilt0 commented Feb 18, 2024

Uh oh!

gupnik commented Feb 19, 2024

Uh oh!

Gilt0 commented Feb 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

Gilt0 commented Feb 14, 2024

Description

Uh oh!

ggwpez commented Feb 14, 2024

Uh oh!

Gilt0 commented Feb 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gupnik commented Feb 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Gilt0 commented Feb 15, 2024

Uh oh!

gupnik left a comment

Choose a reason for hiding this comment

Uh oh!

gupnik commented Feb 17, 2024

Uh oh!

command-bot bot commented Feb 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

command-bot bot commented Feb 17, 2024

Uh oh!

Gilt0 commented Feb 18, 2024

Uh oh!

gupnik commented Feb 19, 2024

Uh oh!

Gilt0 commented Feb 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Gilt0 commented Feb 14, 2024 •

edited

Loading

gupnik commented Feb 15, 2024 •

edited

Loading

command-bot bot commented Feb 17, 2024 •

edited

Loading