Skip to content
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

Topdown Enhancement: Adding validator quorum certificate to topdown gossip finality #1066

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

cryptoAtwill
Copy link
Contributor

@cryptoAtwill cryptoAtwill commented Jul 15, 2024

In this series of PRs, an attempt is made to:

  • Adding validator signature to the gossip payload. This also includes updating the trait interfaces and other side effects (This PR). Previously the ipld-resolver forces any message gossiped to be signed by the a secret key, most likely being the validator key. This PR removes that restriction, any message can be published as long as it's validated by the libp2p peer secret key. Signature requirement is only enforced by application when needed. This makes way for future simplification with basic gossip pubsub from libp2p instead of existing ipld-resolver, which is under-utilised.
  • Collect quorum of signatures and forms a quorum certificate (This PR).
  • Add quorum signature into the topdown finality proposal, Topdown Enhancement: Add quorum cert in "prepare" and "deliver" #1068.

This PR also commented off certain parts of the code base so that at least the code compiles. They will be updated in coming PRs.

@@ -148,8 +156,8 @@ where
pub fn add_block(
&self,
block_height: BlockHeight,
block_hash: Option<V>,
) -> StmResult<(), Error<K>> {
payload: Option<TopdownVote>,
Copy link
Contributor Author

@cryptoAtwill cryptoAtwill Jul 18, 2024

Choose a reason for hiding this comment

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

I dont think add_block is ever needed, vote tally should directly get the data from syncer cache instead of storing its own data. Since syncer cache will be persisted, it will be much more deterministic then something in memory. Removing this now will break tons of code and especially test cases. Mark it as todo maybe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

Successfully merging this pull request may close these issues.

1 participant