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

Reference script integration #3953

Merged
merged 12 commits into from Jun 6, 2022

Conversation

Jimbo4350
Copy link
Contributor

@Jimbo4350 Jimbo4350 commented May 31, 2022

  • Implements a shell script that uses a spending plutus v2 reference script. Currently only build-raw is working with reference scripts.
  • Updates the api and cli to enable the usage of reference scripts
  • Adds documentation outlining how to use reference scripts

@Jimbo4350 Jimbo4350 force-pushed the jordan/reference-script-integration-cli-api-final branch 6 times, most recently from c17663d to 6be0eae Compare June 2, 2022 17:00
@Jimbo4350 Jimbo4350 marked this pull request as ready for review June 3, 2022 16:24
@Jimbo4350 Jimbo4350 force-pushed the jordan/reference-script-integration-cli-api-final branch from 2868852 to 19b83d2 Compare June 3, 2022 18:12
```

To start your babbage cluster you need to run the `example/run/all.sh` shell script.
The remainder of this guide briefly talks about the [shell script example](scripts/plutus/example-reference-script-usage.sh) that automatically does everything for you.
Copy link
Contributor

Choose a reason for hiding this comment

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

'Further, we briefly discuss the shell script example, which automates your following actions'.
@Jimbo4350 Please see if this makes sense^. If not, I'd suggest changing 'automatically does everything for you' into a clearer statement. Like what exactly does it automate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've made a change let me know what you think

and runTxBuildRaw functions

Update the utxo query in the runTxBuild function to also inlclude
reference scripts. This is necessary otherwise the transaction will be
constructed without reference scripts (if they are specified at the cli
level).
@catch-21
Copy link
Contributor

catch-21 commented Jun 6, 2022

Consider renaming --reference-script-file to follow convention of other tx-out flags. For example, it could be changed to --tx-out-reference-script-file

@Jimbo4350 Jimbo4350 force-pushed the jordan/reference-script-integration-cli-api-final branch 2 times, most recently from 7747587 to ac5bdae Compare June 6, 2022 17:03
@catch-21
Copy link
Contributor

catch-21 commented Jun 6, 2022

If we're going to leave --plutus-script-v1 flag in then perhaps add to its help text Specify a plutus script v1 reference script. Note: Ledger rules prevent referenced V1 scripts from being executed

@catch-21
Copy link
Contributor

catch-21 commented Jun 6, 2022

--simple-script-v1       Specify a simple script v1 reference script.
--simple-script-v2       Specify a simple script v2 reference script.

@Jimbo4350 Do these refer to the Shelley(v1) and Allegra(v2) native scripts? If so it may be worth clarifying that in the help text because it wasn't obvious to me.

@Jimbo4350 Jimbo4350 force-pushed the jordan/reference-script-integration-cli-api-final branch from ac5bdae to fc2973b Compare June 6, 2022 17:51
@Jimbo4350 Jimbo4350 mentioned this pull request Jun 6, 2022
38 tasks
@@ -953,7 +966,7 @@ fromShelleyScriptHash = ScriptHash


-- ----------------------------------------------------------------------------
-- The simple native script language
-- The simple simple script language
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
-- The simple simple script language
-- The simple script language

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's super simple ("simple simple") . Thanks for catching this.

@Jimbo4350 Jimbo4350 force-pushed the jordan/reference-script-integration-cli-api-final branch from fc2973b to 8aa99d2 Compare June 6, 2022 18:08
Opt.flag' (AnyScriptLanguage $ SimpleScriptLanguage SimpleScriptV1)
( Opt.long "simple-script-v1"
<> Opt.help "Specify a simple script v1 reference script. \
\See documentation at doc/reference/simple-scripts.md"
Copy link
Contributor

Choose a reason for hiding this comment

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

This document doesn't refer to v1 and v2. How can we make that distinction clear?

Copy link
Contributor

Choose a reason for hiding this comment

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

Specify a simple script v1 (Shelley) reference script
Specify a simple script v2 (Allegra) reference script.
?

Copy link
Contributor

@catch-21 catch-21 Jun 6, 2022

Choose a reason for hiding this comment

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

Also, you could prepend TODO: until they are supported (because that's what you see when you try to use them)

Copy link
Contributor

@JaredCorduan JaredCorduan left a comment

Choose a reason for hiding this comment

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

I made a few minor comments, but overall this is awesome! This was a huge undertaking, thanks @Jimbo4350 !!


In this example we will use the [Required Redeemer](scripts/plutus/scripts/v2/required-redeemer.plutus) Plutus spending script. In order to execute a reference Plutus spending script, we require the following:

- Collateral tx input(s) - these are provided and are forfeited in the event the Plutus script fails to execute.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we ease folks mind by telling them about the fact that a local node will not forward on a failing tx to the relay node unless it is explicitly told to? As you know, I'm sure, the collateral exists to prevent abusive behavior, and under normal circumstances will not be taken.

Copy link
Contributor

Choose a reason for hiding this comment

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

"... in the event this transaction is submitted with a failing Plutus script. Local validation protects against this by default."
?

@@ -151,7 +151,7 @@ $SED -i "${ROOT}/genesis/shelley/genesis.json" \
-e 's/"minFeeB": 0/"minFeeB": 155381/' \
-e 's/"minUTxOValue": 0/"minUTxOValue": 1000000/' \
-e 's/"decentralisationParam": 1.0/"decentralisationParam": 0.7/' \
-e 's/"major": 0/"major": 2/' \
-e 's/"major": 0/"major": 7/' \
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

@Jimbo4350
Copy link
Contributor Author

bors r+

@Jimbo4350
Copy link
Contributor Author

bors r-

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 6, 2022

Canceled.

@Jimbo4350
Copy link
Contributor Author

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 6, 2022

Build succeeded:

@iohk-bors iohk-bors bot merged commit 95c3692 into master Jun 6, 2022
@iohk-bors iohk-bors bot deleted the jordan/reference-script-integration-cli-api-final branch June 6, 2022 21:09
@Jimbo4350
Copy link
Contributor Author

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 6, 2022

Already running a review

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.

None yet

5 participants