search

contrun / ckb-p2p-experiments

1 stars 0 forks source link

Is it possible to do connection upgrade in tentacle? #1

Open contrun opened 1 year ago

contrun commented 1 year ago

Say two peers A and B are behind NATs, peer A wants to connect to peer B. Peer A first establishes a connection C1 to peer B through relay R. Afterwards, we somehow successfully punched a hole and create a direct connection c2 from A to B. Now there are two connections C1 and C2. We want to deliver/receive messages to/from both connections to up level applications. Ideally the direct connection C2 should be preferred. There should be some connection upgrade mechanism to close relayed connection C1 and to use the new connection C2 without affecting up level application. We need to be careful while upgrading the connection as there may be a race condition between the up level application sending/receiving data and closing the unfavorable connection.

contrun commented 1 year ago

Libp2p seems to have book-kept all connections to all peers. It just closes another connection directly.

feat(libp2p): direct connection through relay protocol (DCUtR) by achingbrain · Pull Request #1928 · libp2p/js-libp2p

contrun commented 1 year ago

We need to first make transient connections (e.g. relayed connections) acceptable to tentacle. Currently there can be only one connection to a peer with the same public key.

https://github.com/contrun/ckb-p2p-experiments/blob/b2f3829fd79fe9c7437163334d38d9cec6dce58d/tentacle/tentacle/src/service.rs#L618-L667

We need to thoroughly review the impact of allowing many connections to a peer (identified by its public key).

contrun commented 1 year ago

Currently the connection information (so called session) is also tracked up level application.

In order to send a message to a peer in https://github.com/contrun/ckb-p2p-experiments/blob/b2f3829fd79fe9c7437163334d38d9cec6dce58d/connector/src/lib.rs#L302-L323

we have to obtain a session https://github.com/contrun/ckb-p2p-experiments/blob/b2f3829fd79fe9c7437163334d38d9cec6dce58d/connector/src/lib.rs#L325-L331

This is different from libp2p where https://pkg.go.dev/github.com/libp2p/go-libp2p/core/host where NewStream

    // NewStream opens a new stream to given peer p, and writes a p2p/protocol
    // header with given ProtocolID. If there is no connection to p, attempts
    // to create one. If ProtocolID is "", writes no header.
    // (Thread-safe)
    NewStream(ctx context.Context, p peer.ID, pids ...protocol.ID) (network.Stream, error)

needs only a peer.ID. In other words, all the connection information is book-kept by libp2p the library instead of up level application. In this way, libp2p can look up connection we are going to use with the passed peer.ID. This makes more sense in hole-punching as there may exist many connections between peers (relayed connection or direct connection through hole-punching) and we don't want to leave this book-keeping burden to up level application. This may be a major refactoring for tentacle.

contrun commented 1 year ago

I tried to use peer id instead of session id to send/receive messages in https://github.com/contrun/ckb-p2p-experiments/commit/354e74f16deb4e693cba8113423e0dd7728d2c36 . This commit is reverted because this is a large refactor and there are still many APIs that needs to be changed. In order to iterate more quickly, I decided not to change tentacle itself, but book-keep all the information within the connector trait. The downside of this is that ckb also needs to do the same thing later.