-
-
Notifications
You must be signed in to change notification settings - Fork 80
Freeze indefinite lists and definite lists when decoding #468
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #468 +/- ##
==========================================
+ Coverage 89.86% 89.95% +0.08%
==========================================
Files 33 33
Lines 5003 5007 +4
Branches 759 758 -1
==========================================
+ Hits 4496 4504 +8
+ Misses 327 324 -3
+ Partials 180 179 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Some test cases failed. Most of them seems to be easily fixable. I will look into it. |
|
Fixed the tests. If possible, could you please add the failed test in Mockfrost (test_vesting2.py)? It would help verification. |
|
I'll have to see what is the best way to add them because I think it's occurring when deserializing a complex transaction |
68d6e93 to
10b17b4
Compare
|
@cffls I added test cases that cover all the changed lines. Maybe one can add more test cases to trigger errors for other cbor serializations, but I am not too familiar with all the options there |
When decoding complex Cardano objects, it may happen that internally, there is a map of e.g. PlutusData -> X or Array -> X. This is fine in CBOR, but can lead to issues when translating this into corresponding Python objects which require hashability. I think for this reason, all CBOR lists/maps should be decoded into a frozen variant to avoid errors during translation.
This patch implements the minimal fix to pass a testcase in Mockfrost (test_vesting2.py) (concretely, decoding the script context from cbor may fail here because it contains the datums map which maps arbitrarily complex datums to arbitrary data). However, more instances of decoded dicts and lists are present in serialization and may need to be adapted. I didn't want to do that right away as I may be unaware of things that break when we enforce freezing any decoded datastructure.