-
Notifications
You must be signed in to change notification settings - Fork 247
add staking module differences to block_spec #1335
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
## Walkthrough
The recent updates centralize the block validation process by requiring a single valid signature from a centralized sequencer for block commits. The validator set is now fixed to one, with the removal of aggregator hashes from the block header. Additionally, the block manager now supports out-of-order rollup blocks and includes a termination condition for sequencer double-signing.
## Changes
| Files | Change Summary |
| --- | --- |
| `types/block_spec.md` | Centralized sequencer signature required for `Commit`, `Validators` length set to 1, removed `AggregatorsHash` and `NextAggregatorsHash`. |
| `block/block-manager.md` | Added support for out-of-order rollup blocks and termination condition for sequencer double-signing. |
> 🐇 In the code's gentle flow, a single signature now will show,
> As winter's chill begins to bite, our blocks align with sequenced light. 🌟✨TipsChat with CodeRabbit Bot (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1335 +/- ##
=======================================
Coverage 53.46% 53.46%
=======================================
Files 49 49
Lines 6197 6197
=======================================
Hits 3313 3313
Misses 2573 2573
Partials 311 311 ☔ View full report in Codecov by Sentry. |
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- types/block_spec.md (2 hunks)
Additional comments: 3
types/block_spec.md (3)
87-91: The changes to the
CommitandValidatorsfields reflect a significant shift towards centralization. Ensure that these changes are intentional and align with the overall system design and security model. This centralization could introduce single points of failure and reduce the robustness of the system against attacks or downtime of the central sequencer.94-94: The removal of
AggregatorsHashandNextAggregatorsHashfields simplifies the header structure. Verify that this change is compatible with the rest of the system and that any logic depending on these fields is appropriately updated or removed.108-110: Ensure that the
ProposerAddressvalidation logic is updated to reflect the new centralization changes, as it should now only accept the centralized sequencer's address as valid.
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- block/out-of-order-blocks.png
- block/termination.png
Files selected for processing (1)
- block/block-manager.md (1 hunks)
Files skipped from review due to trivial changes (1)
- block/block-manager.md
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- types/block_spec.md (2 hunks)
Additional comments: 3
types/block_spec.md (3)
87-91: The documentation clearly states the new requirements for the
CommitandValidatorsfields. TheCommitmust have one valid signature from the centralized sequencer, and theValidatorsarray must have a length of exactly one. This reflects the shift to a centralized staking model. It's important to ensure that the codebase reflects these changes and that any logic that previously handled multiple validators is updated accordingly.95-95: The removal of
AggregatorsHashandNextAggregatorsHashfields from theHeaderstruct simplifies the block header structure. The note indicates that Rollkit version A should not process validator set updates from the ABCI application and should maintain the centralized sequencer as the sole validator. This is a significant change that should be clearly communicated to developers and operators to avoid confusion.109-111: The
ProposerAddressfield validation is mentioned to be checked in theVerify()step. Given the move to a centralized sequencer model, it's crucial that theProposerAddresscorresponds to the centralized sequencer's address. Any deviation from this should be flagged as a critical error, as it would indicate a block proposal from an unauthorized entity.
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- block/block-manager.md (1 hunks)
Files skipped from review due to trivial changes (1)
- block/block-manager.md
closes #1220 Instead of #1220, we'll just fix and update the block_spec. Also adds the termination condition, and processing out-of-order rollup blocks from DA. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced a requirement for blocks to have a valid signature from the centralized sequencer. - Implemented support for out-of-order rollup blocks on the Data Availability network. - Added a termination condition for sequencer double-signing scenarios. - **Documentation** - Updated block specification documentation to reflect new validation rules. - Added diagrams to illustrate out-of-order block handling and termination conditions. - **Refactor** - Removed `AggregatorsHash` and `NextAggregatorsHash` fields from block headers to streamline the validation process. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Connor O'Hara <connor@switchboard.xyz>
closes #1220
Instead of #1220, we'll just fix and update the block_spec.
Also adds the termination condition, and processing out-of-order rollup blocks from DA.
Summary by CodeRabbit
New Features
Documentation
Refactor
AggregatorsHashandNextAggregatorsHashfields from block headers to streamline the validation process.