some code review fixes

.gitignore

@@ -4,6 +4,7 @@
 *.dll
 *.so
 *.dylib
+digest
 
 # Test binary, built with `go test -c`
 *.test

actors/builtin/miner/miner_actor.go

@@ -956,9 +956,14 @@ func (a Actor) ProveCommitAggregate(rt Runtime, params *ProveCommitAggregatePara
 		})
 	builtin.RequireNoErr(rt, err, exitcode.ErrIllegalArgument, "aggregate seal verify failed")
 
-	confirmSectorProofsValid(rt, precommitsToConfirm, &builtin.ConfirmSectorProofsParams{
-		PrecomputeRewardPowerStats: false,
-	})
+	var rewret reward.ThisEpochRewardReturn
+	rewretcode := rt.Send(builtin.RewardActorAddr, builtin.MethodsReward.ThisEpochReward, nil, big.Zero(), &rewret)
+	builtin.RequireSuccess(rt, rewretcode, "failed to check epoch baseline power")
+	var pwr power.CurrentTotalPowerReturn
+	powretcode := rt.Send(builtin.StoragePowerActorAddr, builtin.MethodsPower.CurrentTotalPower, nil, big.Zero(), &pwr)
+	builtin.RequireSuccess(rt, powretcode, "failed to check current power")
+
+	confirmSectorProofsValid(rt, precommitsToConfirm, rewret.ThisEpochBaselinePower, rewret.ThisEpochRewardSmoothed, pwr.QualityAdjPowerSmoothed)
 
 	// Compute and burn the aggregate network fee. We need to re-load the state as
 	// confirmSectorProofsValid can change it.
@@ -1064,32 +1069,19 @@ func (a Actor) ConfirmSectorProofsValid(rt Runtime, params *builtin.ConfirmSecto
 	precommittedSectors, err := st.FindPrecommittedSectors(store, params.Sectors...)
 	builtin.RequireNoErr(rt, err, exitcode.ErrIllegalState, "failed to load pre-committed sectors")
 
-	confirmSectorProofsValid(rt, precommittedSectors, params)
+	confirmSectorProofsValid(rt, precommittedSectors,
+		params.RewardStatsThisEpochBaselinePower,
+		params.RewardStatsThisEpochRewardSmoothed,
+		params.PwrTotalQualityAdjPowerSmoothed)
 
 	return nil
 }
 
-func confirmSectorProofsValid(rt Runtime, preCommits []*SectorPreCommitOnChainInfo, params *builtin.ConfirmSectorProofsParams) {
-	var rewardStatsThisEpochBaselinePower big.Int
-	var rewardStatsThisEpochRewardSmoothed smoothing.FilterEstimate
-	var pwrTotalQualityAdjPowerSmoothed smoothing.FilterEstimate
-
-	if params.PrecomputeRewardPowerStats {
-		rewardStatsThisEpochRewardSmoothed = params.RewardStatsThisEpochRewardSmoothed
-		rewardStatsThisEpochBaselinePower = params.RewardStatsThisEpochBaselinePower
-		pwrTotalQualityAdjPowerSmoothed = params.PwrTotalQualityAdjPowerSmoothed
-	} else {
-		var rewret reward.ThisEpochRewardReturn
-		rewretcode := rt.Send(builtin.RewardActorAddr, builtin.MethodsReward.ThisEpochReward, nil, big.Zero(), &rewret)
-		builtin.RequireSuccess(rt, rewretcode, "failed to check epoch baseline power")
-		var pwr power.CurrentTotalPowerReturn
-		powretcode := rt.Send(builtin.StoragePowerActorAddr, builtin.MethodsPower.CurrentTotalPower, nil, big.Zero(), &pwr)
-		builtin.RequireSuccess(rt, powretcode, "failed to check current power")
-
-		rewardStatsThisEpochRewardSmoothed = rewret.ThisEpochRewardSmoothed
-		rewardStatsThisEpochBaselinePower = rewret.ThisEpochBaselinePower
-		pwrTotalQualityAdjPowerSmoothed = pwr.QualityAdjPowerSmoothed
-	}
+func confirmSectorProofsValid(rt Runtime,
+	preCommits []*SectorPreCommitOnChainInfo,
+	rewardStatsThisEpochBaselinePower big.Int,
+	rewardStatsThisEpochRewardSmoothed smoothing.FilterEstimate,
+	pwrTotalQualityAdjPowerSmoothed smoothing.FilterEstimate) {
 
 	circulatingSupply := rt.TotalFilCircSupply()
 
@@ -2126,9 +2118,6 @@ func processEarlyTerminations(rt Runtime) (more bool) {
 	// TODO: We're using the current power+epoch reward. Technically, we
 	// should use the power/reward at the time of termination.
 	// https://github.com/filecoin-project/specs-actors/v6/pull/648
-	// TODO #2: for the fix to issue 799 we precompute these in OnEpochTickEnd and
-	// pass them in to another function. perhaps could do that here too?
-	// but unsure with above TODO.
 	rewardStats := requestCurrentEpochBlockReward(rt)
 	pwrTotal := requestCurrentTotalPower(rt)
 

actors/builtin/miner/miner_test.go

@@ -5029,6 +5029,8 @@ func (h *actorHarness) proveCommitAggregateSector(rt *mock.Runtime, conf proveCo
 		}
 		rt.ExpectSend(builtin.StorageMarketActorAddr, builtin.MethodsMarket.ComputeDataCommitment, &cdcParams, big.Zero(), &cdcRet, exitcode.Ok)
 	}
+	expectQueryNetworkInfo(rt, h)
+
 	// Expect randomness queries for provided precommits
 	var sealRands []abi.SealRandomness
 	var sealIntRands []abi.InteractiveSealRandomness
@@ -5073,7 +5075,7 @@ func (h *actorHarness) proveCommitAggregateSector(rt *mock.Runtime, conf proveCo
 
 	// confirmSectorProofsValid
 	{
-		h.confirmSectorProofsValidInternal(rt, conf, false, precommits...)
+		h.confirmSectorProofsValidInternal(rt, conf, precommits...)
 	}
 
 	// burn networkFee
@@ -5088,12 +5090,7 @@ func (h *actorHarness) proveCommitAggregateSector(rt *mock.Runtime, conf proveCo
 	rt.Verify()
 }
 
-func (h *actorHarness) confirmSectorProofsValidInternal(rt *mock.Runtime, conf proveCommitConf, isPrecomputed bool, precommits ...*miner.SectorPreCommitOnChainInfo) {
-	// expect calls to get network stats if we haven't precomputed
-	if !isPrecomputed {
-		expectQueryNetworkInfo(rt, h)
-	}
-
+func (h *actorHarness) confirmSectorProofsValidInternal(rt *mock.Runtime, conf proveCommitConf, precommits ...*miner.SectorPreCommitOnChainInfo) {
 	// Prepare for and receive call to ConfirmSectorProofsValid.
 	var validPrecommits []*miner.SectorPreCommitOnChainInfo
 	for _, precommit := range precommits {
@@ -5147,7 +5144,7 @@ func (h *actorHarness) confirmSectorProofsValidInternal(rt *mock.Runtime, conf p
 }
 
 func (h *actorHarness) confirmSectorProofsValid(rt *mock.Runtime, conf proveCommitConf, precommits ...*miner.SectorPreCommitOnChainInfo) {
-	h.confirmSectorProofsValidInternal(rt, conf, false, precommits...)
+	h.confirmSectorProofsValidInternal(rt, conf, precommits...)
 	var allSectorNumbers []abi.SectorNumber
 	for _, precommit := range precommits {
 		allSectorNumbers = append(allSectorNumbers, precommit.Info.SectorNumber)
@@ -5156,8 +5153,10 @@ func (h *actorHarness) confirmSectorProofsValid(rt *mock.Runtime, conf proveComm
 	rt.ExpectValidateCallerAddr(builtin.StoragePowerActorAddr)
 
 	rt.Call(h.a.ConfirmSectorProofsValid, &builtin.ConfirmSectorProofsParams{
-		Sectors:                    allSectorNumbers,
-		PrecomputeRewardPowerStats: false,
+		Sectors:                            allSectorNumbers,
+		RewardStatsThisEpochRewardSmoothed: h.epochRewardSmooth,
+		RewardStatsThisEpochBaselinePower:  h.baselinePower,
+		PwrTotalQualityAdjPowerSmoothed:    h.epochQAPowerSmooth,
 	})
 	rt.Verify()
 }

actors/builtin/power/power_actor.go

@@ -428,7 +428,6 @@ func (a Actor) processBatchProofVerifies(rt Runtime) {
 				m,
 				builtin.MethodsMiner.ConfirmSectorProofsValid,
 				&builtin.ConfirmSectorProofsParams{Sectors: successful,
-					PrecomputeRewardPowerStats:         true,
 					RewardStatsThisEpochRewardSmoothed: rewret.ThisEpochRewardSmoothed,
 					RewardStatsThisEpochBaselinePower:  rewret.ThisEpochBaselinePower,
 					PwrTotalQualityAdjPowerSmoothed:    pwr.QualityAdjPowerSmoothed},

actors/builtin/power/power_test.go

@@ -729,7 +729,6 @@ func TestCron(t *testing.T) {
 		rt.SetEpoch(1)
 		actor.createMinerBasic(rt, owner, owner, miner1)
 		actor.createMinerBasic(rt, owner, owner, miner2)
-		//expectQueryNetworkInfo(rt, actor)
 
 		actor.enrollCronEvent(rt, miner1, 2, []byte{})
 		actor.enrollCronEvent(rt, miner2, 2, []byte{})
@@ -1035,7 +1034,6 @@ func TestCronBatchProofVerifies(t *testing.T) {
 		for _, cs := range cs {
 			param := &builtin.ConfirmSectorProofsParams{
 				Sectors:                            cs.sectorNums,
-				PrecomputeRewardPowerStats:         true,
 				RewardStatsThisEpochRewardSmoothed: ac.thisEpochRewardSmoothed,
 				RewardStatsThisEpochBaselinePower:  ac.thisEpochBaselinePower,
 				PwrTotalQualityAdjPowerSmoothed:    st.ThisEpochQAPowerSmoothed,
@@ -1162,7 +1160,6 @@ func (h *spActorHarness) onEpochTickEnd(rt *mock.Runtime, currEpoch abi.ChainEpo
 	for _, cs := range confirmedSectors {
 		param := &builtin.ConfirmSectorProofsParams{
 			Sectors:                            cs.sectorNums,
-			PrecomputeRewardPowerStats:         true,
 			RewardStatsThisEpochRewardSmoothed: h.thisEpochRewardSmoothed,
 			RewardStatsThisEpochBaselinePower:  h.thisEpochBaselinePower,
 			PwrTotalQualityAdjPowerSmoothed:    st.ThisEpochQAPowerSmoothed,

actors/builtin/shared.go

@@ -91,17 +91,10 @@ type MinerAddrs struct {
 	ControlAddrs []addr.Address
 }
 
-// stub for codegen
-// func (b ConfirmSectorProofsParams) MarshalCBOR(w io.Writer) error {
-// 	_, err := w.Write([]byte("aaaaa"))
-// 	return err
-// }
-
 // Note: we could move this alias back to the mutually-importing packages that use it, now that they
 // can instead both alias the v2 version.
 type ConfirmSectorProofsParams struct {
 	Sectors                            []abi.SectorNumber
-	PrecomputeRewardPowerStats         bool
 	RewardStatsThisEpochRewardSmoothed smoothing.FilterEstimate
 	RewardStatsThisEpochBaselinePower  abi.StoragePower
 	PwrTotalQualityAdjPowerSmoothed    smoothing.FilterEstimate

actors/test/commit_post_test.go

@@ -78,7 +78,6 @@ func TestCommitPoStFlow(t *testing.T) {
 					{To: builtin.RewardActorAddr, Method: builtin.MethodsReward.ThisEpochReward},
 					{To: builtin.StoragePowerActorAddr, Method: builtin.MethodsPower.CurrentTotalPower},
 					{To: minerAddrs.IDAddress, Method: builtin.MethodsMiner.OnDeferredCronEvent, SubInvocations: []vm.ExpectInvocation{
-						// XXX-799 this happens twice- see comment in miner_actor.go in processEarlyTerminations function
 						{To: builtin.RewardActorAddr, Method: builtin.MethodsReward.ThisEpochReward},
 						{To: builtin.StoragePowerActorAddr, Method: builtin.MethodsPower.CurrentTotalPower},
 						// The call to burnt funds indicates the overdue precommit has been penalized
@@ -258,7 +257,6 @@ func TestCommitPoStFlow(t *testing.T) {
 					{To: builtin.RewardActorAddr, Method: builtin.MethodsReward.ThisEpochReward},
 					{To: builtin.StoragePowerActorAddr, Method: builtin.MethodsPower.CurrentTotalPower},
 					{To: minerAddrs.IDAddress, Method: builtin.MethodsMiner.OnDeferredCronEvent, SubInvocations: []vm.ExpectInvocation{
-						// XXX-799 this happens twice- see comment in miner_actor.go in processEarlyTerminations function
 						{To: builtin.RewardActorAddr, Method: builtin.MethodsReward.ThisEpochReward},
 						{To: builtin.StoragePowerActorAddr, Method: builtin.MethodsPower.CurrentTotalPower},
 						{To: builtin.StoragePowerActorAddr, Method: builtin.MethodsPower.EnrollCronEvent},

actors/test/committed_capacity_scenario_test.go

@@ -188,7 +188,6 @@ func TestReplaceCommittedCapacitySectorWithDealLadenSector(t *testing.T) {
 					{To: builtin.RewardActorAddr, Method: builtin.MethodsReward.ThisEpochReward},
 					{To: builtin.StoragePowerActorAddr, Method: builtin.MethodsPower.CurrentTotalPower},
 					{To: minerAddrs.IDAddress, Method: builtin.MethodsMiner.OnDeferredCronEvent, SubInvocations: []vm.ExpectInvocation{
-						// XXX-799 this happens twice- see comment in miner_actor.go in processEarlyTerminations function
 						{To: builtin.RewardActorAddr, Method: builtin.MethodsReward.ThisEpochReward},
 						{To: builtin.StoragePowerActorAddr, Method: builtin.MethodsPower.CurrentTotalPower},
 						// pre-commit deposit is burnt
@@ -284,7 +283,6 @@ func TestReplaceCommittedCapacitySectorWithDealLadenSector(t *testing.T) {
 					{To: builtin.RewardActorAddr, Method: builtin.MethodsReward.ThisEpochReward},
 					{To: builtin.StoragePowerActorAddr, Method: builtin.MethodsPower.CurrentTotalPower},
 					{To: minerAddrs.IDAddress, Method: builtin.MethodsMiner.OnDeferredCronEvent, SubInvocations: []vm.ExpectInvocation{
-						// XXX-799 this happens twice- see comment in miner_actor.go in processEarlyTerminations function
 						{To: builtin.RewardActorAddr, Method: builtin.MethodsReward.ThisEpochReward},
 						{To: builtin.StoragePowerActorAddr, Method: builtin.MethodsPower.CurrentTotalPower},
 						// power is removed for old sector and pledge is burnt

actors/test/power_scenario_test.go

@@ -111,6 +111,7 @@ func TestOnEpochTickEnd(t *testing.T) {
 		To:     builtin.StoragePowerActorAddr,
 		Method: builtin.MethodsPower.OnEpochTickEnd,
 		SubInvocations: []vm.ExpectInvocation{
+			// get data from reward and power actors for any eventual calls to confirmsectorproofsparams
 			{To: builtin.RewardActorAddr, Method: builtin.MethodsReward.ThisEpochReward},
 			{To: builtin.StoragePowerActorAddr, Method: builtin.MethodsPower.CurrentTotalPower},
 			{
@@ -135,6 +136,7 @@ func TestOnEpochTickEnd(t *testing.T) {
 		To:     builtin.StoragePowerActorAddr,
 		Method: builtin.MethodsPower.OnEpochTickEnd,
 		SubInvocations: []vm.ExpectInvocation{
+			// get data from reward and power actors for any eventual calls to confirmsectorproofsparams
 			{To: builtin.RewardActorAddr, Method: builtin.MethodsReward.ThisEpochReward},
 			{To: builtin.StoragePowerActorAddr, Method: builtin.MethodsPower.CurrentTotalPower},
 			{

support/mock/mockrt_state_test.go

@@ -52,8 +52,7 @@ func (s State) MarshalCBOR(w io.Writer) error {
 func (s State) UnmarshalCBOR(r io.Reader) error {
 	var cint cbg.CborInt
 	err := cint.UnmarshalCBOR(r)
-	// should the state have been passed-by-reference or something? why is this here?
-	//s.Value = int64(cint)
+	s.Value = int64(cint)
 	return err
 }
 

test-vectors/.gitignore

@@ -1 +1 @@
-determinism/*
+determinism/*