Fix nodes wasting bandwidth during sync due to transactions

nazar-pc commented 2 weeks ago

Is there an existing issue?

[X] I have searched the existing issues

Experiencing problems? Have you tried our Stack Exchange first?

[X] This is not a support question.

Motivation

On a busy network there could be quite some bandwidth occupied by transactions. However, when node is still syncing, there is no point in downloading transactions, which is exactly what is checked here: https://github.com/paritytech/polkadot-sdk/blob/edf79aa972bcf2e043e18065a9bb860ecdbd1a6e/substrate/client/network/transactions/src/lib.rs#L403-L409

The problem is that it is the receiving side! The node is already using a bunch of bandwidth trying to download blocks as fast as possible to import them, there is no point in receiving useless transactions on top of that, especially if they are large.

But there is a double bottom to this problem: wasteful bandwidth usage even when node is synced: https://github.com/paritytech/polkadot-sdk/issues/6433

Together situation can be very bad from my observation on our network for peers with slower network connection.

Request

Do not send transactions to syncing peers

Solution

This issue can be efficiently addressed with suggestions from https://github.com/paritytech/polkadot-sdk/issues/5624 (where frustratingly I'm yet to receive any feedback) where solution is for peers to voluntarily announce to each other if they are synced. This way sender can make a local decision whether it makes sense to propagate transactions to a particular peer or not.

Addressing both this and https://github.com/paritytech/polkadot-sdk/issues/5624 will likely require an RFC.

Are you willing to help with this request?

Yes!

teor2345 commented 2 weeks ago

The way this works in many Bitcoin-derived networks is:

new block and transaction hashes are broadcast by peers
nodes don’t start their mempool until synced
once the mempool has started, nodes choose to download transactions they don’t know about

Is that the kind of solution you’re aiming for here?

In Bitcoin, nodes that push too much unrequested data tend to get banned, to avoid wastage/attacks like this.

nazar-pc commented 2 weeks ago

Is that the kind of solution you’re aiming for here?

Yes, second part of the solution is in https://github.com/paritytech/polkadot-sdk/issues/6433 and is independent from the issue described here

nazar-pc commented 2 weeks ago

Here is how it can be done once synced status information is available (https://github.com/paritytech/polkadot-sdk/issues/5624): https://github.com/autonomys/polkadot-sdk/commit/94a1a8143a89bbe9f938c1939ff68abc1519a305?w=1

bkchr commented 2 weeks ago

We really need a better transaction syncing protocol. There are ideas to use one of the bitcoin set reconciliation protocols. This should probably help here as well.

CC @michalkucharczyk

michalkucharczyk commented 2 weeks ago

Yes, we need improvements here for sure. New protocol is something I'd like to work on in mid-term future. I have some improvements to be made in fork-aware pool. Once they are done I could start looking into new protocol.

paritytech / polkadot-sdk