Add JsonInputArchive overrides to XDRCereal

[drebelsky]

Resolves #5041. Note that the overrides are intentionally hidden behind BUILD_TESTS because we shouldn't use the input overrides on production code (we should store XDR as XDR, not our custom JSON overrides that break every so often). The overloads also don't get used anywhere else, but I think it's probably still worth having them + the tests in the repo, so we can quickly write an ad-hoc tool to parse core JSON if we need to again.

Testing notes
The tests right now are written to check that we can roundtrip 1000 random values of most of our XDR types. Unfortunately, due to the heavy template instantiation, compilation takes ~1 minute on my laptop. With the current configurable values (1000 random values per type, use 1 for levels for xdr::generator_t, limit the autocheck::arbitrary to generating things of at most size 10), the tests take about a minute to run. For reference, 1000 random values for type, limiting autocheck::arbitrary to 5, and using the xdrpp autocheck::generator override took ~17 seconds. Limiting the nesting depth is probably fine since we're checking the nested structures.

Alternatively, we could try roundtripping (by hand or generation) a subset of the particular overloads. This has the benefit of compiling faster. However, it also requires us to actively keep the tests up-to-date (the class list can be updated more intermittently, especially since including things like BucketEntry help gives provide coverage for future types).

There's also a basic set of unit tests that check that the overrides get called.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

[Copilot]

In the JSONInputArchive Asset override, assetCode read from the "assetCode" string is not length-checked before copying into the fixed-size opaque_array in setAlpha(...). If the JSON contains a long string, std::copy(assetCode.begin(), assetCode.end(), alpha.assetCode.begin()) will write past the end of the destination array (UB). Validate assetCode.size() (<= 4 or <= 12 as appropriate) before copying, and throw/fail cleanly when it's out of range.

[Copilot]

The new JSONInputArchive overrides use releaseAssertOrThrow(...), but XDRCereal.h doesn't include util/GlobalChecks.h where it's defined. Add the include (ideally within the BUILD_TESTS block) so test builds don't depend on transitive includes.

[Copilot]

This test file uses releaseAssert(...) in helpers but doesn't include util/GlobalChecks.h where it is defined. Add the missing include to avoid compilation failures that depend on transitive includes.

Suggested change

	#include "util/Decoder.h"
	#include "util/Decoder.h"
	#include "util/GlobalChecks.h"

[Copilot]

<fmt/format.h> and crypto/StrKey.h appear unused in this test file; removing them will reduce compile time and avoid unused-include churn.

Suggested change

	#include "crypto/StrKey.h"
	#include "test/Catch2.h"
	#include "util/Decoder.h"
	#include <fmt/format.h>
	#include "test/Catch2.h"
	#include "util/Decoder.h"

[Copilot]

The new JSONInputArchive enum override uses std::unordered_map but XDRCereal.h doesn't include <unordered_map> (and it also uses std::back_inserter without including ). Add the missing standard headers (and keep them within the BUILD_TESTS guard if you want to limit production impact) to avoid non-deterministic builds relying on transitive includes.

Suggested change

	#include <string>
	#include <string>
	#include <unordered_map>
	#include <iterator>

[Copilot]

The ASSET_CODE_12 validity check uses std::decay<decltype(assetCode)> instead of std::decay_t<...> (or typename std::decay<...>::type). As written, the condition is effectively never true, so the first0 <= 4 rule for AssetCode12 won't be enforced and invalid codes may serialize as strings instead of raw bytes.

[Copilot]

std::string code; is now unused after switching to the archive(...) lambda. Please remove it (or use it) to avoid unused-variable warnings (which may be treated as errors in some builds).

Suggested change

std::string code;

[bboston7]

Thank you for putting this together! Unfortunately, I'm not sure I agree with this approach. I don't think we should be parsing JSONified XDR in stellar-core. There are multiple existing tools to convert between XDR and JSON, and I don't think we should be maintaining an additional one within stellar-core.

I believe we output this JSON as a sort of "human readable" XDR representation. It wasn't intended to be parsed by stellar-core, and I'm concerned about accidentally creating a new, non-standard JSON representation that we're then on the hook to maintain.

As far as debug-ability goes, we should always have access to the underlying XDR whenever we're outputting these JSON strings (be it in archives, meta, etc). If there's any place where JSON in a log file is the sole persisted representation of the data, then I think that's a mistake.

I think the ideal approach to the problem of "we emit non-standard JSON" is something closer to your attempt in #5071, albeit with code generation to avoid the manual type-names-to-strings conversions.

Before embarking on that though, it's worth asking whether we really need this. Was there a request somewhere to be able to ingest this JSON output? Was the raw XDR not available in that instance? Is there maybe a better workflow that the requester could use?