Skip to content

Limit reply size on TMGetObjectByHash 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.

Sign up for GitHub

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

Open
pratikmankawde wants to merge 14 commits into
base: develop
Choose a base branch
from
+10 −3
Open

Limit reply size on TMGetObjectByHash queries #6110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
0eb0495
first draft
pratikmankawde Dec 4, 2025
75d06cc
Merge branch 'develop' into pratik/Charge-higher-fees-for-and-reject-…
pratikmankawde Dec 4, 2025
3252666
cleanup
pratikmankawde Dec 8, 2025
d657bab
Merge branch 'develop' into pratik/Charge-higher-fees-for-and-reject-…
pratikmankawde Dec 8, 2025
138e8ea
removed amendment changes
pratikmankawde Dec 9, 2025
e9b5dcb
added limit to reply size
pratikmankawde Dec 11, 2025
f6fff80
minor clean-up
pratikmankawde Dec 11, 2025
7dddc90
Merge branch 'develop' into pratik/Charge-higher-fees-for-and-reject-…
pratikmankawde Dec 11, 2025
e3d76f2
Merge branch 'develop' into pratik/Charge-higher-fees-for-and-reject-…
pratikmankawde Dec 11, 2025
84fa3ae
subscription test was failing, so trying with longer timeout,
pratikmankawde Dec 11, 2025
6290af1
Update src/xrpld/overlay/detail/PeerImp.cpp
pratikmankawde Dec 12, 2025
5f912ec
Update src/xrpld/overlay/detail/PeerImp.cpp
pratikmankawde Dec 12, 2025
5c3ae67
Merge branch 'develop' into pratik/Charge-higher-fees-for-and-reject-…
pratikmankawde Dec 12, 2025
58b6258
Merge branch 'develop' into pratik/Charge-higher-fees-for-and-reject-…
pratikmankawde Dec 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/test/rpc/Subscribe_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -872,7 +872,7 @@ class Subscribe_test : public beast::unit_test::suite
for (int i = 0; i < numReplies; ++i)
{
std::uint32_t idx{0};
auto reply = wsc.getMsg(100ms);
auto reply = wsc.getMsg(1s);
Copy link
Collaborator

@ximinez ximinez Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did this actually make a difference? Up to now, we've been working under the idea that the problem is port exhaustion, and that the server needs changes, and just re-running the failed jobs until they're fixed.

Copy link
Collaborator Author

@pratikmankawde pratikmankawde Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, tests were failing before this change, and later they passed. Doesn't prove that the issue is just the delay, could be the port exhaustion. But increasing the timeout came to mind as a quick test to see if I can get all green checks. I'll try rerunning the jobs 2-3 times to see if the solution holds. This fix anyway shouldn't be part of this PR.

if (reply)
{
auto r = *reply;
Expand Down
11 changes: 9 additions & 2 deletions src/xrpld/overlay/detail/PeerImp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1351,8 +1351,8 @@
{
// If we've never been in synch, there's nothing we can do
// with a transaction
JLOG(p_journal_.debug()) << "Ignoring incoming transaction: "
<< "Need network ledger";
JLOG(p_journal_.debug())
<< "Ignoring incoming transaction: Need network ledger";
return;
}

Expand Down Expand Up @@ -2618,6 +2618,13 @@
newObj.set_ledgerseq(obj.ledgerseq());

// VFALCO NOTE "seq" in the message is obsolete

// Check if by adding this object, reply has reached its
// limit
if (reply.objects_size() >= Tuning::hardMaxReplyNodes)
{
break;

Check warning on line 2626 in src/xrpld/overlay/detail/PeerImp.cpp

View check run for this annotation

Codecov / codecov/patch

src/xrpld/overlay/detail/PeerImp.cpp#L2626 

Added line #L2626 was not covered by tests
}
}
}
}
Expand Down
Loading