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 FIP-0061 #10629

Merged
merged 5 commits into from
Apr 10, 2023
Merged

feat: implement FIP-0061 #10629

merged 5 commits into from
Apr 10, 2023

Conversation

arajasek
Copy link
Contributor

@arajasek arajasek commented Apr 5, 2023

Related Issues

#10375

Proposed Changes

As described in #10375

Additional Info

Needs:

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

@arajasek arajasek requested a review from a team as a code owner April 5, 2023 19:57
@arajasek arajasek force-pushed the feat/grindability branch 2 times, most recently from c381faa to 304c899 Compare April 6, 2023 14:22
Peer: []byte(m.PeerId),
SealProofType: spt,
var params []byte
if nv <= network.Version10 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change was unearthed, but not caused by FIP-0061 -- it was "working" before because the SealProofType was coincidentally the same value as the PoStProofType, and the power2.CreateMinerParams marshaled to the same bytes as the power3.CreateMinerParams, and so it...worked.

@arajasek arajasek force-pushed the feat/grindability branch 2 times, most recently from 80a37da to 8b35840 Compare April 6, 2023 14:56
@arajasek
Copy link
Contributor Author

arajasek commented Apr 6, 2023

(New tests will pass when actors bundle updated)

Copy link
Contributor

@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.

Tests are sad

    ensemble.go:457: 
        	Error Trace:	/home/circleci/project/itests/ensemble.go:457
        	            				/home/circleci/project/itests/ensemble_presets.go:22
        	            				/home/circleci/project/itests/wdpost_test.go:358
        	Error:      	Received unexpected error:
        	            	starting node:
        	            	    github.com/filecoin-project/lotus/node.New
        	            	        /home/circleci/project/node/builder.go:378
        	            	  - could not build arguments for function "github.com/filecoin-project/lotus/node/modules/lp2p".StartListening.func1
        	            	    	/home/circleci/project/node/modules/lp2p/addrs.go:85:
        	            	    failed to build host.Host:
        	            	    could not build arguments for function "reflect".makeFuncStub
        	            	    	/usr/local/go/src/reflect/asm_amd64.s:28:
        	            	    failed to build lp2p.BaseIpfsRouting:
        	            	    could not build arguments for function "reflect".makeFuncStub
        	            	    	/usr/local/go/src/reflect/asm_amd64.s:28:
        	            	    failed to build dtypes.NetworkName:
        	            	    could not build arguments for function "reflect".makeFuncStub
        	            	    	/usr/local/go/src/reflect/asm_amd64.s:28:
        	            	    failed to build dtypes.AfterGenesisSet:
        	            	    received non-nil error from function "reflect".makeFuncStub
        	            	    	/usr/local/go/src/reflect/asm_amd64.s:28:
        	            	    genesis func failed:
        	            	        github.com/filecoin-project/lotus/node/modules.SetGenesis
        	            	            /home/circleci/project/node/modules/genesis.go:73
        	            	      - make genesis block failed:
        	            	        github.com/filecoin-project/lotus/node/modules/testing.MakeGenesisMem.func1.1
        	            	            /home/circleci/project/node/modules/testing/genesis.go:39
        	            	      - setup miners failed:
        	            	        github.com/filecoin-project/lotus/chain/gen/genesis.MakeGenesisBlock
        	            	            /home/circleci/project/chain/gen/genesis/genesis.go:583
        	            	      - failed to create genesis miner:
        	            	        github.com/filecoin-project/lotus/chain/gen/genesis.SetupStorageMiners
        	            	            /home/circleci/project/chain/gen/genesis/miners.go:168
        	            	      - doExec apply message failed:
        	            	        github.com/filecoin-project/lotus/chain/gen/genesis.doExecValue
        	            	            /home/circleci/project/chain/gen/genesis/util.go:36
        	            	      - implicit message failed with exit code: 16 and error: message failed with backtrace:
        	            	    00: f04 (method 2) -- send aborted with code 16 (16)
        	            	    01: f01 (method 2) -- constructor failed: send aborted with code 16 (16)
        	            	    02: f01000 (method 1) -- proof type Invalid(10) not allowed for new miner actors (16)
        	            	     (RetCode=16)
        	Test:       	TestWindowPostV1P1NV19
--- FAIL: TestWindowPostV1P1NV19 (4.84s)

Comment on lines 36 to 47
case nv >= network.Version19:
switch ssize {
case 2 << 10:
return abi.RegisteredSealProof_StackedDrg2KiBV1, nil
return abi.RegisteredSealProof_StackedDrg2KiBV1_1, nil
case 8 << 20:
return abi.RegisteredSealProof_StackedDrg8MiBV1, nil
return abi.RegisteredSealProof_StackedDrg8MiBV1_1, nil
case 512 << 20:
return abi.RegisteredSealProof_StackedDrg512MiBV1, nil
return abi.RegisteredSealProof_StackedDrg512MiBV1_1, nil
case 32 << 30:
return abi.RegisteredSealProof_StackedDrg32GiBV1, nil
return abi.RegisteredSealProof_StackedDrg32GiBV1_1, nil
case 64 << 30:
return abi.RegisteredSealProof_StackedDrg64GiBV1, nil
return abi.RegisteredSealProof_StackedDrg64GiBV1_1, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It shouldn't be, you're right.

cmd/lotus-bench/main.go Show resolved Hide resolved
t.Fatalf("%+v", err)
}

proofs, skp, err := sealer.GenerateWindowPoSt(context.TODO(), seals[0].ref.ID.Miner, ppt, xsis, randomness)
Copy link
Contributor

Choose a reason for hiding this comment

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

We can just hardcode V1_1 here

Comment on lines +21 to +29
if len(sectorInfo) == 0 {
return nil, xerrors.Errorf("must provide sectors for winning post")
}
ppt, err := sectorInfo[0].SealProof.RegisteredWinningPoStProof()
if err != nil {
return nil, xerrors.Errorf("failed to convert to winning post proof: %w", err)
}

privsectors, skipped, done, err := sb.pubExtendedSectorToPriv(ctx, minerID, sectorInfo, nil, ppt) // TODO: FAULTS?
Copy link
Contributor

Choose a reason for hiding this comment

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

Am I reading this correctly - there is technically no change here, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, no functional change to the Winning PoSt side of things, just the refactor to actually supply the proof type to pubExtendedSectorToPriv (needed for the Window PoSt changes).

storage/wdpost/wdpost_run.go Outdated Show resolved Hide resolved
@arajasek arajasek force-pushed the feat/grindability branch 2 times, most recently from 58cc178 to e3e9624 Compare April 6, 2023 20:27
Copy link
Contributor

@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.

Afaict looks good.

Technically we should be able to make sector skipping more efficient with these changes, but let's maybe not touch working, hard to test PoSt code in a required upgrade.

go.mod Outdated
@@ -46,7 +46,7 @@ require (
github.com/filecoin-project/go-legs v0.4.4
github.com/filecoin-project/go-padreader v0.0.1
github.com/filecoin-project/go-paramfetch v0.0.4
github.com/filecoin-project/go-state-types v0.11.0-alpha-2
github.com/filecoin-project/go-state-types v0.11.0-alpha-3.0.20230406204105-2813ee9cbb0b
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs a tag

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will be updating to the RC shortly.

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.

2 participants