-
Notifications
You must be signed in to change notification settings - Fork 2
[PoC] Network Upgrades PoC (Part 1) #611
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
Conversation
1f07d2f to
0e9a1ba
Compare
|
Unit test coverage:
|
1bab3f0 to
9fb7815
Compare
rwlockbg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great PR - so far this is the state machine logic basically and the underlying mechanism for accepting an upgrade. I presume next PRs will be actual integration. Left some comments, but overall looking nice and nice tests too!
|
love the test fixture and test helper methods!! 🔥 🔥 🔥 putting my approval but little nit: do prefer @rwlockbg suggestion below |
|
@lamafab Do you mind squashing relevant commits (typos, lint, trigger CI). Ideally these changes are made into the commits that introduce them |
|
@0xBEEFCAF3
I'll probably need to rewrite the entire history anyway after following up on Satwiks' recommendations - I"ll cherry-pick all changes and label them accordingly, then force-push. ETA EOD. |
2804471 to
bebf0ac
Compare
|
Regarding: https://github.com/botanix-labs/Macbeth/actions/runs/13976740327/job/39132293647?pr=611 I didn't do any changes in this area, and when running the command locally all tests pass: I'm just going to re-run. |
rwlockbg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am approving it but you need to get hte pipelines green.
|
Btw. I'm also getting the same failure @lamafab https://github.com/botanix-labs/Macbeth/actions/runs/13982320523/job/39149966590?pr=620 |
|
I just renamed a some fields in |
scottmillner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i was able to get all the int tests to pass locally
so when there's another approval happy for it to go in
Botanix Network Upgrade Specification(This spec has been revised with Claude.AI) 1. IntroductionThis document outlines the specification for the Network Upgrade mechanism in the Botanix ecosystem. This mechanism enables coordinated updates to the consensus rules and network protocol while preserving blockchain safety and minimizing the risk of chain splits. 2. Terminology
3. Components3.1 ActivationManagerThe 3.2 Network Upgrade PayloadEach block proposal MAY include a
4. Configuration4.1 ConstantsThe system defines the following global constants:
These global constants ensure network security by requiring a supermajority consensus across a minimum number of validators before any upgrade can proceed, while maintaining a reasonable voting window. 4.2 Default ConfigurationBy default and for the majority of a node's lifetime, the
This configuration:
4.3 Upgrade SignalingWhen a community-driven upgrade proposal is being discussed, validators MAY signal their intentions by configuring the
Key characteristics:
4.4 Upgrade AcceptanceIf an upgrade proposal reaches community consensus and is scheduled for activation, the maintainers MUST release a new Botanix node version with the
Key characteristics:
Validators MUST update to this new version before all upgrade conditions are met to participate in the coordinated upgrade process. 5. Upgrade ConditionsBlocks with the upgraded version MUST only be proposed and backed if ALL of the following are true:
5.1 Approval Rate CalculationThe
By requiring both approval rates to meet quorum, we ensure upgrades only proceed when there is both sufficient community support AND operational readiness. This allows validators to signal support for an upgrade they aren't yet ready to implement, or to prepare for an upgrade they oppose (but will follow if passed by the community). For the calculation, the following values must be retrieved: The exact formula for calculating the aye approval rate is: The exact formula for calculating the compliance approval rate is: The quorum is considered met if and only if:
The ceiling function 6. Process Flow for Compliant ValidatorsThis process flow describe how 6.2 Block Preparation
6.3 Block Validation
6.4 Block Finalization
6.4.1 Syncing ConsiderationsThe
This separation of responsibilities ensures that nodes can reliably sync the canonical chain while maintaining the ability to reject future unwanted upgrades. |
This PR introduces a standalone
ActivationManagerwhich implements our proposed Network Upgrade specification.Early feedback on the specification and enforcement logic is appreciated.
EDIT (2025-03-20): Comments and recommendations have been addressed, ready to merge!
Changes:
thresholdtoapproval_rateacceptingtois_compliantRuntimeVersion { MajorVersion(u16), MinorVersion(u16) }min_validator_countindependently, it's now part of calculation itself. Respectively:This means that if we for example require at least three votes and we have one Aye vote, we calculate
approval_vote = 34and can just checkapproval_vote >= 67- instead of having to doapproval_vote >= 67 && votes_len >= 3.EDIT (2025-03-17): PR is out of Draft and now ready for review!
Unit test coverage -> #611 (comment)
EDIT (2025-03-14): Completed comprehensive unit test suite:
Migrated and expanded all test cases using the improved testing structures! I'll add more documentation and do some final cleanups, and then we're done. I will convert this from a Draft to a PR when appropriate.