Skip to content
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

Fix nodes wasting bandwidth during sync due to transactions #6434

Open
2 tasks done
nazar-pc opened this issue Nov 11, 2024 · 5 comments
Open
2 tasks done

Fix nodes wasting bandwidth during sync due to transactions #6434

nazar-pc opened this issue Nov 11, 2024 · 5 comments
Labels
I5-enhancement An additional feature request.

Comments

@nazar-pc
Copy link
Contributor

Is there an existing issue?

  • I have searched the existing issues

Experiencing problems? Have you tried our Stack Exchange first?

  • 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:

/// Called when peer sends us new transactions
fn on_transactions(&mut self, who: PeerId, transactions: Transactions<B::Extrinsic>) {
// Accept transactions only when node is not major syncing
if self.sync.is_major_syncing() {
trace!(target: LOG_TARGET, "{} Ignoring transactions while major syncing", who);
return
}

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: #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 #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 #5624 will likely require an RFC.

Are you willing to help with this request?

Yes!

@nazar-pc nazar-pc added the I5-enhancement An additional feature request. label Nov 11, 2024
@teor2345
Copy link

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
Copy link
Contributor Author

nazar-pc commented Nov 11, 2024

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

Yes, second part of the solution is in #6433 and is independent from the issue described here

@nazar-pc
Copy link
Contributor Author

nazar-pc commented Nov 12, 2024

Here is how it can be done once synced status information is available (#5624): autonomys@94a1a81?w=1

@bkchr
Copy link
Member

bkchr commented Nov 12, 2024

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I5-enhancement An additional feature request.
Projects
None yet
Development

No branches or pull requests

4 participants