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

cardano-cli conway governance action create-constitution: rework hash arguments #420

Closed
CarlosLopezDeLara opened this issue Nov 1, 2023 · 6 comments · Fixed by #442
Closed
Assignees

Comments

@CarlosLopezDeLara
Copy link
Contributor

CarlosLopezDeLara commented Nov 1, 2023

Remove the metadata word from

--constitution-anchor-url TEXT
( --constitution-anchor-metadata TEXT
| --constitution-anchor-metadata-file FILE
| --constitution-anchor-metadata-hash HASH
)

it should be

--constitution-anchor-url TEXT
( --constitution-anchor-text TEXT
| --constitution-anchor-file FILE
| --constitution-anchor-hash HASH
)

[edit]

Update from a discussion on Slack:

  • We will have --constitution-anchor-url TEXT --constitution-anchor-hash HASH,
  • and a command cardano-cli conway governance hash --file --text to create the HASH
@CarlosLopezDeLara CarlosLopezDeLara added this to the SanchoNet Phase 6 milestone Nov 1, 2023
@smelc smelc self-assigned this Nov 2, 2023
@carlhammann
Copy link
Contributor

@CarlosLopezDeLara, @smelc I'm all for making the flags shorter, but the proposed renaming loses clarity: the file and hash do not include the anchor URL. (An anchor is an URL plus a hash of the contents behind that URL. The text, file, or hash are the options for a "hash source".)

@smelc smelc changed the title cardano-cli conway governance action create-constitution cardano-cli conway governance action create-constitution: rework hash arguments Nov 7, 2023
@smelc
Copy link
Contributor

smelc commented Nov 7, 2023

As of dbbc570, commands affected by this change are:

  • create-constitution (--proposal-anchor-metadata, --proposal-anchor-metadata-file, --constitution-anchor-metadata, --constitution-anchor-metadata-file)
  • update-committee (--proposal-anchor-metadata, --proposal-anchor-metadata-file)
  • create-info (same as previous)
  • create-no-confidence (same as previous)
  • create-protocol-parameters-update (same as previous)
  • create-treasury-withdrawal (same as previous)
  • conway governance vote create (--vote-anchor-metadata, --vote-anchor-metadata-hash)

@smelc
Copy link
Contributor

smelc commented Nov 9, 2023

I'm nearly done with this (PR 442). In the end I propose we have three modes in the conway governance hash command, like b2sum:

  • --text which takes the text to hash directly
  • --text-file which takes a file and treats it as text, and hashes its content
  • --binary-file which takes a treats it as binary, allowing PDF files, compressed files, etc; and hashes its content.

If we go with only --text and --file, we don't clearly state how we treat files, and will end up with unhappy users. Test-wise, those flags will allow us to be platform-agnostic and avoid failures on Windows.

@Jimbo4350
Copy link
Contributor

#442 (review)

@smelc
Copy link
Contributor

smelc commented Nov 9, 2023

I went for --text, --file-text, and --file-binary in the end, as it looked more consistent with existing flags, that tend to differentiate different behaviors by the suffix of the flag, rather than the start of the flag.

@smelc
Copy link
Contributor

smelc commented Nov 9, 2023

@CarlosLopezDeLara> another side effect of the PR is that I harmonized the flags of anchors of vote create with all the other commands:

  • --vote-anchor-url becomes --anchor-url
  • --vote-anchor-metadata-hash becomes --anchor-data-hash

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