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

p2p: handle txns in pubsub validator #6070

Merged

Conversation

algorandskiy
Copy link
Contributor

@algorandskiy algorandskiy commented Jul 15, 2024

Summary

This is a draft implementation on how txHandler logic would look like if fully implemented in pubsub validator.
It loses batch verification part so could be a large performance drawback.

Tried an alternative approach where pubsub validator always returns Ignore and then tx handler calls Relay. This does not work since pubsub discards the second submission as a duplicate.

Test Plan

Extended e2e p2p test to send transactions

Performance

Run scenario1s-p2p: 6781.17 TPS
image

image

Resolves #6052

@algorandskiy algorandskiy added Enhancement experimental p2p Work related to the p2p project labels Jul 15, 2024
@algorandskiy algorandskiy self-assigned this Jul 15, 2024
Copy link

codecov bot commented Jul 15, 2024

Codecov Report

Attention: Patch coverage is 60.93750% with 50 lines in your changes missing coverage. Please review.

Project coverage is 56.10%. Comparing base (48a539f) to head (95184b5).
Report is 22 commits behind head on master.

Files Patch % Lines
network/p2p/pubsubTracer.go 0.00% 24 Missing ⚠️
data/txHandler.go 67.69% 15 Missing and 6 partials ⚠️
network/hybridNetwork.go 0.00% 3 Missing ⚠️
network/p2pNetwork.go 80.00% 1 Missing ⚠️
network/wsNetwork.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6070      +/-   ##
==========================================
- Coverage   56.13%   56.10%   -0.03%     
==========================================
  Files         488      489       +1     
  Lines       69439    69493      +54     
==========================================
+ Hits        38978    38990      +12     
- Misses      27813    27839      +26     
- Partials     2648     2664      +16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@algorandskiy algorandskiy force-pushed the pavel/p2p-pubsub-tx-handler branch from 78fd790 to 355d62e Compare July 17, 2024 16:33
@algorandskiy algorandskiy changed the title WIP: p2p: handle txns in pubsub validator p2p: handle txns in pubsub validator Jul 17, 2024
@algorandskiy algorandskiy marked this pull request as ready for review July 17, 2024 18:51
data/txHandler.go Outdated Show resolved Hide resolved
@algorandskiy algorandskiy marked this pull request as draft July 18, 2024 14:41
@algorandskiy
Copy link
Contributor Author

algorandskiy commented Jul 18, 2024

Added synchronous batch verifier to pubsub validator handler.
Now we are not counting backlog dropped txns correctly.

@algorandskiy algorandskiy marked this pull request as ready for review July 18, 2024 16:54
@algorandskiy
Copy link
Contributor Author

algorandskiy commented Jul 18, 2024

Added a RawTracer interface implementation to track transactionMessagesDroppedFromBacklog and transactionMessagesDupRawMsg metrics

network/p2p/pubsubTracer.go Outdated Show resolved Hide resolved
@algorandskiy algorandskiy force-pushed the pavel/p2p-pubsub-tx-handler branch from 78ae51b to 5977cca Compare July 19, 2024 14:40
data/txHandler.go Outdated Show resolved Hide resolved
data/txHandler.go Outdated Show resolved Hide resolved
data/txHandler.go Show resolved Hide resolved
@algorandskiy algorandskiy requested review from cce and jasonpaulos August 1, 2024 17:11
jasonpaulos
jasonpaulos previously approved these changes Aug 1, 2024
data/txHandler.go Outdated Show resolved Hide resolved
Co-authored-by: Jason Paulos <jasonpaulos@users.noreply.github.com>
jasonpaulos
jasonpaulos previously approved these changes Aug 1, 2024
data/txHandler.go Outdated Show resolved Hide resolved
cce
cce previously approved these changes Aug 8, 2024
@cce cce dismissed stale reviews from jasonpaulos and themself via 8701866 August 8, 2024 14:10
cce
cce previously approved these changes Aug 8, 2024
jasonpaulos
jasonpaulos previously approved these changes Aug 8, 2024
@algorandskiy algorandskiy dismissed stale reviews from jasonpaulos and cce via 95184b5 August 8, 2024 14:48
@algorandskiy
Copy link
Contributor Author

Fixed test after the behavior change

@algorandskiy algorandskiy requested review from cce and jasonpaulos August 8, 2024 14:48
@algorandskiy algorandskiy merged commit 602d950 into algorand:master Aug 8, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement p2p Work related to the p2p project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Txpool: Transaction Broadcasting Inefficiency
3 participants