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

fix: session close also should remove feeler dialing flag #3251

Conversation

driftluo
Copy link
Collaborator

@driftluo driftluo commented Dec 23, 2021

What problem does this PR solve?

dial command will reject by dialing feeler or session keep online

ckb/network/src/network.rs

Lines 332 to 334 in 5b44843

let peer_in_registry = self.with_peer_registry(|reg| {
reg.get_key_by_peer_id(peer_id).is_some() || reg.is_feeler(addr)
});

dialing feeler flag will remove on feeler disconnected

fn disconnected(&mut self, context: ProtocolContextMutRef) {
let session = context.session;
self.network_state.with_peer_registry_mut(|reg| {
reg.remove_feeler(&session.address);
});

But if the remote side turns off the protocol before local has opened it, the local's dialing feeler flag will not be cleared, also, this node can no longer be dial

why has such a delay on open protocol?

outbound: request open -> wait remote response -> open protocol
inbound: wait request -> send back response and open the protocol

There is a slight difference in the opening time of the two sides

Check List

Tests

  • Unit test
  • Integration test

Release note

None: Exclude this PR from the release note.

@driftluo driftluo requested a review from a team as a code owner December 23, 2021 08:25
Makefile Show resolved Hide resolved
Copy link
Collaborator

@yangby-cryptape yangby-cryptape left a comment

Choose a reason for hiding this comment

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

  • Before this PR, the integration test Discovery has 4% chance to be failed.
  • After this PR, run the integration test Discovery more than 100 times without any errors.

@driftluo
Copy link
Collaborator Author

bors r=keroro520,yangby-cryptape

@bors bors bot merged commit da1f26c into nervosnetwork:develop Dec 24, 2021
@driftluo driftluo deleted the fix-feeler-close-not-remove-peer-store-state branch December 24, 2021 06:34
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.

3 participants