-
Notifications
You must be signed in to change notification settings - Fork 487
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
REST API: Fix LedgerStateDelta
JSON encoding
#6106
REST API: Fix LedgerStateDelta
JSON encoding
#6106
Conversation
LedgerStateDelta
JSON encoding
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6106 +/- ##
==========================================
+ Coverage 56.25% 56.26% +0.01%
==========================================
Files 490 490
Lines 69699 69704 +5
==========================================
+ Hits 39206 39222 +16
+ Misses 27825 27822 -3
+ Partials 2668 2660 -8 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a fine fix to make sure the leases can be emitted as valid JSON. On the other hand, I'm struggling to see any value in sending the lease information at all, since it is very explicit already in the transaction (the sender, the lease, and the implied expiration from last valid).
Anyway, I'm ok with this, or zeroing the leases map before creating JSON.
Summary
Removes the
Txleases
field from JSON state delta representations, as this field cannot be properly JSON encoded.Based on #6100
Closes #6097
Test Plan
Test coverage added on all 3 endpoints.