chore(nns): Cleanup constants related to neuron-minimum-dissolve-delay-to-vote #4290
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
If this pull request affects the behavior of any canister owned by
the Governance team, remember to update the corresponding
unreleased_changes.md file(s).
To acknowldge this reminder (and unblock the PR), dismiss this
code review by going to the bottom of the pull request page, and
supply one of the following reasons:
-
Done.
-
No canister behavior changes.
There was a problem hiding this comment.
I would not say that DEFAULT_NEURON_MINIMUM_DISSOLVE_DELAY_TO_VOTE_SECONDS replaces MIN_DISSOLVE_DELAY_FOR_VOTE_ELIGIBILITY_SECONDS (this is not your exact wording, but I think most ppl would say "replace" is equivalent). Rather, the former is removed, because it is determined by a new field in NetworkEconomics. Whereas, tests that previously used 6 months now get that from DEFAULT_NEURON_MINIMUM_DISSOLVE_DELAY_TO_VOTE_SECONDS.
I guess in some future PR, we might want to change DEFAULT_NEURON_MINIMUM_DISSOLVE_DELAY_TO_VOTE_SECONDS to 3 months, once the proposal goes through.
| pub const DEFAULT_NEURON_MINIMUM_DISSOLVE_DELAY_TO_VOTE_SECONDS: u64 = 6 * ONE_MONTH_SECONDS; | ||
|
|
||
| pub const NEURON_MIN_DISSOLVE_DELAY_TO_VOTE_SECONDS_BOUNDS: RangeInclusive<u64> = | ||
| (3 * ONE_MONTH_SECONDS)..=(6 * ONE_MONTH_SECONDS); |
There was a problem hiding this comment.
NC. I assume that a quorum already discussed and decided this.
This seems overly restrictive. Nothing suddenly breaks if the parameter gets set to 2.9 months, right? Nor does anything bad happen if 6.1 months is used, right? I can maybe imagine that something bad might happen if you lower it from 1 second to 0 seconds (0 has a funny way of triggering new behaviors). I can also imagine that maybe INCREASING MDD breaks something (however, I am not specifically aware of anything being broken by a sudden increase).
| // value for the system, but it should be updated to align with [MIN_DISSOLVE_DELAY_FOR_VOTE_ELIGIBILITY_SECONDS]. | ||
| pub const DEFAULT_NEURON_MINIMUM_DISSOLVE_DELAY_TO_VOTE_SECONDS: u64 = | ||
| crate::governance::MIN_DISSOLVE_DELAY_FOR_VOTE_ELIGIBILITY_SECONDS; | ||
| /// The minimum dissolve delay so that a neuron may vote. |
There was a problem hiding this comment.
NCR
When a proposal is created, neurons with dissolve delay (in seconds) less than NetworkEconomics.min_dissolve_delay_seconds receive no ballot (to be filled out) for that proposal. Thus, such neurons cannot vote on the proposal.
| /// The minimum dissolve delay so that a neuron may vote. | ||
| pub const DEFAULT_NEURON_MINIMUM_DISSOLVE_DELAY_TO_VOTE_SECONDS: u64 = 6 * ONE_MONTH_SECONDS; | ||
|
|
||
| pub const NEURON_MIN_DISSOLVE_DELAY_TO_VOTE_SECONDS_BOUNDS: RangeInclusive<u64> = |
There was a problem hiding this comment.
For consistency, with the other constant, s/MIN/MINIMUM/.
| /// The minimum dissolve delay so that a neuron may vote. | ||
| pub const DEFAULT_NEURON_MINIMUM_DISSOLVE_DELAY_TO_VOTE_SECONDS: u64 = 6 * ONE_MONTH_SECONDS; | ||
|
|
||
| pub const NEURON_MIN_DISSOLVE_DELAY_TO_VOTE_SECONDS_BOUNDS: RangeInclusive<u64> = |
There was a problem hiding this comment.
Maybe, a comment that says something like
A proposal that tries to set NetworkEconomics.min_dissolve_delay_seconds must propose a value that falls within this range.
| "neuron_minimum_dissolve_delay_to_vote_seconds ({:?}) must be between three \ | ||
| and six months.", |
There was a problem hiding this comment.
It seems easy that this wording could get out of sync with the constant.
This PR cleans up after the recent change that exposed the (priorly code-constant) neuron-minimum-dissolve-delay-to-vote constraint in NNS Governance. In particular:
MIN_DISSOLVE_DELAY_FOR_VOTE_ELIGIBILITY_SECONDSis removed in favor ofDEFAULT_NEURON_MINIMUM_DISSOLVE_DELAY_TO_VOTE_SECONDS(which has the same value).MIN_DISSOLVE_DELAY_FOR_VOTE_ELIGIBILITY_SECONDS_BOUNDSrenamed asNEURON_MIN_DISSOLVE_DELAY_TO_VOTE_SECONDS_BOUNDS(for consistency) and lifted next to the constantMIN_DISSOLVE_DELAY_FOR_VOTE_ELIGIBILITY_SECONDSthat it bounds.