Skip to content

Conversation

@noot
Copy link
Contributor

@noot noot commented Jul 11, 2025

  • implement libp2p node with basic protocols (ping, identify, autonat, mdns, kademlia) as well as customizable request-response protocols (based on the existing request-response protocols implemented with iroh)
  • implement worker with request handling
  • implement validator and orchestrator with request sending and response handling
  • remove iroh and other now unused code

@noot noot requested a review from Copilot July 11, 2025 03:20

This comment was marked as outdated.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR replaces the existing iroh-based P2P implementation with a new libp2p-based p2p crate, refactors state handling to store keypairs instead of seeds/IDs, and wires up request–response channels in the worker, validator, and orchestrator.

  • Remove old iroh seed/ID utilities and update SystemState to persist p2p_keypair via custom serde functions.
  • Introduce a new Service for P2P in the worker, validator, and orchestrator, using request–response protocols (authentication, hardware challenge, invite, logs, restart).
  • Update CLI, heartbeat, Docker bridge, and API routes to send and receive P2P messages through oneshot channels and p2p::Service.

Reviewed Changes

Copilot reviewed 45 out of 47 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
crates/worker/src/utils/p2p.rs Removed old iroh-based seed and node ID generation utilities.
crates/worker/src/state/system_state.rs Refactored persisted state to store p2p_keypair with custom serde.
crates/worker/src/p2p/mod.rs Added new libp2p-based Service handling request–response protocols.
crates/validator/src/validators/hardware_challenge.rs Switched to sending hardware challenges over channels via p2p.
crates/orchestrator/src/api/routes/nodes.rs Reworked restart/logs routes to use oneshot requests/responses.
Comments suppressed due to low confidence (3)

crates/worker/src/operations/heartbeat/service.rs:179

  • [nitpick] Since p2p_id is always provided, consider making the heartbeat request field non-optional to simplify the signature and remove the Option<String> wrapper.
            p2p_id: Some(p2p_id.to_string()), // TODO: this should always be `Some`

crates/worker/src/p2p/mod.rs:1

  • The new P2P Service has no dedicated unit or integration tests. Consider adding tests for build_p2p_node, run, and message handling to ensure correct protocol behavior.
use anyhow::Context as _;

crates/worker/src/docker/service.rs:184

  • The ${WORKER_P2P_SEED} placeholder replacement was removed but the template may still include it. Ensure any task command templates are updated to remove or handle this placeholder.
                                                    arg.replace("${SOCKET_PATH}", &task_bridge_socket_path)

Copy link
Member

@JannikSt JannikSt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! The basic flow of nodes joining the orchestrator works smooth now! Some comments attached.

@noot noot merged commit 1bb7f87 into main Jul 11, 2025
1 check passed
@noot noot deleted the noot/libp2p-unified branch July 11, 2025 19:45
@Doordashcon
Copy link

nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants