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

fix bootstrapper (genesis block generation) in lotus-soup #6379

Closed
wants to merge 3 commits into from

Conversation

nonsense
Copy link
Member

@nonsense nonsense commented Jun 2, 2021

This PR is fixing the genesis block generation in Lotus, so that lotus-soup testplan's deals end-to-end test passes.

At the moment on master the test plan is failing because the presealed sectors that are added for the initial miner set for genesis are not passing the VM validation for deals:

testing/genesis.go:36    Generating new random genesis block, note that this SHOULD NOT happen unless you are setting up new network
init set t3xg5hxu2acvp276lvdltmoefrr3l2btuunvstlgdtrcryjwiui6amki25uotqnbxtdjcjyucm6o5rykp34d6a t0100                                                                                    init set t3q7j2yu4tjrkbaxggmry6bhp2h3k6vev7xkbb2e6nrhzi2e5wwbxnkih2fsz4e66ml53pbbpnhpdqpmpxo4ma t0101                                                                                    
init set t3xfnw4zxl2jidteskhpgwqyj76ip6qtxxb7uad6kwghepxyx777hm7mzqnljn5szgzebkeh5hqmux5wemelnq t0102                                                                                    
init set t3siyl7azdjzjjnduktipja6pnwyi6goyw72dwtwgfpoouyvbfmyhtg4elbwiiao4rqrwczkl3lz5uizbtvt6a t0103                                                                                    init set t1ceb34gnsc6qk5dt6n7xg6ycwzasjhbxm3iylkiy t0104                                                                                                                                 publishing 10 storage deals on miner t01000 with worker t3siyl7azdjzjjnduktipja6pnwyi6goyw72dwtwgfpoouyvbfmyhtg4elbwiiao4rqrwczkl3lz5uizbtvt6a  

...

VM.Call failure: Deal duration out of bounds. (RetCode=16):

This is happening because the params for the 10 storage deals on miner t01000 look like:

StartEpoch: (abi.ChainEpoch) 0,                                                                                                                                                   
EndEpoch: (abi.ChainEpoch) 129537,    

And minimum deal duration is 180 days.


After I updated chain/gen/genesis/miners.go:176 to increase the preseal duration for the sectors, I hit another problem with the MaxSectorLifetime, so I increased that as well with 10000.

After that I got an error with respect to actual and computed power, so I commented that out.


Obviously this is not yet ready for merging, but the lotus-soup test plan now works, and genesis sectors are generated correctly.

@nonsense nonsense requested review from magik6k and arajasek June 2, 2021 15:05

minerInfos[i].presealExp = (maxPeriods-1)*miner0.WPoStProvingPeriod + pps - 1
//minerInfos[i].presealExp = (maxPeriods-1)*miner0.WPoStProvingPeriod + pps - 1
Copy link
Member Author

Choose a reason for hiding this comment

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

@arajasek this comes up to 129537 which is ~45 days, so when we submit initial deals for presealing (chain/gen/genesis/miners.go:193), the VM throws an error.

Is this value supposed to be equal to 45 days?

Copy link
Member Author

Choose a reason for hiding this comment

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

@arajasek can you explain what is the idea here with (maxPeriods-1)*miner0.WPoStProvingPeriod + pps - 1 ? We seem to be using this value for the initial deal durations when building preseal sectors, and this is not passing the 180 days validation.

Copy link
Contributor

Choose a reason for hiding this comment

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

@nonsense I actually don't know why we do (maxPeriods-1)*miner0.WPoStProvingPeriod + pps - 1, looking through history it seems like it might be legacy (we've "always" done it).

The value definitely shouldn't be ~45 days, though, it should be ~540 days. We need to figure out why that's happening -- it might be because of some funkiness with block times. Does lotus soup use a custom block time?

Copy link
Contributor

Choose a reason for hiding this comment

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

Will try to repro on devnet.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, confirmed it gets 1550097 on a devnet, so I think this is a lotus-soup config thing.

Copy link
Member Author

Choose a reason for hiding this comment

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

@arajasek thanks, that makes sense now - we do reduce the window in the test plan, so that we can test proving failures - it happens at https://github.com/filecoin-project/lotus/blob/master/testplans/lotus-soup/init.go#L45

I think we will want to keep the flexibility to change the proving period, but it looks like it is not correlated to the minimum deal duration...

return cid.Undef, xerrors.Errorf("QualityAdjPower (%s) doesn't match previously calculated qaPow (%s)", pc.QualityAdjPower, qaPow)
}
//if !pc.QualityAdjPower.Equals(qaPow) {
//return cid.Undef, xerrors.Errorf("QualityAdjPower (%s) doesn't match previously calculated qaPow (%s)", pc.QualityAdjPower, qaPow)
Copy link
Member Author

Choose a reason for hiding this comment

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

pc.QualityAdjPower then is equal to 824572800, whereas qaPow is calculated to be 838860800 (we are using 8MiB sectors for the lotus-soup testplans).

@magik6k magik6k added the team/ignite Issues and PRs being tracked by Team Ignite at Protocol Labs label Jun 3, 2021
@nonsense
Copy link
Member Author

nonsense commented Jun 7, 2021

@arajasek thanks for the help. Closing this in favour of #6400 . We might need to revisit this at some point in the future when we introduce integration tests for PoSt faults.

@nonsense nonsense closed this Jun 7, 2021
@nonsense nonsense deleted the nonsense/fix-bootstrapper-lotus-soup branch September 10, 2021 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team/ignite Issues and PRs being tracked by Team Ignite at Protocol Labs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants