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: Implement Boost equivalent deal market in Curio #135

Merged
merged 23 commits into from
Sep 17, 2024

Conversation

LexLuthr
Copy link
Contributor

@LexLuthr LexLuthr commented Aug 7, 2024

How the new market works:

Modules:

Libp2p

Takes in all minerIDs and start a libp2p node for each. Table for storing address, key etc

MK12 Deal handler

Starts listening to deal protocols on each libp2p and then forwards the incoming requests to MK12 market module.

Deal market

Main poller process which starts all relevant modules based on config. For now, we only have one module Mk12. This module exposes a method called ExecuteDeal(). This method is used by Libp2p handler to forward the deal or execution from libp2p.
Deal market also load the ingestor modules (snap and normal deal flow) which assigns the deals from market to sectors

Market tasks

We have 3 different tasks in market with their own pollers based on a deal pipeline state

  1. CommP - Performs commP using remote read of pieces for all deals
  2. PSD - send a PSD message for a set of deals per minerID
  3. FindDeal - Finds and assigns the on chain deal ID to each deals after PSD

Indexing

Indexing task is a IAmBored. The entry is made in a dedicated table after MoveStorage task and this in turn triggers indexing of the deal. Once indexed, deal can be removed from the mk12 deal pipeline and piece/deal metadata is updated.

Index Store

A new store which exclusively is used for indexing based on Cassandra.

Pending

  • When to start offline deal

    • Should there be a manual trigger for all offline deals
    • Should we have import-data equivalent
  • Index-Provider

    • We should probably do our own version instead of using index-provider library
    • lib-ipni should have all we need in communication protocols
  • Libp2p options

Deal Flow

Deal on Libp2p > libp2p hanlder (handle the deal) > Call MK12 modules inside the Deal market > Work through Market tasks > Deal market adds to sector pipeline using Ingestor > Create Indexing task (Move Storage) > Index > GC cleanup of deal pipeline

TODO:

  • Add description of functionality in PR
  • Add UI
  • Figure out Offline deal trigger (? import from file ?)
  • Deal retry with new sector
  • Clean up
  • comment config
  • Add IndexStore tests
  • Add docs
  • Remove GetIndex
  • Use Curio cfg in all new funcs
  • Should we clean up market_offline_urls table after a URL is used? Should this be generic or deal specific?
  • Complete libp2p setup
  • Clean up failed deal/fix GC
  • MarketAddBalance for SP side with PSD wallet
  • Backpressure

Note

Remaining feature will be added in separate PRs. This is ready for review and merge.

@LexLuthr LexLuthr force-pushed the market-migration branch 9 times, most recently from 6b8ec18 to c890aca Compare August 8, 2024 11:32
cmd/curio/config_test.go Show resolved Hide resolved
cmd/curio/tasks/tasks.go Outdated Show resolved Hide resolved
cmd/curio/tasks/tasks.go Show resolved Hide resolved
lib/indexing/indexstore/indexstore.go Outdated Show resolved Hide resolved
market/dealmarket/deal_ingest_common.go Outdated Show resolved Hide resolved
lib/libp2p/libp2p.go Outdated Show resolved Hide resolved
market/dealmarket/deal_ingest_snap.go Outdated Show resolved Hide resolved
tasks/indexing/task_indexing.go Show resolved Hide resolved
tasks/indexing/task_indexing.go Outdated Show resolved Hide resolved
tasks/market/task_find_deal.go Outdated Show resolved Hide resolved
tasks/market/task_find_url.go Outdated Show resolved Hide resolved
tasks/market/task_psd.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@magik6k magik6k left a comment

Choose a reason for hiding this comment

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

First pass, in general this is moving in the right direction.

I'm really excited to start testing this when we get closer to completion, it will make my life so, so much easier.

lib/indexing/indexstore/create.cql Outdated Show resolved Hide resolved
deps/config/types.go Outdated Show resolved Hide resolved
deps/config/types.go Outdated Show resolved Hide resolved
deps/config/types.go Outdated Show resolved Hide resolved
deps/config/types.go Outdated Show resolved Hide resolved
tasks/market/task_find_url.go Outdated Show resolved Hide resolved
tasks/market/task_psd.go Outdated Show resolved Hide resolved
tasks/market/task_psd.go Outdated Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

We might want to add some flag to piece refs / piece park pieces which says that the piece is coming from an external, potentially untrusted source, eventually adding more special handling here.

(Also eventually one day we should add aria2 support here)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also kinda tangential, but would be really cool to have a column in the parked_pieces table for download progress updated e.g. every 5s

tasks/seal/task_movestorage.go Outdated Show resolved Hide resolved
@LexLuthr LexLuthr changed the base branch from main to feat/market August 29, 2024 16:02
* temp: replace lotus to local

* move storiface to Curio

* fix build

* don't refer to storiface2

* make gen

* make remote path test happy

* cleanup storiface
@LexLuthr LexLuthr marked this pull request as ready for review August 30, 2024 16:13
@LexLuthr LexLuthr requested review from snadrus and magik6k September 1, 2024 10:30
Copy link
Collaborator

@magik6k magik6k left a comment

Choose a reason for hiding this comment

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

Wow this is big

Generally I'd say it's looking really good. I expect that as we run this some things will need adjusting, but on a high level this looks quite solid.

Lots of comments are more on the nicpicky side, but there are some that would be nice to address.

Overall feel free to merge into the base branch, the sooner we get this running in the real world the sooner we'll get this shipped.

Makefile Outdated Show resolved Hide resolved
cmd/curio/market.go Outdated Show resolved Hide resolved
cmd/curio/run.go Show resolved Hide resolved
cmd/curio/tasks/tasks.go Outdated Show resolved Hide resolved
deps/config/doc_gen.go Outdated Show resolved Hide resolved
tasks/storage-market/storage_market.go Outdated Show resolved Hide resolved
tasks/storage-market/storage_market.go Show resolved Hide resolved
tasks/storage-market/storage_market.go Outdated Show resolved Hide resolved
tasks/storage-market/task_commp.go Outdated Show resolved Hide resolved
tasks/storage-market/task_psd.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@magik6k magik6k left a comment

Choose a reason for hiding this comment

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

Honestly at this point I wouldn't do further reviews, looks really good.

Some minor comments, but other than that feel free to merge into the staging branch

cmd/curio/market.go Outdated Show resolved Hide resolved
cmd/curio/market.go Outdated Show resolved Hide resolved
deps/config/types.go Outdated Show resolved Hide resolved
market/libp2p/libp2p.go Show resolved Hide resolved
market/libp2p/libp2p.go Outdated Show resolved Hide resolved
market/indexstore/create.cql Outdated Show resolved Hide resolved
@LexLuthr LexLuthr merged commit e2e0411 into feat/market Sep 17, 2024
8 checks passed
@LexLuthr LexLuthr deleted the market-migration branch September 17, 2024 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants