-
-
Notifications
You must be signed in to change notification settings - Fork 73
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
Chang support #371
Chang support #371
Conversation
This requires bumping the python version to 3.10+, because ogmios-python requires this. I also refactored the chain backend to not have built-in kupo support but just provide it as an extension using kupo-wrappers. This way, both OgmiosV5 and OgmiosV6 directly have support for kupo.
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.
I think this is in a good state to merge. @nielstron do you have anything to add?
LGTM |
As a heads up, I am testing this out tonight. It does not appear as though some wallets (i.e. Eternl) supports redeemers as maps yet. I am getting errors when generating cbor with pycardano and then sending to Eternl for signing. Yes, I think this is an Eternl problem, but I did want to put this on the map for anyone else potentially running into this. |
I experienced something similar with Mesh. But Eternal works with platforms like MinSwap, so they do support some form of SC txs that is accepted on Chang? Are we missing something? |
Can confirm that this is the problem. Emurgo/cardano-serialization-lib#693 Right now it seems CSL only uses array style redeemers. So this could likely affect multiple wallets. |
According to this comment we should still allow Redeemers as array: Emurgo/cardano-serialization-lib#693 (comment) Maybe add a switch to the txbuilder for this? @cffls can you look into this since you also added the map support in the first place? |
I did some looking into it but haven't had a chance to try it out. I think there's a one line work around. The Redeemer type is fortunately defined as a list or map. The original redeemer type is still defined, the map was added to it. I think the workaround looks like this:
Obviously if you're building the tx manually you can just set the redeemers as an array. An alternative solution would be to add an option to the txbuilder that specifies redeemer format. By default, it can be set to map, but can be changed to list. Should also be an easy thing to implement. I'll be able to test out in the next few hours. If you have a comment on this approach, please let me know. |
This sounds great @theeldermillenial ! Either approach is fine, maybe just add a function "redeemer_map_to_list" and use that when a flag is set in .build? So both manual builders and the automated builders can benefit from this. |
That's a good idea. I'll try it out. |
Alright, heres my code to swap out the redeemer maps to lists: if (
tx.transaction_witness_set.redeemer is not None
and len(tx.transaction_witness_set.redeemer) > 0
):
tx.transaction_witness_set.redeemer = [
pycardano.Redeemer.from_primitive(k.to_primitive() + v.to_primitive())
for k, v in tx.transaction_witness_set.redeemer.items()
] The problem I'm having is that Eternl can't seem to read the cbor after this. I tested building the cbor both with maps and lists, dumping to cbor, and importing into Eternl. In both cases, it will pull up the tx, but if you look at the witness set, it's basically full of null values. I'll keep hacking away at it, but it looks like this might be more complex than I initially expected. |
Nix that, this seems to work. I'll submit a PR today. I had something that was out of alignment with the change branch. |
Txbuilder always keeps a list of redeemers internally, so the conversion might not be needed. I will try and see if there is a simpler solution to this. |
Pushed a commit for the fix. |
I actually found an additional error that was causing a different issue in Eternl. When the I use empty signing keys when building the transaction with txbuilder. In Eternl, when interacting with the extension through our frontend, we still get the error about redeemers using a map instead of an array, but now I can successfully import the same transaction into Eternl and submit it. I'll pull in the latest changes and test that along with my fix. It'll be tomorrow though because I've been at this and a few other days and the brain is fried. |
Alright, between the fix from Jerry and this, everything seems to be running smooth. |
…376) * Fixed tx imbalance when burning multiple tokens * Added cases and format * Added unit test * Correct unit test * Removed redundant filtering zero-quantity asset * Improved name function (unit test) * Improved unit test * Fix __iadd__ in assets --------- Co-authored-by: Niels <[email protected]> Co-authored-by: Jerry <[email protected]>
Creating this thread to collect discussion about Chang support. Superseded #368
TODO