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: Synthetic-porep #10970

Merged
merged 8 commits into from
Jul 12, 2023
Merged

feat: Synthetic-porep #10970

merged 8 commits into from
Jul 12, 2023

Conversation

snadrus
Copy link
Collaborator

@snadrus snadrus commented Jun 9, 2023

Related Issues

Synthetic PoRep Sealing Implementation for considerable reduction in disk usage by sealing machines during the network proof epochs.

Proposed Changes

This including Rust Proofs, FFI, and Lotus Sealing Scheduler but lacks Network/Protocol code.

Additional Info

https://docs.google.com/document/d/1Ug4uC89Kkwhn3vcbiHETxToIUQ4zXEPa4QcQMPcvRxI/edit#heading=h.277tjch7w86w

https://www.notion.so/pl-strflt/FIP-0059-Synthetic-PoRep-180d995a027647198f6bffebc3eca932

https://www.notion.so/pl-strflt/Synthetic-PoRep-c15c9005f6284a9b9b3adeb6a3cecff1

filecoin-project/FIPs#245

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
  • New features have usage guidelines and / or documentation updates in
  • Tests exist for new functionality or change in behavior
  • CI is green

@snadrus snadrus requested a review from a team as a code owner June 9, 2023 18:09
@snadrus snadrus marked this pull request as draft June 9, 2023 18:10
@snadrus snadrus self-assigned this Jun 9, 2023
Copy link
Contributor

@arajasek arajasek left a comment

Choose a reason for hiding this comment

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

This generally looks good, but we're really gonna need tests in order to gauge correctness. Given that this is just a draft, I'd consider (temporarily) setting network.SyntheticVersion to 20, and writing an itest that generates and submits synthetic proofs. This will not fully pass (it'll get rejected by the actors code), but will at least get some of the code paths exercised.

@@ -208,7 +208,7 @@ type Partition interface {

type SectorOnChainInfo = minertypes.SectorOnChainInfo

func PreferredSealProofTypeFromWindowPoStType(nver network.Version, proof abi.RegisteredPoStProof) (abi.RegisteredSealProof, error) {
func PreferredSealProofTypeFromWindowPoStType(nver network.Version, proof abi.RegisteredPoStProof, configWantSynthetic bool) (abi.RegisteredSealProof, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This file is generated from chain/actors/builtin/miner/actor.go.template. Changes need to be made there, and generated using make actors-gen in order for them to land here.

chain/actors/builtin/miner/miner.go Show resolved Hide resolved

if err = ffi.ClearLayerData(ssize, paths.Cache); err != nil {
log.Warn("failed to GenerateSynthProofs(): ", err)
log.Warnf("num:%d tkt:%v seed:%v sealedCID:%v, unsealedCID:%v", sector.ID.Number, ticket, sealedCID, unsealedCID)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: not actually logging the seed here?

@rjan90 rjan90 mentioned this pull request Jun 14, 2023
@arajasek
Copy link
Contributor

@snadrus Here are 2 tests that can combine to form a good foundation for a test here:

  • TestPledgeSectors, a very simple test that sets up a miner on a "devnet" and pledges sectors on it.
    • You'll want to tweak the miner config to use SynPoRep, of course
  • TestWindowPostV1P1NV19, a recent test that configures the "devnet" used by the test to start at a particular network version, and then tests the PoSt scheduling and generation on it. You can apply something similar to your test, I think.

@snadrus
Copy link
Collaborator Author

snadrus commented Jun 27, 2023

It seems like I need to wait until the actors work is done as this work revolves around the sealing pipeline but those recommended test require the network side to act correctly.

@jennijuju jennijuju changed the title feat: Synthetic-porep: Ready for Actors Impl feat: Synthetic-porep Jul 6, 2023
@magik6k magik6k changed the base branch from master to feat/nv21-skeleton July 12, 2023 10:02
@magik6k magik6k merged commit 933db60 into feat/nv21-skeleton Jul 12, 2023
@magik6k magik6k deleted the synthetic-porep branch July 12, 2023 10:27
@magik6k
Copy link
Contributor

magik6k commented Jul 12, 2023

Merged into #11057

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