Skip to content

Commit

Permalink
[syft/network] network service delete_route and `update_route_prior…
Browse files Browse the repository at this point in the history 
…ity`

 now takes peer's verify key instead of the whole NodePeer object

Co-authored-by: Shubham Gupta <[email protected]>
  • Loading branch information
@khoaguin @shubham3121
khoaguin and shubham3121 committed Apr 9, 2024
1 parent 93d7d53 commit 4f68061
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 25 deletions.
35 changes: 16 additions & 19 deletions packages/syft/src/syft/service/network/network_service.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,6 @@ 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
result = remote_client.api.services.network.add_route(
peer_verify_key=context.credentials,
route=route,
Expand Down Expand Up @@ -507,7 +506,7 @@ def add_route(
if called_by_peer and peer_verify_key != context.credentials:
return SyftError(
message=(
f"The {type(peer_verify_key).__name__}.verify_key: "
f"The {type(peer_verify_key).__name__}: "
f"{peer_verify_key} does not match the signature of the message"
)
)
Expand Down Expand Up @@ -581,10 +580,8 @@ def delete_route_on_peer(
)
remote_client = remote_client.ok()
# ask the remote node to delete 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.delete_route(
peer=self_node_peer,
peer_verify_key=context.credentials,
route=route,
route_id=route_id,
called_by_peer=True,
Expand All @@ -595,7 +592,7 @@ def delete_route_on_peer(
def delete_route(
self,
context: AuthedServiceContext,
peer: NodePeer,
peer_verify_key: SyftVerifyKey,
route: NodeRoute | None = None,
route_id: UID | None = None,
called_by_peer: bool = False,
Expand All @@ -607,28 +604,29 @@ def delete_route(
Args:
context (AuthedServiceContext): The authentication context for the service.
peer (NodePeer): The peer for which the route will be deleted.
peer_verify_key (SyftVerifyKey): The verify key of the remote node peer.
route (NodeRoute): The route to be deleted.
route_id (UID): The UID of the route to be deleted.
called_by_peer (bool): The flag to indicate that it's called by a remote peer.
Returns:
SyftSuccess: If the route is successfully deleted.
SyftError: If there is an error deleting the route.
SyftInfo: If there is only one route left for the peer and
the admin chose not to remove it
"""
if called_by_peer and peer.verify_key != context.credentials:
if called_by_peer and peer_verify_key != context.credentials:
# verify if the peer is truly the one sending the request to delete the route to itself
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__}: "
f"{peer_verify_key} does not match the signature of the message"
)
)

remote_node_peer: NodePeer | SyftError = (
self._get_remote_node_peer_by_verify_key(
context=context, peer_verify_key=peer.verify_key
context=context, peer_verify_key=peer_verify_key
)
)

Expand Down Expand Up @@ -724,9 +722,8 @@ def update_route_priority_on_peer(
f"{peer.id}. Error: {remote_client.err()}"
)
remote_client = remote_client.ok()
self_node_peer: NodePeer = context.node.settings.to(NodePeer)
result = remote_client.api.services.network.update_route_priority(
peer=self_node_peer,
peer_verify_key=context.credentials,
route=route,
priority=priority,
called_by_peer=True,
Expand All @@ -741,7 +738,7 @@ def update_route_priority_on_peer(
def update_route_priority(
self,
context: AuthedServiceContext,
peer: NodePeer,
peer_verify_key: SyftVerifyKey,
route: NodeRoute,
priority: int | None = None,
called_by_peer: bool = False,
Expand All @@ -751,24 +748,24 @@ def update_route_priority(
Args:
context (AuthedServiceContext): The authentication context for the service.
peer (NodePeer): The peer for which the route priority needs to be updated.
peer_verify_key (SyftVerifyKey): The verify key of the peer whose route priority needs to be updated.
route (NodeRoute): The route for which the priority needs to be updated.
priority (int | None): The new priority value for the route. If not
provided, it will be assigned the highest priority among all peers
Returns:
SyftSuccess | SyftError: Successful / Error response
"""
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__}: "
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
12 changes: 6 additions & 6 deletions tests/integration/network/gateway_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ def test_delete_route(set_env_var, gateway_port: int, domain_1_port: int) -> Non

# delete the added route
res = gateway_client.api.services.network.delete_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 @@ -566,14 +566,14 @@ def test_delete_route_on_peer(

# gateway delete the routes for the domain
res = gateway_client.api.services.network.delete_route_on_peer(
peer=domain_peer, route_id=new_route.id
peer_verify_key=domain_peer.verify_key, route_id=new_route.id
)
assert isinstance(res, SyftSuccess)
gateway_peer = domain_client.peers[0]
assert len(gateway_peer.node_routes) == 2

res = gateway_client.api.services.network.delete_route_on_peer(
peer=domain_peer, route=new_route2
peer_verify_key=domain_peer.verify_key, route=new_route2
)
assert isinstance(res, SyftSuccess)
gateway_peer = domain_client.peers[0]
Expand All @@ -582,7 +582,7 @@ def test_delete_route_on_peer(
# gateway deletes the last the route to it for the domain
last_route: NodeRouteType = gateway_peer.node_routes[0]
res = gateway_client.api.services.network.delete_route_on_peer(
peer=domain_peer, route=last_route
peer_verify_key=domain_peer.verify_key, route=last_route
)
assert isinstance(res, SyftSuccess)
assert "There is no routes left" in res.message
Expand Down Expand Up @@ -635,7 +635,7 @@ def test_update_route_priority(

# update the priorities for the routes
res = gateway_client.api.services.network.update_route_priority(
peer=domain_peer, route=new_route, priority=5
peer_verify_key=domain_peer.verify_key, route=new_route, priority=5
)
assert isinstance(res, SyftSuccess)
domain_peer = gateway_client.api.services.network.get_all_peers()[0]
Expand All @@ -645,7 +645,7 @@ def test_update_route_priority(
assert routes_port_priority[new_route.port] == 5

res = gateway_client.api.services.network.update_route_priority(
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 Down

0 comments on commit 4f68061

Please sign in to comment.