fix: market: Reuse the market PubSub in index provider
#8443
Conversation
| // Join the indexer topic using the market's pubsub instance. Otherwise, the provider | ||
| // engine would create its own instance of pubsub down the line in go-legs, which has | ||
| // no validators by default. | ||
| t, err := ps.Join(cfg.TopicName) |
There was a problem hiding this comment.
why should the market process be subscribed to the topic? it shouldn't be receiving or interested in any messages on this topic - it only produced the messages and we don't want it to relay or receive them.
There was a problem hiding this comment.
To publish we also need a topic instance; at least go-legs publisher needs it. Is there a way to publish to a topic without joining it?
Also, the changes here are equivelant to what legs publisher does internally if topic is not set.
Subscribing to a topic (i.e. t.Subscribe) is another explicit call. .Join does not mean subscription I don't think.
There was a problem hiding this comment.
yes, you can publish without subscribing.
There was a problem hiding this comment.
but we normally do it by calling publish directly, without a topic.
There was a problem hiding this comment.
and thats totally ok, getting the topic to publish is the caconical way since topic handles were added.
PubSub instance in index provider enginePubSub instance in index provider and set remaining config
PubSub instance in index provider and set remaining configPubSub in index provider and set all options
|
I think this will change markets to use the standard pubsub setup / validator, while it currently only has the one built in within legs. |
82351d8 to
cc2f805
Compare
PubSub in index provider and set all optionsPubSub in index provider
There was a problem hiding this comment.
Actually this makes tests really unhappy:
ensemble.go:647:
Error Trace: ensemble.go:647
ensemble_presets.go:22
deals_512mb_test.go:29
Error: Received unexpected error:
starting node:
github.com/filecoin-project/lotus/node.New
/home/circleci/project/node/builder.go:373
- could not build arguments for function "github.com/filecoin-project/lotus/node/modules".HandleDeals
/home/circleci/project/node/modules/storageminer.go:305:
failed to build storagemarket.StorageProvider:
could not build arguments for function "reflect".makeFuncStub
/usr/local/go/src/reflect/asm_amd64.s:14:
failed to build provider.Interface:
could not build arguments for function "reflect".makeFuncStub
/usr/local/go/src/reflect/asm_amd64.s:14:
failed to build *pubsub.PubSub:
missing dependencies for function "reflect".makeFuncStub
/usr/local/go/src/reflect/asm_amd64.s:14:
missing types:
- dtypes.BootstrapPeers (did you mean to Provide it?)
- dtypes.DrandBootstrap (did you mean to Provide it?)
- dtypes.DrandSchedule (did you mean to Provide it?)
It would seem that markets don't have a pubsub instance by default, so it should be constructed in the indexer (and maybe there's a bug where it's constructed there multiple times?).
The DI for The issue is that DI for |
Which is expected, and that should mean that it's fine to just use create pubsub in go-legs (but it also seems like we somehow have multiple pubsub instances somehow?) |
The markets process instantiates its own `PubSub` instance with all validators, peer scoring, etc. set up. Use that instane to join the indexing topic, otherwise the default topic instantiated by index-provider internally (via go-legs) has no validators.
Bind drand and bootstrap peers config so that `PubSub` instantiated by `ConfigCommon` has all the dependencies it needs when `PubSub` instance is needed in markets. In ths case, the instance is needed by the index provider engine to announce new indexing advertisements.
cc2f805 to
f369d99
Compare
Codecov Report
@@ Coverage Diff @@
## master #8443 +/- ##
==========================================
+ Coverage 34.56% 40.60% +6.03%
==========================================
Files 681 686 +5
Lines 75174 75415 +241
==========================================
+ Hits 25984 30619 +4635
+ Misses 44340 39483 -4857
- Partials 4850 5313 +463
|
Related Issues
N/A
Proposed Changes
The markets process instantiates its own
PubSubinstance with all validators, peer scoring, etc. set up. Use that instance to join the indexing topic, otherwise the default topic instantiated by index-provider internally (via go-legs) has no validators.Fixes the construction of
PubSubvia dependency injection mechanism in markets by binding its dependencies: bootstrap peers, drand related config.Looks like DI forPubSubin markets is broken since it seems it was never used in markets before; waiting for confirmation that we indeed want validators for the gossipsub used by provider engine before spending time on fixing it.Additional Info
N/A
Checklist
Before you mark the PR ready for review, please make sure that:
<PR type>: <area>: <change being made>fix: mempool: Introduce a cache for valid signaturesPR type: fix, feat, INTERFACE BREAKING CHANGE, CONSENSUS BREAKING, build, chore, ci, docs,perf, refactor, revert, style, testarea: api, chain, state, vm, data transfer, market, mempool, message, block production, multisig, networking, paychan, proving, sealing, wallet, deps