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

Protocol changes for transaction propagation #553

Open
NikVolf opened this issue Jul 8, 2020 · 8 comments
Open

Protocol changes for transaction propagation #553

NikVolf opened this issue Jul 8, 2020 · 8 comments
Assignees
Labels
D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase.

Comments

@NikVolf
Copy link
Contributor

NikVolf commented Jul 8, 2020

Problem

Currently, network protocol does not allow us to announce new transaction hashes or query transactions by hash from the given peer. This leads to the problem where we just send all transactions to all peers using existing /transactions/1 protocol and supplying transaction data with it. Due to possibility that substrate can be used in some degenerate network topologies we also can't choose some subset of peers to propagate transactions only to them (so that they do the same and eventually all peers get all transactions - this will work only on well-connected network with normal topology).

Transferring all transaction data to all peers is very costly in terms of traffic, thus we need to alter the protocol to allow on-demand requests of transaction data and new transactions hashes announcement.

Required changes

  1. Extend transactions protocol to /transactions/2 and add announce transaction hashes message to it. Announce is just a collection of hashes.
  2. Add request-reply protocol for transaction data. Peer provides transaction hash and receives transaction data.

How propagation works now

  1. We receive transaction from some source (RPC, network, offchain).
  2. We propagate this transaction to all peers using /transactions/1 protocol.

How propagation should work with this change

  1. We receive transaction from some source (RPC, network, offchain).
  2. We propagate this transaction to max(5, sqrt(peer_count)) peers using /transactions/1 protocol.
  3. We announce this transaction hash to all peers using /transactions/2 protocol.
  4. We are able to serve request for individual (or batches of) transaction hashes from any peer.
@NikVolf
Copy link
Contributor Author

NikVolf commented Jul 8, 2020

cc @bkchr @tomusdrw @tomaka

@tomaka
Copy link
Contributor

tomaka commented Jul 8, 2020

The change from /transactions/1 to /transactions/2 requires paritytech/substrate#6605

@tomusdrw
Copy link
Contributor

tomusdrw commented Jul 8, 2020

Prior work which I think we should really consider: https://arxiv.org/pdf/1905.10518.pdf (mentioned by @burdges some time ago). Especially: "Low-fanout flooding":

To meet our scalability goal (R1), we limit the flooding done by
public nodes to eight outbound connections even if the total number
of these connections is higher. this way, increasing connectivity
does not increase transaction dissemination cost proportionally.
the decision to relay through outbound connections, but not the
inbound ones, was made to defend against timing attacks

Having the set reconciliation protocol also allows to improve block propagation (separate issue), where instead of sending a full block with all transactions over the wire, we only send the header + transaction hashes (since we know the recipient has the full transactions).
More: https://github.com/bitcoin/bips/blob/master/bip-0152.mediawiki#HeaderAndShortIDs
or Arweave's Blockshadow

@NikVolf
Copy link
Contributor Author

NikVolf commented Jul 8, 2020

We discussed it few times with @burdges already, and (I think) came to conclusion that with our projected TPS and block time this won't work as well as in Bitcoin (due to a lot of large reconciliations needed) or require huge adaptation which research team must first do and experiment on.

@tomusdrw
Copy link
Contributor

tomusdrw commented Jul 8, 2020

Which part do you mean exactly? Limiting flooding to fixed number of peers (or at least putting an upper bound) seems reasonable to me (same as block shadows). With set reconciliation, I agree, we can simply use the hash-announce and request-by-hash thing proposed here (which is based on eth EIP afaict).

@NikVolf
Copy link
Contributor Author

NikVolf commented Jul 8, 2020

Which part do you mean exactly?

reconciliation stuff from paper, algorithm non-linear on number of mempool updates (which essentially is TPS)

@stale
Copy link

stale bot commented Jul 7, 2021

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the A5-stale label Jul 7, 2021
@tomusdrw
Copy link
Contributor

tomusdrw commented Jul 7, 2021

Hey, is anyone still working on this?

No, but would be cool to see someone working on this. Happy to help and guide.

@stale stale bot removed the A5-stale label Jul 7, 2021
@melekes melekes self-assigned this Jan 30, 2023
@altonen altonen transferred this issue from paritytech/substrate Aug 24, 2023
@the-right-joyce the-right-joyce added D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. and removed U3-nice_to_have labels Aug 25, 2023
claravanstaden pushed a commit to Snowfork/polkadot-sdk that referenced this issue Dec 8, 2023
helin6 pushed a commit to boolnetwork/polkadot-sdk that referenced this issue Feb 5, 2024
Bumps [follow-redirects](https://github.com/follow-redirects/follow-redirects) from 1.13.0 to 1.14.7.
- [Release notes](https://github.com/follow-redirects/follow-redirects/releases)
- [Commits](follow-redirects/follow-redirects@v1.13.0...v1.14.7)

---
updated-dependencies:
- dependency-name: follow-redirects
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase.
Projects
Status: Backlog 🗒
Development

No branches or pull requests

6 participants