Snap Deals Lotus Integration: FSM Posting and integration test #7810

ZenGround0 · 2021-12-17T01:25:06Z

Implements the miner node logic to run snap deals upgrades. It also handles posting (winning and window) over upgraded sectors.

There are still occasional bugs in the integration tests that I am investigating, lots of random print and log statements everywhere
FSM error handling is a mess, only the happy path is really written
The market integration is quite a hack, I would like suggestions on how to make it cleaner

jennijuju · 2021-12-17T14:39:04Z

extern/storage-sealing/input.go

@@ -358,6 +370,11 @@ func (m *Sealing) updateInput(ctx context.Context, sp abi.RegisteredSealProof) e

 		for id, sector := range m.openSectors {
 			avail := abi.PaddedPieceSize(ssize).Unpadded() - sector.used
+			// check that sector lifetime is long enough to fit deal
+			if sector.expiration > 0 && sector.expiration < piece.deal.DealProposal.EndEpoch {


should only need the later check cuz dealproposal must >0?

arajasek

Review of the consensus parts of things seems fine, will tackle miner tomorrow. Hopefully.

arajasek · 2021-12-30T21:45:36Z

chain/vm/invoker.go

@@ -74,6 +74,9 @@ func NewActorRegistry() *ActorRegistry {
 }

 func (ar *ActorRegistry) Invoke(codeCid cid.Cid, rt vmr.Runtime, method abi.MethodNum, params []byte) ([]byte, aerrors.ActorError) {
+	if method == 27 {


No, sir. I say to you, no.

magik6k

Looks reasonable, a couple of comments.

The market integration is quite a hack, I would like suggestions on how to make it cleaner

I can't think of any cleaner way to do this, I'd say it's not more hacky than the thing we have right now.

api/api_full.go

chain/vm/invoker.go

chain/vm/syscalls.go

cmd/lotus-seal-worker/main.go

extern/sector-storage/ffiwrapper/verifier_cgo.go

magik6k · 2022-01-03T17:17:24Z

markets/storageadapter/ondealsectorcommitted.go

@@ -110,7 +111,7 @@ func (mgr *SectorCommittedManager) OnDealSectorPreCommitted(ctx context.Context,

 	// Watch for a pre-commit message to the provider.
 	matchEvent := func(msg *types.Message) (bool, error) {
-		matched := msg.To == provider && (msg.Method == miner.Methods.PreCommitSector || msg.Method == miner.Methods.PreCommitSectorBatch)
+		matched := msg.To == provider && (msg.Method == miner.Methods.PreCommitSector || msg.Method == miner.Methods.PreCommitSectorBatch || msg.Method == miner.Methods.ProveReplicaUpdates)


We will need to make sure other market users are aware of this change (filc/boost/others?)

Any advice on how to propagate this information?

markets/storageadapter/ondealsectorcommitted.go

storage/wdpost_changehandler.go

storage/wdpost_run.go

arajasek

Another review wave

chain/vm/syscalls.go

cmd/lotus-miner/sectors.go

extern/sector-storage/ffiwrapper/sealer_cgo.go

extern/sector-storage/ffiwrapper/verifier_cgo.go

arajasek · 2022-01-01T19:52:37Z

extern/sector-storage/ffiwrapper/types.go

+	VerifySeal(proof.SealVerifyInfo) (bool, error)
+	VerifyAggregateSeals(aggregate proof.AggregateSealVerifyProofAndInfos) (bool, error)
+	VerifyReplicaUpdate(update proof.ReplicaUpdateInfo) (bool, error)
+	VerifyWinningPoSt(ctx context.Context, info proof.WinningPoStVerifyInfo, currEpoch abi.ChainEpoch, v network.Version) (bool, error)


new params can go i think

No harm keeping them all up to date right?

arajasek · 2022-01-01T19:53:04Z

extern/sector-storage/ffiwrapper/verifier_cgo.go

 	if err != nil {
 		return nil, nil, xerrors.Errorf("gathering sector info: %w", err)
 	}
+	log.Errorf("got private sectors")


logs are bad

arajasek · 2022-01-01T20:19:11Z

extern/sector-storage/ffiwrapper/verifier_cgo.go

+			cache = paths.UpdateCache
+			sealed = paths.Update
+		} else {
+			log.Errorf("Posting over sector key sector for sector id: %d", s.SectorNumber)


this log and the one above need to go?

- FSM handles the actual cc upgrade process including error states - PoSting (winning and window) works over upgraded and upgrading sectors - Integration test and changes to itest framework to reduce flakes - Update CLI to handle new upgrade - Update dependencies

magik6k

2 small things

api/api_full.go

cmd/lotus-seal-worker/main.go

Base automatically changed from next to master December 17, 2021 03:12

jennijuju reviewed Dec 17, 2021

View reviewed changes

jennijuju force-pushed the feat/snap-deals branch from f5de275 to 784aa86 Compare December 21, 2021 05:12

ZenGround0 force-pushed the feat/snap-deals branch 2 times, most recently from ea1e282 to 0da857f Compare December 21, 2021 21:52

Fatman13 mentioned this pull request Dec 27, 2021

venus同步v15更新 filecoin-project/venus#4642

Closed

BlocksOnAChain requested a review from magik6k December 27, 2021 19:20

BlocksOnAChain added the SnapDeals label Dec 29, 2021

BlocksOnAChain added this to the SnapDeals milestone Dec 29, 2021

ZenGround0 force-pushed the feat/snap-deals branch from a19741f to b98138e Compare December 29, 2021 17:35

ZenGround0 marked this pull request as ready for review December 29, 2021 18:24

ZenGround0 requested a review from a team as a code owner December 29, 2021 18:24

arajasek reviewed Dec 30, 2021

View reviewed changes

magik6k reviewed Jan 3, 2022

View reviewed changes

arajasek reviewed Jan 3, 2022

View reviewed changes

jennijuju modified the milestones: SnapDeals, v1.14.0 Jan 3, 2022

ZenGround0 force-pushed the feat/snap-deals branch 2 times, most recently from e3cd18b to 61e1732 Compare January 5, 2022 08:10

Fatman13 mentioned this pull request Jan 7, 2022

SnapDeal集成 filecoin-project/venus#4680

Closed

ZenGround0 added 5 commits January 10, 2022 15:39

Snap Deals Integration

33f2d24

- FSM handles the actual cc upgrade process including error states - PoSting (winning and window) works over upgraded and upgrading sectors - Integration test and changes to itest framework to reduce flakes - Update CLI to handle new upgrade - Update dependencies

Deflake snap deals integration test

f59bfd6

Deflake more practically

3dc3f55

Fix TooManyMarkedForUpgrade

c309686

doscgen cli

825e2c9

ZenGround0 force-pushed the feat/snap-deals branch from 415360c to 825e2c9 Compare January 10, 2022 10:10

Fix hande deal recover return value bug

d16c5d0

jennijuju removed the SnapDeals label Jan 11, 2022

jennijuju added the release/backport label Jan 11, 2022

magik6k reviewed Jan 11, 2022

View reviewed changes

api/api_full.go Show resolved Hide resolved

cmd/lotus-seal-worker/main.go Show resolved Hide resolved

address review

960759d

arajasek approved these changes Jan 11, 2022

View reviewed changes

arajasek enabled auto-merge January 11, 2022 16:57

arajasek force-pushed the feat/snap-deals branch from 6f656cc to 7382c53 Compare January 11, 2022 17:05

Remove unnecessary params from VerifyWinningPost

d645c5f

arajasek force-pushed the feat/snap-deals branch from 7382c53 to d645c5f Compare January 11, 2022 17:06

arajasek disabled auto-merge January 11, 2022 17:46

arajasek enabled auto-merge January 11, 2022 17:46

arajasek disabled auto-merge January 11, 2022 17:46

arajasek merged commit 207d33e into master Jan 11, 2022

arajasek deleted the feat/snap-deals branch January 11, 2022 17:46

jennijuju removed the release/backport label Jan 11, 2022

Alexey-N-Chernyshov mentioned this pull request Jan 12, 2022

Snapdeals filecoin-project/cpp-filecoin#551

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Snap Deals Lotus Integration: FSM Posting and integration test #7810

Snap Deals Lotus Integration: FSM Posting and integration test #7810

ZenGround0 commented Dec 17, 2021

jennijuju Dec 17, 2021

arajasek left a comment

arajasek Dec 30, 2021

magik6k left a comment

magik6k Jan 3, 2022

ZenGround0 Jan 3, 2022

arajasek left a comment

arajasek Jan 1, 2022

ZenGround0 Jan 3, 2022

arajasek Jan 1, 2022

arajasek Jan 1, 2022

magik6k left a comment

Snap Deals Lotus Integration: FSM Posting and integration test #7810

Snap Deals Lotus Integration: FSM Posting and integration test #7810

Conversation

ZenGround0 commented Dec 17, 2021

Choose a reason for hiding this comment

arajasek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

magik6k left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arajasek 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

Choose a reason for hiding this comment

magik6k left a comment

Choose a reason for hiding this comment