Skip to content

[Merged by Bors] - Refactor Peerdb and PeerManager#2660

Closed
AgeManning wants to merge 21 commits intounstablefrom
disconnected-peer-status
Closed

[Merged by Bors] - Refactor Peerdb and PeerManager#2660
AgeManning wants to merge 21 commits intounstablefrom
disconnected-peer-status

Conversation

@AgeManning
Copy link
Member

Proposed Changes

This is a refactor of the PeerDB and PeerManager. A number of bugs have been surfacing around the connection state of peers and their interaction with the score state.

This refactor tightens the mutability properties of peers such that only specific modules are able to modify the state of peer information preventing inadvertant state changes that can lead to our local peer manager db being out of sync with libp2p.

Further, the logic around connection and scoring was quite convoluted and the distinction between the PeerManager and Peerdb was not well defined. Although these issues are not fully resolved, this PR is step to cleaning up this logic. The peerdb solely manages most mutability operations of peers leaving high-order logic to the peer manager.

A single update_connection_state() function has been added to the peer-db making it solely responsible for modifying the peer's connection state. The way the peer's scores can be modified have been reduced to three simple functions (update_scores(), update_gossipsub_scores() and report_peer()). This prevents any add-hoc modifications of scores and only natural processes of score modification is allowed which simplifies the reasoning of score and state changes.

@AgeManning AgeManning added the ready-for-review The code is ready for review label Oct 1, 2021
Copy link
Contributor

@divagant-martian divagant-martian left a comment

Choose a reason for hiding this comment

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

sneaking back in, LGTM. I added a couple minimal suggestions

@AgeManning
Copy link
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Oct 10, 2021
## Proposed Changes

This is a refactor of the PeerDB and PeerManager. A number of bugs have been surfacing around the connection state of peers and their interaction with the score state. 

This refactor tightens the mutability properties of peers such that only specific modules are able to modify the state of peer information preventing inadvertant state changes that can lead to our local peer manager db being out of sync with libp2p. 

Further, the logic around connection and scoring was quite convoluted and the distinction between the PeerManager and Peerdb was not well defined. Although these issues are not fully resolved, this PR is step to cleaning up this logic. The peerdb solely manages most mutability operations of peers leaving high-order logic to the peer manager. 

A single `update_connection_state()` function has been added to the peer-db making it solely responsible for modifying the peer's connection state. The way the peer's scores can be modified have been reduced to three simple functions (`update_scores()`, `update_gossipsub_scores()` and `report_peer()`). This prevents any add-hoc modifications of scores and only natural processes of score modification is allowed which simplifies the reasoning of score and state changes.
@bors
Copy link

bors bot commented Oct 10, 2021

Build failed:

@AgeManning
Copy link
Member Author

bors r+

@bors
Copy link

bors bot commented Oct 11, 2021

🕐 Waiting for PR status (Github check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

bors bot pushed a commit that referenced this pull request Oct 11, 2021
## Proposed Changes

This is a refactor of the PeerDB and PeerManager. A number of bugs have been surfacing around the connection state of peers and their interaction with the score state. 

This refactor tightens the mutability properties of peers such that only specific modules are able to modify the state of peer information preventing inadvertant state changes that can lead to our local peer manager db being out of sync with libp2p. 

Further, the logic around connection and scoring was quite convoluted and the distinction between the PeerManager and Peerdb was not well defined. Although these issues are not fully resolved, this PR is step to cleaning up this logic. The peerdb solely manages most mutability operations of peers leaving high-order logic to the peer manager. 

A single `update_connection_state()` function has been added to the peer-db making it solely responsible for modifying the peer's connection state. The way the peer's scores can be modified have been reduced to three simple functions (`update_scores()`, `update_gossipsub_scores()` and `report_peer()`). This prevents any add-hoc modifications of scores and only natural processes of score modification is allowed which simplifies the reasoning of score and state changes.
@bors
Copy link

bors bot commented Oct 11, 2021

@bors bors bot changed the title Refactor Peerdb and PeerManager [Merged by Bors] - Refactor Peerdb and PeerManager Oct 11, 2021
@bors bors bot closed this Oct 11, 2021
@AgeManning AgeManning deleted the disconnected-peer-status branch October 12, 2021 05:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-review The code is ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants