Skip to content

Remove deprecated collator-related code in cumulus#9662

Merged
bkchr merged 30 commits intoparitytech:masterfrom
moonbeam-foundation:rq/remove-deprecated-collator-code
Sep 12, 2025
Merged

Remove deprecated collator-related code in cumulus#9662
bkchr merged 30 commits intoparitytech:masterfrom
moonbeam-foundation:rq/remove-deprecated-collator-code

Conversation

@RomarQ
Copy link
Copy Markdown
Contributor

@RomarQ RomarQ commented Sep 5, 2025

Removes collator-related code in cumulus, which has been deprecated for a long time.

Removes an old test, which was adapted in paritytech/cumulus#480 and duplicated by

fn block_building_storage_proof_does_not_include_runtime_by_default() {
let builder = substrate_test_runtime_client::TestClientBuilder::new();
let client = builder.build();
let genesis_hash = client.info().best_hash;
let block = BlockBuilderBuilder::new(&client)
.on_parent_block(genesis_hash)
.with_parent_block_number(0)
.enable_proof_recording()
.build()
.unwrap()
.build()
.unwrap();
let proof = block.proof.expect("Proof is build on request");
let genesis_state_root = client.header(genesis_hash).unwrap().unwrap().state_root;
let backend =
sp_state_machine::create_proof_check_backend::<Blake2Hasher>(genesis_state_root, proof)
.unwrap();
assert!(backend
.storage(&sp_core::storage::well_known_keys::CODE)
.unwrap_err()
.contains("Database missing expected key"),);
}

PoV Recovery Test Updates

Updates the PoV recovery test (cumulus/zombienet/zombienet-sdk/tests/zombie_ci/pov_recovery.rs) to use a more realistic consensus mechanism:

Changes Made

  • Removed: --use-null-consensus flag from test configuration

Rationale

Previous behavior (with null consensus):

  • Nodes operated without real block production
  • PoV recovery mechanisms triggered more frequently
  • Created artificial test conditions that don't reflect production scenarios

New behavior (with actual consensus):

  • Nodes produce blocks normally but don't announce them to peers
  • PoV recovery occurs at a more realistic frequency
  • Better simulates real-world network conditions where blocks may be missed

Impact

This change makes the test more representative of actual network conditions while maintaining the core functionality being tested.

Follow-up

Remove the following lines:

// TODO: Remove after https://github.com/paritytech/cumulus/issues/479
sp_io::storage::set(b":c", &[]);

Review notes

I recommend enabling Hide whitespace option when reviewing the changes:

image

@RomarQ RomarQ changed the title remove deprecated cumulus collator code Removes deprecated collator-related code in cumulus Sep 5, 2025
@RomarQ RomarQ changed the title Removes deprecated collator-related code in cumulus Remove deprecated collator-related code in cumulus Sep 5, 2025
@RomarQ RomarQ marked this pull request as draft September 5, 2025 18:10
@RomarQ RomarQ marked this pull request as ready for review September 5, 2025 18:32
@RomarQ RomarQ marked this pull request as draft September 5, 2025 18:49
@RomarQ RomarQ marked this pull request as ready for review September 5, 2025 22:22
@bkchr bkchr added the T0-node This PR/Issue is related to the topic “node”. label Sep 9, 2025
@bkchr bkchr enabled auto-merge September 9, 2025 15:14
auto-merge was automatically disabled September 9, 2025 15:51

Head branch was pushed to by a user without write access

@RomarQ RomarQ requested review from a team as code owners September 9, 2025 19:50
@RomarQ
Copy link
Copy Markdown
Contributor Author

RomarQ commented Sep 10, 2025

Once #9695 gets merged, will merge the changes here.

@RomarQ
Copy link
Copy Markdown
Contributor Author

RomarQ commented Sep 12, 2025

@lrubasze since you fixed the flakiness and enabled the pov_recovery zombienet tests recently, could you also have a look at the changes in this PR?

@bkchr bkchr added this pull request to the merge queue Sep 12, 2025
Merged via the queue into paritytech:master with commit 136b4cb Sep 12, 2025
258 of 262 checks passed
alvicsam pushed a commit that referenced this pull request Oct 17, 2025
Removes collator-related code in cumulus, which has been deprecated for
a long time.

Removes an old test, which was adapted in
paritytech/cumulus#480 and duplicated by
https://github.com/paritytech/polkadot-sdk/blob/acac0127168dac1d603e4d996cb210ceeddeb5de/substrate/client/block-builder/src/lib.rs#L389-L415

## PoV Recovery Test Updates

Updates the PoV recovery test
(`cumulus/zombienet/zombienet-sdk/tests/zombie_ci/pov_recovery.rs`) to
use a more realistic consensus mechanism:

### Changes Made
- **Removed**: `--use-null-consensus` flag from test configuration

### Rationale

**Previous behavior** (with null consensus):
- Nodes operated without real block production
- PoV recovery mechanisms triggered more frequently
- Created artificial test conditions that don't reflect production
scenarios

**New behavior** (with actual consensus):
- Nodes produce blocks normally but don't announce them to peers
- PoV recovery occurs at a more realistic frequency
- Better simulates real-world network conditions where blocks may be
missed

### Impact

This change makes the test **more representative** of actual network
conditions while maintaining the core functionality being tested.

## Follow-up
Remove the following lines:

https://github.com/paritytech/polkadot-sdk/blob/4acb964059a218be9bac954b4e3803b78b5526bf/cumulus/pallets/parachain-system/src/lib.rs#L993-L994

## Review notes

I recommend enabling `Hide whitespace` option when reviewing the
changes:

<img width="300" alt="image"
src="https://github.com/user-attachments/assets/41f137af-c0b9-435e-af1e-84e51cbdfa23"
/>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T0-node This PR/Issue is related to the topic “node”.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants