Problem (Fix #1142): revised punishment mechanisms not implemented

Solution:
- revised staking and punishment implementation

Update the app_hash in devnet genesis

cleanup unused files

review suggestions

hide validator_snapshot

split up staking_table modules

extract punish from begin_block

add logs and fix nonce contension

Problem: nonce logic makes client difficult to handle

Solution:
- Make nonce to be the count of transactions sent from the staking address

chain-abci/Cargo.toml

@@ -38,6 +38,7 @@ structopt = "0.3"
 secp256k1zkp = { git = "https://github.com/crypto-com/rust-secp256k1-zkp.git", rev = "d50d5998a11064591d86583b297130d6d9636aa5", features = ["recovery", "endomorphism"] }
 blake2 = "0.8"
 parity-scale-codec = { features = ["derive"], version = "1.2" }
+thiserror = "1.0"
 
 [target.'cfg(target_os = "linux")'.dependencies]
 enclave-u-common = { path = "../chain-tx-enclave/enclave-u-common" }

chain-abci/src/app/app_init.rs

@@ -1,6 +1,3 @@
-mod validator_state;
-pub mod validator_state_update;
-
 use std::collections::{BTreeMap, HashMap};
 use std::convert::TryInto;
 use std::mem;
@@ -14,76 +11,50 @@ use serde::{Deserialize, Serialize};
 #[cfg(all(not(feature = "mock-validation"), target_os = "linux"))]
 use crate::enclave_bridge::real::start_zmq;
 use crate::enclave_bridge::EnclaveProxy;
+use crate::staking::StakingTable;
 use chain_core::common::MerkleTree;
 use chain_core::common::Timespec;
 use chain_core::common::{H256, HASH_SIZE_256};
+use chain_core::compute_app_hash;
 use chain_core::init::address::RedeemAddress;
 use chain_core::init::coin::Coin;
 use chain_core::init::config::InitConfig;
 use chain_core::init::config::NetworkParameters;
 use chain_core::state::account::StakedStateDestination;
 use chain_core::state::account::{CouncilNode, StakedState, StakedStateAddress};
-use chain_core::state::tendermint::TendermintValidatorAddress;
 use chain_core::state::tendermint::{BlockHeight, TendermintVotePower};
 use chain_core::state::{ChainState, RewardsPoolState};
 use chain_core::tx::TxAux;
-use chain_core::{compute_app_hash, ChainInfo};
+use chain_core::ChainInfo;
 use chain_storage::account::AccountWrapper;
 use chain_storage::account::StarlingFixedKey;
 use chain_storage::account::{pure_account_storage, AccountStorage};
 use chain_storage::buffer::{
     flush_storage, GetStaking, KVBuffer, StakingBuffer, StakingGetter, StoreKV, StoreStaking,
 };
 use chain_storage::{Storage, StoredChainState};
-use chain_tx_validation::NodeChecker;
-pub use validator_state::ValidatorState;
-use validator_state::ValidatorStateHelper;
 
 /// ABCI app state snapshot
 #[derive(Serialize, Deserialize, Clone, Encode, Decode)]
 pub struct ChainNodeState {
-    /// last processed block height
+    /// last processed block height, set in end block
     pub last_block_height: BlockHeight,
     /// last committed merkle root
     pub last_apphash: H256,
-    /// time in previous block's header or genesis time
+    /// time in current block's header or genesis time, set in begin block
     pub block_time: Timespec,
-    /// state of validators (keys, voting power, punishments, rewards...)
+    /// current block's height or 0 for genesis, set in begin block
+    pub block_height: BlockHeight,
+    /// Indexings of validator states
     #[serde(skip)]
-    pub validators: ValidatorState,
+    pub staking_table: StakingTable,
     /// genesis time
     pub genesis_time: Timespec,
 
     /// The parts of states which involved in computing app_hash
     pub top_level: ChainState,
 }
 
-impl NodeChecker for &ChainNodeState {
-    /// minimal required stake
-    fn minimum_effective_stake(&self) -> Coin {
-        self.minimum_effective()
-    }
-    /// if the TM pubkey/address was/is already used in the consensus
-    fn is_current_validator(&self, address: &TendermintValidatorAddress) -> bool {
-        self.validators.is_current_validator(address)
-    }
-    /// if the staking address was/is already used in the consensus
-    fn is_current_validator_stake(&self, address: &StakedStateAddress) -> bool {
-        self.validators
-            .validator_state_helper
-            .validator_voting_power
-            .contains_key(address)
-    }
-    /// if that combo is to be removed
-    fn is_current_previous_unbond(
-        &self,
-        address: &StakedStateAddress,
-        tm_address: &TendermintValidatorAddress,
-    ) -> bool {
-        self.validators.is_scheduled_for_delete(address, tm_address)
-    }
-}
-
 impl StoredChainState for ChainNodeState {
     fn get_encoded(&self) -> Vec<u8> {
         self.encode()
@@ -99,31 +70,20 @@ impl StoredChainState for ChainNodeState {
 }
 
 impl ChainNodeState {
-    pub fn minimum_effective(&self) -> Coin {
-        if self.validators.number_validators() < self.top_level.network_params.get_max_validators()
-        {
-            self.top_level
-                .network_params
-                .get_required_council_node_stake()
-        } else {
-            (self.validators.lowest_vote_power().as_non_base_coin() + Coin::one())
-                .expect("range of TM vote power < Coin")
-        }
-    }
-
     pub fn genesis(
         genesis_apphash: H256,
         genesis_time: Timespec,
         account_root: StarlingFixedKey,
         rewards_pool: RewardsPoolState,
         network_params: NetworkParameters,
-        validators: ValidatorState,
+        staking_table: StakingTable,
     ) -> Self {
         ChainNodeState {
             last_block_height: BlockHeight::genesis(),
             last_apphash: genesis_apphash,
             block_time: genesis_time,
-            validators,
+            block_height: BlockHeight::genesis(),
+            staking_table,
             genesis_time,
             top_level: ChainState {
                 account_root,
@@ -212,23 +172,15 @@ pub fn check_validators(
     nodes: &[(StakedStateAddress, CouncilNode)],
     mut req_validators: Vec<ValidatorUpdate>,
     distribution: &BTreeMap<RedeemAddress, (StakedStateDestination, Coin)>,
-    network_params: &NetworkParameters,
-) -> Result<ValidatorState, ()> {
+) -> Result<(), ()> {
     let mut validators = Vec::with_capacity(nodes.len());
-    let mut validator_state = ValidatorState::default();
     for (address, node) in nodes.iter() {
         let mut validator = ValidatorUpdate::default();
         let power = get_voting_power(distribution, address);
         validator.set_power(power.into());
         let pk = get_validator_key(&node);
         validator.set_pub_key(pk);
         validators.push(validator);
-        validator_state.add_initial_validator(
-            *address,
-            power,
-            node.clone(),
-            network_params.get_block_signing_window(),
-        );
     }
 
     let fn_sort_key = |a: &ValidatorUpdate| {
@@ -240,7 +192,7 @@ pub fn check_validators(
     req_validators.sort_by_key(fn_sort_key);
 
     if validators == req_validators {
-        Ok(validator_state)
+        Ok(())
     } else {
         Err(())
     }
@@ -282,7 +234,7 @@ pub fn init_app_hash(conf: &InitConfig, genesis_time: Timespec) -> H256 {
 impl<T: EnclaveProxy> ChainNodeApp<T> {
     fn restore_from_storage(
         tx_validator: T,
-        mut last_app_state: ChainNodeState,
+        last_app_state: ChainNodeState,
         genesis_app_hash: [u8; HASH_SIZE_256],
         chain_id: &str,
         storage: Storage,
@@ -307,10 +259,6 @@ impl<T: EnclaveProxy> ChainNodeApp<T> {
         }
         let chain_hex_id = hex::decode(&chain_id[chain_id.len() - 2..])
             .expect("failed to decode two last hex digits in chain ID")[0];
-        last_app_state.validators.validator_state_helper = ValidatorStateHelper::restore(
-            &StakingGetter::new(&accounts, Some(last_app_state.top_level.account_root)),
-            &last_app_state,
-        );
         ChainNodeApp {
             storage,
             accounts,
@@ -364,7 +312,7 @@ impl<T: EnclaveProxy> ChainNodeApp<T> {
 
         if let Some(data) = storage.get_last_app_state() {
             info!("last app state stored");
-            let last_state =
+            let mut last_state =
                 ChainNodeState::decode(&mut data.as_slice()).expect("deserialize app state");
 
             // if tx-query address wasn't provided first time,
@@ -394,6 +342,14 @@ impl<T: EnclaveProxy> ChainNodeApp<T> {
                 }
             }
 
+            // populate the indexing structures in staking table.
+            last_state.staking_table.initialize(
+                &StakingGetter::new(&accounts, Some(last_state.top_level.account_root)),
+                last_state
+                    .top_level
+                    .network_params
+                    .get_required_council_node_stake(),
+            );
             ChainNodeApp::restore_from_storage(
                 tx_validator,
                 last_state,
@@ -483,35 +439,39 @@ impl<T: EnclaveProxy> ChainNodeApp<T> {
             &mut self.storage,
         );
 
-        if let Ok(validator_state) = check_validators(
+        check_validators(
             &nodes,
             req.validators.clone().into_vec(),
             &conf.distribution,
-            &network_params,
-        ) {
-            let genesis_state = ChainNodeState::genesis(
-                genesis_app_hash,
-                genesis_time,
-                new_account_root,
-                rp,
-                network_params,
-                validator_state,
-            );
-            chain_storage::store_genesis_state(
-                &mut kv_store!(self),
-                &genesis_state,
-                self.tx_query_address.is_some(),
-            );
+        )
+        .expect("validators in genesis configuration are not consistent with app_state");
+
+        let val_addresses = nodes.iter().map(|(addr, _)| *addr).collect::<Vec<_>>();
+        let staking_table = StakingTable::from_genesis(
+            &StakingGetter::new(&self.accounts, Some(new_account_root)),
+            network_params.get_required_council_node_stake(),
+            network_params.get_max_validators(),
+            &val_addresses,
+        );
 
-            flush_storage(&mut self.storage, mem::take(&mut self.kv_buffer))
-                .expect("storage io error");
-            self.last_state = Some(genesis_state);
-            self.mempool_state = self.last_state.clone();
+        let genesis_state = ChainNodeState::genesis(
+            genesis_app_hash,
+            genesis_time,
+            new_account_root,
+            rp,
+            network_params,
+            staking_table,
+        );
+        chain_storage::store_genesis_state(
+            &mut kv_store!(self),
+            &genesis_state,
+            self.tx_query_address.is_some(),
+        );
+        flush_storage(&mut self.storage, mem::take(&mut self.kv_buffer)).expect("storage io error");
 
-            ResponseInitChain::new()
-        } else {
-            panic!("validators in genesis configuration are not consistent with app_state")
-        }
+        self.last_state = Some(genesis_state);
+        self.mempool_state = self.last_state.clone();
+        ResponseInitChain::new()
     }
 
     pub fn staking_store(&mut self, buffer_type: BufferType) -> impl StoreStaking + '_ {
@@ -530,6 +490,15 @@ impl<T: EnclaveProxy> ChainNodeApp<T> {
         staking_getter!(self, root, buffer_type)
     }
 
+    pub fn staking_getter_committed(&self) -> StakingGetter<'_> {
+        StakingGetter::new(
+            &self.accounts,
+            self.last_state
+                .as_ref()
+                .map(|state| state.top_level.account_root),
+        )
+    }
+
     pub fn kv_store(&mut self, buffer_type: BufferType) -> impl StoreKV + '_ {
         kv_store!(self, buffer_type)
     }
@@ -544,7 +513,8 @@ impl<T: EnclaveProxy> ChainNodeApp<T> {
         ChainInfo {
             min_fee_computed: min_fee,
             chain_hex_id: self.chain_hex_id,
-            previous_block_time: state.block_time,
+            block_time: state.block_time,
+            block_height: state.block_height,
             unbonding_period: state.top_level.network_params.get_unbonding_period(),
         }
     }