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: Move bootstrap node into collator #727

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

Conversation

aidan46
Copy link
Contributor

@aidan46 aidan46 commented Feb 4, 2025

Description

Move the P2P bootstrap node from the polka-storage-provider-server into the collator. All the examples have been updated to reflect this change.
The zombienet setup has been updated to include the p2p key and the p2p listen address.

@aidan46 aidan46 self-assigned this Feb 4, 2025
@aidan46 aidan46 added the enhancement New feature or request label Feb 4, 2025
@aidan46 aidan46 added this to the Phase 3 milestone Feb 4, 2025
@aidan46 aidan46 linked an issue Feb 4, 2025 that may be closed by this pull request
@aidan46 aidan46 added ready for review Review is needed and removed ready for review Review is needed labels Feb 4, 2025
node/src/cli.rs Outdated Show resolved Hide resolved
node/src/cli.rs Outdated Show resolved Hide resolved
node/src/cli.rs Outdated Show resolved Hide resolved
node/src/command.rs Outdated Show resolved Hide resolved
node/src/service/mod.rs Outdated Show resolved Hide resolved
zombienet/local-testnet.toml Outdated Show resolved Hide resolved
zombienet/private.pem Outdated Show resolved Hide resolved
maat/src/lib.rs Outdated Show resolved Hide resolved
node/src/command.rs Show resolved Hide resolved
primitives/src/p2p.rs Outdated Show resolved Hide resolved
@aidan46 aidan46 force-pushed the 720-feat-move-p2p-bootstrap-into-collator branch from e22fdc2 to 858e03a Compare February 6, 2025 04:45
@aidan46 aidan46 added ready for review Review is needed and removed ready for review Review is needed labels Feb 6, 2025
@cernicc
Copy link
Member

cernicc commented Feb 6, 2025

I am wondering. We already have a peerid which is derived from the provider's address. If I am not mistaken. That peerid is later used in deals etc. How is the p2p peer id used? I imagine those two peerid should be the same?

@cernicc
Copy link
Member

cernicc commented Feb 6, 2025

Sooo. I was thinking about the retrieval. Currently the p2p network and polkadot parachain store different peer ids for the same provider. That means that our custom p2p network only knows how to map the p2p peer id to the multiaddress. The idea for "retrieval name resolver" was that we fetch the peer id for a specific deal from the chain. But that currently wouldn't work because the one stored on the chain is different from the one used by the custom p2p network.

Please correct me if I am wrong.

@aidan46
Copy link
Contributor Author

aidan46 commented Feb 7, 2025

I am wondering. We already have a peerid which is derived from the provider's address. If I am not mistaken. That peerid is later used in deals etc. How is the p2p peer id used? I imagine those two peerid should be the same?

Sooo. I was thinking about the retrieval. Currently the p2p network and polkadot parachain store different peer ids for the same provider. That means that our custom p2p network only knows how to map the p2p peer id to the multiaddress. The idea for "retrieval name resolver" was that we fetch the peer id for a specific deal from the chain. But that currently wouldn't work because the one stored on the chain is different from the one used by the custom p2p network.

Please correct me if I am wrong.

@cernicc The Peer ID on chain and the Peer ID in the P2P network are both derived from the same ED25519 private key. When the storage provider first registers on-chain they create an ED25519 private key, derive the Peer ID using the polka-storage-client and register themselves on-chain with this Peer ID. Basically the Peer ID on-chain is something that is generated externally and is the same that the storage provider uses to register within the P2P network

@aidan46 aidan46 force-pushed the 720-feat-move-p2p-bootstrap-into-collator branch from 9ba91a2 to 067fc01 Compare February 7, 2025 05:52
@aidan46 aidan46 added ready for review Review is needed and removed ready for review Review is needed labels Feb 7, 2025
@aidan46 aidan46 force-pushed the 720-feat-move-p2p-bootstrap-into-collator branch from 067fc01 to ffe5666 Compare February 7, 2025 10:26
@jmg-duarte jmg-duarte added ready for review Review is needed and removed ready for review Review is needed labels Feb 7, 2025
@pete-eiger
Copy link
Contributor

looks great. Should we update the documentation for p2p_key to specify that it is used both for on–chain registration and for the custom P2P network? This may reduce any confusion about why two different peer IDs aren’t generated

Also I like the approach of making the p2p arguments optional except when running as an authority. Perhaps we could have clap enforce these conditionally (if possible) to give users immediate feedback rather than only at runtime?

run-testnet:
openssl genpkey -algorithm ED25519 -out /tmp/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.

nit: maybe we should prefix this /tmp/polka-storage/private.pem, to avoid potential conflicts with different software.

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: Move P2P bootstrap into collator
5 participants