refactor: signature autofilling for Simulate RPC

[tequdev]

High Level Overview of Change

Refactor to enable autofilling of signature-related fields in the Simulate RPC for use in the Signature field (like for Lending, Sponsor feature) as well

Type of Change

• Refactor (non-breaking change that only restructures code)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.5%. Comparing base (e80642f) to head (e4ae71b).
⚠️ Report is 2 commits behind head on develop.

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #5852     +/-   ##
=========================================
- Coverage     79.5%   79.5%   -0.0%     
=========================================
  Files          816     816             
  Lines        72180   72184      +4     
  Branches      8278    8276      -2     
=========================================
- Hits         57362   57358      -4     
- Misses       14818   14826      +8     

Files with missing lines	Coverage Δ
src/xrpld/rpc/handlers/Simulate.cpp	100.0% <100.0%> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[mvadari]

@tequdev is this PR ready to merge?

[tequdev]

@tequdev is this PR ready to merge?

yes👍

[pratikmankawde]

Question/Suggestion: Shouldn't this check !sigObject[jss::Signers].isArray() happen even before this function is called?

[bthomee]

@tequdev do you have a response?

[tequdev]

@pratikmankawde What's the reason for that?

[pratikmankawde]

The name of the function suggests that it fills signatures into objects. But this function is also checking if the json object passed-in has the sub-objects created with correct data types. I think this function should only be called if the object types are correct(Json is valid). Hence the check should happen before.

There should be a separate function to check the validity of the json. Separation of concerns and responsibilities.

Right now we return an error if the json object is malformed. At some point we may want to recreate the sub-objects of proper types.

[mvadari]

That feels unnecessary - why look at the jss:Signers field twice in two different places? Makes way more sense to me to just check before you're going to need to work with it.

[pratikmankawde]

My suggestion was to still do it once, just outside this function and before calling this. We then don't call this function if the field data types are not what we expect. Unless there's a case where sigObject[jss::Signers] is expected to be a scalar.

[mvadari]

Instead of adding a precondition to this function that you have to remember to check before you call it, it feels more robust to check it inside the function. That's also generally the pattern in this repo.