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

Lotus miner needs to use new precommit_batch2 #11139

Closed
3 of 9 tasks
ZenGround0 opened this issue Aug 6, 2023 · 3 comments · Fixed by #11142
Closed
3 of 9 tasks

Lotus miner needs to use new precommit_batch2 #11139

ZenGround0 opened this issue Aug 6, 2023 · 3 comments · Fixed by #11142
Assignees
Labels

Comments

@ZenGround0
Copy link
Contributor

Checklist

  • This is not brainstorming ideas. If you have an idea you'd like to discuss, please open a new discussion on the lotus forum and select the category as Ideas.
  • I have a specific, actionable, and well motivated feature request to propose.

Lotus component

  • lotus daemon - chain sync
  • lotus fvm/fevm - Lotus FVM and FEVM interactions
  • lotus miner/worker - sealing
  • lotus miner - proving(WindowPoSt/WinningPoSt)
  • lotus JSON-RPC API
  • lotus message management (mpool)
  • Other

What is the motivation behind this feature request? Is your feature request related to a problem? Please describe.

As part of direct data onboarding we are finishing a long outstanding migration from a precommit message that does not specify CommD to one that always specifies CommD. This is achieved by deprecating PreCommitSector and PreCommitSectorBatch for PreCommitSectorBatchV2: filecoin-project/builtin-actors#1352

With this change in effect the lotus miner will need to migrate its internal precommit message sending to use the existing precommit message that will not be removed in nv21.

Describe the solution you'd like

All calls to PreCommitSectorBatch and PreCommitSector should be replaced with PreCommitBatchV2. Because the sealing pipeline sector infos hold onto a pointer to CommD and this is the only difference in input parameters between old and new PreCommit methods, this should be a painless code change that only requires swapping out method numbers.

Describe alternatives you've considered

If we don't do this we will be blocked on doing the final stage of precommit message migration inside builtin actors. We will be unable to rely on CommD availability independent of the builtin market which will introduce new sources of user errors into the system (they could precommit without providing CommD and then are unable to finish direct data onboarding flow without precommitting through PreCommitBatch2 all over again)

Additional context

In case you are worried that moving from single PreCommitSector to PreCommitBatchv2 is an invasive change, 99% of codepath is literally exactly the same, we are just forcing precommit messages to enter with slightly different data. See the current implementation: https://github.com/filecoin-project/builtin-actors/blob/master/actors/miner/src/lib.rs#L1704

@jennijuju
Copy link
Member

#11125

@anorth
Copy link
Member

anorth commented Aug 11, 2023

@ZenGround0 @magik6k I think the same thing applies to ProveReplicaUpdate. There are currently two endpoints, the only difference being the older one doesn't require CommD as a parameter, and the newer one does. Direct data onboarding will introduce yet another one, that also requires CommD. As part of this, we should deprecate the old one.

The motivation logic is similar as for precommit. We can't deprecate the old method until Lotus switches to the new one.

@anorth
Copy link
Member

anorth commented Aug 11, 2023

Note that for PRU today, the new method only warns when the provided CommD is incorrect, and then ignores it. In the next network version, it will abort if the provided CommD doesn't correspond with the deals.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants