WIP: Software upgrade spec #2116
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #2116 +/- ##
===========================================
+ Coverage 63.83% 64.86% +1.03%
===========================================
Files 113 115 +2
Lines 6684 6863 +179
===========================================
+ Hits 4267 4452 +185
+ Misses 2133 2127 -6
Partials 284 284 |
| 1. Agree on software upgrade plan via governance (currently via a plaintext proposal). | ||
| 2. Once the upgrade plan is accepted, validators each update their software according to proposal from step 1. | ||
| 3. Each upgraded validator submits a transaction to the SoftwareUpgradeKeeper. | ||
| 4. Once a sufficient quorum of validators has completed step 3, new logic kicks |
There was a problem hiding this comment.
what is "sufficient" here? kind of ambiguous should probably have a number of reference to a governance param or something.
There was a problem hiding this comment.
+1. Something not too close to 2/3rd so we don't risk halting on switch if a couple of validators go off
There was a problem hiding this comment.
Agreed, also we should clarify a bit how the new logic kicks in - conditionals in the code, for now?
| should be heeded instead of the process declared here. | ||
| ``` | ||
|
|
||
| It is recommended that all links point to Github for now, for accountability. |
There was a problem hiding this comment.
What about the cosmos forum? - Is the idea that discussion moves from the forum then onto github for the implementation discussion?
There was a problem hiding this comment.
I think this is just referring to code links (@jaekwon)?
There was a problem hiding this comment.
code links, signatures, everything but discussions.
|
|
||
| Once the proposal from step 1 has been accepted, validators should upgrade | ||
| their software. For the standard upgrade procedure, it is OK for non-validator | ||
| full nodes to upgrade as well. |
There was a problem hiding this comment.
isn't it required that full nodes upgrade as well? - I mean if they don't upgrade they just risk not being able to run transactions
There was a problem hiding this comment.
Depends on the upgrade (e.g. with an upgrade which strictly reduces the set of valid transactions, they don't really need to upgrade).
In most cases they'll need to, the node should warn the user.
| ### Step 3: Update the SoftwareUpgradeKeeper | ||
|
|
||
| The new version of the software should automatically include a (single) signed | ||
| message to the SoftwareUpgradeKeeper w/ the new version of software that the |
|
|
||
| ## Procedure: | ||
|
|
||
| * Deadline: By 2 weeks from after the proposal has been accepted. |
There was a problem hiding this comment.
Should we have some slashing logic for validators that did not submit the message to the SoftwareUpgradeKeeper in case the quorum isn't met?
There was a problem hiding this comment.
I think this could be a good idea.
There was a problem hiding this comment.
Slashing would only occur if 1/3 < number of validator that signalled < quorum when deadline is met, i.e. we don't slash if less than a third of validators signalled. cf #2116 (comment)
| 1. Agree on software upgrade plan via governance (currently via a plaintext proposal). | ||
| 2. Once the upgrade plan is accepted, validators each update their software according to proposal from step 1. | ||
| 3. Each upgraded validator submits a transaction to the SoftwareUpgradeKeeper. | ||
| 4. Once a sufficient quorum of validators has completed step 3, new logic kicks |
There was a problem hiding this comment.
+1. Something not too close to 2/3rd so we don't risk halting on switch if a couple of validators go off
| SOFTWARE_UPGRADE_[XXX]_ENABLED=true, where XXX is the commit hash of the new | ||
| software. | ||
|
|
||
| ### Contingincies |
| The new version of the software should automatically include a (single) signed | ||
| message to the SoftwareUpgradeKeeper w/ the new version of software that the | ||
| validator is running. Even if a validator were to change their software | ||
| version multiple times, the SoftwareUpgradeKeeper ensures that nothing happens |
There was a problem hiding this comment.
I guess the keeper also ensures that validator can only signal a single version to a keeper? (1 to 1 map)
| already taken place elsewhere. To gauge interest in a proposed upgrade, | ||
| users should instead submit a survey proposal with the following template: | ||
|
|
||
| Notethat the `Procedure` section has already been filled out. Do not change |
There was a problem hiding this comment.
Confusing to place that here ("following template:" should be immediately followed by said template)
| should be heeded instead of the process declared here. | ||
| ``` | ||
|
|
||
| It is recommended that all links point to Github for now, for accountability. |
There was a problem hiding this comment.
I think this is just referring to code links (@jaekwon)?
|
|
||
| Once the proposal from step 1 has been accepted, validators should upgrade | ||
| their software. For the standard upgrade procedure, it is OK for non-validator | ||
| full nodes to upgrade as well. |
There was a problem hiding this comment.
Depends on the upgrade (e.g. with an upgrade which strictly reduces the set of valid transactions, they don't really need to upgrade).
In most cases they'll need to, the node should warn the user.
|
|
||
| ## Conflict Resolution | ||
|
|
||
| In case there are conflicting proposals, the most recent proposal to get |
There was a problem hiding this comment.
To get accepted or to pass the SoftwareUpgradeKeeper quorum requirement? What if two proposals are accepted but the latter passes the quorum first?
There was a problem hiding this comment.
We could have a second accepted proposal reset the SoftwareUpgradeKeeper, useful if a proposal is passed and validators start upgrading but discover an issue.
There was a problem hiding this comment.
Oh! This gives me an idea.
Maybe we don't need a second proposal, but just the ability for individual validators to reset/cancel their entry in the keeper's map. A proposal won't pass if the quorum is not met. The only problem then is slashing, but we could have the slashing condition require a minimum quorum. Like if less than a third of validators signal, noone gets slashed.
Only if 1/3 < number of validators that signalled < Switch-triggering quorum and deadline is met do we slash validators that did not signalled.
| until a quorum (SOFTWARE_UPGRADE_QUORUM) are on an identical version of the | ||
| software. | ||
|
|
||
| Only validators can vote here, as delegators do not actually run the validation |
There was a problem hiding this comment.
But with their delegators' stake, presumably, which could be redelegated? How are these votes tallied?
|
|
||
| ## Procedure: | ||
|
|
||
| * Deadline: By 2 weeks from after the proposal has been accepted. |
There was a problem hiding this comment.
I think this could be a good idea.
| 1. Agree on software upgrade plan via governance (currently via a plaintext proposal). | ||
| 2. Once the upgrade plan is accepted, validators each update their software according to proposal from step 1. | ||
| 3. Each upgraded validator submits a transaction to the SoftwareUpgradeKeeper. | ||
| 4. Once a sufficient quorum of validators has completed step 3, new logic kicks |
There was a problem hiding this comment.
Agreed, also we should clarify a bit how the new logic kicks in - conditionals in the code, for now?
| * Audited by: | ||
| - <name/handle> on <date>; report: https://github.com/... | ||
| - <name/handle> on <date>; report: https://github.com/... | ||
|
|
There was a problem hiding this comment.
Can we include a suggestion for signed authorship / endorsement, using ICS 030?
|
|
||
| - [ ] Any new hardware requirements? | ||
| - [ ] Any new bandwidth requirements? | ||
| - [ ] Any dependencies on other proposals? |
There was a problem hiding this comment.
Can these be handled by the SoftwareUpgradeKeeper or are these just "suggested dependencies"?
|
I don't think we should indicate the specific commit hash indicated on-chain, this ties us down to AIB's implementation being canonical. If we really want commit-hash, I think we should prefix it by "implementation name" then. But also there are other non-breaking changes I may want to include, but that would yield a different commit hash. Then I have to either lie, or not take in the non-breaking changes. Valid non-breaking changes include switching to your own fork of tendermint so you can upgrade your mempool, or change something about the peer rating mechanism. |
| * Deadline: By 2 weeks from after the proposal has been accepted. | ||
|
|
||
| Once this proposal has been accepted, validators should upgrade to the new | ||
| software as described above by the deadline. Once the new software is |
There was a problem hiding this comment.
A question here, how can we ensure that the version of software that upgraded to is identical to the version proposed because the proposal is not structured.
|
|
||
| Once this proposal has been accepted, validators should upgrade to the new | ||
| software as described above by the deadline. Once the new software is | ||
| deployed, the new software will automaticaly submit a message to the |
There was a problem hiding this comment.
don't really understand this part - presumably the validator operator keys arent on the machine running the software and dont interact direct with the gaiad they just upgraded. so not clear to me how this should be automatic.
It does make sense that validators should publish something that indicates they upgraded. I'm wondering:
- what is our appetite for not having this extra trigger for now, just letting folks download the new software and have it do a switchover at some height
- shouldn't we do this extra confirmation step through some other means, like including it in votes or proposals, rather than this app-level extra tx?
This spec proposal requires a new SoftwareUpgradeKeeper because the mechanics of that keeper should be different than the governance keeper.
For example, only validators can "vote" (though it should be automatic), and the acceptable values are not yes/no/abstain/veto, but the specific commit hash that the validator already upgraded to.