Support exposing bitswap publicly #967

hannahhoward · 2022-11-20T01:00:22Z

Goals

Implements #951

Uses index provider families to switch to advertising bitswap as an extended provider in the indexer-- this allows a provider to expose booster-bitswap directly as a public node.

Implementation

unfortunately, this has two dependent PRs for the time being, cause the indexprovider package needed a major update which triggered other deps updates -- chore(deps): update index provider 0.9.1 lotus#9688 & Upgrade to index provider v0.9.1 go-fil-markets#766
per Support exposing booster bitswap peer directly to indexer via extended providers #951 description, adds two config vars for public addresses and the location of a private key for booster bitswap
if public addresses are not present, proxy approach still works:
- adds BItswap peer id to config of protocol proxy
- advertises Bitswap records for the main peer id
- NOTE: rather than announcing bitswap per deal any more, we announce bitswap as a supported protocol on an extended provider with the same peer ID as boost
if public address are present, annouces them as extended provider
- extended provider for bitswap is announced in indexer with peerKey + addrs of the bitswap node
- booster-bitswap should be started w/o forwarding
- also makes sure these addresses are advertised instead of the main peer in the support transport endpoint

For discussion

This PR could probably lose the Lotus update if we moved the index provider code we are depending on over to boost. Why are we relying on this code from Lotus? It seems like it should live in Boost.
Do we need to bother with trying to unannounce individual bitswap records?

willscott

This seems generally reasonable.

@ischasny should also review

willscott · 2022-11-20T16:56:27Z

indexprovider/wrapper.go

+		var eps []xproviders.Info
+
+		// add bitswap extended provider as needed
+		if cfg.Dealmaking.BitswapPeerID != "" {


wonder if we can jump directly to this being an array / support multiple bitswap peers?

would rather not jump anything? we'd need to handle multiple private ids too.

willscott · 2022-11-20T16:58:35Z

indexprovider/wrapper.go

+			if err != nil {
+				return nil, err
+			}
+			if len(cfg.Dealmaking.BitswapPublicAddresses) > 0 {


so not setting addresses on your bitswap config means that it'll use the main markets host for advertisements? that seems like the wrong thing to condition on - wouldn't it instead be if the bitswap peer ID matches the host ID?

no. In this case, you still use the bitswap peer id but it now uses the protocol proxy to forward bitswap to that separate peer id while publishing the main boost peer id with the indexer.

willscott · 2022-11-20T17:01:13Z

indexprovider/wrapper.go

 		}
 		// announce all deals on startup in case of a config change
 		lc.Append(fx.Hook{
 			OnStart: func(ctx context.Context) error {
 				go func() {
-					_ = w.IndexerAnnounceAllDeals(ctx)
+					_ = w.AnnounceExtendedProviders(ctx)


this change (from calling IndexerAnnounceAllDeals to calling AnnounceExtendedProviders) will now just update the multiaddrs / if bitswap is running.
Before, that method on startup would try to reconcile what was announced with the known deals.
where will we do that now?

That method just got added to the start with booster-bitswap previous release, so I don't think it's all that important. We could run it though for other reasons?

ischasny · 2022-11-21T12:34:59Z

go.mod

 	github.com/filecoin-project/go-statestore v0.2.0
-	github.com/filecoin-project/index-provider v0.8.1
-	github.com/filecoin-project/lotus v1.18.0-rc3
+	github.com/filecoin-project/index-provider v0.9.2-0.20221119192528-622ddb90df0e


v0.9.2 has been cut, please use it

ischasny · 2022-11-21T12:39:56Z

indexprovider/wrapper.go

+		return nil
+	}
+	adBuilder := xproviders.NewAdBuilder(w.h.ID(), w.h.Peerstore().PrivKey(w.h.ID()), w.h.Addrs())
+	adBuilder.WithExtendedProviders(w.extendedProviders...)


If you want a bitswap endpoint to also be added for the main provider, don't forget to call .WithMetadata(...) on the builder.

hannahhoward · 2022-11-23T04:55:52Z

moved the indexer code cause it was the right thing to do, but still not able to avoid a Lotus deps update

uses index provider families to switch to advertising bitswap as an extended provider -- this allows us to expose booster-bitswap directly

node/config/types.go

dirkmc · 2022-12-05T19:59:05Z

node/modules/retrieval.go

@@ -51,9 +51,20 @@ func NewTransportsListener(cfg *config.Boost) func(h host.Host) (*lp2pimpl.Trans
 			})
 		}
 		if cfg.Dealmaking.BitswapPeerID != "" {
+			addrs := h.Addrs()
+			if len(cfg.Dealmaking.BitswapPublicAddresses) > 0 {


Let's add a comment explaining what this code is doing (and why we need it)

dirkmc · 2022-12-05T20:00:12Z

indexprovider/wrapper.go

+			if err != nil {
+				return nil, err
+			}
+			if len(cfg.Dealmaking.BitswapPublicAddresses) > 0 {


Let's add a comment explaining what this code is doing (and why we need it)

Co-authored-by: dirkmc <dirkmdev@gmail.com>

hannahhoward · 2022-12-08T09:39:52Z

closing in favor of #1030

LexLuthr · 2022-12-08T10:28:53Z

indexprovider/wrapper.go

+	legacyProv  lotus_storagemarket.StorageProvider
+	prov        provider.Interface
+	dagStore    *dagstore.Wrapper
+	meshCreator idxprov.MeshCreator


@hannahhoward Can we copy over idxprov from Lotus to Boost?

hannahhoward force-pushed the feat/update-indexer-announce branch 2 times, most recently from efdd08b to 0ccfa8c Compare November 20, 2022 01:29

hannahhoward requested review from dirkmc and willscott November 20, 2022 01:30

willscott reviewed Nov 20, 2022

View reviewed changes

ischasny reviewed Nov 21, 2022

View reviewed changes

hannahhoward requested a review from masih November 21, 2022 16:41

hannahhoward added 3 commits November 22, 2022 20:57

feat(booster-bitswap): support exposing bitswap publicly

9df537d

uses index provider families to switch to advertising bitswap as an extended provider -- this allows us to expose booster-bitswap directly

chore(docs): cleanup build and docs

83d2fe5

refactor(node): move over Lotus index provider code

975c488

hannahhoward force-pushed the feat/update-indexer-announce branch from d5e4879 to 975c488 Compare November 23, 2022 04:59

jacobheun mentioned this pull request Nov 25, 2022

Announce Bitswap service multiaddrs to the Network Indexers #711

Closed

hannahhoward mentioned this pull request Dec 5, 2022

Release: Booster bitswap #1030

Merged

dirkmc reviewed Dec 5, 2022

View reviewed changes

hannahhoward mentioned this pull request Dec 8, 2022

Decouple Index-provider for Boost from Lotus #1033

Closed

hannahhoward and others added 3 commits December 7, 2022 18:51

Update node/config/types.go

96e008b

Co-authored-by: dirkmc <dirkmdev@gmail.com>

Update node/config/types.go

8b8e1a1

Co-authored-by: dirkmc <dirkmdev@gmail.com>

refactor(indexprovider): clean up bitswap announcements

893d342

hannahhoward closed this Dec 8, 2022

LexLuthr reviewed Dec 8, 2022

View reviewed changes

nonsense deleted the feat/update-indexer-announce branch January 17, 2023 15:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support exposing bitswap publicly #967

Support exposing bitswap publicly #967

hannahhoward commented Nov 20, 2022

willscott left a comment

willscott Nov 20, 2022

hannahhoward Dec 8, 2022

willscott Nov 20, 2022

hannahhoward Dec 8, 2022 •

edited

Loading

willscott Nov 20, 2022

hannahhoward Dec 8, 2022

ischasny Nov 21, 2022

hannahhoward Dec 8, 2022

ischasny Nov 21, 2022

hannahhoward commented Nov 23, 2022

dirkmc Dec 5, 2022

dirkmc Dec 5, 2022

hannahhoward commented Dec 8, 2022

LexLuthr Dec 8, 2022

Support exposing bitswap publicly #967

Support exposing bitswap publicly #967

Conversation

hannahhoward commented Nov 20, 2022

Goals

Implementation

For discussion

willscott left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hannahhoward Dec 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hannahhoward commented Nov 23, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hannahhoward commented Dec 8, 2022

Choose a reason for hiding this comment

hannahhoward Dec 8, 2022 •

edited

Loading