Sponsored Content
Skip to content

fix: correct index for limit in book_offers CLI - #6043

Merged
bthomee merged 5 commits into
developfrom
mvadari/book-offers-cli-fix
Nov 20, 2025
Merged

fix: correct index for limit in book_offers CLI#6043
bthomee merged 5 commits into
developfrom
mvadari/book-offers-cli-fix

Conversation

@mvadari

@mvadari mvadari commented Nov 17, 2025

Copy link
Copy Markdown
Contributor

High Level Overview of Change

This PR fixes an indexing typo in the book_offers CLI processing. This does not affect the HTTPS/WS RPC processing.

It's a fairly straightforward fix - there is a length check on whether the list length is >= 5, but the 5u index is taken instead of the 4u index (as the last in the list).

Context of Change

Reported as a part of the Lending Protocol Attackathon

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Performance (increase or change in throughput and/or latency)
  • Tests (you added tests for code that already exists, or your new feature included in this PR)
  • Documentation update
  • Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
  • Release

API Impact

There is a minor fix in the book_offers CLI. This does not affect the HTTPS/WS RPC processing.

Test Plan

CI passes.

@mvadari
mvadari requested review from a team and vvysokikh1 November 17, 2025 10:45
@mvadari mvadari changed the title Fix index for iLimit in RPCCall.cpp fix: correct index for limit in book_offers CLI Nov 17, 2025
@mvadari mvadari added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label Nov 17, 2025
@mvadari mvadari removed the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label Nov 17, 2025
Comment thread src/test/rpc/RPCCall_test.cpp Outdated
@Tapanito
Tapanito self-requested a review November 17, 2025 12:01
@codecov

codecov Bot commented Nov 17, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.72727% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.6%. Comparing base (6ff495f) to head (532c30e).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
src/xrpld/rpc/detail/RPCCall.cpp 72.7% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           develop   #6043   +/-   ##
=======================================
  Coverage     78.6%   78.6%           
=======================================
  Files          818     818           
  Lines        68983   68989    +6     
  Branches      8246    8241    -5     
=======================================
+ Hits         54196   54210   +14     
+ Misses       14787   14779    -8     
Files with missing lines Coverage Δ
src/xrpld/rpc/detail/RPCCall.cpp 93.7% <72.7%> (-0.4%) ⬇️

... and 3 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mvadari

mvadari commented Nov 17, 2025

Copy link
Copy Markdown
Contributor Author

#6044 is a version of this PR that covers more cases and is broader. We may want to merge this first or go straight to that one, unsure.

@bthomee

bthomee commented Nov 19, 2025

Copy link
Copy Markdown
Collaborator

@Tapanito are there any additional things that need to be addressed, or is this good to go?

@Tapanito

Copy link
Copy Markdown
Contributor

@Tapanito are there any additional things that need to be addressed, or is this good to go?

Good to go @bthomee

@mvadari mvadari added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label Nov 20, 2025
@bthomee
bthomee merged commit a3d4be4 into develop Nov 20, 2025
2 of 3 checks passed
@bthomee
bthomee deleted the mvadari/book-offers-cli-fix branch November 20, 2025 11:37
bthomee pushed a commit that referenced this pull request Apr 7, 2026
This change fixes an indexing typo in the `book_offers` CLI processing, and does not affect the HTTPS/WS RPC processing.
ximinez pushed a commit that referenced this pull request Apr 10, 2026
This change fixes an indexing typo in the `book_offers` CLI processing, and does not affect the HTTPS/WS RPC processing.
@ximinez ximinez added this to the 3.1.3 milestone Apr 15, 2026
@ximinez ximinez modified the milestones: 3.1.3, 3.1.3 (develop) Apr 15, 2026
ximinez pushed a commit that referenced this pull request Apr 22, 2026
This change fixes an indexing typo in the `book_offers` CLI processing, and does not affect the HTTPS/WS RPC processing.
beartec-jpg pushed a commit to beartec-jpg/FalconLedger that referenced this pull request Jun 1, 2026
This change fixes an indexing typo in the `book_offers` CLI processing, and does not affect the HTTPS/WS RPC processing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants