From a59cd3719487be4a2aac6c2478167778be7300c7 Mon Sep 17 00:00:00 2001 From: cheatfate Date: Mon, 25 Nov 2024 19:05:21 +0200 Subject: [PATCH 1/7] Fix observed address is not part of PeerStore[AddressBook]. --- libp2p/peerstore.nim | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/libp2p/peerstore.nim b/libp2p/peerstore.nim index c9d245b078..3d33188e5d 100644 --- a/libp2p/peerstore.nim +++ b/libp2p/peerstore.nim @@ -145,9 +145,21 @@ proc del*(peerStore: PeerStore, peerId: PeerId) {.public.} = for _, book in peerStore.books: book.deletor(peerId) -proc updatePeerInfo*(peerStore: PeerStore, info: IdentifyInfo) = - if info.addrs.len > 0: - peerStore[AddressBook][info.peerId] = info.addrs +proc updatePeerInfo*( + peerStore: PeerStore, + info: IdentifyInfo, + observedAddr: Opt[MultiAddress] = Opt.none(MultiAddress) +) = + let + observed = + if observedAddr.isNone(): + default(seq[MultiAddress]) + else: + @[observedAddr.get()] + addresses = info.addrs & observed + + if len(addresses) > 0: + peerStore[AddressBook][info.peerId] = addresses info.pubkey.withValue(pubkey): peerStore[KeyBook][info.peerId] = pubkey @@ -200,7 +212,7 @@ proc identify*(peerStore: PeerStore, muxer: Muxer) {.async.} = knownAgent = shortAgent muxer.connection.setShortAgent(knownAgent) - peerStore.updatePeerInfo(info) + peerStore.updatePeerInfo(info, stream.observedAddr) finally: await stream.closeWithEOF() From 30658a0fafe455fdd28db64d9b9641053a8b013b Mon Sep 17 00:00:00 2001 From: cheatfate Date: Mon, 25 Nov 2024 22:54:13 +0200 Subject: [PATCH 2/7] Form full P2P address. --- libp2p/peerstore.nim | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/libp2p/peerstore.nim b/libp2p/peerstore.nim index 3d33188e5d..0084b98161 100644 --- a/libp2p/peerstore.nim +++ b/libp2p/peerstore.nim @@ -155,7 +155,13 @@ proc updatePeerInfo*( if observedAddr.isNone(): default(seq[MultiAddress]) else: - @[observedAddr.get()] + if TCP.match(observedAddr): + # We should form full P2P address in case of simple TCP address. + let p2p = MultiAddress.init(multiCodec("p2p"), info.peerId).valueOr: + raiseAssert "Should not be happen" + @[observedAddr.get() & p2p] + else: + @[observedAddr.get()] addresses = info.addrs & observed if len(addresses) > 0: From 42482280cef3d4c94151afa352da94bf0d1cdf8d Mon Sep 17 00:00:00 2001 From: cheatfate Date: Mon, 25 Nov 2024 23:30:49 +0200 Subject: [PATCH 3/7] Fix errors. --- libp2p/peerstore.nim | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/libp2p/peerstore.nim b/libp2p/peerstore.nim index 0084b98161..e16807034c 100644 --- a/libp2p/peerstore.nim +++ b/libp2p/peerstore.nim @@ -32,6 +32,7 @@ import ./peerid, ./peerinfo, ./routing_record, + ./multicodec, ./multiaddress, ./stream/connection, ./multistream, @@ -155,13 +156,17 @@ proc updatePeerInfo*( if observedAddr.isNone(): default(seq[MultiAddress]) else: - if TCP.match(observedAddr): + let res = observedAddr.get() + if TCP.match(res): # We should form full P2P address in case of simple TCP address. - let p2p = MultiAddress.init(multiCodec("p2p"), info.peerId).valueOr: - raiseAssert "Should not be happen" - @[observedAddr.get() & p2p] + let + p2p = MultiAddress.init(multiCodec("p2p"), info.peerId).valueOr: + raiseAssert "Initialization should not fail" + ma = concat(res, p2p).valueOr: + raiseAssert "Concatenation should not fail" + @[ma] else: - @[observedAddr.get()] + @[res] addresses = info.addrs & observed if len(addresses) > 0: From a70d65dd4f33a52471c07f07e3fe1d65f82996ba Mon Sep 17 00:00:00 2001 From: cheatfate Date: Tue, 26 Nov 2024 01:04:40 +0200 Subject: [PATCH 4/7] Restore simple approach. --- libp2p/peerstore.nim | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/libp2p/peerstore.nim b/libp2p/peerstore.nim index e16807034c..d4b4ab0cee 100644 --- a/libp2p/peerstore.nim +++ b/libp2p/peerstore.nim @@ -156,17 +156,7 @@ proc updatePeerInfo*( if observedAddr.isNone(): default(seq[MultiAddress]) else: - let res = observedAddr.get() - if TCP.match(res): - # We should form full P2P address in case of simple TCP address. - let - p2p = MultiAddress.init(multiCodec("p2p"), info.peerId).valueOr: - raiseAssert "Initialization should not fail" - ma = concat(res, p2p).valueOr: - raiseAssert "Concatenation should not fail" - @[ma] - else: - @[res] + @[observedAddr.get()] addresses = info.addrs & observed if len(addresses) > 0: From a688510664281523532f83f2ddbdcab1e00192cc Mon Sep 17 00:00:00 2001 From: cheatfate Date: Tue, 26 Nov 2024 02:19:25 +0200 Subject: [PATCH 5/7] Add `LastSeenBook` to PeerStore. --- libp2p/peerstore.nim | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/libp2p/peerstore.nim b/libp2p/peerstore.nim index d4b4ab0cee..a213ee231c 100644 --- a/libp2p/peerstore.nim +++ b/libp2p/peerstore.nim @@ -64,6 +64,7 @@ type KeyBook* {.public.} = ref object of PeerBook[PublicKey] AgentBook* {.public.} = ref object of PeerBook[string] + LastSeenBook* {.public.} = ref object of PeerBook[Opt[MultiAddress]] ProtoVersionBook* {.public.} = ref object of PeerBook[string] SPRBook* {.public.} = ref object of PeerBook[Envelope] @@ -149,18 +150,12 @@ proc del*(peerStore: PeerStore, peerId: PeerId) {.public.} = proc updatePeerInfo*( peerStore: PeerStore, info: IdentifyInfo, - observedAddr: Opt[MultiAddress] = Opt.none(MultiAddress) + observedAddr: Opt[MultiAddress] = Opt.none(MultiAddress), ) = - let - observed = - if observedAddr.isNone(): - default(seq[MultiAddress]) - else: - @[observedAddr.get()] - addresses = info.addrs & observed - - if len(addresses) > 0: - peerStore[AddressBook][info.peerId] = addresses + if len(info.addrs) > 0: + peerStore[AddressBook][info.peerId] = info.addrs + + peerStore[LastSeenBook][info.peerId] = observedAddr info.pubkey.withValue(pubkey): peerStore[KeyBook][info.peerId] = pubkey From ff7e49089c2942d3361454724ff93aa343126d79 Mon Sep 17 00:00:00 2001 From: cheatfate Date: Tue, 26 Nov 2024 15:58:48 +0200 Subject: [PATCH 6/7] Remove unneeded import. Add `LastSeenBook` to tests. --- libp2p/peerstore.nim | 1 - tests/testswitch.nim | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/libp2p/peerstore.nim b/libp2p/peerstore.nim index a213ee231c..b216daa942 100644 --- a/libp2p/peerstore.nim +++ b/libp2p/peerstore.nim @@ -32,7 +32,6 @@ import ./peerid, ./peerinfo, ./routing_record, - ./multicodec, ./multiaddress, ./stream/connection, ./multistream, diff --git a/tests/testswitch.nim b/tests/testswitch.nim index f58e1d7d6b..dafb9adac0 100644 --- a/tests/testswitch.nim +++ b/tests/testswitch.nim @@ -842,6 +842,9 @@ suite "Switch": switch1.peerStore[AddressBook][switch2.peerInfo.peerId] == switch2.peerInfo.addrs switch1.peerStore[ProtoBook][switch2.peerInfo.peerId] == switch2.peerInfo.protocols + switch1.peerStore[LastSeenBook][switch2.peerInfo.peerId].isSome() + switch2.peerStore[LastSeenBook][switch1.peerInfo.peerId].isSome() + switch1.peerInfo.peerId notin switch2.peerStore[AddressBook] switch1.peerInfo.peerId notin switch2.peerStore[ProtoBook] From dd6eadd55959aac4f41c00c43218038e0c817e62 Mon Sep 17 00:00:00 2001 From: cheatfate Date: Tue, 26 Nov 2024 17:28:13 +0200 Subject: [PATCH 7/7] Do not use switch2 in tests because it has limited capacity == 0. --- tests/testswitch.nim | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/testswitch.nim b/tests/testswitch.nim index dafb9adac0..eb645b14a6 100644 --- a/tests/testswitch.nim +++ b/tests/testswitch.nim @@ -843,7 +843,6 @@ suite "Switch": switch1.peerStore[ProtoBook][switch2.peerInfo.peerId] == switch2.peerInfo.protocols switch1.peerStore[LastSeenBook][switch2.peerInfo.peerId].isSome() - switch2.peerStore[LastSeenBook][switch1.peerInfo.peerId].isSome() switch1.peerInfo.peerId notin switch2.peerStore[AddressBook] switch1.peerInfo.peerId notin switch2.peerStore[ProtoBook]