Zombienet-SDK test for the happy path of the new staking-async flow #8317

tdimitrov · 2025-04-24T08:41:45Z

Adds a zombienet test for the happy case of new staking flow where staking-async pallet is deployed on AH.

The test uses test/fake runtimes for RC and AH-Next. It covers the following scenario:

AH and RC networks are started
Election is started on AH
Election yields the expected number of validators
The new active set is delivered to RC
On session change the active set on RC is changed and AH is notified

The test takes quite a lot of time (~11 minutes) to execute and requires the chainspecs for fake RC and AH-Next to be prebuilt in the root directory of the test module.

substrate/zombienet-sdk/tests/staking_next/tests.rs

kianenigma · 2025-04-24T11:20:42Z

substrate/zombienet-sdk/tests/lib.rs

+// Copyright (C) Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+mod staking_next;


Why have you chosen to have these tests to be in ./substrate vs in ./substrate/frame/staking-async?

Also noting the new name is staking-async, not next.

It's a common pattern to have all zombienet tests in polkadot/zombiened-sdk-tests, substrate/zombienet-sdk-tests, etc.

But I've got no strong preference on this. If you want, I'll move them to to ./substrate/frame/staking-async.

I'll fix the name.

Both are fixed.

kianenigma · 2025-04-24T11:21:18Z

substrate/zombienet-sdk/tests/staking_next/mod.rs

+
+mod tests;
+
+/// Sets `ValidatorCount` in staking pallet to 500.


Suggested change

/// Sets `ValidatorCount` in staking pallet to 500.

/// Sets `ValidatorCount` in staking pallet to `validator_count`.

Also worth adding a comment that this is due to some mysterious bug, otherwise it should not be used/needed.

Has there been any updates on that? do we have any new leads as to why it is needed?

I've added the comment. I still haven't investigated why the value from the chainspec doesn't get set to the storage item.

kianenigma · 2025-04-24T11:22:21Z

substrate/zombienet-sdk/tests/staking_next/mod.rs

+) -> Result<(), anyhow::Error> {
+	let set_validator_count = dynamic::tx("Staking", "set_validator_count", vec![validator_count]);
+	let sudo = dynamic::tx("Sudo", "sudo", vec![set_validator_count.into_value()]);
+	let alice = dev::alice();


This will work fine, but to be more correct you should query Sudo::key here, see who it is, and if it is one of the dev accounts (alice, bob etc.), sign and send it, and if not fail.

In other words, you are assuming alice is sudo, which may or may not be.

kianenigma · 2025-04-24T11:24:38Z

substrate/zombienet-sdk/tests/staking_next/mod.rs

+		})
+		.with_parachain(|p| {
+			p.with_id(1100)
+			.with_chain_spec_path("parachain.json") // TODO: how to autogenerate this?


esp if you move it to ./substrate/frame/staking-next, I think it is okay for now to say in the README.md:

please first run build-and-run-zn.sh compile, and tweak this bash script to have an option to just prepare the two chain-specs, and not run ZN.

substrate/zombienet-sdk/tests/staking_next/tests.rs

kianenigma · 2025-04-24T11:26:48Z

substrate/zombienet-sdk/tests/staking_next/tests.rs

+
+// Events generated by rc_client. Event here doesn't necessary mean 'event in the substrate
+// context'. It can be anything representing a point of interest for the tests.
+enum RcClientEvent {


I think subxt has a cli which allows you to generate a metadata from the wasm file, and within that you have access to the subxt-compliant version of all of the runtime types, including the concrete event types. So you could, instead of this, assert that some events are emitted.

That's true but in this case this event is not a runtime event.
It's an 'event' such as 'something happened in the test which we want to handle and probably change state. In this test there is 100% match between runtime event in test event but in general this might not be the case.

I should pick a better name since this causes confusion.

substrate/zombienet-sdk/tests/staking_next/tests.rs

kianenigma

Would be ideal to simplify the manual-data parsing here, before we extend it to more test scenarios.

Strongly in favor of moving this to staking-async crate, under a new sub-crate over there to keep it more isolated.

tdimitrov · 2025-04-25T10:12:49Z

Would be ideal to simplify the manual-data parsing here, before we extend it to more test scenarios.

The static interface is problematic for CI since we don't want to commit the metadata and at the same time we need it generated somehow. To avoid complicated build scripts I am sticking with the dynamic subxt interface but with some advise from @jsdw I reworked the parsing. I think it's way better now.

Strongly in favor of moving this to staking-async crate, under a new sub-crate over there to keep it more isolated.

I've also moved the test in staking-async, as an ignored integration test. That way CI won't pick it up but we still can run it manually.

jsdw · 2025-04-25T10:25:16Z

The static interface is problematic for CI since we don't want to commit the metadata and at the same time we need it generated somehow.

In case it helps, Subxt does this by spinning up a Substrate node in a build.rs file, downloading metadata from it and then providing an instance of the generated interface from it here:

https://github.com/paritytech/subxt/blob/master/testing/test-runtime/build.rs

(If the node is already spun up at the time the tests start running, you can also generate an interface given a URL to an RPC node ie like this: https://docs.rs/subxt/latest/subxt/attr.subxt.html#connecting-to-a-node-to-download-metadata-via-runtime_metadata_insecure_url--)

But yeah, I appreciate that this adds a little complexity. The static interface def makes life easier though :)

kianenigma · 2025-05-02T13:01:41Z

substrate/frame/staking-async/Cargo.toml

 pallet-bags-list = { workspace = true, default-features = true }
 pallet-balances = { workspace = true, default-features = true }
 rand_chacha = { workspace = true, default-features = true }
 sp-npos-elections = { workspace = true, default-features = true }
 sp-tracing = { workspace = true, default-features = true }
 substrate-test-utils = { workspace = true }
+subxt = { workspace = true }


Let's move them to a separate crate staking-async/zn-test to not increase the compile time of the rust unit tests?

paritytech-workflow-stopper · 2025-05-14T10:00:28Z

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/15017687759
Failed job name: test-linux-stable

tdimitrov added 9 commits April 24, 2025 11:34

zombienet-sdk test prep

4ce41ba

Spawning network works

46ba5ca

Add README

68ba240

Restructure the code a little

5734fd0

Happy case - initial code

94abf12

remove duplicated file

c5caa43

Test works, parsing is messy, code needs a cleanup

f4a2e80

Comments, naming, formatting

91ea999

happy_case - final touches

342554f

tdimitrov added R0-silent The change does not warrant a re-release of the modified crates. T10-tests This PR/Issue is related to tests. labels Apr 24, 2025

tdimitrov requested review from kianenigma and Ank4n April 24, 2025 08:42

tdimitrov commented Apr 24, 2025

View reviewed changes

substrate/zombienet-sdk/tests/staking_next/tests.rs Outdated Show resolved Hide resolved

kianenigma reviewed Apr 24, 2025

View reviewed changes

substrate/zombienet-sdk/tests/staking_next/tests.rs Outdated Show resolved Hide resolved

kianenigma reviewed Apr 24, 2025

View reviewed changes

substrate/zombienet-sdk/tests/staking_next/tests.rs Outdated Show resolved Hide resolved

kianenigma reviewed Apr 24, 2025

View reviewed changes

tdimitrov added 6 commits April 24, 2025 15:18

staking_next -> staking_async

914fe17

Fix formatting in Cargo files

2b3baa5

Use subxt static interface for event parsing

8694bef

Move the test in staking-async

933eefd

Move the test to staking-async

9255217

comment

fbab0f6

tdimitrov requested a review from a team as a code owner April 25, 2025 07:05

Revert to dynamic interface to avoid handling metadata

5126842

fmt

27d40e3

fix feature propagation

6cb7089

md lint

d5b5a23

tdimitrov requested a review from kianenigma April 25, 2025 13:06

tdimitrov added 3 commits April 29, 2025 15:49

Add pallet_root_offences to the test runtime

5f3e93a

Extract utility function in a common module

b37e513

More code extraction

2054335

kianenigma reviewed May 2, 2025

View reviewed changes

tdimitrov added 2 commits May 8, 2025 16:36

Add log target + some renames

b450149

Merge branch 'master' into tsv-ah-staking-tests-2

b286c77

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Zombienet-SDK test for the happy path of the new staking-async flow #8317

Zombienet-SDK test for the happy path of the new staking-async flow #8317

tdimitrov commented Apr 24, 2025

kianenigma Apr 24, 2025

tdimitrov Apr 24, 2025

tdimitrov Apr 25, 2025

kianenigma Apr 24, 2025

kianenigma Apr 24, 2025

tdimitrov Apr 25, 2025

kianenigma Apr 24, 2025

kianenigma Apr 24, 2025

kianenigma Apr 24, 2025

tdimitrov Apr 24, 2025

kianenigma left a comment

tdimitrov commented Apr 25, 2025

jsdw commented Apr 25, 2025

kianenigma May 2, 2025

paritytech-workflow-stopper bot commented May 14, 2025


		mod tests;

		/// Sets `ValidatorCount` in staking pallet to 500.

Zombienet-SDK test for the happy path of the new staking-async flow #8317

Are you sure you want to change the base?

Zombienet-SDK test for the happy path of the new staking-async flow #8317

Conversation

tdimitrov commented Apr 24, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kianenigma left a comment

Choose a reason for hiding this comment

tdimitrov commented Apr 25, 2025

jsdw commented Apr 25, 2025

Choose a reason for hiding this comment

paritytech-workflow-stopper bot commented May 14, 2025