XRPLF / rippled

Decentralized cryptocurrency blockchain daemon implementing the XRP Ledger protocol in C++
https://xrpl.org
ISC License
4.51k stars 1.46k forks source link

Refactor population of transaction JSON #4801

Open Bronek opened 10 months ago

Bronek commented 10 months ago

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:

scottschurr commented 10 months ago

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?

Bronek commented 10 months ago

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