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: Extract common transport code between TCP and WebSocket #70

Closed
2 tasks done
Tracked by #140
lexnv opened this issue Mar 29, 2024 · 1 comment
Closed
2 tasks done
Tracked by #140

transport: Extract common transport code between TCP and WebSocket #70

lexnv opened this issue Mar 29, 2024 · 1 comment
Assignees
Labels
enhancement New feature or request

Comments

@lexnv
Copy link
Collaborator

lexnv commented Mar 29, 2024

Make the transport layer a bit more module and allow the TCP transport code to be reused for WebSocket purposes.

Tasks

Preview Give feedback
  1. enhancement
    lexnv
  2. enhancement
    lexnv

cc #69 @paritytech/networking

@lexnv lexnv added the enhancement New feature or request label Mar 29, 2024
@lexnv lexnv self-assigned this Jun 10, 2024
lexnv added a commit that referenced this issue Jun 13, 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.

---------

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
lexnv added a commit that referenced this issue Jun 13, 2024
Move the dns resolving to a dedicated common module and return specific
errors on failures.

Part of: #70

---------

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
lexnv added a commit that referenced this issue Aug 21, 2024
… error reporting (#206)

The purpose of this PR is to pave the way for making the Identify
protocol more robust, which is currently linked with the low number of
peers and connective issues over a long period of time
- paritytech/polkadot-sdk#4925

This PR adds a coherent `DialError` that exposes the minimal information
users need to know about dial failures.
- paritytech/polkadot-sdk#5239

A new litep2p event is added for reporting multiple dial errors that
occur on different protocols back to the user:

```rust
    /// A list of multiple dial failures.
    ListDialFailures {
        /// List of errors.
        ///
        /// Depending on the transport, the address might be different for each error.
        errors: Vec<(Multiaddr, DialError)>,
    },
```

This event eases the debugging of substrate connectivity issues. At the
same time, it can be used in a future PR to inform back to the Identify
protocol which self-reported addresses of some peers are unreachable:
- #203

### Next Steps
- Add more tests
- Warp sync + sync full nodes since this is touching individual
transports

### Future Work
- The overarching `litep2p::Error` needs a closer look and a
refactoring:
  - #204
  - #128
  
- ConnectionError event for individual transports can be simplified:
  - #205
  
- I've observed some inconsistencies in handling TCP vs WebSocket
connection timeouts. I believe that we can have another pass and share
even more code between them:
  - #70

---------

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Co-authored-by: Dmitry Markin <dmitry@markin.tech>
@lexnv
Copy link
Collaborator Author

lexnv commented Oct 16, 2024

Closing this for now, most of the code is shared between TCP and Websocket. At the moment, there is little to gain from sharing even more code

@lexnv lexnv closed this as completed Oct 16, 2024
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

No branches or pull requests

1 participant