Apply suggestions from code review

Author: jordanschalm

contracts/epochs/FlowEpoch.cdc

@@ -555,11 +555,10 @@ access(all) contract FlowEpoch {
                 nodes.append(FlowIDTableStaking.NodeInfo(nodeID: nodeID))
             }
 
-            let numViewsInStakingAuction = stakingEndView - startView
             let numViewsInDKGPhase = FlowEpoch.configurableMetadata.numViewsInDKGPhase
-            let dkgPhase1FinalView = startView + numViewsInStakingAuction + numViewsInDKGPhase - 1
-            let dkgPhase2FinalView = startView + numViewsInStakingAuction + (2 * numViewsInDKGPhase) - 1
-            let dkgPhase3FinalView = startView + numViewsInStakingAuction + (3 * numViewsInDKGPhase) - 1
+            let dkgPhase1FinalView = stakingEndView + numViewsInDKGPhase
+            let dkgPhase2FinalView = dkgPhase1FinalView + numViewsInDKGPhase
+            let dkgPhase3FinalView = dkgPhase2FinalView + numViewsInDKGPhase
 
             /// emit EpochRecover event
             emit FlowEpoch.EpochRecover(
@@ -697,7 +696,8 @@ access(all) contract FlowEpoch {
         /// This function is intended to update the current epoch configuration when a recovery
         /// transaction needs to be submitted more than once. This function differs 
         /// from recoverNewEpoch because it does not increment the epoch counter. It also 
-        /// does not calculate or distribute rewards avoiding double paying rewards for the same epoch.
+        /// does not calculate or distribute rewards -- calling recoverCurrentEpoch multiple times does not cause multiple reward payouts.
+        /// Rewards for the recovery epoch will be calculated and paid out during the course of the epoch. 
         /// This meta data will be emitted in the EpochRecover service event. This function is used 
         /// within sporks to recover the network from Epoch Fallback Mode (EFM).
         access(all) fun recoverCurrentEpoch(recoveryEpochCounter: UInt64,
@@ -1052,7 +1052,7 @@ access(all) contract FlowEpoch {
 
         self.currentEpochPhase = EpochPhase.EPOCHSETUP
 
-        let dkgPhase1FinalView = proposedEpochMetadata.startView + self.configurableMetadata.numViewsInStakingAuction + self.configurableMetadata.numViewsInDKGPhase - 1
+        let dkgPhase1FinalView = proposedEpochMetadata.stakingEndView + self.configurableMetadata.numViewsInDKGPhase
         let dkgPhase2FinalView = dkgPhase1FinalView + self.configurableMetadata.numViewsInDKGPhase
         let dkgPhase3FinalView = dkgPhase2FinalView + self.configurableMetadata.numViewsInDKGPhase
         emit EpochSetup(counter: proposedEpochMetadata.counter,

lib/go/test/epoch_test_helpers.go

@@ -939,7 +939,7 @@ type testEpochConfig struct {
 	numStakingViews      uint64 // num views for staking auction
 	numDKGViews          uint64 // num views for DKG phase
 	numClusters          uint64 // num collector clusters
-	numEpochAccounts     int    // num collector clusters
+	numEpochAccounts     int    // num accounts to setup for staking
 	randomSource         string // random source
 	rewardIncreaseFactor string // reward increase factor
 }

lib/go/test/flow_epoch_test.go

@@ -1526,8 +1526,8 @@ func TestEpochReset(t *testing.T) {
 //   - epoch recover that specifies unsafeAllowOverwrite = true overwrites the current epoch and does not increment the counter.
 func TestEpochRecover(t *testing.T) {
 
-	// Perform epoch recovery with a new epoch and epoch recover overwriting the current epoch.
-	t.Run("Can recover the epoch and have everything return to normal", func(t *testing.T) {
+	// Perform epoch recovery by transitioning into a new epoch (counter incremented by one)
+	t.Run("Can recover the epoch with a new epoch", func(t *testing.T) {
 		epochConfig := &testEpochConfig{
 			startEpochCounter:    startEpochCounter,
 			numEpochViews:        numEpochViews,
@@ -1653,7 +1653,7 @@ func TestEpochRecover(t *testing.T) {
 				endView        uint64 = 160
 				targetDuration uint64 = numEpochViews
 				// invalid epoch counter when recovering the current epoch the counter should equal the current epoch counter
-				epochCounter  uint64 = startEpochCounter + 100
+				epochCounter  uint64 = startEpochCounter + 1
 				targetEndTime uint64 = expectedTargetEndTime(epochTimingConfigResult, epochCounter)
 			)
 			args := getRecoveryTxArgs(env, ids, startView, stakingEndView, endView, targetDuration, targetEndTime, epochCounter)
@@ -1693,7 +1693,7 @@ func TestEpochRecover(t *testing.T) {
 				endView        uint64 = 160
 				targetDuration uint64 = numEpochViews
 				// invalid epoch counter when recovering the current epoch the counter should equal the current epoch counter
-				epochCounter  uint64 = startEpochCounter + 100
+				epochCounter  uint64 = startEpochCounter
 				targetEndTime uint64 = expectedTargetEndTime(epochTimingConfigResult, epochCounter)
 			)
 			args := getRecoveryTxArgs(env, ids, startView, stakingEndView, endView, targetDuration, targetEndTime, epochCounter)
@@ -2006,8 +2006,8 @@ func getRecoveryTxArgs(
 		cadence.NewUInt64(endView),
 		cadence.NewUInt64(targetDuration),
 		cadence.NewUInt64(targetEndTime),
-		cadence.NewArray(collectorClusters), // collectorClusters
-		cadence.NewArray(clusterQcVoteData), // clusterQCVoteData
+		cadence.NewArray(collectorClusters),
+		cadence.NewArray(clusterQcVoteData),
 		cadence.NewArray(dkgPubKeysCdc),
 		cadence.NewDictionary([]cadence.KeyValuePair{{
 			Key:   cadence.String(nodeIds[0]),
@@ -2045,9 +2045,9 @@ func verifyEpochRecoverGovernanceTx(
 		}
 	}
 	for i, dkgKeyCdc := range args[8].(cadence.Array).Values {
+		// strip `"` characters because the Cadence fmt.Stringer implementation adds them.
 		dkgPubKeys[i] = strings.ReplaceAll(dkgKeyCdc.String(), `"`, "")
 	}
-	numStakingViews := stakingEndView - startView
 	// seed is not manually set when recovering the epoch, it is randomly generated
 	metadataFields := getEpochMetadata(t, b, env, cadence.NewUInt64(epochCounter))
 	seed := strings.ReplaceAll(metadataFields["seed"].String(), `"`, "")
@@ -2073,9 +2073,9 @@ func verifyEpochRecoverGovernanceTx(
 		finalView:          endView,
 		collectorClusters:  args[6].(cadence.Array).Values,
 		randomSource:       seed,
-		dkgPhase1FinalView: startView + numStakingViews + numDKGViews - 1,
-		dkgPhase2FinalView: startView + numStakingViews + (2 * numDKGViews) - 1,
-		dkgPhase3FinalView: startView + numStakingViews + (3 * numDKGViews) - 1,
+		dkgPhase1FinalView: stakingEndView + numDKGViews,
+		dkgPhase2FinalView: stakingEndView + (2 * numDKGViews),
+		dkgPhase3FinalView: stakingEndView + (3 * numDKGViews),
 		targetDuration:     targetDuration,
 		targetEndTime:      targetEndTime,
 		numberClusterQCs:   len(args[6].(cadence.Array).Values),