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 Redeemer Deserialization Issue #374

Merged
merged 2 commits into from
Sep 18, 2024
Merged

Conversation

bhatt-deep
Copy link
Contributor

Problem

The recent changes to the redeemer structure introduced a deserialization error when processing transactions. The new format (dictionary with tuple keys) was incompatible with the existing deserialization logic, which expected a list of redeemers.

Solution

This PR updates the witness.py file to handle both the new and old redeemer formats:

  • Modified the _get_redeemers function to process both dictionary and list formats.

@nielstron
Copy link
Contributor

Hi @bhatt-deep thank for the submission, looks good.
Out of curiosity - this is only necessary when trying to submit txs to pre-chang networks? Or does it also have some use in post-chang networks?

@bhatt-deep
Copy link
Contributor Author

Hi @nielstron, thanks for your question.

This change is needed for post-chang networks. The issue we faced was a problem with how PyCardano handles redeemers during serialization and deserialization in the chang branch, though everything worked fine before. I think this line is causing the issue:
https://github.com/Python-Cardano/pycardano/pull/371/files#diff-d8177155e5a3a27a67d01fdd3e88078e01bc853cd1d49ed0356aa6258de4dea7R1049

To explain further:

  1. Transactions submitted using the chang branch succeeded on the blockchain but failed when deserialized in PyCardano.
  2. The problem arises when reconstructing the transaction from CBOR. You can check it by running:
    print(Transaction.from_cbor(signed_tx.to_cbor()))
  3. In the _get_redeemers function, the redeemer structure changed. Instead of a list, it's now a dictionary with tuple keys. Here's an example:
    {(0, 2): [CBORTag(1281, []), [2763715, 604504597]]}
  4. This new structure didn't match the old deserialization logic, which expected a list:
    [Redeemer.from_primitive(redeemer) for redeemer in data]
  5. The error we got was:
    Error submitting transaction: ['list'] typed value is required for deserialization. Got <class 'tuple'>: (0, 2)
    

@bhatt-deep bhatt-deep force-pushed the chang branch 2 times, most recently from a8d5828 to 7a80ff1 Compare September 13, 2024 23:46
@cffls
Copy link
Collaborator

cffls commented Sep 14, 2024

Nice! I didn't think of the deserialization when writing this. Could you please add a unit test for it? Otherwise, looks good to me.

@bhatt-deep
Copy link
Contributor Author

Thanks! I'll add the unit test for deserialization and push the changes soon.

@bhatt-deep
Copy link
Contributor Author

@cffls have added unit tests.

@cffls cffls merged commit 7727c72 into Python-Cardano:chang Sep 18, 2024
@cffls
Copy link
Collaborator

cffls commented Sep 18, 2024

Thank you @bhatt-deep !

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.

3 participants