ConfirmSectorProofsValid: batch VerifyDealsOnSectorProveCommit

[ZenGround0]

From builtin/miner/miner_actor.go:ConfirmSectorProofsValid:

		// Check (and activate) storage deals associated to sector. Abort if checks failed.
		// return DealWeight for the deal set in the sector
		// TODO: we should batch these calls...

I believe this is referring to batching multiple sectors in a single call from miner to market instead of one call per sector.

[anorth]

See #483 and be careful not to introduce another problem of that sort.

[Stebalien]

Q: How should we handle errors here? At the moment, we skip each sector with invalid deals independently (allowing the other sectors to be confirmed).

[anorth]

I think we want to maintain that property. The request to the market actor should really be a crude batch, associating each sector id with its deals. The response should indicate which ones succeeded. The market actor will need to be careful to persist state changes only for the successful sector groups as atomic units.

[anorth]

Even if we don't batch these, as low hanging fruit we should at least not make this call if the sector has no deals.

[aarshkshah1992]

@anorth The PR to NOT call ActivateDeals if there are no deals has been merged. Should we keep this issue open in case we want to do this batching in the future or should we close it and remove the TODO from the code ?

[anorth]

Please remove the TODO, but yes let's keep this issue open.

[anorth]

We've determined this isn't high importance for network launch. It's an optimization to execution time, but unlikely a large contributor to overall chain validation time (it hasn't shown up in any profiles yet). I've reduced it to P3.

cc @wadealexc

[ZenGround0]

This is a good option for further optimizing aggregated prove commit.