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

CIP-1694 | Updates and semantics #847

Merged
merged 2 commits into from
Jul 9, 2024

Conversation

Hornan7
Copy link
Contributor

@Hornan7 Hornan7 commented Jun 25, 2024

  • Replace "New Constitutional Committee" governance action by "Update Committee"
  • Replace "Update to the Constitution" governance action by "New Constitution". This is how it is called on-chain when submitting this governance action and when querying its tag. I also changed it for semantic reasons.
  • Replace "Proposal policy" by "Guardrails Script". This wording is mainly used broadly by everyone and is also the wording used in the Interim constitution Article VIII, Apendix 1, (Automated Checking ("Guardrails Script")).
  • Update each parameters name tags as seen on chain. (conway era)
  • Replace "Pre-defined Drep" name by "Pre-defined voting option" at 3 different places in the CIP-1694 (Which already got me in big trouble, but lets not go there. =P)

- Replace "New Constitutional Committee" governance action by "Update Committee"
- Replace "Update to the Constitution" governance action by "New Constitution". This is how it is called on-chain when submitting this governance action and when querying the its tag. I also changed it for semantic reasons. 
- Replace "Proposal policy" by "Guardrails Script". This wording is mainly used broadly by everyone and is also the wording used in the Interim constitution Article VIII, Apendix 1, (Automated Checking ("Guardrails Script")).
- Update each parameters name tags as seen on chain. (conway era)
- Replace "Pre-defined Drep" name by "Pre-defined voting option" at 3 different placed in the CIP-1694 (Which got me in big trouble, but lets not go there. =P)
Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

Looks good to me & will endorse as soon as it passes one or two authoritative reviews.

@Ryun1 - cross-referencing proposed CIP-0120? | Constitution specification #796 which still uses the term "updated version" - when according to the changes here it would be an entirely "new" constitution (please correct me if I've misunderstood).

cc @WhatisRT @lehins @thenic95 @stevenj

@WhatisRT
Copy link
Contributor

Note that the proposal policy isn't restricted to being a guardrails script. In fact, the main motivation to introduce it was to put some restriction on the treasury which people really cared about in the Edinburgh meetup and apparently everyone has forgotten about since then.

For the parameter names there are two versions of them, a short and a long version. The names as they are currently in the CIP should align with those written in previous ledger specs. I think we should really have some kind of dictionary of protocol parameters that is properly maintained. There are the CIPs for each era, but they haven't been maintained (I can spot some old names in there, and they match the naming in this CIP better than the proposed names here btw.) and they aren't particularly useful if you have a specific question about some parameter. So I'm somewhat hesitant to change those names that are actually still in use somewhere without having a proper source of truth.

@rphair do you think such a dictionary would make sense as a CIP? The issue is that it'd have to be actively maintained, so it wouldn't be static. But the upside would be that all those naming discussions would be a lot more public than they currently are.

@rphair
Copy link
Collaborator

rphair commented Jun 26, 2024

@WhatisRT I recall & know well what you mean about the parameter CIPs being neglected. This effort ended with an internal disagreement (documented on one of these GitHub CIP PR threads: I think the one that's still unmerged) about whether these CIPs would be full updates (superseding the old ones) or cumulative. So I don't want to push that approach since whatever impetus there may be this year to maintain such a documentation system might not exist next year.

Likewise with CIPs we would have the problem of editorial process behind every single little parameter definition change: even without considering addition to editor workload (@Ryun1 @Crypto2099 & I would get in the habit probably of rubber-stamping the PRs that came in from "authoritative" sources)... but rather what happens whenever there's a disagreement from the community, which we are obligated to resolve by consensus... and those disagreements about wording, behaviour, or versioning haven't been promptly or authoritatively resolved in the past.

So I would recommend a GitHub structure that would allow updates by whichever GitHub users the Ledger team considers "authoritative": a repo on the IntersectMBO or input-output-hk accounts to only house a GitHub Wiki: like https://github.com/input-output-hk/cardano-node-wiki/wiki - you can see that pages are automatically alphabetised onto a sidebar, which is a pain in the neck for navigation but I think would suit the parameter listings very well.

Note we cannot use cardano-foundation/CIPs/wiki since this will have to host the Wiki content I've been working on for the CIP process itself. 😇

p.s. like we have on Wikipedia (more or less, through its "talk" pages), if there are "naming discussions" these can be hosted in the Discussions section of the same repository.

@WhatisRT
Copy link
Contributor

Thank you! I think in that case I'll just put it somewhere in https://github.com/IntersectMBO/formal-ledger-specifications. It's still public and it makes sense for it to be there, it'll probably just be less visible.

@Hornan7
Copy link
Contributor Author

Hornan7 commented Jun 26, 2024

My goal here was to have something that reflect what the community would have to deal with everyday in the governance state of the ledger on Conway era. Which explain my choice to use the most recent names for those params. I also strongly believe the long version would make it easier for the community to understand them. They also reflect the ones being used in the interim constitution, which would also help for better understanding of their relation with the guardrail script (or proposal policy) 😅
I also made sure that the PR was editable by maintainers because I was expecting those discussions. 😃👍
I can change the wordings according to what you guys prefer but I would honestly try to stick with the most recent version of those names IMO since we are entering Conway era very soon. 🥰

Let me know if there is anything you would like me to change or remove, and I will make the modifications accordingly. 😃👍

@WEEDisFOOD
Copy link

This is very cool Mike!

@lehins
Copy link
Contributor

lehins commented Jun 27, 2024

Very nice! Thank you @Hornan7

Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

I think this could well be approved & merged before the next CIP meeting (https://hackmd.io/@cip-editors/92) but I've put it for Last Check there anyway to make sure we get CIP editor sign-off on this & merge no later than that date.

@rphair rphair added the State: Last Check Review favourable with disputes resolved; staged for merging. label Jun 27, 2024
Copy link
Contributor

@WhatisRT WhatisRT left a comment

Choose a reason for hiding this comment

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

I looked at the draft constitution and it also it a 'Guardrails Script'. So apparently that's considered the official name now and I withdraw that point. I think it's really stupid that no one involved with that bothered to make a PR here or let me know, but what's done is done.

As for the protocol parameter names, I started making a table and found a few more inconsistencies. I suspect that there'll be more renamings of them in the future, but maybe that doesn't really matter for this PR. Other CIPs usually had consistent names with the spec but I have a feeling that very few people actually care about that. So go ahead with it.

@rphair rphair requested review from Ryun1 and Crypto2099 June 27, 2024 14:33
@Ryun1
Copy link
Collaborator

Ryun1 commented Jul 3, 2024

Thanks for this @Hornan7
Sharing this with stakeholders 💪

@kevinhammond
Copy link
Contributor

On the parameter name issue, we need canonical versions of the names. The ones here should be consistent with those used in the guardrails document, which were intended to be the common names that users would need to interact with when making changes (i.e. not necessarily the ledger names, which are primarily internal). There's more convergence on the newer names, fortunately, but some of the older ones can be rather confusing. When looking, I found about 5 different conventions in use.

@kevinhammond
Copy link
Contributor

I looked at the draft constitution and it also it a 'Guardrails Script'. So apparently that's considered the official name now and I withdraw that point. I think it's really stupid that no one involved with that bothered to make a PR here or let me know, but what's done is done.

The guardrails script is a proposal policy that enforces the automatable guardrails (which could include treasury withdrawals). In principle, there could be different proposal policies, but this seems unlikely to happen on Cardano. Some naming changes/fixes were proposed to the CIP but I don't know whether they made it as a PR?

@kevinhammond
Copy link
Contributor

@WhatisRT I recall & know well what you mean about the parameter CIPs being neglected. This effort ended with an internal disagreement (documented on one of these GitHub CIP PR threads: I think the one that's still unmerged) about whether these CIPs would be full updates (superseding the old ones) or cumulative. So I don't want to push that approach since whatever impetus there may be this year to maintain such a documentation system might not exist next year.

I ran out of energy on this one. IMO, the sensible thing is to have a living document that doesn't involve looking at 4-5 unrelated CIPs to determine the current set of parameters. This document aims to do that:

https://docs.cardano.org/about-cardano/explore-more/parameter-guide/

@rphair
Copy link
Collaborator

rphair commented Jul 3, 2024

thanks @kevinhammond - since we have no open CIP PRs for parameter definitions or changes, and since that approach hasn't been proposed for the Conway ledger era, and since the only unmerged one has recently been closed:

... I personally feel fine about not keeping the protocol parameters on the CIP repository and think that either of the approaches suggested above would work. cc @Ryun1 @Crypto2099 @KtorZ

Copy link
Collaborator

@Crypto2099 Crypto2099 left a comment

Choose a reason for hiding this comment

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

I think these changes make sense w.r.t. the context of CIP-1694 and making is more user-friendly to read and align between this document and the "guardrails script" that is included as part of the Interim Constitution document.

That said, I do believe that the lexicon and/or glossary of terms linking between the on-ledger names and the "human friendly" names does fall into the purview of the CIP Editors as, were someone post-Chang to propose that a protocol parameter should be renamed, we need some forum for that discussion and debate to take place in. cc: @rphair

Copy link
Collaborator

@Ryun1 Ryun1 left a comment

Choose a reason for hiding this comment

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

thanks Mike!

Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

Good to see this ready to merge now that we are (in the process of) separating the side issue about the parameter inventory into a separately documented GitHub issue.

@Ryun1
Copy link
Collaborator

Ryun1 commented Jul 9, 2024

We can move discussions of protocol parameter definitions/naming over to #852.
@kevinhammond @WhatisRT @Crypto2099 @rphair

@Ryun1 Ryun1 merged commit 2736a9a into cardano-foundation:master Jul 9, 2024
@Hornan7 Hornan7 deleted the CIP-1694-Update branch July 28, 2024 02:28
Hornan7 added a commit to Hornan7/CIPs that referenced this pull request Jul 28, 2024
- Correction to the French version related to PR cardano-foundation#847
rphair pushed a commit that referenced this pull request Jul 28, 2024
- Correction to the French version related to PR #847
@rphair rphair removed the State: Last Check Review favourable with disputes resolved; staged for merging. label Sep 3, 2024
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.

8 participants