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.

Sign up for GitHub

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

cmd: allow to set the retrieval peerID, not just the retrieval multiaddr #430

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

cmd: allow to set the retrieval peerID, not just the retrieval multiaddr #430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/provider/daemon.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ func daemonCommand(cctx *cli.Context) error {
engine.WithHttpPublisherAnnounceAddr(cfg.Ingest.HttpPublisher.AnnounceMultiaddr),
engine.WithPubsubAnnounce(!cfg.DirectAnnounce.NoPubsubAnnounce),
engine.WithSyncPolicy(syncPolicy),
engine.WithRetrievalPeerID(cfg.ProviderServer.RetrievalPeerID),
engine.WithRetrievalAddrs(cfg.ProviderServer.RetrievalMultiaddrs...),
)
if err != nil {
Expand Down
3 changes: 3 additions & 0 deletions cmd/provider/internal/config/providerserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ package config
type ProviderServer struct {
// ListenMultiaddr is the multiaddr string for the node's listen address
ListenMultiaddr string
// RetrievalPeerID is the peer ID to advertise for data retrieval.
// Defaults to the provider's libp2p host peer ID.
RetrievalPeerID string
// RetrievalMultiaddrs are the addresses to advertise for data retrieval.
// Defaults to the provider's libp2p host listen addresses.
RetrievalMultiaddrs []string
Expand Down
16 changes: 16 additions & 0 deletions engine/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,22 @@ func WithRetrievalAddrs(addrs ...string) Option {
}
}

// WithRetrievalPeerID sets the peerID of where to get the content corresponding to an indexing advertisement.
// If unspecified, the libp2p host peer ID are used.
// See: WithHost
func WithRetrievalPeerID(peerID string) Option {
Copy link
Member

Choose a reason for hiding this comment

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

This is bound to provider libp2p host, since the identity is needed for signing of advertisements.

Have you considered setting the engine host option instead?

Copy link
Author

Choose a reason for hiding this comment

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

You mean just using WithHost and set the full retrieval host? Wouldn't that cause problems?

Copy link
Member

Choose a reason for hiding this comment

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

It shouldn't. The identity of a provider is tied to what it uses to sign advertisements and where chain is retrievable from.
The reason for RetrievalAddrs option existing is that a libp2p node may have any public addr through DNS for example, that may not directly be known by it. This is similar to setting public hostname in a traditional HTTP server.

But the peer ID (or identity by extension) must be known to the host itself and must match the ID other peers expect to encounter when fetching ad chain from it.

Copy link

@EnchanterIO EnchanterIO Feb 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

For a bite more ctx, we have this PR deployed and it generates the following advertisements:

CID:          baguqeera7miiexnupzmlrusq2s5u2xd4ngqrsqormpjdtl6hkq73w2xqtkca
...
Signature: ✅ valid
Signed by: advertisement publisher

Copy link
Member

Choose a reason for hiding this comment

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

@EnchanterIO What other options are set in the engine that is deployed? I suspect the reason that works is that the peer ID set via option introduced by this PR matches the host identity?

Copy link

Choose a reason for hiding this comment

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

It does not match the identity, contrary, the problem we try to address with this PR is that if retrievalAddr is not empty, the host id (identity of index provider) is inserted into adv - but we want our separate p2p-server bitswap provider peer id coming from this option to be inserted into adv instead.

Copy link
Member

Choose a reason for hiding this comment

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

Got it thank you

In IPNI terminology they are referred to as "Provider" and "Publisher". Provider provides the actual downloadable content , and Publisher tells the network of what is available for download from where i.e. expose the advertisement chain.

IIUC what you'd like to be able to do is to use the engine as a publisher for providers that may reside elsewhere. Right?

Related note: I recommend looking at Extended Providers in IPNI spec.

Copy link
Author

@MichaelMure MichaelMure Feb 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

IIUC what you'd like to be able to do is to use the engine as a publisher for providers that may reside elsewhere. Right?

That is correct.

return func(o *options) error {
if len(peerID) != 0 {
pId, err := peer.Decode(peerID)
if err != nil {
return fmt.Errorf("bad peer ID %q: %w", peerID, err)
}
o.provider.ID = pId
}
return nil
}
Copy link
Collaborator

@gammazero gammazero Feb 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This functionality is already available by using the WithProvider and WithPrivateKey options. The WithProvider option sets the peer and addresses for the provider to put in indexing advertisements. This is different than WithHost which sets the host that is publishing the advertisements (libp2p server).

Setting the provider ID should also be accompanied by a call to WithPrivateKey, to set the private key associated with the provider peer ID being set. This way the advertisements will be signed with the provider's key.

The only reason this PR currently works is because the indexer is currently configured to allow any ID to publish on behalf of a provider. This means the advertisement will be accepted if it has a valid signature from anyone. This configuration is long overdue for being locked down so that the indexer only accepts advertisements signed by the provider or signed by a peer from a specific list of delegated publishers for that provider.

}

func WithSyncPolicy(syncPolicy *policy.Policy) Option {
return func(o *options) error {
o.syncPolicy = syncPolicy
Expand Down
Loading