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

Txpool: Transaction Broadcasting Inefficiency #6052

Closed
sarvalabs-karthik opened this issue Jul 6, 2024 · 7 comments · Fixed by #6070
Closed

Txpool: Transaction Broadcasting Inefficiency #6052

sarvalabs-karthik opened this issue Jul 6, 2024 · 7 comments · Fixed by #6070
Assignees
Labels
p2p Work related to the p2p project

Comments

@sarvalabs-karthik
Copy link

After reviewing the Algorand code, I noticed that transactions are broadcasted to other peers using a pub-sub mechanism without immediate validation. Wouldn’t this approach allow someone to send invalid transactions, leading to high bandwidth consumption?

@sarvalabs-karthik sarvalabs-karthik changed the title Transaction Broadcasting Inefficiency Txpool: Transaction Broadcasting Inefficiency Jul 6, 2024
@urtho
Copy link
Contributor

urtho commented Jul 6, 2024

End users submit transactions via REST endpoints on API nodes, not via gossip network directly.
Transactions are validated there against the ledger state before getting sent over gossip network to relays.

Relays have adaptive rate limiters to deal with "spam" being sent directly over the gossip.

@sarvalabs-karthik
Copy link
Author

sarvalabs-karthik commented Jul 7, 2024

I agree that there is no issue if tx is submitted through RPC. In case of p2p network, there is an app rate limiter which can be bypassed by firing txns with app ids ranging from 1 to 2^64. But the issue is that, all the nodes in the network keep propagating the messages eventhough they are invalid.
In case of Ethereum, each node gossips or broadcasts the txn after full validation.

In simple words, I would modify my algorand node code to send invalid txns in p2p publishing.

@urtho
Copy link
Contributor

urtho commented Jul 7, 2024

Transaction costs are so low on Algorand that it is easier to spam the network with valid TXNs - no code modifications needed. The current hub and spoke topology handles x100 spikes just fine.

But P2P implementation is not yet complete so this is a good time for feedback.

@sarvalabs-karthik
Copy link
Author

I have a question regarding the deduplication process for raw transaction groups. I noticed that a rotating salted cache is used instead of a digest cache. Could you please explain the reasoning behind this choice? Specifically, I am interested in understanding the advantages or considerations that led to the decision to use a rotating salted cache for this purpose.

@sarvalabs-karthik
Copy link
Author

sarvalabs-karthik commented Jul 15, 2024

Hello @algorandskiy ,

I am tagging you here as you might be the right person to clear my doubt regarding the tx-related salted cache.

@algorandskiy
Copy link
Contributor

There is a PR where the deduplication for wsnet handler was introduced: #4806
Blake2 hashing function selected because of its speed, rotating salt added to increase randomness since blake2 is relatively new and might (or might not) have collisions.

@algorandskiy
Copy link
Contributor

Thank you for bringing the pubsub issue to attention. It is indeed an problem and needs to be addressed.

@algorandskiy algorandskiy self-assigned this Jul 15, 2024
@algorandskiy algorandskiy added the p2p Work related to the p2p project label Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2p Work related to the p2p project
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants