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

[BUG] - cli build cmd requires protocol params to be provided in order to embed datum in transaction #4058

Closed
Tracked by #4144 ...
catch-21 opened this issue Jun 16, 2022 · 3 comments · Fixed by #4431
Closed
Tracked by #4144 ...
Assignees
Labels
bug Something isn't working comp: cardano-cli era: babbage in-scope This item is being worked and will be part of an upcoming release priority medium issues/PRs that SHOULD be addressed. This should be done for the release, but acceptable if it doesn priority: medium A bug that needs to be addressed after ongoing stories and tasks. For instance can be planned for ne type: bug Something is not working user type: internal Created by an IOG employee Vasil

Comments

@catch-21
Copy link
Contributor

catch-21 commented Jun 16, 2022

Internal/External
Internal if an IOHK staff member.

Area
Other Any other topic (Delegation, Ranking, ...).

Summary
When using build --tx-out-datum-embed-* flag to embed datum in the transaction whilst attaching its hash to the output, an error is seen on submit if --protocol-params-file is not provided. If protocol params are required then build should complain that it is missing.

Steps to reproduce
Steps to reproduce the behaviour:

  1. build a transaction using --tx-out-datum-embed-value without providing --protocol-params-file
  2. sign and submit that transaction

Actual behaviour
No error is displayed on build but the following error is seen on submit:

Command failed: transaction submit  Error: Error while submitting tx: ShelleyTxValidationError ShelleyBasedEraBabbage (ApplyTxError [UtxowFailure (FromAlonzoUtxowFail (PPViewHashesDontMatch SNothing (SJust (SafeHash "9c365c30b7badac0e2efb11828f011ed4566496145d3f0cef93dff8b13d7c932"))))])

Expected behavior
Either protocol-params are not necessary and it should just work, or, the build cmd should complain it is missing like it does for plutus script execution:

Command failed: transaction build  Error: Transaction uses Plutus scripts but does not provide the protocol parameters to hash

System info (please complete the following information):

  • OS Name: 20.04.4 LTS (Focal Fossa)
  • Node/Cli version: 6471c31

Acceptance Criteria

Given I'm a user using build command to build a transaction body:

  • Build command does not require pparams.json to build a valid transaction.
  • the flag --protocol-params-file is removed from the command help/usage
  • If the user provides the flag --protocol-params-file the cli prints out a warning of deprecation and ignores the provided pparams file. So that we don't break existing tooling.
@catch-21 catch-21 added bug Something isn't working Vasil and removed Vasil labels Jun 16, 2022
@catch-21
Copy link
Contributor Author

This also exists in Alonzo era so leaving the Vasil label off

@dorin100 dorin100 added the Vasil label Jun 19, 2022
@CarlosLopezDeLara CarlosLopezDeLara added priority low Issues/RPs that are low priority issues/PRs in relation to a minimum Shelley testnet and Shelley mai priority medium issues/PRs that SHOULD be addressed. This should be done for the release, but acceptable if it doesn 1.35.1 Include in 1.35.1 and removed priority low Issues/RPs that are low priority issues/PRs in relation to a minimum Shelley testnet and Shelley mai labels Jun 27, 2022
@CarlosLopezDeLara CarlosLopezDeLara added 1.35.2 and removed 1.35.1 Include in 1.35.1 labels Jul 8, 2022
@CarlosLopezDeLara CarlosLopezDeLara added in-scope This item is being worked and will be part of an upcoming release and removed 1.35.2 labels Jul 22, 2022
@CarlosLopezDeLara CarlosLopezDeLara moved this to 📋 Backlog in Node CLI/API 2022 Aug 11, 2022
@CarlosLopezDeLara CarlosLopezDeLara moved this from 📋 Backlog to 🔖 Ready in Node CLI/API 2022 Aug 11, 2022
@Jimbo4350
Copy link
Contributor

We can totally eliminate this in the build command by not requiring the user to specify the protocol params to begin with. We already query the protocol params here. We can query the node first and then replace <*> validateProtocolParameters era mpparams with <*> return (BuildTxWith $ Just pparams).

@dorin100
Copy link

@CarlosLopezDeLara I think this ticket needs Acceptance Criteria too.

@CarlosLopezDeLara CarlosLopezDeLara moved this from 🔖 Sprint backlog to 🏗 In progress in Node CLI/API 2022 Sep 13, 2022
LudvikGalois pushed a commit that referenced this issue Sep 14, 2022
LudvikGalois pushed a commit that referenced this issue Sep 27, 2022
@dorin100 dorin100 added type: bug Something is not working user type: internal Created by an IOG employee labels Oct 21, 2022
@dorin100 dorin100 added priority: medium A bug that needs to be addressed after ongoing stories and tasks. For instance can be planned for ne comp: cardano-cli era: babbage labels Oct 21, 2022
LudvikGalois pushed a commit that referenced this issue Oct 24, 2022
iohk-bors bot added a commit that referenced this issue Oct 31, 2022
4431: Infer protocol params in transaction build r=LudvikGalois a=LudvikGalois

We now ignore the `--protocol-params-file` in the `transaction build` command. Instead we
fetch it from the node directly. If the user still supplies it, we display a warning, but otherwise
continue.

Fixes #4058

Co-authored-by: Robert 'Probie' Offner <[email protected]>
Jimbo4350 pushed a commit that referenced this issue Jan 5, 2023
Jimbo4350 pushed a commit that referenced this issue Jan 5, 2023
Jimbo4350 pushed a commit that referenced this issue Jan 11, 2023
newhoggy pushed a commit to IntersectMBO/cardano-api that referenced this issue May 23, 2023
newhoggy pushed a commit to IntersectMBO/cardano-cli that referenced this issue May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working comp: cardano-cli era: babbage in-scope This item is being worked and will be part of an upcoming release priority medium issues/PRs that SHOULD be addressed. This should be done for the release, but acceptable if it doesn priority: medium A bug that needs to be addressed after ongoing stories and tasks. For instance can be planned for ne type: bug Something is not working user type: internal Created by an IOG employee Vasil
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants