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

PoS Validator Management #793

Conversation

AeonSw4n
Copy link
Contributor

@AeonSw4n AeonSw4n commented Nov 6, 2023

No description provided.

@AeonSw4n AeonSw4n requested a review from a team as a code owner November 6, 2023 11:45
@AeonSw4n
Copy link
Contributor Author

AeonSw4n commented Nov 6, 2023

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

@AeonSw4n AeonSw4n force-pushed the p/pos-validator-connection-management branch from d127346 to 819ca19 Compare November 6, 2023 12:17
)

type ConsensusController struct {
lock sync.RWMutex
fastHotStuffEventLoop *consensus.FastHotStuffEventLoop
blockchain *Blockchain
server *Server

handshake *HandshakeController
Copy link
Contributor

Choose a reason for hiding this comment

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

All of the changes in this file are related to peer management for the validator set, and related to maintaining connections between nodes. The changes introduced by this PR are not directly related to maintaining state of the blockchain or state of the Fast-HotStuff event loop based on internal events (block proposal, vote, timeout) and external messages from peers (blocks, vote, timeouts)

IMO, we should keep this ConsensusController scoped to just maintaining state blockchain and state of the Fast-HotStuff event loop based on the above events and and messages.

IMO, it's clearer to move all of the peer management, validator handshake, and validator connection maintenance from this file to a separate file, ex: pos_validator_management.go that's scoped to just these three items. The loop to re-sync the validator set may live in server or in the new file. IMO, this the ConsensusController is not the best place to house it though.

@AeonSw4n AeonSw4n force-pushed the p/pos-validator-handshake branch from 8253a2b to 222d970 Compare November 8, 2023 03:06
@AeonSw4n AeonSw4n force-pushed the p/pos-validator-connection-management branch from 819ca19 to dac5441 Compare November 8, 2023 03:06
@AeonSw4n AeonSw4n mentioned this pull request Nov 8, 2023
@AeonSw4n AeonSw4n force-pushed the p/pos-validator-handshake branch from 222d970 to 19a5515 Compare November 22, 2023 11:10
@AeonSw4n AeonSw4n force-pushed the p/pos-validator-connection-management branch from dac5441 to 725aa2d Compare November 22, 2023 11:10
@AeonSw4n AeonSw4n force-pushed the p/pos-validator-handshake branch from 19a5515 to 08faef7 Compare November 27, 2023 16:32
@AeonSw4n AeonSw4n force-pushed the p/pos-validator-connection-management branch from 725aa2d to 96daf1b Compare November 27, 2023 16:32
This reverts commit e44c26f.
@AeonSw4n AeonSw4n force-pushed the p/pos-validator-handshake branch from 08faef7 to 8fc6254 Compare December 7, 2023 13:29
@AeonSw4n AeonSw4n force-pushed the p/pos-validator-connection-management branch from 7b4ad8e to 0df796e Compare December 7, 2023 13:29
@AeonSw4n AeonSw4n closed this Dec 7, 2023
@lazynina lazynina deleted the p/pos-validator-connection-management branch April 9, 2024 20:18
Sign up for free to 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.

2 participants