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 | Guard security-relevant protocol parameter changes behind SPO votes #622

Merged
merged 4 commits into from
Feb 6, 2024

Conversation

WhatisRT
Copy link
Contributor

These changes came out of recent discussions with security researchers. Guarding certain parameters behind SPO votes results in preserving the current security and liveness properties of the system without having to make further security assumptions.

This is WIP for now, since I'm not sure yet whether the list of parameters guarded behind this extra condition is final.

@Ryun1 Ryun1 changed the title CIP-1694: Guard security-relevant protocol parameter changes behind SPO votes CIP-1694 | Guard security-relevant protocol parameter changes behind SPO votes Nov 20, 2023
@Ryun1 Ryun1 added the Category: Ledger Proposals belonging to the 'Ledger' category. label Nov 20, 2023
CIP-1694/README.md Outdated Show resolved Hide resolved
Also clarify that the proposal policy only applies to protocol
parameter updates and treasury withdrawals.
@WhatisRT WhatisRT marked this pull request as ready for review December 21, 2023 13:28
@lehins
Copy link
Contributor

lehins commented Jan 4, 2024

We'll need to add this new parameter to the list of security params: IntersectMBO/cardano-ledger#3952

@Ryun1
Copy link
Collaborator

Ryun1 commented Jan 9, 2024

Whilst we are editing CIP-1694, would it be possible to add a note about script-based constitutional committee credentials?
As these made it into the ledger design post the CIP, and I have found that most people are unaware of these credentials because of this.

@WhatisRT
Copy link
Contributor Author

I've added that CC members can be scripts, that the new PParam minFeeRefScriptsCoinsPerByte is security-relevant and fixed the outdated claim that we go into no-confidence when we don't have enough active CC members.

Comment on lines +1317 to +1328
The security relevant protocol parameters are:
* `maxBBSize`
* `maxTxSize`
* `maxBHSize`
* `maxValSize`
* `maxBlockExUnits`
* `minFeeA`
* `minFeeB`
* `coinsPerUTxOByte`
* `govActionDeposit`
* `minFeeRefScriptsCoinsPerByte`

Copy link
Contributor

@Hornan7 Hornan7 Jan 31, 2024

Choose a reason for hiding this comment

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

If this can be done at the protocol level, you should add dvtMotionNoConfidence and pvtMotionNoConfidence to this list as well and remove the CC rights to vote on these 2 parameters that were designed originaly to remove them.

They already cannot vote on "Motion of no confidence" and "Update Committee" governance actions, for obvious reasons. Which is why I think, their right to vote on these 2 parameters should be given to SPOs instead in my opinion. 😄👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately that can't happen due to technical reasons, since dvtMotionNoConfidence and dvtMotionNoConfidence are not individual protocol parameters, but instead they are thresholds that are part of a threshold set, as such they can't be voted on individually. In other words, whenever a proposal is submitted that updates Stake Pool or DRep thresholds, all of the thresholds must be provided.

Moreover, drastic changes to the CIP are not possible at this stage.

The formal specification is complete and undergoing the audit at this point. Any changes right now will cause a significant delay to the release.

That being said, if you believe such functionality is very important, feel free to submit a CIP for the next era.

Copy link

Choose a reason for hiding this comment

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

Thank you for this explanation @lehins
Would you happen to have more information on the mentioned "threshold set" and what they contain?
Are the sets the different governance action thresholds for DReps and SPOs?

Copy link
Contributor

Choose a reason for hiding this comment

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

There are two separate protocol parameters one for DReps and another for Stake Pools. Both of which contain all the relevant thresholds.

Copy link
Contributor

@Hornan7 Hornan7 Feb 6, 2024

Choose a reason for hiding this comment

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

Would you happen to have more information on the mentioned "threshold set" and what they contain?
Are the sets the different governance action thresholds for DReps and SPOs?

@thenic95 Here are the 2 "threshold set" grouped in square brackets:
image

What he meant is:
when you submit a PP governance action, you have to include all Protocol Parameters of the same "set" in the proposal file even if you change only one parameter from the same "set".

Thanks for the info @lehins 🥇

@Ryun1
Copy link
Collaborator

Ryun1 commented Feb 4, 2024

I have forwarded this PR to the Intersect governance parameter working group for comment :)

@Hornan7
Copy link
Contributor

Hornan7 commented Feb 5, 2024

The only thing I would add is that, since we have the CC and Dreps votes on govActionDeposit already, I don't think adding SPOs votes would be nessesary. Unless we end up with no Committee at all.

But in that case, if you see this as a security issue, why all other deposits wouldn't be equally at risk? If risk there is. (dRepdeposit, poolDeposit, keyDeposit)
Because technicaly speaking, if the risk of having govActionDeposit not being protected by SPO would add the possibility of someone raising it to stall governance on Cardano, what would hold em to do the same with DReps deposit, Stake pool deposit, stake key deposit and hold anyone else from registering as a DRep, SPOs or their own stake credentials?

I can understand the importance of adding layers of difficulty to all other ones though (Transaction related ones), but I really don't think govActionDeposit is a big of a deal.

@WhatisRT
Copy link
Contributor Author

WhatisRT commented Feb 5, 2024

This list of security-relevant parameters comes from multiple sessions with security researchers where we analyzed how the chain might break if certain parameters were set to bad values. There were some strict criteria for a parameter to be included, one of which being unrecoverability. govActionDeposit is unrecoverable if set too high: set it to a number over the total amount of lovelace in the system and you can never propose a governance action again. Other deposits don't have this problem, and they don't affect anyone who isn't already registered. There are issues with raising these too high, but the change is reversible.

While this may sound unrealistic, the point of the security-relevant parameters is to cover all possible attacks that satisfy these criteria. You only need to trust the SPOs to trust that the chain will not suffer from these attacks, no matter what happens with DReps & the CC.

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.

Positive feedback from stake holders confirmed at CIP meeting today.

@rphair rphair merged commit 1777164 into cardano-foundation:master Feb 6, 2024
Ryun1 pushed a commit to Ryun1/CIPs that referenced this pull request Feb 20, 2024
…SPO votes (cardano-foundation#622)

* Guard security-relevant protocol parameter changes behind SPO votes

* Replace the 'security group' by 'security relevant parameters'

Also clarify that the proposal policy only applies to protocol
parameter updates and treasury withdrawals.

* Minor fixes and additions

* Fix outdated wording
Ryun1 pushed a commit to Ryun1/CIPs that referenced this pull request Mar 6, 2024
…SPO votes (cardano-foundation#622)

* Guard security-relevant protocol parameter changes behind SPO votes

* Replace the 'security group' by 'security relevant parameters'

Also clarify that the proposal policy only applies to protocol
parameter updates and treasury withdrawals.

* Minor fixes and additions

* Fix outdated wording
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Ledger Proposals belonging to the 'Ledger' category.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants