-
Notifications
You must be signed in to change notification settings - Fork 471
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ledger: Implement JSON encoding and decoding for ledgercore.StateDelta #6100
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6100 +/- ##
==========================================
- Coverage 56.25% 55.89% -0.36%
==========================================
Files 490 490
Lines 69699 69765 +66
==========================================
- Hits 39206 38995 -211
- Misses 27825 28061 +236
- Partials 2668 2709 +41 ☔ View full report in Codecov by Sentry. |
IMO if one needs I let others to comment before closing this. |
@algorandskiy I agree it may not make sense to expose this as JSON. The only real value I can think of is as a debugging tool, but as you pointed out, converting the response from msgpack to JSON can achieve that as well. I think there are two choices to fix the problem here:
While 2 is technically a breaking change, it is simpler and there is likely little to no JSON usage of the affected endpoints. |
The only problem with converting msgpack to JSON is that JSON doesn't support using objects as keys, requiring extra conversion steps. I think it's a lot easier if JSON can be requested directly, with no conversion needed. |
Closing as #6106 was merged. |
Summary
Implements
MarshalJSON()
andUnmarshalJSON()
methods on theledgercore.StateDelta
struct that can properly encode maps with objects as keys.Test Plan
To test JSON encoding/decoding on
ledgercore.StateDelta
, I've added a unit test that uses thejson.Marshal()
to encode theStateDelta
, thenjson.Unmarshal()
to decode it. If the initialStateDelta
value matches the decoded value, the test passes. Otherwise, it fails. All tests are inledger/ledgercore/statedelta_test.go
.