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.

Sign up for GitHub

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

Jump to bottom

Add update proposal governance action #152

Closed
wants to merge 4 commits into from
Closed

Add update proposal governance action #152

wants to merge 4 commits into from

Conversation

Jimbo4350
Copy link
Contributor

Changelog

- description: |
    Add update proposal governance action
# uncomment types applicable to the change:
  type:
  - feature        # introduces a new feature
  # - breaking       # the API has changed in a breaking way
  - compatible     # the API has changed but is non-breaking
  # - optimisation   # measurable performance improvements
  # - improvement    # QoL changes e.g. refactoring
  # - bugfix         # fixes a defect
  # - test           # fixes/modifies tests
  # - maintenance    # not directly related to the code
  # - release        # related to a new release preparation
  # - documentation  # change in code docs, haddocks...

Context

Additional context for the PR goes here.

If the PR fixes a particular issue please provide a
link
to the issue.

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • The change log section in the PR description has been filled in
  • New tests are added if needed and existing tests are updated. These may include:
    • golden tests
    • property tests
    • round trip tests
    • integration tests
      See Running tests for more details
  • The version bounds in .cabal files are updated
  • CI passes. See note on CI. The following CI checks are required:
    • Code is linted with hlint. See .github/workflows/check-hlint.yml to get the hlint version
    • Code is formatted with stylish-haskell. See .github/workflows/stylish-haskell.yml to get the stylish-haskell version
    • Code builds on Linux, MacOS and Windows for ghc-8.10.7 and ghc-9.2.7
  • The changelog section in the PR is updated to describe the change
  • Self-reviewed the diff

@Jimbo4350 Jimbo4350 mentioned this pull request Aug 7, 2023
Closed
@Jimbo4350 Jimbo4350 force-pushed the jordan/add-update-proposal-gov-action branch 2 times, most recently from 6bcfba3 to e49029c Compare August 7, 2023 19:54
CarlosLopezDeLara
CarlosLopezDeLara reviewed Aug 7, 2023
View reviewed changes
...est/cardano-cli-golden/files/golden/help/conway_governance_action_create-update-proposal.cli
Usage: cardano-cli conway governance action create-update-proposal
[--protocol-major-version NATURAL
--protocol-minor-version NATURAL]
[--decentralization-parameter RATIONAL]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[--decentralization-parameter RATIONAL] <-- This no longer exists in Conway

CarlosLopezDeLara
CarlosLopezDeLara reviewed Aug 7, 2023
View reviewed changes
...est/cardano-cli-golden/files/golden/help/conway_governance_action_create-update-proposal.cli
[--protocol-major-version NATURAL
--protocol-minor-version NATURAL]
[--decentralization-parameter RATIONAL]
[ --extra-entropy HEX
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[--extra-entropy HEX | --reset-extra-entropy] No longer exists in Conway

CarlosLopezDeLara
CarlosLopezDeLara reviewed Aug 7, 2023
View reviewed changes
...est/cardano-cli-golden/files/golden/help/conway_governance_action_create-update-proposal.cli
)
[--cost-model FILE]
--out-file FILE

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does not seem to include protocol parameters introduced with Conway.

  • governance voting thresholds ($P1$, $P{2a}$, $P{2b}$, $P3$, $P4$, $P{5a}$, $P{5b}$, $P{5c}$, $P6$, $P7$, $Q1$, $Q{2b}$, $Q_4$)
  • constitutional committee term limits
  • governance action expiration
  • governance action deposit (govDeposit)
  • DRep deposit amount (drepDeposit)
  • DRep activity period (drepActivity)
  • minimal constitutional committee size
  • maximum term limit (in epochs) of the constitutional committee

@carbolymer carbolymer self-requested a review August 7, 2023 20:26
@Jimbo4350 Jimbo4350 force-pushed the jordan/add-update-proposal-gov-action branch from e49029c to 642516e Compare August 8, 2023 13:08
newhoggy
newhoggy reviewed Aug 8, 2023
View reviewed changes
cardano-cli/src/Cardano/CLI/EraBased/Run/Governance/Actions.hs
case constit of
ConstitutionFromFile fp -> do
cBs <- liftIO $ BS.readFile $ unFile fp
_utf8EncodedText <- firstExceptT GovernanceActionsCmdNonUtf8EncodedConstitution . hoistEither $ Text.decodeUtf8' cBs
Copy link
Contributor

@newhoggy newhoggy Aug 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason this is unused?

Is it to verify the byte string contains valid text?

disassembler
disassembler reviewed Aug 9, 2023
View reviewed changes
cardano-cli/src/Cardano/CLI/EraBased/Options/Governance/Actions.hs
pGovernanceActionCreateUpdateProposal :: CardanoEra era -> Maybe (Parser (GovernanceActionCmds era))
pGovernanceActionCreateUpdateProposal =
featureInEra Nothing (\cOn -> Just $
subParser "create-update-proposal"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be create-param-proposal., or something to similar to distinguish that it's an update to parameters.

CarlosLopezDeLara
CarlosLopezDeLara reviewed Aug 9, 2023
View reviewed changes
...est/cardano-cli-golden/files/golden/help/conway_governance_action_create-update-proposal.cli
@@ -0,0 +1,130 @@
Usage: cardano-cli conway governance action create-update-proposal
Copy link
Contributor

@CarlosLopezDeLara CarlosLopezDeLara Aug 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Jimbo4350 create-update-proposal is a bit vague, it might be protocol-parameter-change, protocol-parameter-proposal, or even better, a shorter one pparam-change which is closer to how it is called on the CIP.

@Jimbo4350
Copy link
Contributor Author

subsumed by #170

@Jimbo4350 Jimbo4350 closed this Aug 15, 2023
@Jimbo4350 Jimbo4350 deleted the jordan/add-update-proposal-gov-action branch April 12, 2024 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CarlosLopezDeLara CarlosLopezDeLara CarlosLopezDeLara left review comments

@disassembler disassembler disassembler left review comments

@newhoggy newhoggy newhoggy left review comments

@carbolymer carbolymer carbolymer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Jimbo4350 @newhoggy @carbolymer @disassembler @CarlosLopezDeLara