fix(node): remove obsolete ProtocolInfo from chain_manager

Author: drcpu-github

Diff for: data_structures/src/chain/mod.rs

@@ -43,7 +43,7 @@ use crate::{
     },
     get_environment, get_protocol_version, get_protocol_version_activation_epoch,
     proto::{
-        versioning::{ProtocolInfo, ProtocolVersion, Versioned, VersionedHashable},
+        versioning::{ProtocolVersion, Versioned, VersionedHashable},
         ProtobufConvert,
     },
     staking::prelude::*,
@@ -76,9 +76,6 @@ pub trait Hashable {
 /// Data structure holding critical information about the chain state and protocol constants
 #[derive(Debug, Serialize, Deserialize, PartialEq, Clone, Default)]
 pub struct ChainInfo {
-    /// Keeps track of protocol versions and their related data (e.g. activation epoch)
-    pub protocol: ProtocolInfo,
-
     /// Blockchain valid environment
     pub environment: Environment,
 

Diff for: data_structures/src/lib.rs

@@ -126,6 +126,19 @@ pub fn set_environment(environment: Environment) {
     }
 }
 
+/// Protocol version that we are running.
+pub fn get_protocol_info() -> ProtocolInfo {
+    // This unwrap is safe as long as the lock is not poisoned.
+    // The lock can only become poisoned when a writer panics.
+    let protocol_info = PROTOCOL.read().unwrap();
+
+    ProtocolInfo {
+        current_version: protocol_info.current_version,
+        all_versions: protocol_info.all_versions.clone(),
+        all_checkpoints_periods: protocol_info.all_checkpoints_periods.clone(),
+    }
+}
+
 /// Protocol version that we are running.
 pub fn get_protocol_version(epoch: Option<Epoch>) -> ProtocolVersion {
     // This unwrap is safe as long as the lock is not poisoned.
@@ -168,10 +181,6 @@ pub fn load_protocol_info(info: ProtocolInfo) {
     *protocol_info = info;
 }
 
-pub fn initialize_default(default_checkpoint_period: u16) {
-    register_protocol_version(ProtocolVersion::default(), 0, default_checkpoint_period);
-}
-
 pub fn clear_protocol_info() {
     log::info!("Clearing all protocol versions");
     let mut protocol_info = PROTOCOL.write().unwrap();

Diff for: node/src/actors/chain_manager/actor.rs

@@ -13,9 +13,7 @@ use witnet_data_structures::{
     },
     data_request::DataRequestPool,
     get_environment, get_protocol_version_activation_epoch, get_protocol_version_period,
-    initialize_default, load_protocol_info,
     proto::versioning::ProtocolVersion,
-    refresh_protocol_version,
     staking::prelude::*,
     superblock::SuperBlockState,
     types::LastBeacon,
@@ -264,7 +262,6 @@ impl ChainManager {
                                 checkpoint: 0,
                                 hash_prev_vrf: hash_prev_block,
                             },
-                            ..ChainInfo::default()
                         };
 
                         let bootstrap_committee = chain_info
@@ -376,20 +373,6 @@ impl ChainManager {
                     chain_info.highest_block_checkpoint.hash_prev_block
                 );
 
-                // Load protocol info into global state (overwrites whatever was set through
-                // configuration). If possible, derives the current protocol version from that
-                // info and the current epoch. This essentially allows a node to catch up with
-                // a new protocol version if the transition happened while it was down.
-                load_protocol_info(chain_info.protocol.clone());
-                // If the protocol info we loaded above originates from a default initialization
-                // of chain info, the default protocol versions are not registered yet. We cannot
-                // use the Default trait to fix this because we do not have access to the required
-                // checkpoint period from consensus constants in the data structures module.
-                if get_protocol_version_activation_epoch(ProtocolVersion::default()) == u32::MAX {
-                    initialize_default(consensus_constants.checkpoints_period);
-                }
-                refresh_protocol_version(chain_info.highest_block_checkpoint.checkpoint);
-
                 // We may need to update the epoch constants in both the sessions and epoch manager to
                 // correctly calculate the current epoch after wit/2 is activated while the configuration
                 // has not been updated yet.

Diff for: node/src/actors/chain_manager/handlers.rs

@@ -12,12 +12,12 @@ use futures::future::Either;
 use itertools::Itertools;
 use witnet_data_structures::{
     chain::{
-        tapi::ActiveWips, Block, ChainInfo, ChainState, CheckpointBeacon, ConsensusConstantsWit2,
+        tapi::ActiveWips, Block, ChainState, CheckpointBeacon, ConsensusConstantsWit2,
         DataRequestInfo, Epoch, Hash, Hashable, NodeStats, PublicKeyHash, SuperBlockVote,
         SupplyInfo, SupplyInfo2, ValueTransferOutput,
     },
     error::{ChainInfoError, TransactionError::DataRequestNotFound},
-    get_protocol_version,
+    get_protocol_info, get_protocol_version, get_protocol_version_activation_epoch,
     proto::versioning::ProtocolVersion,
     refresh_protocol_version,
     staking::{
@@ -143,12 +143,8 @@ impl Handler<EpochNotification<EveryEpochPayload>> for ChainManager {
         // The best candidate must be cleared on every epoch
         let best_candidate = self.best_candidate.take();
 
-        // Make sure that the protocol version in the chain state is kept up to date
+        // Update the global protocol version state if necessary
         let expected_protocol_version = get_protocol_version(self.current_epoch);
-        if let Some(ChainInfo { protocol, .. }) = &mut self.chain_state.chain_info {
-            protocol.current_version = expected_protocol_version;
-        }
-        // Also update the global protocol version state if necessary
         let current_protocol_version = get_protocol_version(None);
         if expected_protocol_version != current_protocol_version {
             refresh_protocol_version(current_epoch);
@@ -2038,11 +2034,8 @@ impl Handler<GetSupplyInfo2> for ChainManager {
         let current_staked_supply = self.chain_state.stakes.total_staked().nanowits();
 
         let wit1_block_reward = chain_info.consensus_constants.initial_block_reward;
-        let wit2_activated = chain_info.protocol.current_version == ProtocolVersion::V2_0;
-        let wit2_activation_epoch = chain_info
-            .protocol
-            .all_versions
-            .get_activation_epoch(chain_info.protocol.current_version);
+        let wit2_activated = get_protocol_version(None) == ProtocolVersion::V2_0;
+        let wit2_activation_epoch = get_protocol_version_activation_epoch(ProtocolVersion::V2_0);
         let wit2_block_reward =
             ConsensusConstantsWit2::default().get_validator_block_reward(current_epoch);
 
@@ -2438,12 +2431,7 @@ impl Handler<GetProtocolInfo> for ChainManager {
     type Result = <GetProtocolInfo as Message>::Result;
 
     fn handle(&mut self, _msg: GetProtocolInfo, _ctx: &mut Self::Context) -> Self::Result {
-        let chain_info = &self.chain_state.chain_info;
-
-        match chain_info {
-            None => Ok(None),
-            Some(ChainInfo { protocol, .. }) => Ok(Some(protocol.clone())),
-        }
+        Ok(Some(get_protocol_info()))
     }
 }
 

Diff for: node/src/actors/chain_manager/mod.rs

@@ -1032,13 +1032,6 @@ impl ChainManager {
                             scheduled_epoch,
                             checkpoint_period,
                         );
-                        // Register the 2_0 protocol into chain state (namely, chain info) so that
-                        // the scheduled activation data eventually gets persisted into storage.
-                        chain_info.protocol.register(
-                            scheduled_epoch,
-                            ProtocolVersion::V2_0,
-                            checkpoint_period,
-                        );
 
                         if let Some(epoch_constants) = &mut self.epoch_constants {
                             match epoch_constants.set_values_for_wit2(
@@ -3856,7 +3849,7 @@ mod tests {
             KeyedSignature, OutputPointer, PartialConsensusConstants, PublicKey, SecretKey,
             Signature, StakeOutput, ValueTransferOutput,
         },
-        proto::versioning::{ProtocolInfo, VersionedHashable},
+        proto::versioning::VersionedHashable,
         transaction::{
             CommitTransaction, DRTransaction, MintTransaction, RevealTransaction, StakeTransaction,
             StakeTransactionBody, UnstakeTransaction, UnstakeTransactionBody, VTTransaction,
@@ -4016,7 +4009,6 @@ mod tests {
                     hash_prev_block: Hash::SHA256([1; 32]),
                 },
                 highest_vrf_output: CheckpointVRF::default(),
-                protocol: ProtocolInfo::default(),
             });
 
             assert_eq!(
@@ -4438,7 +4430,6 @@ mod tests {
                     hash_prev_block: Hash::SHA256([1; 32]),
                 },
                 highest_vrf_output: CheckpointVRF::default(),
-                protocol: ProtocolInfo::default(),
             });
             chain_manager.chain_state.reputation_engine = Some(ReputationEngine::new(1000));
             chain_manager.vrf_ctx = Some(VrfCtx::secp256k1().unwrap());
@@ -4573,7 +4564,6 @@ mod tests {
                     hash_prev_block: Hash::SHA256([1; 32]),
                 },
                 highest_vrf_output: CheckpointVRF::default(),
-                protocol: ProtocolInfo::default(),
             });
             let out_ptr = OutputPointer {
                 transaction_id: "0000000000000000000000000000000000000000000000000000000000000001"
@@ -4685,7 +4675,6 @@ mod tests {
                 hash_prev_block: Hash::SHA256([1; 32]),
             },
             highest_vrf_output: CheckpointVRF::default(),
-            protocol: ProtocolInfo::default(),
         });
         chain_manager.chain_state.stakes = stakes;
 

Diff for: node/src/storage_mngr/node_migrations.rs

@@ -78,16 +78,32 @@ fn migrate_chain_state_v3_to_v4(old_chain_state_bytes: &[u8]) -> Vec<u8> {
     .concat()
 }
 
+fn migrate_chain_state_v4_to_v5(old_chain_state_bytes: &[u8]) -> Vec<u8> {
+    let db_version: u32 = 5;
+    let db_version_bytes = db_version.to_le_bytes();
+
+    // Removal of fields in ChainState v5:
+    let protocol_info = ProtocolInfo::default();
+    let protocol_info_bytes = serialize(&protocol_info).unwrap();
+
+    [
+        &db_version_bytes,
+        &old_chain_state_bytes[4..5],
+        &old_chain_state_bytes[5 + protocol_info_bytes.len()..],
+    ]
+    .concat()
+}
+
 fn migrate_chain_state(mut bytes: Vec<u8>) -> Result<ChainState, failure::Error> {
     loop {
         let version = check_chain_state_version(&bytes);
-        log::debug!("Chain state version as read from storage is {:?}", version);
+        log::info!("Chain state version as read from storage is {:?}", version);
 
         match version {
             Ok(0) => {
                 // Migrate from v0 to v2
                 bytes = migrate_chain_state_v0_to_v2(&bytes);
-                log::debug!("Successfully migrated ChainState v0 to v2");
+                log::info!("Successfully migrated ChainState v0 to v2");
             }
             Ok(2) => {
                 // Migrate from v2 to v3
@@ -96,14 +112,19 @@ fn migrate_chain_state(mut bytes: Vec<u8>) -> Result<ChainState, failure::Error>
                 // and the UTXOs are stored in separate keys. But that operation is done in the
                 // ChainManager on initialization, here we just update the db_version field.
                 migrate_chain_state_v2_to_v3(&mut bytes);
-                log::debug!("Successfully migrated ChainState v2 to v3");
+                log::info!("Successfully migrated ChainState v2 to v3");
             }
             Ok(3) => {
                 // Migrate from v3 to v4
                 bytes = migrate_chain_state_v3_to_v4(&bytes);
-                log::debug!("Successfully migrated ChainState v3 to v4");
+                log::info!("Successfully migrated ChainState v3 to v4");
             }
             Ok(4) => {
+                // Migrate from v3 to v4
+                bytes = migrate_chain_state_v4_to_v5(&bytes);
+                log::info!("Successfully migrated ChainState v4 to v5");
+            }
+            Ok(5) => {
                 // Latest version
                 // Skip the first 4 bytes because they are used to encode db_version
                 return match deserialize(&bytes[4..]) {
@@ -207,7 +228,7 @@ pub fn put_chain_state_in_batch<K>(
 where
     K: serde::Serialize + 'static,
 {
-    let db_version: u32 = 4;
+    let db_version: u32 = 5;
     // The first byte of the ChainState db_version must never be 0 or 1,
     // because that can be confused with version 0.
     assert!(db_version.to_le_bytes()[0] >= 2);

Diff for: node/tests/serialization.rs

@@ -1,7 +1,7 @@
 use bincode::{deserialize, serialize};
 use serde::{de::DeserializeOwned, Serialize};
 use std::fmt::Debug;
-use witnet_data_structures::{chain::*, proto::versioning::ProtocolInfo};
+use witnet_data_structures::chain::*;
 
 fn t<T>(al: T)
 where
@@ -57,7 +57,6 @@ fn chain_state() {
             checkpoint: 0,
             hash_prev_vrf: bootstrap_hash,
         },
-        protocol: ProtocolInfo::default(),
     };
     let c = ChainState {
         chain_info: Some(chain_info),