-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Charge higher fees for and reject heavy TMGetObjectByHashLimit queries #6110
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
base: develop
Are you sure you want to change the base?
Charge higher fees for and reject heavy TMGetObjectByHashLimit queries #6110
Conversation
…heavy-TMGetObjectByHash-queries
| return; | ||
| } | ||
|
|
||
| // Charge heavier fee for large requests (>256 objects) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why 256? What should be these incremental boundaries?
| } | ||
| else | ||
| { | ||
| fee_.update( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the minimum fee be feeModerateBurdenPeer or feeTrivialPeer?
src/xrpld/overlay/detail/PeerImp.cpp
Outdated
| // (only when fix is enabled) | ||
| if (replyBudgetBytes > 0 && | ||
| replyBytes + dataSz + 64 > replyBudgetBytes) | ||
| break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we break here, meaning we send whatever we accumulated or just not send anything?
Context of Change
Complete details of the context of the change and the change itself is shared in the ticket associated with this.
I am not duplicating the details here to avoid redundancy and confusion.
I will add details of the test and impact in the ticket itself.
Type of Change
.gitignore, formatting, dropping support for older tooling)