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

feat: Add P2P functionality #666

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
Open

feat: Add P2P functionality #666

wants to merge 9 commits into from

Conversation

aidan46
Copy link
Contributor

@aidan46 aidan46 commented Jan 7, 2025

Description

This PR adds bootstrap and registration p2p node functionality to the polka-storage-provider-server. The node type is selected by passing the node-type argument into the CLI (bootstrap or registration). Configuration is read from a TOML file, see the example in the p2p-example-config folder. The P2P network supports an ED25519 private key in PEM file format.

The generate-peer-id command has been added to the polka-storage-provider-client. This command generates a peer ID from an ED25519 public key PEM file and writes it to a file or stdout, depending on the --file argument.

Note:
I have not added any P2P discovery, this will be implemented in the fetching application. I can add a discovery example if needed.

Add P2P bootstrap and register behaviour to the polka-storage-provider-server.
@aidan46 aidan46 self-assigned this Jan 7, 2025
@aidan46 aidan46 added the enhancement New feature or request label Jan 7, 2025
@aidan46 aidan46 added this to the Phase 3 milestone Jan 7, 2025
@aidan46 aidan46 linked an issue Jan 7, 2025 that may be closed by this pull request
@aidan46 aidan46 linked an issue Jan 7, 2025 that may be closed by this pull request
Cargo.toml Show resolved Hide resolved
storage-provider/client/src/commands/mod.rs Show resolved Hide resolved
storage-provider/client/src/commands/mod.rs Outdated Show resolved Hide resolved
storage-provider/server/p2p-example-config/private.pem Outdated Show resolved Hide resolved
storage-provider/server/p2p-example-config/bootstrap.toml Outdated Show resolved Hide resolved
storage-provider/server/src/main.rs Show resolved Hide resolved
storage-provider/server/src/main.rs Show resolved Hide resolved
storage-provider/server/src/p2p.rs Outdated Show resolved Hide resolved
@aidan46 aidan46 force-pushed the feat/peer-resolver branch from 13fb051 to a3e3803 Compare January 7, 2025 12:40
@@ -52,24 +55,29 @@ pub enum P2PError {
P2PTransportError(#[from] libp2p::TransportError<std::io::Error>),
}

fn create_keypair<P: AsRef<Path> + std::fmt::Debug>(path: P) -> Result<Keypair, P2PError> {
pub fn create_keypair<P: AsRef<Path> + std::fmt::Debug>(path: P) -> Result<Keypair, P2PError> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it need to be a P2P error? Or do they wrap around something more specific we could use?

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we also support the @ dialect or its not practical?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this function the P2P error wraps around ed25519_dalek::pkcs8::Error and libp2p::identity::DecodingError.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean support to @ dialect when using a PEM file and otherwise put the private key straight into the config file?

Copy link
Contributor

Choose a reason for hiding this comment

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

In this function the P2P error wraps around ed25519_dalek::pkcs8::Error and libp2p::identity::DecodingError.

We could probably use the lower one for more accurate errors? Not sure, not super important

Do you mean support to @ dialect when using a PEM file and otherwise put the private key straight into the config file?

Yes

@aidan46 aidan46 added the ready for review Review is needed label Jan 10, 2025
@aidan46 aidan46 added ready for review Review is needed and removed ready for review Review is needed labels Jan 10, 2025
@@ -0,0 +1,3 @@
keypair = "@storage-provider/server/p2p-example-config/private.pem"
Copy link
Contributor

Choose a reason for hiding this comment

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

wrong path, should be to the '@examples`

@@ -198,7 +198,7 @@ impl pallet_storage_provider::Config for Test {
type RuntimeEvent = RuntimeEvent;
type Randomness = DummyRandomnessGenerator<Self>;
type AuthorVrfHistory = DummyRandomnessGenerator<Self>;
type PeerId = BoundedVec<u8, ConstU32<32>>; // Max length of SHA256 hash
type PeerId = BoundedVec<u8, ConstU32<42>>; // https://github.com/libp2p/specs/blob/master/peer-ids/peer-ids.md#peer-ids
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it should be a const in primitives?

token: CancellationToken,
) -> Result<(), P2PError> {
info!("Starting P2P bootstrap node");
let tracker = TaskTracker::new();
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason for using tracker here? Can't we just spawn the task and await it in a select?

rendezvous_point,
rendezvous_point_address,
None,
Namespace::from_static("polka-storage"),
Copy link
Contributor

Choose a reason for hiding this comment

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

const


/// Register the peer with the rendezvous point.
/// The ttl is how long the peer will remain registered in seconds.
async fn register(
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand why we need to have two different swarms :( I'll need to read up on that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ready for review Review is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: Implement public key to Peer ID conversion feat: Implement resolver in sp-server
3 participants