search

IntersectMBO / cardano-cli

This repository contains sources for the command-line interface (CLI) tool for interacting with the Cardano blockchain.
Apache License 2.0
39 stars 14 forks source link

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

Closed reeshavacharya closed 2 weeks ago

reeshavacharya commented 2 weeks ago

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
  2. 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
  3. 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 commented 2 weeks ago

cc @CarlosLopezDeLara

CarlosLopezDeLara commented 2 weeks ago

@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 https://github.com/IntersectMBO/cardano-cli/pull/877