Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

substrate-test-runtime migrated to "pure" frame runtime #13737

Merged
merged 87 commits into from
May 4, 2023

Conversation

michalkucharczyk
Copy link
Contributor

@michalkucharczyk michalkucharczyk commented Mar 28, 2023

This PR migrates the substrate-test-runtime to "pure" frame runtime.

New substrate-test-pallet was introduced. It implements all old Extrinsics used here and there in various substrate's modules tests.

Little runtime API and functionality cleanup was also done: unused staff was removed.

Since the runtime storage and call flow (mainly due to usage of frame::executive, frame::system, frame::balances) was changed, many tests required some updates. This PR addresses that. Each test change is provided in separate commit for reviewing convenience.

Hand written transfer functionality, exhaust_resources, nonce, priority, propagate flag, requires/provides tags for extrinsic was replaced by legit frame mechanisms (frame::balances, CheckNonce, CheckWeight, and substrate-test-pallet specific CheckSubstrateCall SignedExtension).

Creation of GenesisConfig storage for substrate-test-runtime was also re-fectored.

Partially addresses: #9782

ConsensusLog::NextEpochData is now added by pallet_babe as
pallet_babe::SameAuthoritiesForever trigger is used in runtime config.
test-substrate-runtime is now using frame::executive to finalize the
block. during finalization the digests stored during block execution are
checked against header digests:
https://github.com/paritytech/substrate/blob/aaa9a3631d29f757552ffcc8b97aa7091c0b27b0/frame/executive/src/lib.rs#L585-L591

It makes impossible to directly manipulate header's digets, w/o
depositing logs into system pallet storage `Digest<T: Config>`.

Instead of this dedicated extrinsic allowing to store logs items
(MmrRoot / AuthoritiesChange) is used.
test-substrate-runtime is now using frame::executive to finalize the
block. during finalization the digest logs stored during block execution are
checked against header digest logs:
https://github.com/paritytech/substrate/blob/aaa9a3631d29f757552ffcc8b97aa7091c0b27b0/frame/executive/src/lib.rs#L585-L591

It makes impossible to directly manipulate header's digets, w/o
depositing logs into system pallet storage `Digest<T: Config>`.

Instead of this dedicated extrinsic allowing to store logs items
(ScheduledChange / ForcedChange and DigestItem::Other) is used.
The size of unchecked extrinsic was increased. The pattern used in test will
be placed at the end of scale-encoded buffer.
Transfer transaction processing was slightly improved, test was
adjusted.
Runtime extrinsic size was increased. Size of data read during block
execution was also increased due to usage of new pallets in runtime.

Sizes were adjusted in tests.
cargo update -p substrate-test-runtime -p substrate-test-runtime-client
@michalkucharczyk michalkucharczyk added B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Mar 28, 2023
@michalkucharczyk michalkucharczyk requested a review from a team March 29, 2023 07:37
client/consensus/beefy/src/tests.rs Outdated Show resolved Hide resolved
client/consensus/beefy/src/tests.rs Outdated Show resolved Hide resolved
client/basic-authorship/src/basic_authorship.rs Outdated Show resolved Hide resolved
client/basic-authorship/src/lib.rs Outdated Show resolved Hide resolved
client/network/bitswap/src/lib.rs Outdated Show resolved Hide resolved
test-utils/runtime/src/lib.rs Outdated Show resolved Hide resolved
test-utils/runtime/src/lib.rs Outdated Show resolved Hide resolved
client/rpc/src/dev/tests.rs Outdated Show resolved Hide resolved
client/service/src/lib.rs Outdated Show resolved Hide resolved
@davxy
Copy link
Member

davxy commented Mar 30, 2023

LGTM. And mostly silly nits.

But before green light I would like to hear opinions about:

  • client "unit" tests depending on correctness of FRAME pallet (in babe so far)
  • eventually spreading this 1st class treatment to the other RuntimeApi components as well (e.g. embedding aura/grandpa pallet in test runtime as well). That is all the stuff we're implementing the runtime Api for.

@michalkucharczyk
Copy link
Contributor Author

bot rebase

@paritytech-processbot
Copy link

Rebased

@michalkucharczyk
Copy link
Contributor Author

bot rebase

@paritytech-processbot
Copy link

Rebased

Copy link
Contributor

@skunert skunert left a comment

Choose a reason for hiding this comment

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

Looks good, only super nitpicks!

client/basic-authorship/src/basic_authorship.rs Outdated Show resolved Hide resolved
client/consensus/grandpa/src/tests.rs Outdated Show resolved Hide resolved
client/consensus/grandpa/src/tests.rs Outdated Show resolved Hide resolved
client/network/bitswap/src/lib.rs Outdated Show resolved Hide resolved
client/service/test/src/client/mod.rs Outdated Show resolved Hide resolved
test-utils/runtime/src/extrinsic.rs Outdated Show resolved Hide resolved
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-check-each-crate-macos
Logs: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2780696

@michalkucharczyk
Copy link
Contributor Author

bot merge

@paritytech-processbot paritytech-processbot bot merged commit ebe0c08 into master May 4, 2023
@paritytech-processbot paritytech-processbot bot deleted the mku-refactor-substrate-test-runtime branch May 4, 2023 16:20
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
…3737)

* substrate-test-runtime migrated to pure-frame based

* test block builder: helpers added

* simple renaming

* basic_authorship test adjusted

* block_building storage_proof test adjusted

* babe: tests: should_panic expected added

* babe: tests adjusted

ConsensusLog::NextEpochData is now added by pallet_babe as
pallet_babe::SameAuthoritiesForever trigger is used in runtime config.

* beefy: tests adjusted

test-substrate-runtime is now using frame::executive to finalize the
block. during finalization the digests stored during block execution are
checked against header digests:
https://github.com/paritytech/substrate/blob/aaa9a3631d29f757552ffcc8b97aa7091c0b27b0/frame/executive/src/lib.rs#L585-L591

It makes impossible to directly manipulate header's digets, w/o
depositing logs into system pallet storage `Digest<T: Config>`.

Instead of this dedicated extrinsic allowing to store logs items
(MmrRoot / AuthoritiesChange) is used.

* grandpa: tests adjusted

test-substrate-runtime is now using frame::executive to finalize the
block. during finalization the digest logs stored during block execution are
checked against header digest logs:
https://github.com/paritytech/substrate/blob/aaa9a3631d29f757552ffcc8b97aa7091c0b27b0/frame/executive/src/lib.rs#L585-L591

It makes impossible to directly manipulate header's digets, w/o
depositing logs into system pallet storage `Digest<T: Config>`.

Instead of this dedicated extrinsic allowing to store logs items
(ScheduledChange / ForcedChange and DigestItem::Other) is used.

* network:bitswap: test adjusted

The size of unchecked extrinsic was increased. The pattern used in test will
be placed at the end of scale-encoded buffer.

* runtime apis versions adjusted

* storage keys used in runtime adjusted

* wasm vs native tests removed

* rpc tests: adjusted

Transfer transaction processing was slightly improved, test was
adjusted.

* tests: sizes adjusted

Runtime extrinsic size was increased. Size of data read during block
execution was also increased due to usage of new pallets in runtime.

Sizes were adjusted in tests.

* cargo.lock update

cargo update -p substrate-test-runtime -p substrate-test-runtime-client

* warnings fixed

* builders cleanup: includes / std

* extrinsic validation cleanup

* txpool: benches performance fixed

* fmt

* spelling

* Apply suggestions from code review

Co-authored-by: Davide Galassi <[email protected]>

* Apply code review suggestions

* Apply code review suggestions

* get rid of 1063 const

* renaming: UncheckedExtrinsic -> Extrinsic

* test-utils-runtime: further step to pure-frame

* basic-authorship: tests OK

* CheckSubstrateCall added + tests fixes

* test::Transfer call removed

* priority / propagate / no sudo+root-testing

* fixing warnings + format

* cleanup: build2/nonce + format

* final tests fixes

all tests are passing

* logs/comments removal

* should_not_accept_old_signatures test removed

* make txpool benches work again

* Cargo.lock reset

* format

* sudo hack removed

* txpool benches fix+cleanup

* .gitignore reverted

* rebase fixing + unsigned cleanup

* Cargo.toml/Cargo.lock cleanup

* force-debug feature removed

* mmr tests fixed

* make cargo-clippy happy

* network sync test uses unsigned extrinsic

* cleanup

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

* push_storage_change signed call remove

* GenesisConfig cleanup

* fix

* fix

* GenesisConfig simplified

* storage_keys_works: reworked

* storage_keys_works: expected keys in vec

* storage keys list moved to substrate-test-runtime

* substrate-test: some sanity tests + GenesisConfigBuilder rework

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <[email protected]>

* Apply suggestions from code review

* Review suggestions

* fix

* fix

* beefy: generate_blocks_and_sync block_num sync with actaul value

* Apply suggestions from code review

Co-authored-by: Davide Galassi <[email protected]>

* Update test-utils/runtime/src/genesismap.rs

Co-authored-by: Davide Galassi <[email protected]>

* cargo update -p sc-rpc -p sc-transaction-pool

* Review suggestions

* fix

* doc added

* slot_duration adjusted for Babe::slot_duration

* small doc fixes

* array_bytes::hex used instead of hex

* tiny -> medium name fix

* Apply suggestions from code review

Co-authored-by: Sebastian Kunert <[email protected]>

* TransferData::try_from_unchecked_extrinsic -> try_from

* Update Cargo.lock

---------

Co-authored-by: parity-processbot <>
Co-authored-by: Davide Galassi <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: Sebastian Kunert <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants