Handle invalid marker parameter in grpc call

[cindyyan317]

Handle invalid marker parameter in grpc call

Context of Change

end_marker is used to limit the range of ledger entries to fetch
If end_marker is less than marker, crash can happen at https://github.com/XRPLF/rippled/blob/develop/src/xrpld/rpc/handlers/LedgerData.cpp#L185

Type of Change

• [ x] Bug fix (non-breaking change which fixes an issue)

Before

rippled server crashes if end_marker is smaller than marker.

After

Return code = InvalidArgument desc = end marker malformed error to grpc caller.

Test

Test manually

This PR is to address the crash. Developing unit test setup for grpc server methods is beyond the scope of this PR.

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 78.1%. Comparing base (c1c2b5b) to head (c7c5cd7).

Files with missing lines	Patch %	Lines
src/xrpld/rpc/handlers/LedgerData.cpp	0.0%	6 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           develop   #5317   +/-   ##
=======================================
  Coverage     78.1%   78.1%           
=======================================
  Files          790     790           
  Lines        67908   67909    +1     
  Branches      8228    8229    +1     
=======================================
+ Hits         53024   53025    +1     
  Misses       14884   14884           

Files with missing lines	Coverage Δ
src/xrpld/rpc/handlers/LedgerData.cpp	57.1% <0.0%> (-0.6%)	⬇️

... and 1 file with indirect coverage changes

[ximinez]

In addition to the change below, is it possible to write some unit tests for this functionality?

[cindyyan317]

In addition to the change below, is it possible to write some unit tests for this functionality?

@ximinez
I believe the current version of rippled doesn't have a unit test setup for gRPC methods. Based on my limited understanding of rippled, setting up unit tests for gRPC seems more complex than fixing the crash itself.

If there's existing unit test scaffolding for gRPC calls, similar to env.rpc, I'm happy to add tests. Otherwise, I'd prefer to focus on fixing the crash in this PR rather than setting up additional framework components.

[ximinez]

If there's existing unit test scaffolding for gRPC calls, similar to env.rpc, I'm happy to add tests. Otherwise, I'd prefer to focus on fixing the crash in this PR rather than setting up additional framework components.

I thought we had some, but I can't find them now, so I guess we can move forward as-is.

[ximinez]

Looks good

[vlntb]

[nit]: It might be useful to differentiate between null and wrong position of marker. I.e.:

    auto key = uint256::fromVoidChecked(request.end_marker());

    if (!key)
    {
        return {response, {grpc::StatusCode::INVALID_ARGUMENT, "end marker malformed"}};
    }

    if (*key < startKey)
    {
        return {response, {grpc::StatusCode::INVALID_ARGUMENT, "end marker out of range"}};
    }

    e = ledger->sles.upper_bound(*key);

[vlntb]

LGTM, happy to approve once nit comment is answered.
p.s. I checked and can confirm that it is not very easy to add unit-test in this case.