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.

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

swarm/src/toggle: Ignore listen upgr errors when disabled #1945

Merged
merged 3 commits into from
Feb 4, 2021

Conversation

mxinden
Copy link
Member

@mxinden mxinden commented Feb 4, 2021

A disabled ToggleProtoHandler can receive listen upgrade errors in the
following two cases:

  1. Protocol negotiation on an incoming stream failed with no protocol being
    agreed on.

  2. When combining ProtocolsHandler implementations a single ProtocolsHandler
    might be notified of an inbound upgrade error unrelated to its own upgrade
    logic. For example when nesting a ToggleProtoHandler in a
    ProtocolsHandlerSelect the former might receive an inbound upgrade error
    even when disabled.

ToggleProtoHandler should ignore the error in both of these cases.

Fixes paritytech/polkadot#2373.

A disabled `ToggleProtoHandler` can receive listen upgrade errors in the
following two cases:

1. Protocol negotiation on an incoming stream failed with no protocol being
   agreed on.

2. When combining `ProtocolsHandler` implementations a single `ProtocolsHandler`
   might be notified of an inbound upgrade error unrelated to its own upgrade
   logic. For example when nesting a `ToggleProtoHandler` in a
   `ProtocolsHandlerSelect` the former might receive an inbound upgrade error
   even when disabled.

`ToggleProtoHandler` should ignore the error in both of these cases.
@mxinden
Copy link
Member Author

mxinden commented Feb 4, 2021

In case this patch finds approval, I would like to relase libp2p-swarm v0.27.2 unless anyone objects.

Copy link
Contributor

@romanb romanb left a comment

Choose a reason for hiding this comment

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

👍 Feel free to cut releases after merging.

@mxinden
Copy link
Member Author

mxinden commented Feb 4, 2021

releases

I planned to only publish a new libp2p-swarm patch release. Is there some other crate you would like to be released in conjunction?

@romanb
Copy link
Contributor

romanb commented Feb 4, 2021

I planned to only publish a new libp2p-swarm patch release. Is there some other crate you would like to be released in conjunction?

Not necessarily. I only used plural in case you think it would be desirable to have a libp2p crate release that has the minimum bound for the libp2p-swarm patch version introduced with this PR.

Copy link
Member

@tomaka tomaka left a comment

Choose a reason for hiding this comment

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

👍

@mxinden mxinden merged commit c897df3 into libp2p:master Feb 4, 2021
@mxinden
Copy link
Member Author

mxinden commented Feb 4, 2021

libp2p-swarm v0.27.2 tagged and published.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic originating from libp2p_swarm
3 participants