-
Notifications
You must be signed in to change notification settings - Fork 236
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: dial_feeler(..)
function dial identify protocol
#4784
base: develop
Are you sure you want to change the base?
fix: dial_feeler(..)
function dial identify protocol
#4784
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all sessions open with identify,then it will open others https://github.com/nervosnetwork/ckb/blob/develop/network/src/protocols/identify/mod.rs#L466-L476
As your said, is it just a bad function name with bad comments?
Lines 488 to 501 in b8f655a
Note "... Dial just feeler protocol ..."
Lines 477 to 486 in b8f655a
|
dial feeler success should mark this dial is feeler, but dial identify not self.with_peer_registry_mut(|reg| {
reg.add_feeler(&addr);
}); These two dial behaviors are mutually exclusive:
|
If I was correct, it should be:
Am I right? (then I will add this comment into the code) |
no, feeler never opened on dial identify ckb/network/src/protocols/identify/mod.rs Lines 483 to 486 in b8f655a
|
Am I correct this time? |
dial_identify:
dial_feeler:
|
Lines 488 to 501 in b8f655a
But, in above code, |
I think a better description here is, "After successfully sending the command, mark the feeler", there is no dial completed at this time |
So, can I say:
For example, the return of |
What problem does this PR solve?
The
dial_feeler(..)
function dial with the identify protocol acctually.Check List
Tests
Release note