-
-
Notifications
You must be signed in to change notification settings - Fork 76
Added IndefiniteDecoder for round trip plutusdata serialization #431
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
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #431 +/- ##
==========================================
- Coverage 88.95% 88.79% -0.16%
==========================================
Files 33 33
Lines 4744 4757 +13
Branches 1134 1136 +2
==========================================
+ Hits 4220 4224 +4
- Misses 369 374 +5
- Partials 155 159 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Fixed QA issues. Not entirely sure why but sometimes a test for roundtrip transaction serialization fails. It does not appear to be reproducible. |
Looks like some non-determinism is introduced, which caused the test to fail randomly. |
Also, it seems that the customized function |
The reason why it failed non-deterministically was that the order of the elements in a set isn't preserved when deserializing, which led to mismatched order when compared with the original data. |
@cffls Wait...so does this mean that the c-extension was also causing the issues related to sets? I'll try to fix the other outstanding issues tonight. |
Yes, it is forcing the code to use c-extension and making it impossible to monkey patch any default behavior, e.g. decoding tag.258 into a set, which doesn't preserve the order. |
I wonder if that explains some of the other occasional behavior I would have that cause hash errors. I could never reproduce them and they happened infrequently. Sigh... |
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.
LGTM, thanks!
This PR adds support for IndefiniteDecoder, a special CBOR decoder class that returns indefinite lists as an
IndefiniteList
rather than alist
as a way to support roundtrip, reproduciblePlutusData
serialization.There are two new unit tests that demonstrate the need for this PR in the serialization tests.
The first is
test_datum_round_trip
, that creates a customPlutusData
class, serializes to CBOR and deserializes. Without theIndefiniteDecoder
, this test fails.The second test is
test_datum_raw_round_trip
, that creates a customPlutusdata
class, serializes to CBOR and deserializes asRawPlutusData
. This test is currently marked as an expected fail test, since this PR does not resolve this issue. Attempts to resolve this problem causes issues with other round trip serialization tests, and warrants further investigation.closes #331