circuit-relay-server limits and shades of transient

[marcus-pousette]

• Version: 1.8.1

• Subsystem: registrar circuit relay server, registrar

Severity:

Medium or less

Description:

Since limits are applied to connection by default, all circuit relay connections will become transient. This means protocol handles that have set notifyOnTransient to false will not receive onConnect events for these connections.

But, are non-unlimited circuit relay connections really transient in the same way as the one we get from identify? What is an intuitive way on listen to topology changes:

A. notifyOnTransient set to true, and filter out all transient connections except the relayed one or b. notifyOnTransient set to false, but set applyDefaultLimit: false on the circuit relay server config?

It feeeels, like transient could be replace with limits property object

and then every connection would have this property and we can use that to filter out unwanted connections from the protocol handler, for example one that will timeout to soon, or does not allow us to send that much data

[achingbrain]

all circuit relay connections will become transient

This was true up until about a month ago when we merged https://github.com/libp2p/js-libp2p/pull/2575 - now relayed connections are only marked transient if the relay server actually applies limits.

It feeeels, like transient could be replace with limits property object

I think this might be a good idea. I'm certainly keen to move away from the 'transient' language in favour of 'limited' to better align with the changes around naming in [email protected] and showing the actual limits instead of having an opaque boolean value would make things a bit easier to work with.

E.g. the .limits object could have bytes and seconds properties that decrease as the allowances are used up.

We can then remove runOnTransientConnection/notifyOnTransient from libp2p.handle and libp2p.register (though keep it for libp2p.dialProtocol? Or is it fair to say you are on your own if you dial a protocol using a circuit relay address?) and all stream handlers and topology listeners would be invoked regardless of the limit, then they can make the decision to use the connection or not themselves.

These are all (very) breaking changes so would cause [email protected] to be released.

But, are non-unlimited circuit relay connections really transient in the same way as the one we get from identify?

I'm not sure I understand this question. What connections do you get from identify?

[marcus-pousette]

This was true up until about a month ago when we merged https://github.com/libp2p/js-libp2p/pull/2575 - now relayed connections are only marked transient if the relay server actually applies limits.

Yeap, and it seems like the parameterless constructor by default applies limits, so all circuit relay connections are now transient.

I think this might be a good idea. I'm certainly keen to move away from the 'transient' language in favour of 'limited' to better align with the changes around naming in [email protected] and showing the actual limits instead of having an opaque boolean value would make things a bit easier to work with.

We can then remove runOnTransientConnection/notifyOnTransient from libp2p.handle and libp2p.register (though keep it for libp2p.dialProtocol? Or is it fair to say you are on your own if you dial a protocol using a circuit relay address?) and all stream handlers and topology listeners would be invoked regardless of the limit, then they can make the decision to use the connection or not themselves.

Make sense!

I'm not sure I understand this question. What connections do you get from identify?

If I remember correctly, the identify protocol initiates a transient connection which is closed afterwards (?), not sure how that would perfectly fit in the { data, timeout } limit object though. Because that thing is going to close as soon as that the identification is done.

Also when I think about a generalisation of the concept, we could have other properties of the limits, where the limit or constraint is actually some control logic, that for example allows a peer to only transmit 100mb per day or something, or only transmit data that follow certain patterns.

[achingbrain]

If I remember correctly, the identify protocol initiates a transient connection which is closed afterwards (?)

Identify runs on every newly opened connection, it doesn't open or close connections itself.

The way everything currently works, a connection is only transient if it's via a relay, and that relay has applied data/time limits.

[achingbrain]

it seems like the parameterless constructor by default applies limits, so all circuit relay connections are now transient.

Can you link to the code you are talking about? The .transient property is only set to true if it's explicitly passed in, so omitting the parameter would have it be false.

The parameter is passed in from the upgrader, and this is only true if the relay has applied limits to the outbound or inbound connection.

[marcus-pousette]

Can you link to the code you are talking about? The .transient property is only set to true if it's explicitly passed in, so omitting the parameter would have it be false.

The parameter is passed in from the upgrader, and this is only true if the relay has applied limits to the outbound or inbound connection.

I might be wrong but

https://github.com/libp2p/js-libp2p/blob/944935f8dbcc1083e4cb4a02b49a0aab3083d3d9/packages/transport-circuit-relay-v2/src/server/reservation-store.ts#L51

applyDefaultLimits will be true unless explicitly setting it to false

So the peerstore get filled up with limited connections (by default (?))

https://github.com/libp2p/js-libp2p/blob/944935f8dbcc1083e4cb4a02b49a0aab3083d3d9/packages/transport-circuit-relay-v2/src/server/reservation-store.ts#L95

and we fetch it here

https://github.com/libp2p/js-libp2p/blob/944935f8dbcc1083e4cb4a02b49a0aab3083d3d9/packages/transport-circuit-relay-v2/src/server/index.ts#L341

then with the transport we have this line

https://github.com/libp2p/js-libp2p/blob/944935f8dbcc1083e4cb4a02b49a0aab3083d3d9/packages/transport-circuit-relay-v2/src/transport/transport.ts#L273C20-L273C26

and I assume the status is sent over from the server to the client and will mark the connection as transient (since status.limit will not be null)?

[achingbrain]

applyDefaultLimits will be true unless explicitly setting it to false

Ah yes - this is by design. My mistake - I thought you meant the Connection constructor, not that of the Circuit Relay server.

If we disable limits by default, all peers become open relays which makes them ripe for abuse. This was the case for Circuit Relay v1 and it became very expensive to run a relay so most people didn't bother.

See the rationale section of the Circuit Relay V2 spec for more on that, but in brief we want people to run limited relays as a public good, this makes it easier for browser nodes to become dialable via WebRTC and for quic nodes to be able to perform hole punching, so limits are enabled by default.

There are some valid use cases for being an open relay which is why you can disable limits but it's an opt-in thing.