Skip to content

[Cirrus] Add IsExpired status to Poll#707

Merged
fassadlr merged 5 commits intostratisproject:release/1.1.0.0from
quantumagi:expirepolls
Sep 20, 2021
Merged

[Cirrus] Add IsExpired status to Poll#707
fassadlr merged 5 commits intostratisproject:release/1.1.0.0from
quantumagi:expirepolls

Conversation

@quantumagi
Copy link
Contributor

@quantumagi quantumagi commented Sep 15, 2021

@quantumagi quantumagi requested a review from fassadlr September 15, 2021 05:59
@quantumagi quantumagi changed the title Add IsExpired status to Poll [Cirrus] Add IsExpired status to Poll Sep 15, 2021
@quantumagi quantumagi marked this pull request as draft September 15, 2021 13:09
@quantumagi quantumagi marked this pull request as ready for review September 16, 2021 08:06
Copy link
Contributor

@fassadlr fassadlr left a comment

Choose a reason for hiding this comment

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

@quantumagi I'm mostly good with this, however, do we need to do this via BIP9 as opposed to an activation height? All masternodes are required to upgrade when we upgrade the SidechainMasternode branch, so it is kinda something we control. In other words, if we push to that branch and they have to restart their MN due to an windows update, they are forced to upgrade.

Perhaps an activation height is just easier?

@quantumagi
Copy link
Contributor Author

quantumagi commented Sep 17, 2021

@fassadlr , The BIP9 activation would just make the upgrade a bit smoother. Otherwise, if only a few nodes upgrade straight away then they will be faced with errors due to federations that differ from what is expected by the non-upgraded majority. If we use BIP9 then it will eventually only be the non-upgraded 10% that will face errors that will require them to upgrade as well. That said, it may still depend a lot on how the upgrade is managed. WDYT?

BTW we could add the activation height stuff as a separate PR and remove a few files from this one. See #713.

@fassadlr
Copy link
Contributor

@fassadlr , The BIP9 activation would just make the upgrade a bit smoother. Otherwise, if only a few nodes upgrade straight away then they will be faced with errors due to federations that differ from what is expected by the non-upgraded majority. If we use BIP9 then it will eventually only be the non-upgraded 10% that will face errors that will require them to upgrade as well. That said, it may still depend a lot on how the upgrade is managed. WDYT?

BTW we could add the activation height stuff as a separate PR and remove a few files from this one. See #713.

The masternode users are generally very active with regards to keeping their nodes upgraded and like I said, we control the upgrade by indirectly forcing them to upgrade when they restart their node. This can be due to Windows updates etc. If we set an activation height 2 weeks from release and announce in the channel daily, 99% of the guys will be upgraded by the time activation happens. I know that the other guys prefer activation heights as well, so perhaps lets move it to that? WDYT?

@fassadlr
Copy link
Contributor

Otherwise, the PR looks 👍

@quantumagi
Copy link
Contributor Author

@fassadlr , Sounds good. I've made the change.

Copy link
Contributor

@fassadlr fassadlr left a comment

Choose a reason for hiding this comment

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

We just need to set the activation heights and agree on the poll expiry period 👍

@fassadlr fassadlr merged commit c6ae67a into stratisproject:release/1.1.0.0 Sep 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants