feat(network): add proxied nodes to info endpoint #2818
Conversation
github-actions
bot
added
network
packages/trackerless-network/src/proto/packages/dht/protos/DhtRpc.client.ts
Outdated
Show resolved
Hide resolved
| super({ | ||
| createConnectorFacade: () => new SimulatorConnectorFacade(localPeerDescriptor, simulator), | ||
| metricsContext: new MetricsContext(), | ||
| allowIncomingPrivateConnections: false | ||
| allowIncomingPrivateConnections |
There was a problem hiding this comment.
Are private connections and proxy connections the same thing? (Do we plan to use private connections to something else, too?)
E.g. we have this comment in ConnectionManager:
// Used to filter proxy connections from the connections view
private remotePrivateConnections: Set<DhtAddress> = new Set()
There was a problem hiding this comment.
Proxy connections is a layer2 concept so having a wider conecpt in the lower layer makes sense
There was a problem hiding this comment.
Also Not all proxy-connections are private connections. Nodes only use privateClientMode when they only have proxy client connections. If there are any streams where the node participates in normally private mode is disabled
There was a problem hiding this comment.
Proxy connections is a layer2 concept so having a wider conecpt in the lower layer makes sense
Ah ok, wasn't aware of this fact 👍
We could modify/remove this "Used to filter proxy connections from the connections view" comment as it is bit misleading.
There was a problem hiding this comment.
Also, not about this PR, but why the remotePrivateConnections has the remote-prefix? Is the whole connection private, i.e. both the local and remote see treat it specially? (If that's the case, we could remove the remote-prefix. If it is private only on the remote side, we could rename the accessor to match the field name.)
There was a problem hiding this comment.
There isn't a real need to maintain the state of the locally private connections as the entire connection manager is in private mode if they exist at all. The remote-prefix is kept to make it clear that they are set by someone else
There was a problem hiding this comment.
Could remove the comment yeah
Summary
Added proxied nodes to the to info endpoint