Skip to content

Commit

Permalink
[syft/network] network service add_route now takes peer's verify key
Browse files Browse the repository at this point in the history
instead of the whole NodePeer object

Co-authored-by: Shubham Gupta <[email protected]>
  • Loading branch information
khoaguin and shubham3121 committed Apr 9, 2024
1 parent 78a3ac5 commit 93d7d53
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 15 deletions.
15 changes: 7 additions & 8 deletions packages/syft/src/syft/service/network/network_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -476,9 +476,8 @@ def add_route_on_peer(
remote_client = remote_client.ok()
# ask the remote node to add the route to the self node
# note that the `.to` transform gives a NodePeer object without its routes
self_node_peer: NodePeer = context.node.settings.to(NodePeer)
result = remote_client.api.services.network.add_route(
peer=self_node_peer,
peer_verify_key=context.credentials,
route=route,
called_by_peer=True,
)
Expand All @@ -488,7 +487,7 @@ def add_route_on_peer(
def add_route(
self,
context: AuthedServiceContext,
peer: NodePeer,
peer_verify_key: SyftVerifyKey,
route: NodeRoute,
called_by_peer: bool = False,
) -> SyftSuccess | SyftError:
Expand All @@ -497,24 +496,24 @@ def add_route(
Args:
context (AuthedServiceContext): The authentication context of the remote node.
peer (NodePeer): The peer representing the remote node.
peer_verify_key (SyftVerifyKey): The verify key of the remote node peer.
route (NodeRoute): The route to be added.
called_by_peer (bool): The flag to indicate that it's called by a remote peer.
Returns:
SyftSuccess | SyftError
"""
# verify if the peer is truly the one sending the request to add the route to itself
if called_by_peer and peer.verify_key != context.credentials:
if called_by_peer and peer_verify_key != context.credentials:
return SyftError(
message=(
f"The {type(peer).__name__}.verify_key: "
f"{peer.verify_key} does not match the signature of the message"
f"The {type(peer_verify_key).__name__}.verify_key: "
f"{peer_verify_key} does not match the signature of the message"
)
)
# get the full peer object from the store to update its routes
remote_node_peer: NodePeer | SyftError = (
self._get_remote_node_peer_by_verify_key(context, peer.verify_key)
self._get_remote_node_peer_by_verify_key(context, peer_verify_key)
)
if isinstance(remote_node_peer, SyftError):
return remote_node_peer
Expand Down
18 changes: 11 additions & 7 deletions tests/integration/network/gateway_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ def test_add_route(set_env_var, gateway_port: int, domain_1_port: int) -> None:
new_route = HTTPNodeRoute(host_or_ip="localhost", port=10000)
domain_peer: NodePeer = gateway_client.api.services.network.get_all_peers()[0]
res = gateway_client.api.services.network.add_route(
peer=domain_peer, route=new_route
peer_verify_key=domain_peer.verify_key, route=new_route
)
assert isinstance(res, SyftSuccess)
domain_peer = gateway_client.api.services.network.get_all_peers()[0]
Expand All @@ -364,7 +364,7 @@ def test_add_route(set_env_var, gateway_port: int, domain_1_port: int) -> None:
# adding another route to the domain
new_route2 = HTTPNodeRoute(host_or_ip="localhost", port=10001)
res = gateway_client.api.services.network.add_route(
peer=domain_peer, route=new_route2
peer_verify_key=domain_peer.verify_key, route=new_route2
)
assert isinstance(res, SyftSuccess)
domain_peer = gateway_client.api.services.network.get_all_peers()[0]
Expand All @@ -374,7 +374,7 @@ def test_add_route(set_env_var, gateway_port: int, domain_1_port: int) -> None:

# add an existed route to the domain and check its priority gets updated
res = gateway_client.api.services.network.add_route(
peer=domain_peer, route=domain_peer.node_routes[0]
peer_verify_key=domain_peer.verify_key, route=domain_peer.node_routes[0]
)
assert "route already exists" in res.message
assert isinstance(res, SyftSuccess)
Expand All @@ -389,7 +389,7 @@ def test_add_route(set_env_var, gateway_port: int, domain_1_port: int) -> None:

# add another existed route (port 10000)
res = gateway_client.api.services.network.add_route(
peer=domain_peer, route=domain_peer.node_routes[1]
peer_verify_key=domain_peer.verify_key, route=domain_peer.node_routes[1]
)
assert "route already exists" in res.message
assert isinstance(res, SyftSuccess)
Expand Down Expand Up @@ -430,7 +430,7 @@ def test_delete_route(set_env_var, gateway_port: int, domain_1_port: int) -> Non
new_route = HTTPNodeRoute(host_or_ip="localhost", port=10000)
domain_peer: NodePeer = gateway_client.api.services.network.get_all_peers()[0]
res = gateway_client.api.services.network.add_route(
peer=domain_peer, route=new_route
peer_verify_key=domain_peer.verify_key, route=new_route
)
assert isinstance(res, SyftSuccess)
domain_peer = gateway_client.api.services.network.get_all_peers()[0]
Expand Down Expand Up @@ -467,6 +467,10 @@ def test_add_route_on_peer(set_env_var, gateway_port: int, domain_1_port: int) -
port=domain_1_port, email="[email protected]", password="changethis"
)

# Remove existing peers
assert isinstance(_remove_existing_peers(domain_client), SyftSuccess)
assert isinstance(_remove_existing_peers(gateway_client), SyftSuccess)

# connecting the domain to the gateway
result = domain_client.connect_to_gateway(gateway_client)
assert isinstance(result, SyftSuccess)
Expand Down Expand Up @@ -612,11 +616,11 @@ def test_update_route_priority(
new_route2 = HTTPNodeRoute(host_or_ip="localhost", port=10001)
domain_peer: NodePeer = gateway_client.api.services.network.get_all_peers()[0]
res = gateway_client.api.services.network.add_route(
peer=domain_peer, route=new_route
peer_verify_key=domain_peer.verify_key, route=new_route
)
assert isinstance(res, SyftSuccess)
res = gateway_client.api.services.network.add_route(
peer=domain_peer, route=new_route2
peer_verify_key=domain_peer.verify_key, route=new_route2
)
assert isinstance(res, SyftSuccess)

Expand Down

0 comments on commit 93d7d53

Please sign in to comment.