-
Notifications
You must be signed in to change notification settings - Fork 722
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
Add CBOR support to transaction build and transaction build-raw commands #3483
Conversation
The PR introduces the following changes to the
And the following changes to the
|
This resolves #3472. Notes about implementation are within the issue. |
2b0262b
to
a4c1358
Compare
<> Opt.help (helpTextForFile ++ " The file must follow the special \ | ||
\JSON schema for script data.") | ||
) | ||
|
||
pScriptDataValue = | ||
pScriptDataFile = ScriptDataFile (Text.pack ("--" ++ dataFlagPrefix ++ "-file option is deprecated")) <$> |
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.
Why is this being deprecated?
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 --tx-in-redeemer-json-file
is clearer than --tx-in-redeemer-file
, although I'm not sure if our deprecation policy allows this.
If we prefer to keep the original option name then I can revert that portion.
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'd say keep the original option name
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.
Done!
a4c1358
to
2f6d874
Compare
this is going to be very helpful, thank you @newhoggy ! |
6be6754
to
fa61bb0
Compare
@JaredCorduan Do you have a recommended way of getting CBOR files for testing? |
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.
LGTM! I think some of the lines go over the 100 char limit though.
3473089
to
b631c9c
Compare
Added some newlines to keep under the 100 char limit. |
b631c9c
to
33121d1
Compare
@newhoggy in the ledger tests, we use this ruby program to generate random CBOR from a CDDL spec: https://rubygems.org/gems/cddl The CDDL for datums is here: https://github.com/input-output-hk/cardano-ledger/blob/6fccece6415df6e8864985ddefdc30e0c1a6b8b6/eras/alonzo/test-suite/cddl-files/alonzo.cddl#L271-L276 If you want something simpler, I would suggest make a few examples conforming to that CDDL that use a variety of definite and indefinite lists and maps. |
stuff Use serialiser from plutus instead Use serialiser from plutus instead
Added some $ xxd scripts/plutus/data/42.datum.cbor
00000000: 182a .*
$ xxd scripts/plutus/data/typed-42.datum.cbor
00000000: d879 9f18 2aff .y..*. Which correspond to the JSON datum files: $ cat scripts/plutus/data/42.datum
{"int":42}%
$ cat scripts/plutus/data/typed-42.datum
{"constructor":0,"fields":[{"int":42}]}% Which according to https://cbor.nemo157.com/ are annotated as follows:
|
Tested by hand editing |
33121d1
to
2d48896
Compare
bors merge |
Build succeeded: |
Note, this deprecates options like
--tx-in-datum-file
, replacing them with the more descriptive--tx-in-datum-json-file