Improvements after review

Signed-off-by: Kostiantyn Stepaniuk <[email protected]>

src/esperanza/finalizationstate.cpp

@@ -155,8 +155,8 @@ void FinalizationState::InstaJustify() {
   m_last_justified_epoch = m_current_epoch - 1;
 
   if (m_current_epoch > 1) {
-    uint32_t prev_justified = m_current_epoch - 2;
-    if (GetCheckpoint(prev_justified).m_is_justified) {
+    uint32_t to_be_finalized = m_current_epoch - 2;
+    if (GetCheckpoint(to_be_finalized).m_is_justified) {
       cp.m_is_finalized = true;
       m_last_finalized_epoch = m_last_justified_epoch;
     }
@@ -202,19 +202,14 @@ ufp64::ufp64_t FinalizationState::GetCollectiveRewardFactor() {
     return 0;
   }
 
-  // we use "m_current_epoch - 2" epoch to retrieve dynasty votes because:
-  // -1 as m_current_epoch is already incremented inside of InitializeEpoch
-  // before this function is invoked
-  // -1 because when we store votes we use vote.m_target_epoch which is always
-  // one below the m_current_epoch
-  const uint32_t checkpoint_epoch = m_current_epoch - 2;
+  assert(m_last_finalized_epoch == m_current_epoch - 2);
 
   ufp64::ufp64_t curVoteFraction = ufp64::div_2uint(
-      GetCheckpoint(checkpoint_epoch).GetCurDynastyVotes(m_expected_source_epoch),
+      GetCheckpoint(m_last_finalized_epoch).GetCurDynastyVotes(m_expected_source_epoch),
       m_cur_dyn_deposits);
 
   ufp64::ufp64_t prevVoteFraction = ufp64::div_2uint(
-      GetCheckpoint(checkpoint_epoch).GetPrevDynastyVotes(m_expected_source_epoch),
+      GetCheckpoint(m_last_finalized_epoch).GetPrevDynastyVotes(m_expected_source_epoch),
       m_prev_dyn_deposits);
 
   ufp64::ufp64_t voteFraction = ufp64::min(curVoteFraction, prevVoteFraction);
@@ -259,10 +254,6 @@ uint64_t FinalizationState::GetDepositSize(const uint160 &validatorAddress) cons
   }
 }
 
-uint32_t FinalizationState::GetExpectedSourceEpoch() const {
-  return m_expected_source_epoch;
-}
-
 uint32_t FinalizationState::GetRecommendedTargetEpoch() const {
   return m_recommended_target_epoch;
 }

src/esperanza/finalizationstate.h

@@ -133,7 +133,6 @@ class FinalizationState : public FinalizationStateData {
 
   uint64_t GetDepositSize(const uint160 &validatorAddress) const;
 
-  uint32_t GetExpectedSourceEpoch() const;
   uint32_t GetRecommendedTargetEpoch() const;
 
   Vote GetRecommendedVote(const uint160 &validatorAddress) const;

src/test/esperanza/finalizationstate_utils.cpp

@@ -85,6 +85,10 @@ void FinalizationStateSpy::shuffle() {
 
 #undef ConstRand
 
+uint32_t FinalizationStateSpy::GetExpectedSourceEpoch() const {
+  return m_expected_source_epoch;
+}
+
 void FinalizationStateSpy::CreateAndActivateDeposit(const uint160 &validator_address, CAmount deposit_size) {
   BOOST_REQUIRE_EQUAL(GetCurrentEpoch(), 0);
 

src/test/esperanza/finalizationstate_utils.h

@@ -50,6 +50,7 @@ class FinalizationStateSpy : public FinalizationState {
     return m_settings.bounty_fraction_denominator;
   }
 
+  uint32_t GetExpectedSourceEpoch() const;
   void CreateAndActivateDeposit(const uint160 &validator_address, CAmount deposit_size);
   void CreateDeposit(const uint160 &finalizer_address, CAmount deposit_size);
 

test/functional/finalization_vote.py

@@ -152,16 +152,7 @@ def run_test(self):
         generate_block(node0, count=4)
         prev_tx = finalizer1.decoderawtransaction(prev_tx)['txid']
 
-        # test that node recognizes old and invalid votes.
-        # tx = make_vote_tx(finalizer1, address1, node0.getblockhash(30), 1, 2, prev_tx)
-        # assert_raises_rpc_error(-26, 'bad-vote-invalid', node0.sendrawtransaction, tx)
-        # tx = make_vote_tx(finalizer1, address1, node0.getblockhash(30), 2, 3, prev_tx)
-        # assert_raises_rpc_error(-26, 'bad-vote-invalid', node0.sendrawtransaction, tx)
-        # tx = make_vote_tx(finalizer1, address1, node0.getblockhash(30), 7, 6, prev_tx)
-        # assert_raises_rpc_error(-26, 'bad-vote-invalid', node0.sendrawtransaction, tx)
-        self.log.info('Tested outdated and invalid vote votes')
-
-        # check that make_vote_tx works as expected (we really rely on this guy on tests above)
+        # check that make_vote_tx works as expected
         tx = make_vote_tx(finalizer1, address1, node0.getblockhash(30), 5, 6, prev_tx)
         node0.sendrawtransaction(tx)
         self.wait_for_vote_and_disconnect(finalizer=finalizer2, node=node0)
@@ -175,6 +166,17 @@ def run_test(self):
                                          'validators': 3})
         self.log.info('make_vote_tx works together with real finalizers')
 
+        # test that node recognizes old and invalid votes.
+        tx = make_vote_tx(finalizer1, address1, node0.getblockhash(30), 1, 2, prev_tx)
+        assert_raises_rpc_error(-26, 'bad-vote-invalid', node0.sendrawtransaction, tx)
+        tx = make_vote_tx(finalizer1, address1, node0.getblockhash(30), 2, 3, prev_tx)
+        assert_raises_rpc_error(-26, 'bad-vote-invalid', node0.sendrawtransaction, tx)
+        tx = make_vote_tx(finalizer1, address1, node0.getblockhash(30), 7, 9, prev_tx)
+        assert_raises_rpc_error(-26, 'bad-vote-invalid', node0.sendrawtransaction, tx)
+        tx = make_vote_tx(finalizer1, address1, node0.getblockhash(30), 7, 6, prev_tx)
+        assert_raises_rpc_error(-26, 'bad-vote-invalid', node0.sendrawtransaction, tx)
+        self.log.info('Tested outdated and invalid vote votes')
+
         # UNIT-E TODO: there is a know issue https://github.com/dtr-org/unit-e/issues/643
         # that finalizer doesn't vote after processing the checkpoint.
         # Once it's resolved, the bellow test must be uncommented