Handle missing SC_ADDRESS_TYPEs in XDRCereal JSONOutputArchive overrides #4761
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
graydon
force-pushed
the
missing-cereal-cases
branch
from
ebe7b37 to
b63d6ab
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I stumbled on this missing case when debugging a previous issue -- printing XDR to JSON strings caused a crash due to missing cases.
I don't think it matters a ton how well this renders, as it's "just for debugging": we don't read back any of these and they're only emitted in debug messages and diagnostic tools like "dump-xdr". But I figured it'd be best to do it "well enough", would appreciate feedback on (a) whether the muxed address rendering seems sensible (it's a copy of what we do in muxed addresses outside SC_ADDRESS_TYPEs) and (b) whether it's ok to leave the claimable balances and liquidity pools as just not-otherwise-differentiated IDs, or if I ought to differentiate them as well in separate sub-nvps.