Skip to content

Always send full parent header, not only hash, part of collation response#8939

Merged
Sajjon merged 14 commits intomasterfrom
cyon/always_send_pull_parent_head_data_in_collation_response_7733
Jun 30, 2025
Merged

Always send full parent header, not only hash, part of collation response#8939
Sajjon merged 14 commits intomasterfrom
cyon/always_send_pull_parent_head_data_in_collation_response_7733

Conversation

@Sajjon
Copy link
Contributor

@Sajjon Sajjon commented Jun 23, 2025 •
edited
Loading

Implementation of #7733

Description

Instead of conditionally sending the full parent header in the collation response we now always send it (never the hash of it).

@Sajjon Sajjon marked this pull request as ready for review June 23, 2025 12:55
@Sajjon Sajjon added T0-node This PR/Issue is related to the topic “node”. T8-polkadot This PR/Issue is related to/affects the Polkadot network. labels Jun 23, 2025
@Sajjon
Copy link
Contributor Author

Sajjon commented Jun 23, 2025

/cmd prdoc --audience Node Dev --bump patch

@github-actions
Copy link
Contributor

github-actions bot commented Jun 23, 2025

Command "prdoc --audience Node Dev --bump patch" has failed ❌! See logs here

@Sajjon
Copy link
Contributor Author

Sajjon commented Jun 23, 2025

/cmd prdoc --audience node_dev --bump patch

@Sajjon Sajjon requested a review from alindima June 23, 2025 14:42
sandreim
sandreim reviewed Jun 23, 2025
View reviewed changes
polkadot/node/network/collator-protocol/src/collator_side/mod.rs Outdated
} else {
ParentHeadData::OnlyHash(parent_head_data_hash)
};
let parent_head_data =
Copy link
Contributor

@sandreim sandreim Jun 23, 2025

Choose a reason for hiding this comment

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

In send_collation( we should also be able to get rid of the match since all validators now support CollationWithParentHeadData.

Copy link
Contributor Author

@Sajjon Sajjon Jun 24, 2025

Choose a reason for hiding this comment

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

is this what you had in mind: 1eeb80b ?

Copy link
Contributor

@alindima alindima Jun 24, 2025

Choose a reason for hiding this comment

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

I wouldn't define a new data type just for this purpose, just pass both values to the send_collation

Copy link
Contributor

@alindima alindima Jun 24, 2025

Choose a reason for hiding this comment

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

actually, we don't even use the hash here

Copy link
Contributor Author

@Sajjon Sajjon Jun 24, 2025

Choose a reason for hiding this comment

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

Ok I think I've made sensible changes now.

@alindima
Copy link
Contributor

alindima commented Jun 24, 2025

I did not need to update any failing unit tests, all tests were passing
But I will try to see if I can add a new one asserting that we always the full header.

See decode_collation_response

alexggh
alexggh reviewed Jun 24, 2025
View reviewed changes
alindima
alindima approved these changes Jun 24, 2025
View reviewed changes
Copy link
Contributor

@alindima alindima left a comment

Choose a reason for hiding this comment

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

LGTM, but let's fix the formatting

@Sajjon
Copy link
Contributor Author

Sajjon commented Jun 24, 2025

/cmd fmt

@paritytech-workflow-stopper
Copy link

paritytech-workflow-stopper bot commented Jun 24, 2025

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

@Sajjon Sajjon requested review from alexggh and sandreim June 25, 2025 08:03
@Sajjon Sajjon added this pull request to the merge queue Jun 30, 2025
Merged via the queue into master with commit 0447d26 Jun 30, 2025
262 of 263 checks passed
@Sajjon Sajjon deleted the cyon/always_send_pull_parent_head_data_in_collation_response_7733 branch June 30, 2025 13:30
ordian added a commit that referenced this pull request Jul 1, 2025
@ordian
Merge branch 'master' into ao-fix-disabling-race
7951995 
* master:
  EPMB/unsigned: fixed multi-page winner computation (#8987)
  Always send full parent header, not only hash, part of collation response (#8939)
ordian added a commit that referenced this pull request Jul 24, 2025
@ordian
Merge branch 'master' into ao-fix-issue-64
5cd6006 
* master: (91 commits)
  Add extra information to the harmless error logs during validate_transaction (#9047)
  `sp-tracing`: Remove `test-utils` feature (#9063)
  add try-state check for staking roles -- staker cannot be nominator a… (#9034)
  net/discovery: File persistence for `AddrCache` (#8839)
  dispute-coordinator: handle race with offchain disabling (#9050)
  Align parameters for `EventEmitter::emit_sent_event` (#9057)
  Fetch parent block `api_version` (#9059)
  [XCM Precompile] Rename functions and improve docs in the Solidity interface (#9023)
  Cleanup and improvements for `ControlledValidatorIndices` (#8896)
  reenable 0001-parachains-pvf (#9046)
  Add optional auto-rebag within on-idle (#8684)
  Fix flaxy 0003-block-building-warp-sync test - one more approach (#8974)
  [Staking] [AHM] Fixes insufficient slashing of nominators (and some other small issues). (#8937)
  chore: Bump bounded-collections dep (#9004)
  XCMP and DMP improvements (#8860)
  EPMB/unsigned: fixed multi-page winner computation (#8987)
  Always send full parent header, not only hash, part of collation response (#8939)
  revive: Precompiles should return dummy code when queried (#9001)
  Fix confusing log messages in network protocol behaviour (#8819)
  Fix pallet_migrations benchmark when FailedMigrationHandler emits events (#8694)
  ...
alvicsam pushed a commit that referenced this pull request Oct 17, 2025
@Sajjon @github-actions @alvicsam
Always send full parent header, not only hash, part of collation resp…
70800ee 
…onse (#8939)

Implementation of #7733

# Description
Instead of **conditionally** sending the full parent header in the
collation response we now **always** send it (never the hash of it).

---------

Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
RomarQ pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Dec 3, 2025
@Sajjon @github-actions @RomarQ
Always send full parent header, not only hash, part of collation resp…
91b82d6 
…onse (paritytech#8939)

Implementation of paritytech#7733

# Description
Instead of **conditionally** sending the full parent header in the
collation response we now **always** send it (never the hash of it).

---------

Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alindima alindima alindima approved these changes

@alexggh alexggh alexggh approved these changes

@sandreim sandreim sandreim approved these changes

Assignees

No one assigned

Labels

T0-node This PR/Issue is related to the topic “node”. T8-polkadot This PR/Issue is related to/affects the Polkadot network.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Sajjon @alindima @alexggh @sandreim @pgherveou