rippled icon indicating copy to clipboard operation
rippled copied to clipboard

Refactor population of transaction JSON

Open Bronek opened this issue 1 year ago • 2 comments

Currently we have multiple locations performing the same task of populating JSON with transaction data, taking into account client-selected API version. As we add more API versions in the future, all these locations will accumulate complexity, while essentially doing the same task. Ideally we should improve code cohesion / reduce duplication by pulling all this logic into single location e.g. STTx::populateJson(Json::Value& target, ReadView const& ledger, unsigned apiVersion ... etc.) (not necessarily inside STTx , just an example).

There are two challenges that need to be solved:

  • first challenge is pulling the necessary data while managing dependencies (levelization) on ledger, master ledger, context etc. These data are (using JSON names):
    • validated
    • close_time_iso / date
    • ledger_index / inLedger
    • ledger_hash
  • second challenge is related to how we pre-populate transaction JSON for any API version inNetworkOPs.cpp and then adjust it accordingly for a specific version:
    • perhaps want to populate JSON for a specific API version,
    • then create function that will transform this JSON from one API version to another ?

Bronek avatar Nov 03 '23 18:11 Bronek

I'll mention that clio has exactly the same problems and the added difficulty that it wants to remain in lock step with what rippled supports. You might check in with the clio folks to see if there's some way these changes can be available to both rippled and clio?

scottschurr avatar Nov 03 '23 19:11 scottschurr

Also, by "currently" I actually mean since https://github.com/XRPLF/rippled/pull/4775

Bronek avatar Nov 03 '23 20:11 Bronek