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

transport: Introduce common listener for tcp and websocket #147

Merged
merged 20 commits into from
Jun 13, 2024

Conversation

lexnv
Copy link
Collaborator

@lexnv lexnv commented Jun 11, 2024

This PR aims to remove duplicated code for socket listener between TCP and WebSocket transports.

Changes done:

  • Extended code logic to facilitate both TCP and WebSockets
  • Moved the listener code to a dedicated common module
  • Provided fairness for polling the listener stream by introducing a rotating index
  • Traits to convert between multiaddr <-> socket addr
  • Removed unnecessary cloning of listen addresses

Part of #70.

lexnv added 11 commits June 10, 2024 13:22
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
@lexnv lexnv added the enhancement New feature or request label Jun 11, 2024
@lexnv lexnv self-assigned this Jun 11, 2024
Copy link
Collaborator

@dmitry-markin dmitry-markin left a comment

Choose a reason for hiding this comment

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

Looks good!

I would, may be, go one step further and make SocketListener completely unaware of whether it deals with TCP or WebSocket. Basically, the main difference is the code path in get_socket_address(), and may be this can be made external to SocketListener. The PR already simplifies a lot and is good in it's current form, so we can do this in a follow-up if you consider this valuable/doable.

src/transport/common/listener.rs Outdated Show resolved Hide resolved
src/transport/common/listener.rs Outdated Show resolved Hide resolved
src/transport/common/listener.rs Show resolved Hide resolved
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Copy link
Collaborator

@dmitry-markin dmitry-markin left a comment

Choose a reason for hiding this comment

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

Everything looks good!

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
@lexnv lexnv merged commit 70819df into master Jun 13, 2024
8 checks passed
@lexnv lexnv deleted the lexnv/refactor-tcp branch June 13, 2024 09:55
dmitry-markin added a commit that referenced this pull request Jun 14, 2024
## [0.6.0] - 2024-06-14    
    
This release introduces breaking changes into `kad` module. The API has
been extended as following:
    
- An event `KademliaEvent::IncomingRecord` has been added.    
- New methods `KademliaHandle::store_record()` /
`KademliaHandle::try_store_record()` have been introduced.
    
This allows implementing manual incoming DHT record validation by
configuring `Kademlia` with `IncomingRecordValidationMode::Manual`.
    
Also, it is now possible to enable `TCP_NODELAY` on sockets.    
    
Multiple refactorings to remove the code duplications and improve the
implementation robustness have been done.
    
### Added    
    
- Support manual DHT record insertion
([#135](#135))
- transport: Make `TCP_NODELAY` configurable
([#146](#146))
    
### Changed    
    
- transport: Introduce common listener for tcp and websocket
([#147](#147))
- transport/common: Share DNS lookups between TCP and WebSocket
([#151](#151))
    
### Fixed    
    
- ping: Make ping fault tolerant wrt outbound substreams races
([#133](#133))
- crypto/noise: Make noise fault tolerant
([#142](#142))
- protocol/notif: Fix panic on missing peer state
([#143](#143))
- transport: Fix erroneous handling of secondary connections
([#149](#149))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants