Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(rpc): rpc crashes when a peer has no addresses #1507

Merged
merged 1 commit into from
Jan 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 23 additions & 23 deletions tests/v2/test_jsonrpc_waku.nim
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import
libp2p/protocols/pubsub/rpc/message
import
../../waku/v1/node/rpc/hexstrings,
../../waku/v2/node/peer_manager/peer_manager,
../../waku/v2/node/waku_node,
../../waku/v2/node/jsonrpc/[store_api,
relay_api,
Expand All @@ -33,7 +34,8 @@ import
../../waku/v2/protocol/waku_filter/client,
../../waku/v2/utils/peers,
../../waku/v2/utils/time,
./testlib/common
./testlib/common,
../test_helpers

template sourceDir*: string = currentSourcePath.rsplit(DirSep, 1)[0]
const sigPath = sourceDir / ParDir / ParDir / "waku" / "v2" / "node" / "jsonrpc" / "jsonrpc_callsigs.nim"
Expand Down Expand Up @@ -404,34 +406,22 @@ procSuite "Waku v2 JSON-RPC API":
await allFutures([node1.stop(), node2.stop(), node3.stop()])

asyncTest "Admin API: get managed peer information":
# Create a couple of nodes
let
nodeKey1 = crypto.PrivateKey.random(Secp256k1, rng[])[]
node1 = WakuNode.new(nodeKey1, ValidIpAddress.init("0.0.0.0"), Port(60220))
nodeKey2 = crypto.PrivateKey.random(Secp256k1, rng[])[]
node2 = WakuNode.new(nodeKey2, ValidIpAddress.init("0.0.0.0"), Port(60222))
peerInfo2 = node2.peerInfo
nodeKey3 = crypto.PrivateKey.random(Secp256k1, rng[])[]
node3 = WakuNode.new(nodeKey3, ValidIpAddress.init("0.0.0.0"), Port(60224))
peerInfo3 = node3.peerInfo

await allFutures([node1.start(), node2.start(), node3.start()])

await node1.mountRelay()
await node2.mountRelay()
await node3.mountRelay()
# Create 3 nodes and start them with relay
let nodes = toSeq(0..<3).mapIt(WakuNode.new(generateKey(), ValidIpAddress.init("0.0.0.0"), Port(60220+it*2)))
await allFutures(nodes.mapIt(it.start()))
await allFutures(nodes.mapIt(it.mountRelay()))

# Dial nodes 2 and 3 from node1
await node1.connectToNodes(@[constructMultiaddrStr(peerInfo2)])
await node1.connectToNodes(@[constructMultiaddrStr(peerInfo3)])
await nodes[0].connectToNodes(@[constructMultiaddrStr(nodes[1].peerInfo)])
await nodes[0].connectToNodes(@[constructMultiaddrStr(nodes[2].peerInfo)])

# RPC server setup
let
rpcPort = Port(8552)
ta = initTAddress(bindIp, rpcPort)
server = newRpcHttpServer([ta])

installAdminApiHandlers(node1, server)
installAdminApiHandlers(nodes[0], server)
server.start()

let client = newRpcHttpClient()
Expand All @@ -443,15 +433,25 @@ procSuite "Waku v2 JSON-RPC API":
response.len == 2
# Check peer 2
response.anyIt(it.protocol == WakuRelayCodec and
it.multiaddr == constructMultiaddrStr(peerInfo2))
it.multiaddr == constructMultiaddrStr(nodes[1].peerInfo))
# Check peer 3
response.anyIt(it.protocol == WakuRelayCodec and
it.multiaddr == constructMultiaddrStr(peerInfo3))
it.multiaddr == constructMultiaddrStr(nodes[2].peerInfo))

# Artificially remove the address from the book
nodes[0].peerManager.peerStore[AddressBook][nodes[1].peerInfo.peerId] = @[]
nodes[0].peerManager.peerStore[AddressBook][nodes[2].peerInfo.peerId] = @[]

# Verify that the returned addresses are empty
let responseEmptyAdd = await client.get_waku_v2_admin_v1_peers()
check:
responseEmptyAdd[0].multiaddr == ""
responseEmptyAdd[1].multiaddr == ""

await server.stop()
await server.closeWait()

await allFutures([node1.stop(), node2.stop(), node3.stop()])
await allFutures(nodes.mapIt(it.stop()))

asyncTest "Admin API: get unmanaged peer information":
let
Expand Down
18 changes: 14 additions & 4 deletions waku/v2/node/jsonrpc/admin_api.nim
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,22 @@ proc constructMultiaddrStr*(wireaddr: MultiAddress, peerId: PeerId): string =

proc constructMultiaddrStr*(peerInfo: PeerInfo): string =
# Constructs a multiaddress with both location (wire) address and p2p identity
if peerInfo.listenAddrs.len == 0:
return ""
constructMultiaddrStr(peerInfo.listenAddrs[0], peerInfo.peerId)

proc constructMultiaddrStr*(remotePeerInfo: RemotePeerInfo): string =
# Constructs a multiaddress with both location (wire) address and p2p identity
if remotePeerInfo.addrs.len == 0:
return ""
constructMultiaddrStr(remotePeerInfo.addrs[0], remotePeerInfo.peerId)

proc constructMultiaddrStr*(storedInfo: StoredInfo): string =
# Constructs a multiaddress with both location (wire) address and p2p identity
if storedInfo.addrs.len == 0:
return ""
constructMultiaddrStr(storedInfo.addrs[0], storedInfo.peerId)

proc installAdminApiHandlers*(node: WakuNode, rpcsrv: RpcServer) =

## Admin API version 1 definitions
Expand Down Expand Up @@ -65,7 +75,7 @@ proc installAdminApiHandlers*(node: WakuNode, rpcsrv: RpcServer) =
# Map managed peers to WakuPeers and add to return list
wPeers.insert(node.peerManager.peerStore
.peers(WakuRelayCodec)
.mapIt(WakuPeer(multiaddr: constructMultiaddrStr(toSeq(it.addrs.items)[0], it.peerId),
.mapIt(WakuPeer(multiaddr: constructMultiaddrStr(it),
protocol: WakuRelayCodec,
connected: it.connectedness == Connectedness.Connected)),
wPeers.len) # Append to the end of the sequence
Expand All @@ -74,7 +84,7 @@ proc installAdminApiHandlers*(node: WakuNode, rpcsrv: RpcServer) =
# Map WakuFilter peers to WakuPeers and add to return list
wPeers.insert(node.peerManager.peerStore
.peers(WakuFilterCodec)
.mapIt(WakuPeer(multiaddr: constructMultiaddrStr(toSeq(it.addrs.items)[0], it.peerId),
.mapIt(WakuPeer(multiaddr: constructMultiaddrStr(it),
protocol: WakuFilterCodec,
connected: it.connectedness == Connectedness.Connected)),
wPeers.len) # Append to the end of the sequence
Expand All @@ -83,7 +93,7 @@ proc installAdminApiHandlers*(node: WakuNode, rpcsrv: RpcServer) =
# Map WakuSwap peers to WakuPeers and add to return list
wPeers.insert(node.peerManager.peerStore
.peers(WakuSwapCodec)
.mapIt(WakuPeer(multiaddr: constructMultiaddrStr(toSeq(it.addrs.items)[0], it.peerId),
.mapIt(WakuPeer(multiaddr: constructMultiaddrStr(it),
protocol: WakuSwapCodec,
connected: it.connectedness == Connectedness.Connected)),
wPeers.len) # Append to the end of the sequence
Expand All @@ -92,7 +102,7 @@ proc installAdminApiHandlers*(node: WakuNode, rpcsrv: RpcServer) =
# Map WakuStore peers to WakuPeers and add to return list
wPeers.insert(node.peerManager.peerStore
.peers(WakuStoreCodec)
.mapIt(WakuPeer(multiaddr: constructMultiaddrStr(toSeq(it.addrs.items)[0], it.peerId),
.mapIt(WakuPeer(multiaddr: constructMultiaddrStr(it),
protocol: WakuStoreCodec,
connected: it.connectedness == Connectedness.Connected)),
wPeers.len) # Append to the end of the sequence
Expand Down