Epochs validating and mining metrics fix (#880)

* create reproduction of Case 3 validators incorrectly getting  epochs_validating_and_mining incremented.

* refactor tower update metrics, tests not passing

* lazy reset of counters happens at verify stage of towerstate.

* functional test for lazy miner state reset

* adjust TowerStateResource view to include actual_count_proofs_in_epoch

* change json-rpc method for getting miner state.

json-rpc/src/data.rs

@@ -295,10 +295,11 @@ pub fn get_miner_state(
     db: &dyn DbReader,
     version: u64,
     account: AccountAddress,
-    // ledger_info: &LedgerInfoWithSignatures,
+    ledger_info: &LedgerInfoWithSignatures,
 ) -> Result<TowerStateResourceView, JsonRpcError> {
+    let epoch = ledger_info.ledger_info().epoch();
     match get_account_state(db, account, version)? {
-        Some(s) => TowerStateResourceView::try_from(s).map_err(Into::into),
+        Some(s) => TowerStateResourceView::from_state_and_epoch(s, epoch).map_err(Into::into),
         None => Err(JsonRpcError::internal_error("No account state found".to_owned())),
     }
 

json-rpc/src/methods.rs

@@ -381,7 +381,7 @@ impl<'a> Handler<'a> {
         &self,
         params: GetTowerStateParams,
     ) -> Result<TowerStateResourceView, JsonRpcError> {
-        data::get_miner_state(self.service.db.borrow(), self.version(), params.account)
+        data::get_miner_state(self.service.db.borrow(), self.version(), params.account, &self.ledger_info)
     }
 
     async fn get_oracle_upgrade_state(

json-rpc/types/src/views.rs

@@ -1442,28 +1442,69 @@ pub struct TowerStateResourceView {
     pub count_proofs_in_epoch: u64,
     pub epochs_validating_and_mining: u64,
     pub contiguous_epochs_validating_and_mining: u64,
-    pub epochs_since_last_account_creation: u64
+    pub epochs_since_last_account_creation: u64,
+    // ADDED FIELDS FROM ORIGINAL MOVE STRUCT
+    // the actual count of proofs in epoch considering the lazy computation
+    pub actual_count_proofs_in_epoch: u64
 }
 
-impl TryFrom<AccountState> for TowerStateResourceView {
-    type Error = Error;
+impl TowerStateResourceView {
+  pub fn from_state_and_epoch(state: AccountState, this_epoch: u64) -> Result<TowerStateResourceView, Error>  {
 
-    fn try_from(state: AccountState) -> Result<TowerStateResourceView, Error> {
+      let mut actual_count_proofs_in_epoch = 0;
       if let Some(m) = state.get_miner_state()? {
+        if m.latest_epoch_mining == this_epoch {
+         actual_count_proofs_in_epoch =  m.count_proofs_in_epoch;
+        }
+
         Ok(TowerStateResourceView {
             previous_proof_hash: BytesView::from( m.previous_proof_hash),
             verified_tower_height: m.verified_tower_height, // user's latest verified_tower_height
             latest_epoch_mining: m.latest_epoch_mining,
             count_proofs_in_epoch: m.count_proofs_in_epoch,
             epochs_validating_and_mining: m.epochs_validating_and_mining,
             contiguous_epochs_validating_and_mining: m.contiguous_epochs_validating_and_mining,
-            epochs_since_last_account_creation: m.epochs_since_last_account_creation
+            epochs_since_last_account_creation: m.epochs_since_last_account_creation,
+            // the proof count adjusted for lazy computation
+            actual_count_proofs_in_epoch,
         })
       } else {
         bail!("could not get tower state")
       }
-    }
-}
+  }
+}
+
+// impl TryFrom<AccountState> for TowerStateResourceView {
+//     type Error = Error;
+
+//     fn try_from(state: AccountState) -> Result<TowerStateResourceView, Error> {
+//       let this_epoch = match state.get_configuration_resource()?{
+//           Some(cr) => cr.epoch(),
+//           None => bail!("cannot get epoch data from account state"),
+//       };
+
+//       let mut actual_count_proofs_in_epoch = 0;
+//       if let Some(m) = state.get_miner_state()? {
+//         if m.latest_epoch_mining == this_epoch {
+//          actual_count_proofs_in_epoch =  m.count_proofs_in_epoch;
+//         }
+
+//         Ok(TowerStateResourceView {
+//             previous_proof_hash: BytesView::from( m.previous_proof_hash),
+//             verified_tower_height: m.verified_tower_height, // user's latest verified_tower_height
+//             latest_epoch_mining: m.latest_epoch_mining,
+//             count_proofs_in_epoch: m.count_proofs_in_epoch,
+//             epochs_validating_and_mining: m.epochs_validating_and_mining,
+//             contiguous_epochs_validating_and_mining: m.contiguous_epochs_validating_and_mining,
+//             epochs_since_last_account_creation: m.epochs_since_last_account_creation,
+//             // the proof count adjusted for lazy computation
+//             actual_count_proofs_in_epoch,
+//         })
+//       } else {
+//         bail!("could not get tower state")
+//       }
+//     }
+// }
 
 //////// 0L ////////
 #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]

language/diem-framework/modules/0L/Reconfigure.move → language/diem-framework/modules/0L/EpochBoundary.move

@@ -8,7 +8,7 @@
 
 
 address 0x1 {
-module EpochBoundary { // TODO: Rename to Boundary
+module EpochBoundary {
     use 0x1::CoreAddresses;
     use 0x1::Subsidy;
     use 0x1::NodeWeight;
@@ -17,7 +17,6 @@ module EpochBoundary { // TODO: Rename to Boundary
     use 0x1::Globals;
     use 0x1::Vector;
     use 0x1::Stats;
-    use 0x1::ValidatorUniverse;
     use 0x1::AutoPay;
     use 0x1::Epoch;
     use 0x1::DiemConfig;
@@ -43,7 +42,7 @@ module EpochBoundary { // TODO: Rename to Boundary
 
         process_fullnodes(vm, nominal_subsidy_per);
 
-        process_validators(vm, subsidy_units, outgoing_compliant_set);
+        process_validators(vm, subsidy_units, *&outgoing_compliant_set);
 
         let proposed_set = propose_new_set(vm, height_start, height_now);
 
@@ -52,7 +51,7 @@ module EpochBoundary { // TODO: Rename to Boundary
             DiemAccount::slow_wallet_epoch_drip(vm, Globals::get_unlock());
             // update_validator_withdrawal_limit(vm);
         };
-        reset_counters(vm, proposed_set, height_now)
+        reset_counters(vm, proposed_set, outgoing_compliant_set, height_now)
     }
 
     // process fullnode subsidy
@@ -157,15 +156,13 @@ module EpochBoundary { // TODO: Rename to Boundary
         proposed_set
     }
 
-    fun reset_counters(vm: &signer, proposed_set: vector<address>, height_now: u64) {
+    fun reset_counters(vm: &signer, proposed_set: vector<address>, outgoing_compliant: vector<address>, height_now: u64) {
 
         // Reset Stats
         Stats::reconfig(vm, &proposed_set);
 
-        // Migrate TowerState list from elegible: in case there is no minerlist 
-        // struct, use eligible for migrate_eligible_validators
-        let eligible = ValidatorUniverse::get_eligible_validators(vm);
-        TowerState::reconfig(vm, &eligible);
+        // Migrate TowerState list from elegible.
+        TowerState::reconfig(vm, &outgoing_compliant);
 
         // Reconfigure the network
         DiemSystem::bulk_update_validators(vm, proposed_set);

language/diem-framework/modules/0L/Globals.move

@@ -93,7 +93,7 @@ module Globals {
           max_validators_per_set: 100,
           subsidy_ceiling_gas: 296 * COIN_SCALING_FACTOR,
           vdf_difficulty: 100,
-          epoch_mining_thres_lower: 1,
+          epoch_mining_thres_lower: 2, // Note: in test harness, the validators are in epoch 0 where they have 1 proof already committed from mock genesis ceremony.
           epoch_mining_thres_upper: 1000, // upper bound unlimited
           epoch_slow_wallet_unlock: 10,
         }
@@ -105,7 +105,7 @@ module Globals {
           max_validators_per_set: 100,
           subsidy_ceiling_gas: 8640000 * COIN_SCALING_FACTOR,
           vdf_difficulty: 120000000,
-          epoch_mining_thres_lower: 1,
+          epoch_mining_thres_lower: 1, // in testnet, staging, we don't want to wait too long between proofs.
           epoch_mining_thres_upper: 72, // upper bound enforced at 20 mins per proof.
           epoch_slow_wallet_unlock: 10000000,
         }