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

transport/manager: Investigate connection closed without primary and secondary connections #239

Open
lexnv opened this issue Sep 5, 2024 · 0 comments
Open

transport/manager: Investigate connection closed without primary and secondary connections #239

lexnv opened this issue Sep 5, 2024 · 0 comments

Comments

@lexnv
Copy link
Collaborator

lexnv commented Sep 5, 2024

Substrate panic triggered on debug_assert:

Thread 'tokio-runtime-worker' panicked at 'assertion failed: false', /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/litep2p-0.6.2/src/transport/manager/mod.rs:858

Code path:

litep2p/src/transport/manager/mod.rs

Lines 850 to 859 in a27d007

None => {
tracing::warn!(
target: LOG_TARGET,
?peer,
?connection_id,
"non-primary connection was closed but secondary connection doesn't exist",
);
debug_assert!(false);
Err(Error::InvalidState)

Substrate issue: paritytech/polkadot-sdk#5595
Grafana link: https://grafana.teleport.parity.io/goto/sR83XkeSg?orgId=1
@lexnv lexnv mentioned this issue Sep 24, 2024
Merged
lexnv added a commit that referenced this issue Oct 28, 2024
@lexnv @dmitry-markin
peer_state: Robust state machine transitions (#251)
f112b59 
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:
- #239
- #238
- #237

---------

Signed-off-by: Alexandru Vasile <[email protected]>
Co-authored-by: Dmitry Markin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@lexnv