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

make decodeTx not so strict when policy key is missing #3194

Merged
merged 3 commits into from
Mar 23, 2022

Conversation

paweljakubas
Copy link
Contributor

@paweljakubas paweljakubas commented Mar 22, 2022

adp-1549

  • I have make sure checking for policy key is only taking place when minting/burning is on.

Comments

Issue Number

@paweljakubas paweljakubas self-assigned this Mar 22, 2022
@piotr-iohk
Copy link
Contributor

I've run e2e suite against this PR -> https://github.com/input-output-hk/cardano-wallet/runs/5646577836?check_suite_focus=true

Most of the tests are now passing (i.e. the fix addresses some of the failures), but not all of them.

The remaining failures are:

  • Plutus transactions that mint some tokens
  • Constructing the minting transaction

Plutus:

rspec ./spec/e2e_spec.rb:102 # Cardano Wallet E2E tests E2E Balance -> Sign -> Submit game
rspec ./spec/e2e_spec.rb:109 # Cardano Wallet E2E tests E2E Balance -> Sign -> Submit mint-burn
rspec ./spec/e2e_spec.rb:200 # Cardano Wallet E2E tests E2E Balance -> Sign -> Submit currency

These are Plutus scripts that actually mint some tokens and they fail with the same error:

{"code":"invalid_wallet_type",
"message":"It seems the wallet lacks a policy public key. It's therefore not possible to create a minting or burning transaction. Please restore the wallet from a mnemonic phrase instead of an account public key."}

Construct mint tx:
Also constructing mint transaction still fails:

$ curl -X POST http://localhost:8090/v2/wallets/2269611a3c10b219b0d38d74b004c298b76d16a9/transactions-construct \
-d '{"mint_burn":[{"operation":{"mint":{"receiving_address":"addr_test1qq223dyrt96yx79rz3yda2vs6xjx28ts2ax39hp8cxq2p0shpgwhzykzha0z6w27gxsps40gurvxjt2vxh26009q5wqqxpj2ey","amount":{"quantity":1,"unit":"assets"}}},"policy_script_template":"cosigner#0","asset_name":"1111"}]}' \
-H "Content-Type: application/json" | jq

{
  "code": "invalid_wallet_type",
  "message": "It seems the wallet lacks a policy public key. It's therefore not possible to create a minting or burning transaction. Please restore the wallet from a mnemonic phrase instead of an account public key."
}

@paweljakubas paweljakubas force-pushed the paweljakubas/adp-1549/fix-decode-tx branch from fe066fa to 860a27c Compare March 23, 2022 12:28
@paweljakubas
Copy link
Contributor Author

@piotr-iohk plutus scripts now should pass. construct tx should not proceed when policy key is missing. I will add a way to set this key (via endpoint as it is not possible to do it with db migrations) in next PR.

@piotr-iohk
Copy link
Contributor

piotr-iohk commented Mar 23, 2022

@piotr-iohk plutus scripts now should pass. construct tx should not proceed when policy key is missing. I will add a way to set this key (via endpoint as it is not possible to do it with db migrations) in next PR.

Executed e2e suite against this branch after changes again.

>> cardano-node and cardano-wallet versions:
v2022-01-18 (git revision: 860a27c18ba5bca2392c0f4ea6cdd74812f275db)
cardano-node 1.34.1 - linux-x86_64 - git rev 73f9a746362695dc2cb63ba757fbcabb81733d23

All is passing now:

https://github.com/input-output-hk/cardano-wallet/actions/runs/2028501957

Note: Construct mint tx would still fail (there is no such test in e2e suite yet.), but I understand that's a topic for another PR.

@paweljakubas
Copy link
Contributor Author

bors r+

iohk-bors bot added a commit that referenced this pull request Mar 23, 2022
3194: make decodeTx not so strict when policy key is missing r=paweljakubas a=paweljakubas

adp-1549

<!--
Detail in a few bullet points the work accomplished in this PR.

Before you submit, don't forget to:

* 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 <!--, so that this text does not appear
     in merge commit messages.

* Don't waste reviewers' time:
   ✓ If it's a draft, select the Create Draft PR option.
   ✓ Self-review your changes to make sure nothing unexpected slipped through.

* Try to make your intent clear:
   ✓ Write a good Description that explains what this PR is meant to do.
   ✓ Jira will detect and link to this PR once created, but you can also
     link this PR in the description of the corresponding Jira ticket.
   ✓ Highlight what Testing you have done.
   ✓ Acknowledge any changes required to the Documentation.
-->


- [x] I have make sure checking for policy key is only taking place when minting/burning is on. 

### Comments

<!-- Additional comments, links, or screenshots to attach, if any. -->

### Issue Number

<!-- Reference the Jira/GitHub issue that this PR relates to, and which requirements it tackles.
  Note: Jira issues of the form ADP- will be auto-linked. -->


Co-authored-by: Pawel Jakubas <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Mar 23, 2022

Build failed:

  test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs:2083:13:
  1) Cardano.Wallet.Shelley.Transaction.balanceTransaction, when passed unresolved inputs, may fail
       *** Gave up! Passed only 99 tests; 4 discarded tests (14% unknown txins).

#3183

@paweljakubas
Copy link
Contributor Author

bors retry

iohk-bors bot added a commit that referenced this pull request Mar 23, 2022
3194: make decodeTx not so strict when policy key is missing r=paweljakubas a=paweljakubas

adp-1549

<!--
Detail in a few bullet points the work accomplished in this PR.

Before you submit, don't forget to:

* 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 <!--, so that this text does not appear
     in merge commit messages.

* Don't waste reviewers' time:
   ✓ If it's a draft, select the Create Draft PR option.
   ✓ Self-review your changes to make sure nothing unexpected slipped through.

* Try to make your intent clear:
   ✓ Write a good Description that explains what this PR is meant to do.
   ✓ Jira will detect and link to this PR once created, but you can also
     link this PR in the description of the corresponding Jira ticket.
   ✓ Highlight what Testing you have done.
   ✓ Acknowledge any changes required to the Documentation.
-->


- [x] I have make sure checking for policy key is only taking place when minting/burning is on. 

### Comments

<!-- Additional comments, links, or screenshots to attach, if any. -->

### Issue Number

<!-- Reference the Jira/GitHub issue that this PR relates to, and which requirements it tackles.
  Note: Jira issues of the form ADP- will be auto-linked. -->


Co-authored-by: Pawel Jakubas <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Mar 23, 2022

Build failed:

 test/unit/Cardano/Wallet/Api/ServerSpec.hs:107:5:
  1) Cardano.Wallet.Api.Server, API Server, binds to the local interface
       uncaught exception: IOException of type NoSuchThing
       kevent: does not exist (No such file or directory)

  To rerun use: --match "/Cardano.Wallet.Api.Server/API Server/binds to the local interface/"

@paweljakubas
Copy link
Contributor Author

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Mar 23, 2022

Build succeeded:

@iohk-bors iohk-bors bot merged commit a72ca89 into master Mar 23, 2022
@iohk-bors iohk-bors bot deleted the paweljakubas/adp-1549/fix-decode-tx branch March 23, 2022 20:59
WilliamKingNoel-Bot pushed a commit that referenced this pull request Mar 23, 2022
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.

2 participants