fix: dial_feeler(..) function dial identify protocol

[chaoticlonghair]

What problem does this PR solve?

The dial_feeler(..) function dial with the identify protocol acctually.

Check List

Tests

• Unit test
• Integration test
• Manual test (add detailed scripts or steps below)
• No code ci-runs-only: [ quick_checks,linters ]

Release note

None: Exclude this PR from the release note.

[driftluo]

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

[chaoticlonghair]

all sessions open with identify，then it will open others ...

As your said, is it just a bad function name with bad comments?

• I just confused why a function named dial_feeler(..) but dial with the identify protocol acctually.

ckb/network/src/network.rs

Lines 488 to 501 in b8f655a

	/// Dial just feeler protocol
	pub fn dial_feeler(&self, p2p_control: &ServiceControl, addr: Multiaddr) {
	if let Err(err) = self.dial_inner(
	p2p_control,
	addr.clone(),
	TargetProtocol::Single(SupportProtocols::Identify.protocol_id()),
	) {
	debug!("dial_feeler error {err}");
	} else {
	self.with_peer_registry_mut(|reg| {
	reg.add_feeler(&addr);
	});
	}
	}

Note

"... Dial just feeler protocol ..."

• If dial with the feeler is inside the identify protocol, why don't just call dial_identify(..), with error handling.

ckb/network/src/network.rs

Lines 477 to 486 in b8f655a

	/// Dial just identify protocol
	pub fn dial_identify(&self, p2p_control: &ServiceControl, addr: Multiaddr) {
	if let Err(err) = self.dial_inner(
	p2p_control,
	addr,
	TargetProtocol::Single(SupportProtocols::Identify.protocol_id()),
	) {
	debug!("dial_identify error: {err}");
	}
	}

• dial_identify(..) will also dial with feeler protocol, too.

[driftluo]

If dial with the feeler is inside the identify protocol, why don't just call dial_identify(..), with error handling.

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：

• dial feeler will open identify and feeler, then close this session
• dial identify will open identify and all protocols except feeler, and keep connected

[chaoticlonghair]

• dial identify will open identify and all protocols except feeler, and keep connected

If I was correct, it should be:

dial identify will open identify with all supported protocols inside it, then close feeler, and keep session connected

Am I right? (then I will add this comment into the code)

[driftluo]

dial identify will open identify with all supported protocols inside it, then close feeler, and keep the session connected

no, feeler never opened on dial identify

ckb/network/src/protocols/identify/mod.rs

Lines 483 to 486 in b8f655a

	id != &SupportProtocols::Feeler.protocol_id()
	&& id != &SupportProtocols::RelayV2.protocol_id()
	} else {
	id != &SupportProtocols::Feeler.protocol_id()

[chaoticlonghair]

no, feeler never opened on dial identify

dial_identify:

• A dial identify to B: connected.

dial_feeler:

• A dial identify to B: connected.
• A mark feeler.
• A dial feeler to B. (or B dial feeler to A in callback?)

Am I correct this time?

[driftluo]

dial_identify：

• A dial identify to B: connected
• A‘s identify open all protocols except feeler to B

dial_feeler:

• A mark feeler
• A dial identify to B: connected
• A‘s identify open feeler to B

[chaoticlonghair]

dial_feeler:

• A mark feeler
• A dial identify to B: connected
• A‘s identify open feeler to B

ckb/network/src/network.rs

Lines 488 to 501 in b8f655a

	/// Dial just feeler protocol
	pub fn dial_feeler(&self, p2p_control: &ServiceControl, addr: Multiaddr) {
	if let Err(err) = self.dial_inner(
	p2p_control,
	addr.clone(),
	TargetProtocol::Single(SupportProtocols::Identify.protocol_id()),
	) {
	debug!("dial_feeler error {err}");
	} else {
	self.with_peer_registry_mut(|reg| {
	reg.add_feeler(&addr);
	});
	}
	}

But, in above code, dial_inner is before add_feeler.
So, in my understood, dial-identify.is_ok() --> mark-feeler --> dial-feeler.

[driftluo]

But dial_inner is before add_feeler.

I think a better description here is, "After successfully sending the command, mark the feeler", there is no dial completed at this time

[chaoticlonghair]

there is no dial completed at this time

So, can I say:

In theory, the order is not strong (or strict) promised.
Just based on experience in most cases (maybe 99.9999999%).

For example, the return of dial_inner is hanging for a while.