Skip to content
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

Fix plutus script deserialization #293

Merged

Conversation

theeldermillenial
Copy link
Contributor

When either a PlutusV1Script or a PlutusV2Script is added as a script input and serialized to cbor, any attempt to deserialize throws a deserialization error. This is because there is no explicit catch in _restore_typed_primitive for bytes data to parse into the appropriate data type except for ByteString.

This PR takes a conservative approach to resolving this issue, by only looking for classes named PlutusVxScript with a bytes values and casts them to the appropriate script type.

A more general approach would be to detect if the parsing object is a class and the value is bytes and using the abstracted parsing object to wrap the bytes object, and do this for all bytes values. However, that was not implemented here because it wasn't clear if this could have bad side effects.

I was able to develop a minimal test example that failed before the fix and passed after the fix was implemented.

fixes #251

@codecov-commenter
Copy link

codecov-commenter commented Jan 10, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (d1ec439) 84.63% compared to head (4d94b3b) 84.61%.

Files Patch % Lines
pycardano/serialization.py 50.00% 1 Missing and 1 partial ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #293      +/-   ##
==========================================
- Coverage   84.63%   84.61%   -0.02%     
==========================================
  Files          26       26              
  Lines        3071     3075       +4     
  Branches      752      754       +2     
==========================================
+ Hits         2599     2602       +3     
- Misses        353      354       +1     
  Partials      119      119              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@nielstron nielstron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Thank you for this fix!

@nielstron nielstron merged commit 8cbf815 into Python-Cardano:main Jan 10, 2024
11 checks passed
@nielstron nielstron mentioned this pull request Jan 12, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot deserialize PlutusV2 script attached to transaction
3 participants