-
-
Notifications
You must be signed in to change notification settings - Fork 76
Fix reference scripts fetched from ogmios #254
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
This fixed the issue I had in #252 in the tool I was using |
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the GitHub App Integration for your organization. Read more. @@ Coverage Diff @@
## main #254 +/- ##
==========================================
- Coverage 85.14% 85.14% -0.01%
==========================================
Files 26 26
Lines 2996 2995 -1
Branches 719 719
==========================================
- Hits 2551 2550 -1
Misses 335 335
Partials 110 110
|
The scripts are provided in the format that correspond to the preimage of the various hashes calculated from scripts. For some unknown reasons, few interfaces like the cardano-cli have opted for encoding an already cbor-encoded binary object as cbor, resulting in this double-cbor-wrapping (dare I say it?) non-sense (albeit I have no doubt it wasn't intentional). Using only the "raw" flat-encoded binary form of the script is unsatisfactory IMO because it still requires being cbor-encoded before hashing to obtain a valid preimage. So the most meaningful form is the cbor-flat-encoded script. Now, why do we have 2 different binary encoding format, and one of them sometimes applied twice is a discussion for another day. |
Thanks for this insight. So
|
@@ -467,9 +467,9 @@ def _utxo_from_ogmios_result(self, result) -> UTxO: | |||
script = output.get("script", None) | |||
if script: | |||
if "plutus:v2" in script: | |||
script = PlutusV2Script(cbor2.loads(bytes.fromhex(script["plutus:v2"]))) | |||
script = PlutusV2Script(bytes.fromhex(script["plutus:v2"])) |
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.
@cffls bump, this needs a decision and IMO should be merged as is |
Maybe we should add a test case for it? Ogmios is run in the test environment afaik |
Yes, we should add a test case for it before merging. I will try add one and see if it works. |
88f0b0b
to
b075c41
Compare
Same fix as for kupo (#253) for #252
@KtorZ maybe you can chime in on what exactly the format of scripts in utxos is in kupo/ogmios? Its mentioned to be "raw script" from which I would actually understand 0 cbor wrapping but it appears to be exactly once?