Skip to content

Spec for staking module differences - #1220

Closed
S1nus wants to merge 7 commits into
mainfrom
connor/staking-module-spec
Closed

Spec for staking module differences#1220
S1nus wants to merge 7 commits into
mainfrom
connor/staking-module-spec

Conversation

@S1nus

@S1nusS1nus commented Oct 2, 2023

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

github-actionsBot commented Oct 2, 2023

Copy link
Copy Markdown
Contributor
PR Preview Action v1.4.4
🚀 Deployed preview to https://rollkit.github.io/rollkit/pr-preview/pr-1220/
on branch gh-pages at 2023-10-02 21:51 UTC

@codecov

codecovBot commented Oct 2, 2023

Copy link
Copy Markdown

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (eb7bdc0) 56.08% compared to head (513f44d) 56.07%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #1220 +/- ##
==========================================
- Coverage 56.08% 56.07% -0.02% 
==========================================
Files 63 63 Lines 6820 6820 ==========================================
- Hits 3825 3824 -1 - Misses 2606 2607 +1 
Partials 389 389 

see 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@S1nus
S1nus marked this pull request as ready for review October 2, 2023 22:31

Rollkit will support customizable functionality for verifying the correctness of a block proposer. However, it currently works by delegating this functionality to the Application, by checking that the `AggregatorsHash` of a header corresponds to the `NextAggregatorsHash` of the previous, where setting the `NextAggregtorsHash` is the responsibility of the ABCI Application.

## Assumptions and Considerations

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should include here our assumptions around how tendermint staking works.
This would allow auditors to better verify our assumptions.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

"tendermint staking" does not exist. the ABCI App simply sets the validator set

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, we use the staking module for that.

@nashqueuenashqueue left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This spec aims to check how we use the staking module vs. how it is used in tendermint. Please elaborate on that.


Rollkit will support customizable functionality for verifying the correctness of a block proposer. However, it currently works by delegating this functionality to the Application, by checking that the `AggregatorsHash` of a header corresponds to the `NextAggregatorsHash` of the previous, where setting the `NextAggregtorsHash` is the responsibility of the ABCI Application.

## Assumptions and Considerations

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, we use the staking module for that.

@S1nus

S1nus commented Oct 4, 2023

Copy link
Copy Markdown
ContributorAuthor

This spec aims to check how we use the staking module vs. how it is used in tendermint. Please elaborate on that.

don't we use it in exactly the same way?

@nashqueue

Copy link
Copy Markdown
Contributor

This spec aims to check how we use the staking module vs. how it is used in tendermint. Please elaborate on that.

don't we use it in exactly the same way?

The point of this spec is to exactly figure this out and to make sure to capture the nuances if there are differences.

@Manav-Aggarwal
Manav-Aggarwal marked this pull request as draft November 8, 2023 14:38
@S1nus

S1nus commented Nov 14, 2023

Copy link
Copy Markdown
ContributorAuthor

As of #1291, we decided that Rollkit vA will control the proposer entirely, ignoring all validator set updates from the ABCI app.

This is being implemented in the Header.Verify(...) method, where it enforces that the ProposerAddress must be equal to the ProposerAddress of the previous header. If implemented properly, this ensures the entire chain is signed by the same person (the centralized sequencer).

@S1nusS1nus closed this Nov 14, 2023
github-merge-queueBot pushed a commit that referenced this pull request Nov 20, 2023
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>
gupadhyaya pushed a commit that referenced this pull request Nov 24, 2023
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>
@tac0turtle
tac0turtle deleted the connor/staking-module-spec branch March 20, 2025 11:57
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@S1nus@nashqueue@MSevey