-
Notifications
You must be signed in to change notification settings - Fork 293
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
Protocol level changes to improve idle clients #3234
Comments
The first point makes sense and I agree with your assessment regarding the single-way ping limitation for the "pong peer". I'll work on implementing this shortly. |
I was looking at this today and noticed that it will unfortunately break the ping related statistics for inbound peers. Notably the One potential solution is to use the aforementioned 3-way ping/pong/ack so that outbound peers can determine it from the While it would perhaps be most correct to introduce a new |
I'll think a bit more about this, but it seems reasonable to me (each peer will only ever see 1 inbound pong, so there's no ambiguity). |
After thinking on it a little more, while I'm fairly confident not introducing a new message and reusing pong for the ack would work, I've come to the conclusion that I don't think it really would be a good approach. The reasoning is because choosing when to do it or not for proper backwards compatibility still really requires a new protocol version (or at least some type of mechanism to differentiate, but that is customarily the protocol version). Given that a new protocol version would essentially be required anyway, there is little point in not introducing a new This is all presuming that we want to do a 3-way ping/pong/ack. Another alternative is to deprecate those fields, but then it would need two versions and something still would have to be done to populate them in the mean time. |
Mobile OSs have low limits for CPU usage for apps in the background. It is also important to limit CPU as much as possible to conserve battery. Based on some recent (informal) profile sessions, I'd like to propose a few changes to the P2P protocol to help with these (and other idle client usages):
1. Make pings be the responsibility of the connecting (i.e. outbound) peer as opposed to both peers performing it.
Both peers performing pings is wasteful, because they are both performing the same action: namely, checking whether the remote is alive. However, a single ping/pong pair of messages should mostly (see limitation below) suffice to ensure the connection is still working, as both peers both send and receive one message.
Each ping (and indeed every message) involves performing a hash, so limiting the amount of pings/pongs processed and leaving them the responsibility of the outbound peer allows mobile/SPV/client peers to better modulate the CPU usage related to the connection keep alive logic.
Technically, switching from two-way to single-way ping does have a limitation: the "pong peer" (i.e. the one that received the ping and is responding with the pong), doesn't know if the other peer has actually received its message. While it can assume the other peer is still alive (because it received its ping), it cannot know for sure. To fix this limitation, we'd have to switch to a 3-way handshake ping method.
However, I don't believe the prior limitation is significant, because if the pong fails to be received, the remote peer will disconnect anyway. In a worst case scenario (where the network connection drops without any additional packets going around) at most one ping interval delay will be spent waiting for new messages (which could happen at any time anyway).
2. Make tx invs optionalEdit: I just learned MsgVersion.DisableRelayTx exists, but isn't currently exposed on the wallet, so I'll work on exposing that there.
End-user clients in a background/idle state have very little interest in mempool transactions. First, because the transactions are unlikely to be for them, and second because they don't usually relay them.Therefore, being able to negotiate with the remote peer so that it does not send any tx invs could be used to reduce CPU usage when idle, by removing the need to receive and process incoming inv messages with tx data.This could be made as an interactive protocol (a local peer could enable/disable receiving invs multiple times within the span of a single connection), however that has the drawback of leaking when the peer is likely active in the software, therefore negotiating only once per connection (during the initial protocol negotiation stage) seems better to me.Even without the interactive protocol, this change has the downside of additional metadata leaks about the type of client the peer is running. In particular, it would make it easier to fingerprint mobile clients if they are always connecting to nodes and disabling receiving tx invs. The counterpoint to this argument is that a client could be configured for additional privacy by not disabling invs if they desired the extra privacy protection.The text was updated successfully, but these errors were encountered: