cleanup: miner: remove markets and deal-making from Lotus Miner #12005

aarshkshah1992 · 2024-05-16T15:25:53Z

Related Issues

Closes #12004

Proposed Changes

The use of legacy markets was deprecated from Lotus Miner a long time ago in favour of Boost. This PR removes all legacy markets related code from the Lotus Miner. Lotus no longer depends on go-graphsync, go-data-transfer, go-fil-markets, dagstore and index-provider .

All Markets related functionality has been removed from the Lotus Miner CLI and the API. Specifically, the following Market APIs have been removed from a Lotus Miner API as part of this PR:

         MarketImportDealData(ctx context.Context, propcid cid.Cid, path string) error //perm:write
	// MarketListRetrievalDeals is deprecated, returns empty list
	MarketListRetrievalDeals(ctx context.Context) ([]struct{}, error)                                                                                                                    //perm:read
	MarketGetDealUpdates(ctx context.Context) (<-chan storagemarket.MinerDeal, error)                                                                                                    //perm:read
	MarketListIncompleteDeals(ctx context.Context) ([]storagemarket.MinerDeal, error)                                                                                                    //perm:read
	MarketSetAsk(ctx context.Context, price types.BigInt, verifiedPrice types.BigInt, duration abi.ChainEpoch, minPieceSize abi.PaddedPieceSize, maxPieceSize abi.PaddedPieceSize) error //perm:admin
	MarketGetAsk(ctx context.Context) (*storagemarket.SignedStorageAsk, error)                                                                                                           //perm:read
	MarketSetRetrievalAsk(ctx context.Context, rask *retrievalmarket.Ask) error                                                                                                          //perm:admin
	MarketGetRetrievalAsk(ctx context.Context) (*retrievalmarket.Ask, error)                                                                                                             //perm:read
	MarketListDataTransfers(ctx context.Context) ([]DataTransferChannel, error)                                                                                                          //perm:write
	MarketDataTransferUpdates(ctx context.Context) (<-chan DataTransferChannel, error)                                                                                                   //perm:write
	// MarketDataTransferDiagnostics generates debugging information about current data transfers over graphsync
	MarketDataTransferDiagnostics(ctx context.Context, p peer.ID) (*TransferDiagnostics, error) //perm:write
	// MarketRestartDataTransfer attempts to restart a data transfer with the given transfer ID and other peer
	MarketRestartDataTransfer(ctx context.Context, transferID datatransfer.TransferID, otherPeer peer.ID, isInitiator bool) error //perm:write
	// MarketCancelDataTransfer cancels a data transfer with the given transfer ID and other peer
	MarketCancelDataTransfer(ctx context.Context, transferID datatransfer.TransferID, otherPeer peer.ID, isInitiator bool) error //perm:write
	MarketPendingDeals(ctx context.Context) (PendingDealInfo, error)                                                             //perm:write
	MarketPublishPendingDeals(ctx context.Context) error                                                                         //perm:admin
	MarketRetryPublishDeal(ctx context.Context, propcid cid.Cid) error                                                           //perm:admin

	// DagstoreListShards returns information about all shards known to the
	// DAG store. Only available on nodes running the markets subsystem.
	DagstoreListShards(ctx context.Context) ([]DagstoreShardInfo, error) //perm:read

	// DagstoreInitializeShard initializes an uninitialized shard.
	//
	// Initialization consists of fetching the shard's data (deal payload) from
	// the storage subsystem, generating an index, and persisting the index
	// to facilitate later retrievals, and/or to publish to external sources.
	//
	// This operation is intended to complement the initial migration. The
	// migration registers a shard for every unique piece CID, with lazy
	// initialization. Thus, shards are not initialized immediately to avoid
	// IO activity competing with proving. Instead, shard are initialized
	// when first accessed. This method forces the initialization of a shard by
	// accessing it and immediately releasing it. This is useful to warm up the
	// cache to facilitate subsequent retrievals, and to generate the indexes
	// to publish them externally.
	//
	// This operation fails if the shard is not in ShardStateNew state.
	// It blocks until initialization finishes.
	DagstoreInitializeShard(ctx context.Context, key string) error //perm:write

	// DagstoreRecoverShard attempts to recover a failed shard.
	//
	// This operation fails if the shard is not in ShardStateErrored state.
	// It blocks until recovery finishes. If recovery failed, it returns the
	// error.
	DagstoreRecoverShard(ctx context.Context, key string) error //perm:write

	// DagstoreInitializeAll initializes all uninitialized shards in bulk,
	// according to the policy passed in the parameters.
	//
	// It is recommended to set a maximum concurrency to avoid extreme
	// IO pressure if the storage subsystem has a large amount of deals.
	//
	// It returns a stream of events to report progress.
	DagstoreInitializeAll(ctx context.Context, params DagstoreInitializeAllParams) (<-chan DagstoreInitializeAllEvent, error) //perm:write

	// DagstoreGC runs garbage collection on the DAG store.
	DagstoreGC(ctx context.Context) ([]DagstoreShardResult, error) //perm:admin

	// DagstoreRegisterShard registers a shard manually with dagstore with given pieceCID
	DagstoreRegisterShard(ctx context.Context, key string) error //perm:admin

	// IndexerAnnounceDeal informs indexer nodes that a new deal was received,
	// so they can download its index
	IndexerAnnounceDeal(ctx context.Context, proposalCid cid.Cid) error //perm:admin

	// IndexerAnnounceAllDeals informs the indexer nodes aboutall active deals.
	IndexerAnnounceAllDeals(ctx context.Context) error //perm:admin

	// DagstoreLookupPieces returns information about shards that contain the given CID.
	DagstoreLookupPieces(ctx context.Context, cid cid.Cid) ([]DagstoreShardInfo, error) //perm:admin
	
	DealsImportData(ctx context.Context, dealPropCid cid.Cid, file string) error //perm:admin
	DealsConsiderOnlineStorageDeals(context.Context) (bool, error)               //perm:admin
	DealsSetConsiderOnlineStorageDeals(context.Context, bool) error              //perm:admin
	DealsConsiderOnlineRetrievalDeals(context.Context) (bool, error)             //perm:admin
	DealsSetConsiderOnlineRetrievalDeals(context.Context, bool) error            //perm:admin
	DealsPieceCidBlocklist(context.Context) ([]cid.Cid, error)                   //perm:admin
	DealsSetPieceCidBlocklist(context.Context, []cid.Cid) error                  //perm:admin
	DealsConsiderOfflineStorageDeals(context.Context) (bool, error)              //perm:admin
	DealsSetConsiderOfflineStorageDeals(context.Context, bool) error             //perm:admin
	DealsConsiderOfflineRetrievalDeals(context.Context) (bool, error)            //perm:admin
	DealsSetConsiderOfflineRetrievalDeals(context.Context, bool) error           //perm:admin
	DealsConsiderVerifiedStorageDeals(context.Context) (bool, error)             //perm:admin
	DealsSetConsiderVerifiedStorageDeals(context.Context, bool) error            //perm:admin
	DealsConsiderUnverifiedStorageDeals(context.Context) (bool, error)           //perm:admin
	DealsSetConsiderUnverifiedStorageDeals(context.Context, bool) error          //perm:admin

	PiecesListPieces(ctx context.Context) ([]cid.Cid, error)                                 //perm:read
	PiecesListCidInfos(ctx context.Context) ([]cid.Cid, error)                               //perm:read
	PiecesGetPieceInfo(ctx context.Context, pieceCid cid.Cid) (*piecestore.PieceInfo, error) //perm:read
	PiecesGetCIDInfo(ctx context.Context, payloadCid cid.Cid) (*piecestore.CIDInfo, error)   //perm:read

However, the following APIs have still been kept around to enable users to interact with the built-in Market Actor.

        // StateMarketBalance looks up the Escrow and Locked balances of the given address in the Storage Market
	StateMarketBalance(context.Context, address.Address, types.TipSetKey) (MarketBalance, error) //perm:read
	// StateMarketParticipants returns the Escrow and Locked balances of every participant in the Storage Market
	StateMarketParticipants(context.Context, types.TipSetKey) (map[string]MarketBalance, error) //perm:read
	// StateMarketDeals returns information about every deal in the Storage Market
	StateMarketDeals(context.Context, types.TipSetKey) (map[string]*MarketDeal, error) //perm:read
	// StateMarketStorageDeal returns information about the indicated deal
	StateMarketStorageDeal(context.Context, abi.DealID, types.TipSetKey) (*MarketDeal, error) //perm:read
	
        // MarketListDeals enlists all deals in the storage market actor
	MarketListDeals(ctx context.Context) ([]*MarketDeal, error) //perm:read
        DealsList(ctx context.Context) ([]*MarketDeal, error) //perm:admin
	
        // MarketAddBalance adds funds to the market actor
	MarketAddBalance(ctx context.Context, wallet, addr address.Address, amt types.BigInt) (cid.Cid, error) //perm:sign
	// MarketGetReserved gets the amount of funds that are currently reserved for the address
	MarketGetReserved(ctx context.Context, addr address.Address) (types.BigInt, error) //perm:sign
	// MarketReserveFunds reserves funds for a deal
	MarketReserveFunds(ctx context.Context, wallet address.Address, addr address.Address, amt types.BigInt) (cid.Cid, error) //perm:sign
	// MarketReleaseFunds releases funds reserved by MarketReserveFunds
	MarketReleaseFunds(ctx context.Context, addr address.Address, amt types.BigInt) error //perm:sign
	// MarketWithdraw withdraws unlocked funds from the market actor
	MarketWithdraw(ctx context.Context, wallet, addr address.Address, amt types.BigInt) (cid.Cid, error) //perm:sign

Additional Info

Checklist

Before you mark the PR ready for review, please make sure that:

Commits have a clear commit message.
PR title is in the form of of <PR type>: <area>: <change being made>
- example: fix: mempool: Introduce a cache for valid signatures
- PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
- area, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps
If the PR affects users (e.g., new feature, bug fix, system requirements change), update the CHANGELOG.md and add details to the UNRELEASED section.
New features have usage guidelines and / or documentation updates in
- Lotus Documentation
- Discussion Tutorials
Tests exist for new functionality or change in behavior
CI is green

github-actions · 2024-05-16T15:26:15Z

All checks have passed

ZenGround0 · 2024-05-16T16:03:04Z

You can (and imo should) remove the graphsync module from the fullnode with markets gone. There are comments saying it's used by other subsystems but it's just markets afaik.

aarshkshah1992 · 2024-05-17T04:25:48Z

@ZenGround0 Yup. You will see it all in the final draft once it is ready for review :)

aarshkshah1992 · 2024-05-20T11:45:19Z

Tested this on a local devnet node and things look good. Was able to seal and activate sectors (submit WindowPosts for them)

aarshshah@Aarshs-MacBook-Pro lotus % lotus-miner sectors list
ID  State    OnChain  Active  Expiration                    Deals  
0   Proving  YES      YES     3675537 (in 24 weeks 2 days)  CC     
1   Proving  YES      YES     3675537 (in 24 weeks 2 days)  CC     
2   Proving  YES      YES     1549540 (in 10 weeks 1 day)   CC     
3   Proving  YES      YES     1549548 (in 10 weeks 1 day)   CC

aarshshah@Aarshs-MacBook-Pro lotus % lotus-miner proving check 0
deadline  partition  sector  status
0         0          0       good
0         0          1       good
aarshshah@Aarshs-MacBook-Pro lotus % lotus-miner proving check 1
deadline  partition  sector  status
1         0          2       good
1         0          3       good

aarshshah@Aarshs-MacBook-Pro lotus % lotus-miner proving deadlines
Miner: t01000
deadline  open      partitions  sectors (faults)  proven partitions
0         17:03:03  1           2 (0)             0
1         17:07:03  1           2 (0)             0
2         17:11:03  0           0 (0)             0
3         17:15:03  0           0 (0)             0  (current)
4         17:19:03  0           0 (0)             0
5         17:23:03  0           0 (0)             0
6         17:27:03  0           0 (0)             0
7         17:31:03  0           0 (0)             0
8         17:35:03  0           0 (0)             0
9         17:39:03  0           0 (0)             0
10        17:43:03  0           0 (0)             0
11        17:47:03  0           0 (0)             0
12        17:51:03  0           0 (0)             0
13        17:55:03  0           0 (0)             0
14        17:59:03  0           0 (0)             0
15        18:03:03  0           0 (0)             0
16        18:07:03  0           0 (0)             0
17        18:11:03  0           0 (0)             0
18        18:15:03  0           0 (0)             0
19        18:19:03  0           0 (0)             0
20        18:23:03  0           0 (0)             0
21        18:27:03  0           0 (0)             0
22        18:31:03  0           0 (0)             0
23        18:35:03  0           0 (0)             0
24        18:39:03  0           0 (0)             0
25        18:43:03  0           0 (0)             0
26        18:47:03  0           0 (0)             0
27        18:51:03  0           0 (0)             0
28        18:55:03  0           0 (0)             0
29        18:59:03  0           0 (0)             0
30        19:03:03  0           0 (0)             0
31        19:07:03  0           0 (0)             0
32        19:11:03  0           0 (0)             0
33        19:15:03  0           0 (0)             0
34        19:19:03  0           0 (0)             0
35        19:23:03  0           0 (0)             0
36        19:27:03  0           0 (0)             0
37        19:31:03  0           0 (0)             0
38        19:35:03  0           0 (0)             0
39        19:39:03  0           0 (0)             0
40        19:43:03  0           0 (0)             0
41        19:47:03  0           0 (0)             0
42        19:51:03  0           0 (0)             0
43        19:55:03  0           0 (0)             0
44        19:59:03  0           0 (0)             0
45        20:03:03  0           0 (0)             0
46        20:07:03  0           0 (0)             0
47        20:11:03  0           0 (0)             0

…-provider

… feat/remove-markets-provider

api/api_storage.go

rvagg · 2024-05-23T10:27:13Z

cmd/lotus-miner/init_service.go

-			return xerrors.Errorf("at least one module must be enabled")
-		}
-
-		// we should remove this as soon as we have more service types and not just `markets`


rvagg · 2024-05-23T10:30:41Z

node/config/types.go

+	Fees         MinerFeeConfig
+	Addresses    MinerAddressConfig
+	HarmonyDB    HarmonyDB
+	EnableLibp2p bool


do we even need this anymore? can we just force a false when we call ConfigCommon() because it's not used anywhere else in miner is it?

@rvagg We need it in the itests so that the Miner and the Full Node can talk to each other.

lotus/itests/kit/ensemble.go

Line 937 in 42743e7

err = other.NetConnect(context.Background(), addr)

connects one full node to the provided full nodes

not the miners though, only full nodes

node/builder_chain.go

rvagg · 2024-05-23T10:35:42Z

node/builder_miner.go

 	return Options(

 		Override(new(v1api.FullNode), modules.MakeUuidWrapper),
 		// Needed to instantiate pubsub used by index provider via ConfigCommon
 		Override(new(dtypes.DrandSchedule), modules.BuiltinDrandConfig),
 		Override(new(dtypes.BootstrapPeers), modules.BuiltinBootstrap),
 		Override(new(dtypes.DrandBootstrap), modules.DrandBootstrap),
-		ConfigCommon(&cfg.Common, enableLibp2pNode),
+		ConfigCommon(&cfg.Common, cfg.EnableLibp2p),


Suggested change

ConfigCommon(&cfg.Common, cfg.EnableLibp2p),

ConfigCommon(&cfg.Common, false),

As per comment below, I don't think we need this option, it doesn't need a host to listen for anything anymore does it?

@rvagg We need this flag as enabling this flag allows Miners to talk to the deamon over libp2p in the testkit.

don't they use RPC though? the miners need to talk together but I thought the rest was all just RPC

rvagg · 2024-05-23T10:36:58Z

node/impl/storminer.go

-	Transport         dtypes.ProviderTransport          `optional:"true"`
-	DealPublisher     *storageadapter.DealPublisher     `optional:"true"`
-	SectorBlocks      *sectorblocks.SectorBlocks        `optional:"true"`
-	Host              host.Host                         `optional:"true"`


another hint at not needing libp2p in the miner

See my comment below.

node/modules/client.go

rvagg · 2024-05-23T10:45:41Z

Can't comment inline but there's also these:

RelayIndexerMessages in node/modules/services.go
IndexerIngestTopic in build/params_shared_funcs.go

rvagg · 2024-05-23T10:48:07Z

Looks pretty good to me! Again I'd like to have Curio ppl input on this because there's some scary big changes in here, although they look right to my not-so-trained-eye.

… feat/remove-markets-provider

aarshkshah1992 · 2024-05-30T19:44:51Z

@rvagg Rebased on master/parent branch and addressed your review comments.

rvagg · 2024-05-31T01:21:20Z

itests/kit/ensemble.go

 		cfg.Subsystems.EnableMining = m.options.subsystems.Has(SMining)
 		cfg.Subsystems.EnableSealing = m.options.subsystems.Has(SSealing)
 		cfg.Subsystems.EnableSectorStorage = m.options.subsystems.Has(SSectorStorage)
 		cfg.Subsystems.EnableSectorIndexDB = m.options.subsystems.Has(SHarmony)
-		cfg.Dealmaking.MaxStagingDealsBytes = m.options.maxStagingDealsBytes
+		cfg.EnableLibp2p = true


Is this right? it's false elsewhere by default now, can we remove this or switch it off and false the second argument to node.StorageMiner() below like it is for normal node instantiation?

I'm unsure what the libp2p functionality is for in the miner with markets gone now, maybe it can go all together, including that second argument to node.StorageMiner()?

rvagg · 2024-05-31T01:25:25Z

lgtm except I'm still not convinced on the libp2p thing, and that it's even needed on a storage node.

or is it the case that in the testkit you can have a node that is both a full node and miner? but if it's a full node, it should have its own libp2p stack from the full node assembly process; where is libp2p getting involved in any of the storage miner work now?

aarshkshah1992 added 4 commits May 16, 2024 19:04

remove EnableMarkets flag

d1be22a

remove market subsystem

5672c5f

remove dagstore

789848b

remove index provider

ee3a83d

aarshkshah1992 marked this pull request as draft May 16, 2024 15:26

aarshkshah1992 mentioned this pull request May 16, 2024

cleanup: Lotus client: remove markets and deal-making from Lotus Client #11999

Open

8 tasks

aarshkshah1992 changed the title ~~[WIP] Remove Markets/deal-making from Miner and Full Node~~ [WIP] cleanup: markets: remove markets and deal-making from Lotus Miner and Full Node May 16, 2024

aarshkshah1992 added 6 commits May 17, 2024 12:28

remove graphsync and data-transfer

aea0b8f

remove markets

5b09bbc

go mod tidy

13a34c6

fix cbor gen deps

96579ff

remove deal making from config

5596c96

remove eol alert

a0f0d0d

aarshkshah1992 marked this pull request as ready for review May 17, 2024 10:36

aarshkshah1992 requested review from rvagg, ZenGround0 and rjan90 May 17, 2024 10:37

aarshkshah1992 changed the title ~~[WIP] cleanup: markets: remove markets and deal-making from Lotus Miner and Full Node~~ cleanup: markets: remove markets and deal-making from Lotus Miner and Full Node May 17, 2024

aarshkshah1992 added 3 commits May 20, 2024 17:16

Merge remote-tracking branch 'origin/master' into feat/remove-markets…

a7728c3

…-provider

go mod tidy

84ba02d

Merge remote-tracking branch 'origin/feat/remove-markets-client' into…

86824d7

… feat/remove-markets-provider

rvagg mentioned this pull request May 22, 2024

chore: deps: update ipni #12025

Closed

8 tasks

aarshkshah1992 changed the title ~~cleanup: markets: remove markets and deal-making from Lotus Miner and Full Node~~ cleanup: markets: remove markets and deal-making from Lotus Miner May 22, 2024

update go mod

149e0a7

aarshkshah1992 changed the title ~~cleanup: markets: remove markets and deal-making from Lotus Miner~~ cleanup: miner: remove markets and deal-making from Lotus Miner API & CLI May 22, 2024

aarshkshah1992 changed the title ~~cleanup: miner: remove markets and deal-making from Lotus Miner API & CLI~~ cleanup: miner: remove markets and deal-making from Lotus Miner May 22, 2024

rvagg reviewed May 23, 2024

View reviewed changes

api/api_storage.go Show resolved Hide resolved

rvagg reviewed May 23, 2024

View reviewed changes

node/builder_chain.go Outdated Show resolved Hide resolved

rvagg reviewed May 23, 2024

View reviewed changes

node/modules/client.go Outdated Show resolved Hide resolved

This was linked to issues May 26, 2024

Depreciation plan for the Legacy Lotus Markets sub-system #10267

Open

Remove legacy markets code in Lotus-Miner #10533

Open

aarshkshah1992 added 3 commits May 31, 2024 00:47

Merge remote-tracking branch 'origin/feat/remove-markets-client' into…

3ea92f9

… feat/remove-markets-provider

make jen

42743e7

changes as per review

01b9ab6

rvagg reviewed May 31, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cleanup: miner: remove markets and deal-making from Lotus Miner #12005

cleanup: miner: remove markets and deal-making from Lotus Miner #12005

aarshkshah1992 commented May 16, 2024 •

edited

github-actions bot commented May 16, 2024 •

edited

ZenGround0 commented May 16, 2024

aarshkshah1992 commented May 17, 2024

aarshkshah1992 commented May 20, 2024 •

edited

rvagg May 23, 2024

rvagg May 23, 2024

aarshkshah1992 May 30, 2024 •

edited

rvagg May 31, 2024

rvagg May 23, 2024

aarshkshah1992 May 30, 2024

rvagg May 31, 2024

rvagg May 23, 2024

aarshkshah1992 May 30, 2024

rvagg commented May 23, 2024

rvagg commented May 23, 2024

aarshkshah1992 commented May 30, 2024

rvagg May 31, 2024

rvagg commented May 31, 2024

	ConfigCommon(&cfg.Common, cfg.EnableLibp2p),
	ConfigCommon(&cfg.Common, false),

cleanup: miner: remove markets and deal-making from Lotus Miner #12005

Are you sure you want to change the base?

cleanup: miner: remove markets and deal-making from Lotus Miner #12005

Conversation

aarshkshah1992 commented May 16, 2024 • edited

Related Issues

Proposed Changes

Additional Info

Checklist

github-actions bot commented May 16, 2024 • edited

ZenGround0 commented May 16, 2024

aarshkshah1992 commented May 17, 2024

aarshkshah1992 commented May 20, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aarshkshah1992 May 30, 2024 • edited

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

Choose a reason for hiding this comment

rvagg commented May 23, 2024

rvagg commented May 23, 2024

aarshkshah1992 commented May 30, 2024

Choose a reason for hiding this comment

rvagg commented May 31, 2024

aarshkshah1992 commented May 16, 2024 •

edited

github-actions bot commented May 16, 2024 •

edited

aarshkshah1992 commented May 20, 2024 •

edited

aarshkshah1992 May 30, 2024 •

edited