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: fix identify disconnect #3052

Merged
merged 1 commit into from
Sep 24, 2021

Conversation

driftluo
Copy link
Collaborator

@driftluo driftluo commented Sep 24, 2021

What problem does this PR solve?

fix https://github.com/nervosnetwork/ckb/runs/3673717980#step:4:1915 this ci fail

  1. assert fail because the assert is wrong here, the registered behavior is before received, and the discovery in the test is broadcast every second. It is impossible to determine whether the node3 is just in the process of trying to connect when asserting. It can also be changed to wait_connect_state(&node3, 1), but I don’t think it’s necessary, it’s no problem to delete it directly
  2. When identify is closed, we should first check whether the current node information is banned and then insert the peer store, otherwise, the banned address may be spread

Check List

Tests

  • Unit test

Release note

Title Only: Include only the PR title in the release note.

@driftluo
Copy link
Collaborator Author

bors r=keroro520,yangby-cryptape

bors bot added a commit that referenced this pull request Sep 24, 2021
3052: fix: fix identify disconnect r=keroro520,yangby-cryptape a=driftluo

### What problem does this PR solve?

fix https://github.com/nervosnetwork/ckb/runs/3673717980#step:4:1915 this ci fail

1. assert fail because the assert is wrong here, the `registered` behavior is before `received`, and the `discovery` in the test is broadcast every second. It is impossible to determine whether the node3 is just in the process of trying to connect when asserting. It can also be changed to `wait_connect_state(&node3, 1)`, but I don’t think it’s necessary, it’s no problem to delete it directly 
2. When identify is closed, we should first check whether the current node information is banned and then insert the peer store, otherwise, the banned address may be spread 

### Check List

Tests

- Unit test

### Release note

```release-note
Title Only: Include only the PR title in the release note.
```



Co-authored-by: driftluo <[email protected]>
@bors
Copy link
Contributor

bors bot commented Sep 24, 2021

This PR was included in a batch that successfully built, but then failed to merge into develop. It will not be retried.

Additional information:

{"message":"All comments must be resolved.","documentation_url":"https://docs.github.com/articles/about-protected-branches"}

@driftluo
Copy link
Collaborator Author

bors r=keroro520,yangby-cryptape

1 similar comment
@yangby-cryptape
Copy link
Collaborator

bors r=keroro520,yangby-cryptape

@bors bors bot merged commit 976f76d into nervosnetwork:develop Sep 24, 2021
@driftluo driftluo deleted the fix-identify-disconnect branch September 25, 2021 07:27
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