Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[stable2407] Backport #6603 #6668

Open
wants to merge 1 commit into
base: stable2407
Choose a base branch
from
Open

[stable2407] Backport #6603 #6668

wants to merge 1 commit into from
+18 −5

Conversation

paritytech-cmd-bot-polkadot-sdk[bot]
Copy link

Backport #6603 into stable2407 from lexnv.

See the documentation on how to use this bot.

@lexnv @github-actions
litep2p/req-resp: Always provide main protocol name in responses (#6603)
e9bbbee 
Request responses are initialized with a main protocol name, and
optional protocol names as a fallback.

Running litep2p in kusama as a validator has surfaced a `debug_asserts`
coming from the sync component:

https://github.com/paritytech/polkadot-sdk/blob/3906c578c96d97a8a099a4bdac4685acbe375a7c/substrate/client/network/sync/src/strategy/chain_sync.rs#L640-L646

The issue is that we initiate a request-response over the main protocol
name `/genesis/sync/2` but receive a response over the legacy procotol
`ksm/sync/2`. This behavior is correct because litep2p propagates to the
higher levels the protocol that responded.

In contrast, libp2p provides the main protocol name regardless of
negotiating a legacy protocol.

Because of this, higher level components assume that only the main
protocol name will respond.
To not break this assumption, this PR alings litep2p shim layer with the
libp2p behavior.

Closes: #6581

---------

Signed-off-by: Alexandru Vasile <[email protected]>
Co-authored-by: Dmitry Markin <[email protected]>
(cherry picked from commit 2a0b268)
@paritytech-cmd-bot-polkadot-sdk paritytech-cmd-bot-polkadot-sdk bot mentioned this pull request Nov 27, 2024
Merged
@github-actions github-actions bot added the A3-backport Pull request is already reviewed well in another branch. label Nov 27, 2024
@github-actions GitHub Actions
Copy link

This pull request is amending an existing release. Please proceed with extreme caution,
as to not impact downstream teams that rely on the stability of it. Some things to consider:

  • Backports are only for 'patch' or 'minor' changes. No 'major' or other breaking change.
  • Should be a legit fix for some bug, not adding tons of new features.
  • Must either be already audited or trivial (not sure audit).
Emergency Bypass

If you really need to bypass this check: add validate: false to each crate
in the Prdoc where a breaking change is introduced. This will release a new major
version of that crate and all its reverse dependencies and basically break the release.

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable 2/3
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7826019

@EgorPopelyaev
Copy link
Contributor

@lexnv Should we wait for Batsi to ✅ or I can merge it as is?

@lexnv
Copy link
Contributor

lexnv commented Nov 27, 2024 •
edited
Loading

Can merge it as is, this is litep2p impacting only 🙏

dq: is 2407 considered stale? If that is the case we can rely on the newer 2409 and 2412 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@lexnv lexnv

Labels
A3-backport Pull request is already reviewed well in another branch.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@paritytech-cicd-pr @EgorPopelyaev @lexnv