Merge casper-network#4549

4549: Rewards calculation fixes r=AlexanderLimonov a=fizyk20 This PR fixes the issues with calculating rewards after an upgrade from 1.x to 2.0: - The immediate switch blocks are no longer rewarded. - Rewards calculation code passes the correct protocol versions to the data storage layer. - Data storage layer takes the protocol version into account in requests for era validators, total supply and seigniorage rate. - Upgrade tests set 2.0.x as the upgraded version. Co-authored-by: Bartłomiej Kamiński <[email protected]>
teonite · Feb 22, 2024 · ce92a85 · ce92a85
2 parents c52231d + b08f3cb
commit ce92a85
Show file tree

Hide file tree

Showing 10 changed files with 126 additions and 84 deletions.
diff --git a/ci/nctl_upgrade.sh b/ci/nctl_upgrade.sh
@@ -86,7 +86,7 @@ function dev_branch_settings() {
     pushd "$(get_path_to_remotes)"
     RC_VERSION="$(ls --group-directories-first -d */ | sort -r | head -n 1 | tr -d '/')"
 
-    [[ "$RC_VERSION" =~ (.*[^0-9])([0-9])(.)([0-9]+) ]] && INCREMENT="${BASH_REMATCH[1]}$((${BASH_REMATCH[2]} + 1))${BASH_REMATCH[3]}${BASH_REMATCH[4]}"
+    [[ "$RC_VERSION" =~ (.*[^0-9])([0-9])(.)([0-9]+) ]] && INCREMENT="2.0${BASH_REMATCH[3]}${BASH_REMATCH[4]}"
 
     RC_VERSION=$(echo "$RC_VERSION" | sed 's/\./\_/g')
     INCREMENT=$(echo "$INCREMENT" | sed 's/\./\_/g')

diff --git a/node/src/components/contract_runtime/rewards.rs b/node/src/components/contract_runtime/rewards.rs
@@ -4,7 +4,8 @@ mod tests;
 use std::{collections::BTreeMap, ops::Range, sync::Arc};
 
 use casper_storage::data_access_layer::{
-    EraValidatorsRequest, RoundSeigniorageRateResult, TotalSupplyResult,
+    EraValidatorsRequest, RoundSeigniorageRateRequest, RoundSeigniorageRateResult,
+    TotalSupplyRequest, TotalSupplyResult,
 };
 use futures::stream::{self, StreamExt as _, TryStreamExt as _};
 
@@ -32,6 +33,7 @@ impl<T> ReactorEventT for T where T: Send + From<StorageRequest> + From<Contract
 
 #[derive(Debug)]
 pub(crate) struct CitedBlock {
+    protocol_version: ProtocolVersion,
     height: u64,
     era_id: EraId,
     proposer: PublicKey,
@@ -41,6 +43,21 @@ pub(crate) struct CitedBlock {
     is_genesis: bool,
 }
 
+impl CitedBlock {
+    fn from_executable_block(block: ExecutableBlock, protocol_version: ProtocolVersion) -> Self {
+        Self {
+            protocol_version,
+            era_id: block.era_id,
+            height: block.height,
+            proposer: *block.proposer,
+            rewarded_signatures: block.rewarded_signatures,
+            state_root_hash: Digest::default(),
+            is_switch_block: block.era_report.is_some(),
+            is_genesis: block.era_id.is_genesis(),
+        }
+    }
+}
+
 #[derive(Debug)]
 pub(crate) struct RewardsInfo {
     eras_info: BTreeMap<EraId, EraInfo>,
@@ -78,7 +95,7 @@ pub enum RewardsError {
 }
 
 impl RewardsInfo {
-    pub async fn new_from_storage<REv: ReactorEventT>(
+    pub async fn new<REv: ReactorEventT>(
         effect_builder: EffectBuilder<REv>,
         protocol_version: ProtocolVersion,
         signature_rewards_max_delay: u64,
@@ -114,15 +131,13 @@ impl RewardsInfo {
             "blocks fetched",
         );
 
-        let eras_info = Self::create_eras_info(
-            effect_builder,
-            current_era_id,
-            protocol_version,
-            cited_blocks.iter(),
-        )
-        .await?;
+        let eras_info =
+            Self::create_eras_info(effect_builder, current_era_id, cited_blocks.iter()).await?;
 
-        cited_blocks.push(executable_block.into());
+        cited_blocks.push(CitedBlock::from_executable_block(
+            executable_block,
+            protocol_version,
+        ));
 
         Ok(RewardsInfo {
             eras_info,
@@ -143,7 +158,6 @@ impl RewardsInfo {
     async fn create_eras_info<REv: ReactorEventT>(
         effect_builder: EffectBuilder<REv>,
         current_era_id: EraId,
-        protocol_version: ProtocolVersion,
         mut cited_blocks: impl Iterator<Item = &CitedBlock>,
     ) -> Result<BTreeMap<EraId, EraInfo>, RewardsError> {
         let oldest_block = cited_blocks.next();
@@ -165,21 +179,22 @@ impl RewardsInfo {
             .chain(cited_blocks.filter(|&block| block.is_switch_block))
             .map(|block| {
                 let state_root_hash = block.state_root_hash;
+                let protocol_version = block.protocol_version;
                 let era = if block.is_switch_block {
                     block.era_id.successor()
                 } else {
                     block.era_id
                 };
 
-                (era, state_root_hash)
+                (era, protocol_version, state_root_hash)
             })
             .collect();
 
         let num_eras_to_fetch =
             eras_and_state_root_hashes.len() + usize::from(oldest_block_is_genesis);
 
         let mut eras_info: BTreeMap<_, _> = stream::iter(eras_and_state_root_hashes)
-            .then(|(era_id, state_root_hash)| async move {
+            .then(|(era_id, protocol_version, state_root_hash)| async move {
                 let era_validators_result = effect_builder
                     .get_era_validators_from_contract_runtime(EraValidatorsRequest::new(
                         state_root_hash,
@@ -197,7 +212,10 @@ impl RewardsInfo {
                     .ok_or_else(|| RewardsError::FailedToFetchEraValidators(state_root_hash))?
                     .1;
 
-                let total_supply = match effect_builder.get_total_supply(state_root_hash).await {
+                let total_supply_request =
+                    TotalSupplyRequest::new(state_root_hash, protocol_version);
+                let total_supply = match effect_builder.get_total_supply(total_supply_request).await
+                {
                     TotalSupplyResult::RootNotFound
                     | TotalSupplyResult::MintNotFound
                     | TotalSupplyResult::ValueNotFound(_)
@@ -207,8 +225,10 @@ impl RewardsInfo {
                     TotalSupplyResult::Success { total_supply } => total_supply,
                 };
 
+                let seignorate_rate_request =
+                    RoundSeigniorageRateRequest::new(state_root_hash, protocol_version);
                 let seignorate_rate = match effect_builder
-                    .get_round_seigniorage_rate(state_root_hash)
+                    .get_round_seigniorage_rate(seignorate_rate_request)
                     .await
                 {
                     RoundSeigniorageRateResult::RootNotFound
@@ -342,17 +362,19 @@ pub(crate) async fn fetch_data_and_calculate_rewards_for_era<REv: ReactorEventT>
         "starting the rewards calculation"
     );
 
-    if current_era_id.is_genesis() {
-        // Special case: genesis block does not yield any reward, because there is no block
-        // producer, and no previous blocks whose signatures are to be rewarded:
+    if current_era_id.is_genesis()
+        || current_era_id == chainspec.protocol_config.activation_point.era_id()
+    {
+        // Special case: genesis block and immediate switch blocks do not yield any reward, because
+        // there is no block producer, and no signatures from previous blocks to be rewarded:
         Ok(chainspec
             .network_config
             .accounts_config
             .validators()
             .map(|account| (account.public_key.clone(), U512::zero()))
             .collect())
     } else {
-        let rewards_info = RewardsInfo::new_from_storage(
+        let rewards_info = RewardsInfo::new(
             effect_builder,
             chainspec.protocol_version(),
             chainspec.core_config.signature_rewards_max_delay,
@@ -499,6 +521,7 @@ async fn collect_past_blocks_batched<REv: From<StorageRequest>>(
 impl From<Block> for CitedBlock {
     fn from(block: Block) -> Self {
         Self {
+            protocol_version: block.protocol_version(),
             era_id: block.era_id(),
             height: block.height(),
             proposer: block.proposer().clone(),
@@ -509,17 +532,3 @@ impl From<Block> for CitedBlock {
         }
     }
 }
-
-impl From<ExecutableBlock> for CitedBlock {
-    fn from(block: ExecutableBlock) -> Self {
-        Self {
-            era_id: block.era_id,
-            height: block.height,
-            proposer: *block.proposer,
-            rewarded_signatures: block.rewarded_signatures,
-            state_root_hash: Digest::default(),
-            is_switch_block: block.era_report.is_some(),
-            is_genesis: block.era_id.is_genesis(),
-        }
-    }
-}
diff --git a/node/src/components/rpc_server/rpcs/docs.rs b/node/src/components/rpc_server/rpcs/docs.rs
@@ -27,7 +27,7 @@ use super::{
 use crate::effect::EffectBuilder;
 
 pub(crate) const DOCS_EXAMPLE_PROTOCOL_VERSION: ProtocolVersion =
-    ProtocolVersion::from_parts(1, 5, 3);
+    ProtocolVersion::from_parts(2, 0, 0);
 
 const DEFINITIONS_PATH: &str = "#/components/schemas/";
 

diff --git a/node/src/effect.rs b/node/src/effect.rs
@@ -1992,11 +1992,10 @@ impl<REv> EffectBuilder<REv> {
     /// Returns the total supply from the given `root_hash`.
     ///
     /// This operation is read only.
-    pub(crate) async fn get_total_supply(self, state_hash: Digest) -> TotalSupplyResult
+    pub(crate) async fn get_total_supply(self, request: TotalSupplyRequest) -> TotalSupplyResult
     where
         REv: From<ContractRuntimeRequest>,
     {
-        let request = TotalSupplyRequest::new(state_hash);
         self.make_request(
             move |responder| ContractRuntimeRequest::GetTotalSupply { request, responder },
             QueueKind::ContractRuntime,
@@ -2009,12 +2008,11 @@ impl<REv> EffectBuilder<REv> {
     /// This operation is read only.
     pub(crate) async fn get_round_seigniorage_rate(
         self,
-        state_hash: Digest,
+        request: RoundSeigniorageRateRequest,
     ) -> RoundSeigniorageRateResult
     where
         REv: From<ContractRuntimeRequest>,
     {
-        let request = RoundSeigniorageRateRequest::new(state_hash);
         self.make_request(
             move |responder| ContractRuntimeRequest::GetRoundSeigniorageRate { request, responder },
             QueueKind::ContractRuntime,

diff --git a/resources/local/chainspec.toml.in b/resources/local/chainspec.toml.in
@@ -1,6 +1,6 @@
 [protocol]
 # Protocol version.
-version = '1.0.0'
+version = '2.0.0'
 # Whether we need to clear latest blocks back to the switch block just before the activation point or not.
 hard_reset = false
 # This protocol version becomes active at this point.

diff --git a/resources/production/chainspec.toml b/resources/production/chainspec.toml
@@ -1,6 +1,6 @@
 [protocol]
 # Protocol version.
-version = '1.5.3'
+version = '2.0.0'
 # Whether we need to clear latest blocks back to the switch block just before the activation point or not.
 hard_reset = true
 # This protocol version becomes active at this point.