Reference scripts impl - publication of a script #4086

paweljakubas · 2023-08-10T16:15:33Z

logic in constructTransaction on api and Cardano.Wallet level
extension of TransactionCtx
use the added field in mkUnsignedTx
fix toCardanoTxOut
adjust fee when reference script is present
integration test showing reference script publication

Comments

PR tackles publication of reference script to be used in later transactions by using reference inputs when minting/burning.
Script is propagated in blockchain in the first of TxOut of a given transaction. It is shown in integration tests that construction/signing/submitting of such a transaction works, plus when decoding transaction a script is present in witness count as expected.

Issue Number

https://cardanofoundation.atlassian.net/browse/ADP-3090

Anviking · 2023-08-22T15:03:07Z

lib/wallet/src/Cardano/Wallet/Write/Tx/Balance.hs

@@ -901,6 +906,8 @@ selectAssets era (ProtocolParameters pp) utxoAssumptions outs redeemers
                , txChange = view #skeletonChange skeleton
                , txPaymentTemplate = view #template <$>
                    assumedInputScriptTemplate utxoAssumptions
+                , txReferenceScript =


As said in DM: The changes to Write.{Balance, SizeEstimation, UTxOAssumptions} shouldn't be needed. As long as the new inline-script-containing TxOut is part of the partial tx given to balanceTx, then balanceTx will know of its size and fee-contribution. So it should just work with the changes reverted.

thanks! indeed adjusting estimateTxSize was not needed!

HeinrichApfelmus

The implementation and test look sensible to me, but it appears to me that the validation checks when parsing a reference script template do not match the specification. 🤔

(In addition, I believe that the specific validation checks here should not be performed — definitely for the validity interval one; less definitely for the checks on the number of cosigners, as there is some correlation to which minting scripts that we allow.)

HeinrichApfelmus · 2023-08-30T17:25:15Z

lib/wallet/api/http/Cardano/Wallet/Api/Http/Shelley/Server.hs

+            wrongMintingTemplate script =
+                isLeft (validateScriptOfTemplate RecommendedValidation script)
+                || countCosigners script /= (1 :: Int)
+                || existsNonZeroCosigner script


Why are these conditions checked? According to the specification (swagger.yaml), any script template should be acceptable. 🤔

script template allows more than one cosigner in general. But here we are using script for minting/burning in SHELLEY (not in shared style) so we need to make sure one cosigner is present.

That's alright with me — all I'm asking for is that this fact is highlighted in the specification. 🤓📓 Could you amend the documents mint-burn.md and swagger.yaml to describe the allowed values for reference_policy_script_template?

(By definition, if the implementation is different from the specification, that's a bug.)

ah, ok. done

Thanks for adding a description of the restriction to mint-burn.md — could you add it to swagger.yaml as well?

lib/wallet/api/http/Cardano/Wallet/Api/Http/Shelley/Server.hs

HeinrichApfelmus · 2023-09-12T14:03:44Z

specifications/api/mint-burn.md

@@ -16,7 +16,7 @@ Specifically:

 1. Creation of a transaction output that contains a minting script and is suitable for use as reference input.

-    In the `reference_policy_script_template` field, you can optionally add a script template. The HTTP endpoint will map this script template into a script using the wallet's policy public key, and this script will be included in the first transaction output (i.e. at index `0`) of the transaction.
+    In the `reference_policy_script_template` field, you can optionally add a script template. The HTTP endpoint will map this script template into a script using the wallet's policy public key, and this script will be included in the first transaction output (i.e. at index `0`) of the transaction. It is worth mentioning that for shelley wallets script template needs to include one cosigner only.


Suggested change

In the `reference_policy_script_template` field, you can optionally add a script template. The HTTP endpoint will map this script template into a script using the wallet's policy public key, and this script will be included in the first transaction output (i.e. at index `0`) of the transaction. It is worth mentioning that for shelley wallets script template needs to include one cosigner only.

In the `reference_policy_script_template` field, you can optionally add a script template. The HTTP endpoint will map this script template into a script using the wallet's policy public key, and this script will be included in the first transaction output (i.e. at index `0`) of the transaction. For Shelley-style wallets, the script template must contain a single cosigner only, but it may include time locks.

HeinrichApfelmus · 2023-09-12T14:06:23Z

lib/wallet/api/http/Cardano/Wallet/Api/Http/Shelley/Server.hs

+            wrongMintingTemplate script =
+                isLeft (validateScriptOfTemplate RecommendedValidation script)
+                || countCosigners script /= (1 :: Int)
+                || existsNonZeroCosigner script


Thanks for adding a description of the restriction to mint-burn.md — could you add it to swagger.yaml as well?

lib/wallet/api/http/Cardano/Wallet/Api/Http/Shelley/Server.hs

HeinrichApfelmus

Thanks! 😊 No major objections anymore. Could you address the remaining comments? — Small change to description in mint-burn.md, and I would like to see a change to swagger.yaml as well.

…etail in a few bullet points the work accomplished in this PR. Before you submit, don't forget to: CODE-OF-CONDUCT.md CONTRIBUTING.md LICENSE MAINTAINERS.md README.md cabal.project default.nix docker-compose.yml docs flake.lock flake.nix floskell.json fourmolu.yaml hie-direnv.yaml justfile lib nix prototypes reports scripts shell.nix specifications test touch.me.CI weeder.dhall Make sure the GitHub PR fields are correct: ✓ Set a good Title for your PR. ✓ Assign yourself to the PR. ✓ Assign one or more reviewer(s). ✓ Link to a Jira issue, and/or other GitHub issues or PRs. ✓ In the PR description delete any empty sections and all text commented in  - [x] logic in `constructTransaction` on api and `Cardano.Wallet` level - [x] extension of `TransactionCtx` - [x] use the added field in `mkUnsignedTx` - [x] fix `toCardanoTxOut` - [x] adjust fee when reference script is present - [x] integration test showing reference script publication ### Comments PR tackles publication of reference script to be used in later transactions by using reference inputs when minting/burning. Script is propagated in blockchain in the first of TxOut of a given transaction. It is shown in integration tests that construction/signing/submitting of such a transaction works, plus when decoding transaction a script is present in witness count as expected.  ### Issue Number https://cardanofoundation.atlassian.net/browse/ADP-3090  Source commit: d6ef2b1

- [x] update `ApiMinBurnFromInput` to accommodate policy id - [x] update swagger and spec - [x] regenerate golden and make sure unit tests pass - [x] incorporate new `ApiMinBurnData` inside `constructTransaction` - [x] introduce ScriptSource - [x] handle from input case to `mkUnsignedTx` - [x] adjust `mkUnsignedTx` - [x] show the case in integration testing` ### Comments builds on top of #4086  ### Issue Number adp-3090

…the work accomplished in this PR. Before you submit, don't forget to: CODE-OF-CONDUCT.md CONTRIBUTING.md LICENSE MAINTAINERS.md README.md cabal.project default.nix docker-compose.yml docs flake.lock flake.nix floskell.json fourmolu.yaml hie-direnv.yaml justfile lib nix prototypes reports scripts shell.nix specifications test touch.me.CI weeder.dhall Make sure the GitHub PR fields are correct: ✓ Set a good Title for your PR. ✓ Assign yourself to the PR. ✓ Assign one or more reviewer(s). ✓ Link to a Jira issue, and/or other GitHub issues or PRs. ✓ In the PR description delete any empty sections and all text commented in  - [x] update `ApiMinBurnFromInput` to accommodate policy id - [x] update swagger and spec - [x] regenerate golden and make sure unit tests pass - [x] incorporate new `ApiMinBurnData` inside `constructTransaction` - [x] introduce ScriptSource - [x] handle from input case to `mkUnsignedTx` - [x] adjust `mkUnsignedTx` - [x] show the case in integration testing` ### Comments builds on top of #4086  ### Issue Number adp-3090  Source commit: 6157932

paweljakubas self-assigned this Aug 10, 2023

paweljakubas force-pushed the paweljakubas/adp-3090/reference-scripts-impl branch from bbb30a4 to ba4f707 Compare August 11, 2023 14:03

paweljakubas force-pushed the paweljakubas/adp-3090/reference-scripts-impl branch from ba4f707 to 2ed37b0 Compare August 22, 2023 10:03

paweljakubas mentioned this pull request Aug 22, 2023

Support reference scripts #4070

Closed

1 task

Anviking reviewed Aug 22, 2023

View reviewed changes

paweljakubas force-pushed the paweljakubas/adp-3090/reference-scripts-impl branch from 2ed37b0 to 491776d Compare August 23, 2023 10:49

paweljakubas changed the title ~~Reference scripts impl~~ Reference scripts impl - publication of script Aug 23, 2023

paweljakubas marked this pull request as ready for review August 23, 2023 12:28

paweljakubas requested review from Anviking, HeinrichApfelmus and jonathanknowles August 23, 2023 12:28

paweljakubas changed the title ~~Reference scripts impl - publication of script~~ Reference scripts impl - publication of a script Aug 23, 2023

paweljakubas force-pushed the paweljakubas/adp-3090/reference-scripts-impl branch 3 times, most recently from 5830774 to 72ec1b3 Compare August 29, 2023 12:41

paweljakubas mentioned this pull request Aug 29, 2023

Use reference input #4114

Merged

8 tasks

paweljakubas force-pushed the paweljakubas/adp-3090/reference-scripts-impl branch from 72ec1b3 to 7022fd3 Compare August 30, 2023 06:25

HeinrichApfelmus requested changes Aug 30, 2023

View reviewed changes

paweljakubas force-pushed the paweljakubas/adp-3090/reference-scripts-impl branch 4 times, most recently from a78a6b3 to 5edc807 Compare September 6, 2023 10:45

paweljakubas requested a review from HeinrichApfelmus September 7, 2023 09:40

paweljakubas added 7 commits September 8, 2023 14:55

parseReferenceScript part 1

bcf82c4

parseReferenceScript part 2

a7db812

compute referenceScript from template

c8bedc7

extend TransactionCtx

c1e788a

sneak reference script to shelley layer

13831cc

sneak to toCardanoTxOut

8b0eefa

conclude toCardanoTxOut

598bd19

paweljakubas added 4 commits September 8, 2023 14:55

integration test - part 1

ae0ca59

integration test - part 2

7b498aa

post rebase fix

7d4caec

add script template remark in spec

bf1b84a

paweljakubas force-pushed the paweljakubas/adp-3090/reference-scripts-impl branch from 718ddae to bf1b84a Compare September 8, 2023 12:57

get rid of validity check during publication of the script

88073da

HeinrichApfelmus reviewed Sep 12, 2023

View reviewed changes

HeinrichApfelmus approved these changes Sep 12, 2023

View reviewed changes

paweljakubas added 2 commits September 12, 2023 16:19

more docs polish

f3cc31f

some fix

baa6679

paweljakubas added this pull request to the merge queue Sep 12, 2023

Merged via the queue into master with commit d6ef2b1 Sep 12, 2023
2 checks passed

paweljakubas deleted the paweljakubas/adp-3090/reference-scripts-impl branch September 12, 2023 16:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reference scripts impl - publication of a script #4086

Reference scripts impl - publication of a script #4086

paweljakubas commented Aug 10, 2023 •

edited

Loading

Anviking Aug 22, 2023 •

edited

Loading

paweljakubas Aug 23, 2023

HeinrichApfelmus left a comment

HeinrichApfelmus Aug 30, 2023

paweljakubas Sep 7, 2023

HeinrichApfelmus Sep 7, 2023

paweljakubas Sep 8, 2023

HeinrichApfelmus Sep 12, 2023

HeinrichApfelmus Sep 12, 2023

paweljakubas Sep 12, 2023

HeinrichApfelmus Sep 12, 2023

HeinrichApfelmus left a comment

Reference scripts impl - publication of a script #4086

Reference scripts impl - publication of a script #4086

Conversation

paweljakubas commented Aug 10, 2023 • edited Loading

Comments

Issue Number

Anviking Aug 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HeinrichApfelmus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HeinrichApfelmus left a comment

Choose a reason for hiding this comment

paweljakubas commented Aug 10, 2023 •

edited

Loading

Anviking Aug 22, 2023 •

edited

Loading