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.

Sign up for GitHub

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

Jump to bottom

peer_state: Robust state machine transitions #251

Merged
merged 46 commits into from
Oct 28, 2024
Merged

peer_state: Robust state machine transitions #251

merged 46 commits into from
Oct 28, 2024

Conversation

lexnv
Copy link
Collaborator

@lexnv lexnv commented Sep 24, 2024

This PR refactors the peer state. Previously the AddressStore and PeerState where intertwined. The AddressStore contained an optional ConnectionID, while the PeerState states contained a mandatory score.

This PR separates the address store and the peer state, while moving the logic of the peer state transitioning to a separate module. While at it, have added documentation and testing for state machine transitions.

Changes include:

  • Secondary connection from the PeerContext is merged into the PeerState
  • Connection Ids are removed from the Address Store
  • Transport manager is refactored to use the new state machine, this keeps a minimal code around in the manager

Testing Done

  • added extra tests to the peer state
  • tested with subp2p-explorer for discovering kusama

This builds upon #250 for a bigger refactor effort to track addresses in a healthier

Aims at improving:

lexnv added 27 commits September 24, 2024 13:23
@lexnv
address: Add score constants to the address store
59dbbf8 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
address: Convert DialError to appropriate score
ae7799a 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
address: Implement eviction algorithm for the AddressStore
ac5d731 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
manager: Keep track of the address for future dials
a2356ec 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
manager: Make add_known_address more robust to track multiple peer addrs
5355e85 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
manager: Track addresses better for dial and open failure
ecb436b 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
manager: Update addresses on connection established
bf066d1 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
manager/handle: Keep lock for shorter time
b052c5d 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
address/tests: Adjust testing
da52056 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
address/tests: Add tests for eviction and insertion
6c7f399 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
manager/tests: Adjust testing to new address store interface
5b88ea4 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
peer_state: Introduce connection record without address score
d575812 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
peer_state: Refactor peerstate for minimal state machine
6494256 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
peer_state: Implement peer state transitions isolated from manager
7029015 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
peer_state/tests: Add tests for peerstate transitions
3f4cdc9 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
address_store: Remove connection ID from the store
6bf358b 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
types: Remove secondary connection ID and use new peerstate
c9205df 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
manager: Handle opening addresses generically
25a5c5e 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
manager: Refactor dialing on multiple addresses
a792d0e 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
manager: Refactor dial addresses
c7c5ed4 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
manager: Refactor on dial failure
b137d0a 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
manager: Refacotr on connection closed
f4d46a9 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
manager: Refactor on connection established
a795fd4 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
manager: Refactor on connection opened
7c8242b 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
manager: Refactor on open failure
f7e6d88 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
manager/tests: Adjust testing to the new interface
10cb1ed 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
manager/handle: Use new peerstate interface
728c563 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv lexnv added the enhancement New feature or request label Sep 24, 2024
@lexnv lexnv self-assigned this Sep 24, 2024
@lexnv lexnv mentioned this pull request Oct 2, 2024
Closed
dmitry-markin
dmitry-markin approved these changes Oct 22, 2024
View reviewed changes
Copy link
Collaborator

@dmitry-markin dmitry-markin left a comment

Choose a reason for hiding this comment

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

With the state machine extracted into PeerState the code indeed easier to comprehend 🎉

I will also need to review #250 before you are able to merge both PRs into master.

src/transport/manager/peer_state.rs Outdated Show resolved Hide resolved
src/transport/manager/peer_state.rs Outdated Show resolved Hide resolved
src/transport/manager/peer_state.rs Outdated Show resolved Hide resolved
src/transport/manager/peer_state.rs Outdated Show resolved Hide resolved
src/transport/manager/peer_state.rs Outdated Show resolved Hide resolved
src/transport/manager/mod.rs Outdated Show resolved Hide resolved
src/transport/manager/mod.rs Show resolved Hide resolved
src/transport/manager/mod.rs Outdated Show resolved Hide resolved
src/transport/manager/mod.rs Show resolved Hide resolved
src/transport/manager/mod.rs Outdated Show resolved Hide resolved
Base automatically changed from lexnv/enhanve-address-store to master October 25, 2024 14:03
lexnv and others added 18 commits October 25, 2024 17:04
@lexnv @dmitry-markin
Update src/transport/manager/peer_state.rs
450c109 
Co-authored-by: Dmitry Markin <[email protected]>
@lexnv @dmitry-markin
Update src/transport/manager/peer_state.rs
16e1294 
Co-authored-by: Dmitry Markin <[email protected]>
@lexnv @dmitry-markin
Update src/transport/manager/peer_state.rs
ebe8853 
Co-authored-by: Dmitry Markin <[email protected]>
@lexnv @dmitry-markin
Update src/transport/manager/peer_state.rs
2a33ab5 
Co-authored-by: Dmitry Markin <[email protected]>
@lexnv @dmitry-markin
Update src/transport/manager/peer_state.rs
caaa3f1 
Co-authored-by: Dmitry Markin <[email protected]>
@lexnv @dmitry-markin
Update src/transport/manager/peer_state.rs
f2b4285 
Co-authored-by: Dmitry Markin <[email protected]>
@lexnv @dmitry-markin
Update src/transport/manager/peer_state.rs
4614c7d 
Co-authored-by: Dmitry Markin <[email protected]>
@lexnv @dmitry-markin
Update src/transport/manager/mod.rs
c8180ab 
Co-authored-by: Dmitry Markin <[email protected]>
@lexnv
peer_state: Fix clippy
2105dd2 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
peer_state: Address other possible states
896c47b 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
peer_state: Warn if connection opened from other sources
4a09f3f 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
peerstate: Add warns when opening -> connected
8fa12bb 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
peer_state: Warn about peerId mismatch
22406c6 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
manager: Replace assert with warn
764b769 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
Merge remote-tracking branch 'origin/master' into lexnv/improve-peer-…
14d7f0f 
…state

Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
peerstate: Downgrade warn to trace
b7ba361 
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
manager: Rename Self::open_addresses to
6e5868e 
`supported_transports_addresses`

Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv
manager: Link todo issue
c2ee9b8 
Signed-off-by: Alexandru Vasile <[email protected]>
dmitry-markin
dmitry-markin approved these changes Oct 28, 2024
View reviewed changes
src/transport/manager/peer_state.rs Outdated
Comment on lines 464 to 466
address.with(Protocol::P2p(
Multihash::from_bytes(&peer.to_bytes()).expect("valid peer id"),
))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
address.with(Protocol::P2p(
Multihash::from_bytes(&peer.to_bytes()).expect("valid peer id"),
))
address.with(Protocol::P2p(peer.as_ref()))

src/transport/manager/peer_state.rs Show resolved Hide resolved
@lexnv
Copy link
Collaborator Author

lexnv commented Oct 28, 2024

Double checked with subp2p-explorer bench-cli, everything looks good, thanks for the review 🙏

@lexnv lexnv merged commit f112b59 into master Oct 28, 2024
8 checks passed
@lexnv lexnv deleted the lexnv/improve-peer-state branch October 28, 2024 17:06
@lexnv lexnv mentioned this pull request Nov 1, 2024
Merged
lexnv added a commit that referenced this pull request Nov 4, 2024
@lexnv @dmitry-markin
chore: Release litep2p version 0.8.0 (#280)
af0535c 
## [0.8.0] - 2024-11-01

This release adds support for content provider advertisement and
discovery to Kademlia protocol implementation (see libp2p
[spec](https://github.com/libp2p/specs/blob/master/kad-dht/README.md#content-provider-advertisement-and-discovery)).
Additionally, the release includes several improvements and memory leak
fixes to enhance the stability and performance of the litep2p library.

### Added

- kad: Providers part 8: unit, e2e, and `libp2p` conformance tests
([#258](#258))
- kad: Providers part 7: better types and public API, public addresses &
known providers ([#246](#246))
- kad: Providers part 6: stop providing
([#245](#245))
- kad: Providers part 5: `GET_PROVIDERS` query
([#236](#236))
- kad: Providers part 4: refresh local providers
([#235](#235))
- kad: Providers part 3: publish provider records (start providing)
([#234](#234))

### Changed

- transport_service: Improve connection stability by downgrading
connections on substream inactivity
([#260](#260))
- transport: Abort canceled dial attempts for TCP, WebSocket and Quic
([#255](#255))
- kad/executor: Add timeout for writting frames
([#277](#277))
- kad: Avoid cloning the `KademliaMessage` and use reference for
`RoutingTable::closest`
([#233](#233))
- peer_state: Robust state machine transitions
([#251](#251))
- address_store: Improve address tracking and add eviction algorithm
([#250](#250))
- kad: Remove unused serde cfg
([#262](#262))
- req-resp: Refactor to move functionality to dedicated methods
([#244](#244))
- transport_service: Improve logs and move code from tokio::select macro
([#254](#254))

### Fixed

- tcp/websocket/quic: Fix cancel memory leak
([#272](#272))
- transport: Fix pending dials memory leak
([#271](#271))
- ping: Fix memory leak of unremoved `pending_opens`
([#274](#274))
- identify: Fix memory leak of unused `pending_opens`
([#273](#273))
- kad: Fix not retrieving local records
([#221](#221))

---------

Signed-off-by: Alexandru Vasile <[email protected]>
Co-authored-by: Dmitry Markin <[email protected]>
@lexnv lexnv mentioned this pull request Nov 4, 2024
Merged
github-merge-queue bot pushed a commit to paritytech/polkadot-sdk that referenced this pull request Nov 5, 2024
@lexnv @dmitry-markin
litep2p: Update litep2p to v0.8.0 (#6353)
94389a9 
This PR updates litep2p to the latest release.

- `KademliaEvent::PutRecordSucess` is renamed to fix word typo
- `KademliaEvent::GetProvidersSuccess` and
`KademliaEvent::IncomingProvider` are needed for bootnodes on DHT work
and will be utilized later


### Added

- kad: Providers part 8: unit, e2e, and `libp2p` conformance tests
([#258](paritytech/litep2p#258))
- kad: Providers part 7: better types and public API, public addresses &
known providers ([#246](paritytech/litep2p#246))
- kad: Providers part 6: stop providing
([#245](paritytech/litep2p#245))
- kad: Providers part 5: `GET_PROVIDERS` query
([#236](paritytech/litep2p#236))
- kad: Providers part 4: refresh local providers
([#235](paritytech/litep2p#235))
- kad: Providers part 3: publish provider records (start providing)
([#234](paritytech/litep2p#234))

### Changed

- transport_service: Improve connection stability by downgrading
connections on substream inactivity
([#260](paritytech/litep2p#260))
- transport: Abort canceled dial attempts for TCP, WebSocket and Quic
([#255](paritytech/litep2p#255))
- kad/executor: Add timeout for writting frames
([#277](paritytech/litep2p#277))
- kad: Avoid cloning the `KademliaMessage` and use reference for
`RoutingTable::closest`
([#233](paritytech/litep2p#233))
- peer_state: Robust state machine transitions
([#251](paritytech/litep2p#251))
- address_store: Improve address tracking and add eviction algorithm
([#250](paritytech/litep2p#250))
- kad: Remove unused serde cfg
([#262](paritytech/litep2p#262))
- req-resp: Refactor to move functionality to dedicated methods
([#244](paritytech/litep2p#244))
- transport_service: Improve logs and move code from tokio::select macro
([#254](paritytech/litep2p#254))

### Fixed

- tcp/websocket/quic: Fix cancel memory leak
([#272](paritytech/litep2p#272))
- transport: Fix pending dials memory leak
([#271](paritytech/litep2p#271))
- ping: Fix memory leak of unremoved `pending_opens`
([#274](paritytech/litep2p#274))
- identify: Fix memory leak of unused `pending_opens`
([#273](paritytech/litep2p#273))
- kad: Fix not retrieving local records
([#221](paritytech/litep2p#221))

See release changelog for more details:
https://github.com/paritytech/litep2p/releases/tag/v0.8.0

cc @paritytech/networking

---------

Signed-off-by: Alexandru Vasile <[email protected]>
Co-authored-by: Dmitry Markin <[email protected]>
lexnv added a commit to paritytech/polkadot-sdk that referenced this pull request Nov 15, 2024
@lexnv @dmitry-markin
litep2p: Update litep2p to v0.8.0 (#6353)
1cac380 
This PR updates litep2p to the latest release.

- `KademliaEvent::PutRecordSucess` is renamed to fix word typo
- `KademliaEvent::GetProvidersSuccess` and
`KademliaEvent::IncomingProvider` are needed for bootnodes on DHT work
and will be utilized later

- kad: Providers part 8: unit, e2e, and `libp2p` conformance tests
([#258](paritytech/litep2p#258))
- kad: Providers part 7: better types and public API, public addresses &
known providers ([#246](paritytech/litep2p#246))
- kad: Providers part 6: stop providing
([#245](paritytech/litep2p#245))
- kad: Providers part 5: `GET_PROVIDERS` query
([#236](paritytech/litep2p#236))
- kad: Providers part 4: refresh local providers
([#235](paritytech/litep2p#235))
- kad: Providers part 3: publish provider records (start providing)
([#234](paritytech/litep2p#234))

- transport_service: Improve connection stability by downgrading
connections on substream inactivity
([#260](paritytech/litep2p#260))
- transport: Abort canceled dial attempts for TCP, WebSocket and Quic
([#255](paritytech/litep2p#255))
- kad/executor: Add timeout for writting frames
([#277](paritytech/litep2p#277))
- kad: Avoid cloning the `KademliaMessage` and use reference for
`RoutingTable::closest`
([#233](paritytech/litep2p#233))
- peer_state: Robust state machine transitions
([#251](paritytech/litep2p#251))
- address_store: Improve address tracking and add eviction algorithm
([#250](paritytech/litep2p#250))
- kad: Remove unused serde cfg
([#262](paritytech/litep2p#262))
- req-resp: Refactor to move functionality to dedicated methods
([#244](paritytech/litep2p#244))
- transport_service: Improve logs and move code from tokio::select macro
([#254](paritytech/litep2p#254))

- tcp/websocket/quic: Fix cancel memory leak
([#272](paritytech/litep2p#272))
- transport: Fix pending dials memory leak
([#271](paritytech/litep2p#271))
- ping: Fix memory leak of unremoved `pending_opens`
([#274](paritytech/litep2p#274))
- identify: Fix memory leak of unused `pending_opens`
([#273](paritytech/litep2p#273))
- kad: Fix not retrieving local records
([#221](paritytech/litep2p#221))

See release changelog for more details:
https://github.com/paritytech/litep2p/releases/tag/v0.8.0

cc @paritytech/networking

---------

Signed-off-by: Alexandru Vasile <[email protected]>
Co-authored-by: Dmitry Markin <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmitry-markin dmitry-markin dmitry-markin approved these changes

Assignees

@lexnv lexnv

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lexnv @dmitry-markin