identify: fix observed address handling

[ivan4th]

If a connection was closed before receiving an identify response, the corresponding observed address could be wrongly removed as the connection counter for the local address was being decremented on each disconnect even if no messages were consumed by IDService previously.

The issues began after #2793

Patch by @sukunrt

[sukunrt]

Oops! sorry about that!

I believe this smaller patch will fix the issue.

diff --git a/p2p/protocol/identify/obsaddr.go b/p2p/protocol/identify/obsaddr.go
index 4437c4b0..fc1c100c 100644
--- a/p2p/protocol/identify/obsaddr.go
+++ b/p2p/protocol/identify/obsaddr.go
@@ -452,6 +452,12 @@ func (o *ObservedAddrManager) removeConn(conn connMultiaddrs) {
 	o.mu.Lock()
 	defer o.mu.Unlock()
 
+	observedTWAddr, ok := o.connObservedTWAddrs[conn]
+	if !ok {
+		return
+	}
+	delete(o.connObservedTWAddrs, conn)
+
 	// normalize before obtaining the thinWaist so that we are always dealing
 	// with the normalized form of the address
 	localTW, err := thinWaistForm(o.normalize(conn.LocalMultiaddr()))
@@ -467,11 +473,6 @@ func (o *ObservedAddrManager) removeConn(conn connMultiaddrs) {
 		delete(o.localAddrs, string(localTW.Addr.Bytes()))
 	}
 
-	observedTWAddr, ok := o.connObservedTWAddrs[conn]
-	if !ok {
-		return
-	}
-	delete(o.connObservedTWAddrs, conn)
 	observer, err := getObserver(conn.RemoteMultiaddr())
 	if err != nil {
 		return

[ivan4th]

@sukunrt yes, that also fixes the problem, thanks!

Updated the PR

[sukunrt]

Thanks @ivan4th

[MarcoPolo]

@ivan4th would a patch release containing this fix help you?

[ivan4th]

@MarcoPolo yes, that would help indeed, thank you!

[ivan4th]

Oops, just noticed that when replacing my patch, I also reverted the testing code. Added it back in #2828