fix: better error handling in account RPCs

[mvadari]

High Level Overview of Change

This PR adds a check for whether the account and ident params in several account-based RPCs are strings. If not, it returns invalidParams. Some error messages are also improved to be more specific.

Note: This PR looks a lot larger than it is; it is really just the same change(s) applied over and over again across several RPCs.

Context of Change

rippled currently returns Internal error in this scenario.

The error in the logs is Caught throw: Type is not convertible to string

Type of Change

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

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)

This is a bugfix, not a breaking change, but there isn't an option for that.

Test Plan

Added unit tests that fail before this patch and now pass.

Future Tasks

Audit other RPCs for this sort of issue.

[codecov]

Codecov Report

Attention: Patch coverage is 83.72093% with 7 lines in your changes missing coverage. Please review.

Project coverage is 71.4%. Comparing base (f3bcc65) to head (fe1ebed).

Additional details and impacted files

@@           Coverage Diff           @@
##           develop   #5065   +/-   ##
=======================================
  Coverage     71.3%   71.4%           
=======================================
  Files          796     796           
  Lines        67031   67055   +24     
  Branches     10885   10869   -16     
=======================================
+ Hits         47808   47847   +39     
+ Misses       19223   19208   -15     

Files	Coverage Δ
src/xrpld/rpc/handlers/AccountInfo.cpp	88.2% <100.0%> (+0.4%)	⬆️
src/xrpld/rpc/handlers/AccountLines.cpp	87.5% <100.0%> (+0.2%)	⬆️
src/xrpld/rpc/handlers/AccountTx.cpp	85.1% <100.0%> (+0.1%)	⬆️
src/xrpld/rpc/handlers/NoRippleCheck.cpp	100.0% <100.0%> (ø)
src/xrpld/rpc/handlers/AccountChannels.cpp	81.2% <50.0%> (-0.6%)	⬇️
...rc/xrpld/rpc/handlers/AccountCurrenciesHandler.cpp	97.5% <90.9%> (-2.5%)	⬇️
src/xrpld/rpc/handlers/AccountObjects.cpp	95.3% <77.8%> (+1.7%)	⬆️
src/xrpld/rpc/handlers/AccountOffers.cpp	88.8% <57.1%> (+0.3%)	⬆️

... and 7 files with indirect coverage changes

[mvadari]

This change could also be done by editing parseBase58 to return an empty std::optional instead of crashing. However, this would result in an rpcACT_MALFORMED (in most cases) instead of rpcINVALID_PARAMS, which I believe differs from what Clio returns in this case.

[godexsoft]

Looks fine to me. Leaving a few nits and questions 👍

[godexsoft]

nit: can be avoided via immediately invoked lambda

[mvadari]

Given that the lambda would be handling errors, it'll be more readable etc. if it's left as-is, and probably wouldn't have any performance improvements to switch.

[godexsoft]

nit: Same improvement possible

[ximinez]

Because this branch is based on master, RippledCode.cmake needs to be updated to reflect the renaming of AccountLinesRPC_test.cpp to AccountLines_test.cpp.

[mvadari]

Closing as this patch is included in #5078