updates to book_changes RPC command

[ckeshava]

High Level Overview of Change

Updates to the book_changes RPC command. Pending clarification, I believe this PR also fixes #5033 and #5036 (improves the latency of the command)

Context of Change

1. accepts shortcut strings (current, closed, validated) as inputs to ledger_index parameter in the RPC command.
2. returns validated: true/false in the RPC output

Type of Change

• [x] 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)
• [x] 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

• [ ] Public API: New feature (new methods and/or new fields)
• [ ] Public API: Breaking change (in general, breaking changes should only impact the next api_version)
• [ ] libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
• [ ] Peer protocol change (must be backward compatible or bump the peer protocol version)

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 76.2%. Comparing base (9a6af9c) to head (75cb233). Report is 1 commits behind head on develop.

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #5096     +/-   ##
=========================================
+ Coverage     76.1%   76.2%   +0.1%     
=========================================
  Files          760     760             
  Lines        61548   61550      +2     
  Branches      8160    8124     -36     
=========================================
+ Hits         46834   46884     +50     
+ Misses       14714   14666     -48     

Files with missing lines	Coverage Δ
src/xrpld/rpc/BookChanges.h	14.4% <100.0%> (+14.4%)	:arrow_up:
src/xrpld/rpc/handlers/BookOffers.cpp	98.8% <100.0%> (+6.0%)	:arrow_up:

... and 9 files with indirect coverage changes

[Bronek]

@godexsoft you might want to test this branch with clio.

[ckeshava]

@Bronek thanks for the quick review!

What do you mean by USER context? I feel the getLedgerByContext should only be used by the consensus mechanism (and Full History Nodes) to collect+store all ledgers. Its in-efficient for a tiny or medium resources validator to engage in acquisition of ledgers.

The getLedgerByContext method can also be removed in the ledger_request RPC call. I expect to see similar issues in that RPC call. Check my comment here: https://github.com/XRPLF/rippled/issues/5036#issuecomment-2279269503

[Bronek]

What do you mean by USER context? I feel the getLedgerByContext should only be used by the consensus mechanism (and Full History Nodes) to collect+store all ledgers. Its in-efficient for a tiny or medium resources validator to engage in acquisition of ledgers.

USER methods are accessible to users, ADMIN methods are accessible to admins only (i.e. they require a password and are bound to specific ip/port). The purpose of getLedgerByContext, as exposed by ledger_request is to allow downloading ledgers from other nodes, which is what InboundLedgers::acquire does. This has uses outside of consensus, e.g. replaying transactions. This is nothing to do with node size, and it will be slow by definition.

The getLedgerByContext method can also be removed in the ledger_request RPC call. I expect to see similar issues in that RPC call. Check my comment here: #5036 (comment)

Nope, it should not be removed. There is a reason why we need to be able to download ledgers from other nodes, and why we need to have an ADMIN method for this action.