feat: enable manual identify

[wemeetagain]

In some applications, automatically dialing identify is not desired, and more manual control is useful.

To that end, this PR contains two edits that act together

• Add an additional identify init boolean option runOnConnectionOpen, which controls whether to listen to 'connection:open' events to trigger identify
• Add a return value to the identify method

[maschad]

I see the potential benefits of having more control over when streams are opened, but could you elaborate a bit more on your use case? I don't think this is in accordance with the spec since identify is used to inform known peers about changes which will happen whilst the node is running (which suggests it should always be run when opening a new connection).

[wemeetagain]

The spec doesn't say anything about when the identify protocol should be dialed. I think the 'dial on connection open' behavior is just a common practice that is useful amd sufficient for most usecases. (Identify/push, a separate protocol, is used to inform peers about changes, but again, the spec gives no commands about when that protocol must or should be dialed)

In our usecase, we have another protocol that we want to dial first on connection open (We call it the 'status' protocol). Then depending on the result of that, either success or failure (note that failure can be failure to support that protocol, or responding to that protocol with a response that we don't like), the peer will either be disconnected or more protocols will be dialed, along with identify, which we use mainly for the agent version string.
We'd like to avoid the unnecessary identify in the case that the first protocol result is a failure.

[maschad]

The spec doesn't say anything about when the identify protocol should be dialed. I think the 'dial on connection open' behavior is just a common practice that is useful amd sufficient for most usecases. (Identify/push, a separate protocol, is used to inform peers about changes, but again, the spec gives no commands about when that protocol must or should be dialed)

That's true I do agree that the matter of when is more open to interpretation, so since this would be enabled by default this doesn't change anything functionally.

In our usecase, we have another protocol that we want to dial first on connection open (We call it the 'status' protocol). Then depending on the result of that, either success or failure (note that failure can be failure to support that protocol, or responding to that protocol with a response that we don't like), the peer will either be disconnected or more protocols will be dialed, along with identify, which we use mainly for the agent version string.
We'd like to avoid the unnecessary identify in the case that the first protocol result is a failure.

Thanks for explaining, I think this is reasonable.

[achingbrain]

LGTM in theory, though I fear people will call this function without specifying the only-run-manually config option which will lead to hard-to-predict stream closures due to the default stream limits so this introduces a bit of a foot gun.

This PR is missing tests, please can you add some to prevent future regressions.

[achingbrain]

Nearly there, just a few small things to address.