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

Treasury Withdrawal action can be created without specifying --funds-receiving-stake-verification-key-file and --transfer #876

Closed
reeshavacharya opened this issue Aug 28, 2024 · 2 comments · Fixed by #877

Comments

@reeshavacharya
Copy link

Description

Treasury Withdrawal action is being created even when the funds receiving stake verification key file and transfer amount are not specified, which are mandatory fields for a valid proposal.

Steps to Reproduce

  1. Create treasury withdrawal action without --funds-receiving-stake-verification-key-file and --transfer
cardano-cli conway governance action create-treasury-withdrawal --anchor-url https://raw.githubusercontent.com/Ryun1/metadata/main/cip108/treasury-withdrawal.jsonld --anchor-data-hash 154ac6b0da5a4c6b185e9b12d89427bf13ca9f4703fe0a5118151a6832229b4a --governance-action-deposit 100000000000 --deposit-return-stake-verification-key-file /home/niraj/.cardano/keys/stake.vkey --out-file create-treasury-withdrawal.proposal --testnet --constitution-script-hash edcd84c10e36ae810dc50847477083069db796219b39ccde790484e0
  1. Build a tx with the proposal
 cardano-cli conway transaction build --proposal-file create-treasury-withdrawal.proposal --proposal-script-file /home/niraj/.cardano/keys/guardrails-script.plutus --proposal-redeemer-value {} --tx-in-collateral a4749ccac547b98840f0970f4157bc58f0918713e35899845192536cbe8a5d02#0 --tx-in a4749ccac547b98840f0970f4157bc58f0918713e35899845192536cbe8a5d02#0 --tx-in a75808079882522af7b596fcc2431d83127b7e0922adc61dc1ecdcdf5ed48771#0 --out-file /home/niraj/.cardano/keys/propose-gov-action-create-treasury-withdrawal_tx.raw --change-address addr_test1qq7rypzjfl4ydy62t9e9e6tkhwul99d2u9q2c9paqrywacemmw9rfes5q4g8uqep2ydlftaulfq23tv8l9c39fthw9zsm60shz --socket-path /home/niraj/.cardano/sancho/node.socket --testnet-magic=4
  1. Sign and Submit
cardano-cli conway transaction sign --tx-body-file /home/niraj/.cardano/keys/propose-gov-action-create-treasury-withdrawal_tx.raw --signing-key-file /home/niraj/.cardano/keys/payment.skey --out-file /home/niraj/.cardano/keys/propose-gov-action-create-treasury-withdrawal_signed_tx.json
cardano-cli conway transaction submit --tx-file /home/niraj/.cardano/keys/propose-gov-action-create-treasury-withdrawal_signed_tx.json --socket-path /home/niraj/.cardano/sancho/node.socket --testnet-magic=4

The transaction was submitted:

Transaction submitted : 6d84c9cbd85257226488871b5ea7aae19f2c34f3b3e562b4856527f0bd31dbbc
GovAction Id          : 6d84c9cbd85257226488871b5ea7aae19f2c34f3b3e562b4856527f0bd31dbbc#0

The missing fields should be mandatory. This could lead to invalid proposals being processed.

Additional Context

cardano-node 9.1.0
cardano-cli 9.2.1.0
git rev 176f99e51155cb3eaa0711db1c3c969d67438958

Possible Solution

Making the options for --funds-receiving-stake-verification-key-file and --transfer for create-treasury-withdrawal action mandatory.

@smelc
Copy link
Contributor

smelc commented Aug 28, 2024

cc @CarlosLopezDeLara

@CarlosLopezDeLara
Copy link
Contributor

CarlosLopezDeLara commented Aug 28, 2024

@reeshavacharya @smelc

I discussed this with @lehins a couple of weeks ago. Technically this is a valid proposal according to the current ledger rules. i.e. Ledger can take an empty map there. However we agreed that indeed, it would be better to have a check to not allow this type of proposal. In the end, this proposal makes no sense. Ledger will implement a check for this before Chang +1, we are not in a hurry because in the bootstrap phase Treasury withdrawals are not allowed anyways.

On the CLI side, we will definitely make it mandatory field.

Edit: This PR fixes the issue #877

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants