feat(kad): report changes to our mode as an event

[dhuseby]

Description

Previously, the kademlia protocol would only log changes to the internal mode. With this patch, we now also emit an event which allows users to code against the internal state of the kademlia protocol.

Fixes #4310.

Notes & open questions

I added the code to emit the event in the reconfigure_mode function on the assumption that all mode changes result in that function being called.

Change checklist

• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• A changelog entry has been made in the appropriate crates
• I have force pushed to break the squash commit system we use around here.

[dhuseby]

This PR replaces the messed up previous PR #4341

[thomaseizinger]

Thanks for the creating a new PR!

The kademlia tests are still failing (correctly) because we are now emitting an additional event. For example, you'd have to add your new event to the following list:

rust-libp2p/protocols/kad/tests/client_mode.rs

Lines 22 to 23 in 1fff308

	[MyBehaviourEvent::Identify(_), MyBehaviourEvent::Identify(_), MyBehaviourEvent::Kad(KademliaEvent::RoutingUpdated { peer, .. })],
	[MyBehaviourEvent::Identify(_), MyBehaviourEvent::Identify(_)],

[thomaseizinger]

I am adding this to the next milestone because it is a breaking change. Conventionally, we also set these PRs to draft to not accidentally merge them! :)

[dhuseby]

So I was looking at the behavior of this change and I was wondering about what we want as the correct behavior. In the case where the set_mode function is called with a new mode (instead of None), there will be no event emitted because it never results in a call to the determine_mode_from_external addresses function which is the only place we emit the event.

Is it correct to say we only emit the event whenever we are in auto_mode and we determine our mode from the external addresses?

Also, the current behavior means that there's often—but not always—an initial event emitted from the first time a we try to determine our mode from the external addresses. There is a corner case where set_mode is called, auto_mode is set to false, before the swarm starts. In that case no mode changed events will ever be emitted.

I suppose this all is as it should be. If you set the mode manually and turn of auto_mode in doing so, you probably don't want any mode changed events emitted. If you never set the mode manually or you disable the manual mode setting, you're putting the behavior into auto_mode and it will report any changes via a mode changed event.

[dhuseby]

no force pushes... closed in lieu of PR #4503

[thomaseizinger]

So I was looking at the behavior of this change and I was wondering about what we want as the correct behavior. In the case where the set_mode function is called with a new mode (instead of None), there will be no event emitted because it never results in a call to the determine_mode_from_external addresses function which is the only place we emit the event.

Is it correct to say we only emit the event whenever we are in auto_mode and we determine our mode from the external addresses?

Also, the current behavior means that there's often—but not always—an initial event emitted from the first time a we try to determine our mode from the external addresses. There is a corner case where set_mode is called, auto_mode is set to false, before the swarm starts. In that case no mode changed events will ever be emitted.

I suppose this all is as it should be. If you set the mode manually and turn of auto_mode in doing so, you probably don't want any mode changed events emitted. If you never set the mode manually or you disable the manual mode setting, you're putting the behavior into auto_mode and it will report any changes via a mode changed event.

Yep, that is my thinking too! If you set the mode manually, you don't need an event to tell you because you already have a code path where you can add whatever logic you want to do as a result of the new mode.