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

fix!: make connection securing abortable #2662

Merged
merged 5 commits into from
Aug 30, 2024

Conversation

achingbrain
Copy link
Member

To allow doing things like having a single AbortSignal that can be used as a timeout for incoming connection establishment, allow passing it as an option to the ConnectionEncrypter secureOutbound and secureInbound methods.

Previously we'd wrap the stream to be secured in an AbortableSource, however this has some serious performance implications and it's generally better to just use a signal to cancel an ongoing operation instead of racing every chunk that comes out of the source.

BREAKING CHANGE: the final argument to secureOutbound and secureInbound in the ConnectionEncrypter interface is now an options object

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if necessary (this includes comments as well)
  • I have added tests that prove my fix is effective or that my feature works

@achingbrain achingbrain added the version-2.0 PRs that will be released in libp2p v2 label Aug 14, 2024
@achingbrain achingbrain requested a review from a team as a code owner August 14, 2024 10:50
@achingbrain achingbrain mentioned this pull request Aug 14, 2024
27 tasks
To allow doing things like having a single `AbortSignal` that can
be used as a timeout for incoming connection establishment, allow
passing it as an option to the `ConnectionEncrypter` `secureOutbound`
and `secureInbound` methods.

Previously we'd wrap the stream to be secured in an `AbortableSource`,
however this has some [serious performance implications](ChainSafe/js-libp2p-gossipsub#361)
and it's generally better to just use a signal to cancel an ongoing
operation instead of racing every chunk that comes out of the source.
@achingbrain achingbrain force-pushed the fix/make-connection-securing-abortable branch from 786ba3c to 277f755 Compare August 14, 2024 11:09
@achingbrain achingbrain merged commit 86a62f5 into release-v2.0 Aug 30, 2024
24 checks passed
@achingbrain achingbrain deleted the fix/make-connection-securing-abortable branch August 30, 2024 11:06
achingbrain added a commit that referenced this pull request Sep 6, 2024
To allow doing things like having a single `AbortSignal` that can be used as a timeout for incoming connection establishment, allow passing it as an option to the `ConnectionEncrypter` `secureOutbound` and `secureInbound` methods.

Previously we'd wrap the stream to be secured in an `AbortableSource`, however this has some [serious performance implications](ChainSafe/js-libp2p-gossipsub#361) and it's generally better to just use a signal to cancel an ongoing operation instead of racing every chunk that comes out of the source.

BREAKING CHANGE: the final argument to `secureOutbound` and `secureInbound` in the `ConnectionEncrypter` interface is now an options object

---------

Co-authored-by: chad <chad.nehemiah94@gmail.com>
achingbrain added a commit that referenced this pull request Sep 6, 2024
To allow doing things like having a single `AbortSignal` that can be used as a timeout for incoming connection establishment, allow passing it as an option to the `ConnectionEncrypter` `secureOutbound` and `secureInbound` methods.

Previously we'd wrap the stream to be secured in an `AbortableSource`, however this has some [serious performance implications](ChainSafe/js-libp2p-gossipsub#361) and it's generally better to just use a signal to cancel an ongoing operation instead of racing every chunk that comes out of the source.

BREAKING CHANGE: the final argument to `secureOutbound` and `secureInbound` in the `ConnectionEncrypter` interface is now an options object

---------

Co-authored-by: chad <chad.nehemiah94@gmail.com>
achingbrain added a commit that referenced this pull request Sep 6, 2024
To allow doing things like having a single `AbortSignal` that can be used as a timeout for incoming connection establishment, allow passing it as an option to the `ConnectionEncrypter` `secureOutbound` and `secureInbound` methods.

Previously we'd wrap the stream to be secured in an `AbortableSource`, however this has some [serious performance implications](ChainSafe/js-libp2p-gossipsub#361) and it's generally better to just use a signal to cancel an ongoing operation instead of racing every chunk that comes out of the source.

BREAKING CHANGE: the final argument to `secureOutbound` and `secureInbound` in the `ConnectionEncrypter` interface is now an options object

---------

Co-authored-by: chad <chad.nehemiah94@gmail.com>
@achingbrain achingbrain mentioned this pull request Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
version-2.0 PRs that will be released in libp2p v2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants