Merge #703 #704

703: Problem: dev-utils generates incorrect app hash r=tomtau a=yihuang

Solution:
- dev-utils use `genesis_time` from `genesis.json`, rather than `dev-utils.json`
- Remove `genesis_time` field from `dev-utils.json`


704: Problem (CRO-668): app hash keep changing on every empty block r=tomtau a=yihuang

Solution:
- Fix the bug: `last_distribution_time` not updated after rewards distribution.
- Added unit testing to prevent this happen again.


Co-authored-by: yihuang <yi.codeplayer@gmail.com>

chain-abci/src/app/rewards.rs

@@ -58,6 +58,7 @@ impl<T: EnclaveProxy> ChainNodeApp<T> {
         {
             return None;
         }
+        top_level.rewards_pool.last_distribution_time = state.block_time;
         self.rewards_pool_updated = true;
 
         let mut total_staking = Coin::zero();
@@ -207,4 +208,39 @@ mod tests {
         // rewards decrease
         assert!(reward2 > Coin::zero() && reward2 < reward1);
     }
+
+    #[test]
+    fn empty_block_should_not_change_app_hash() {
+        let (env, storage, account_storage) = ChainEnv::new(Coin::max(), Coin::zero(), 1);
+        let mut app = env.chain_node(storage, account_storage);
+        let _rsp_init_chain = app.init_chain(&env.req_init_chain());
+
+        let mut req = env.req_begin_block(1, 0);
+        let start_block_time = env
+            .init_config
+            .network_params
+            .rewards_config
+            .distribution_period as u64;
+        req.mut_header()
+            .set_time(seconds_to_timestamp(start_block_time));
+        app.begin_block(&req);
+        app.end_block(&RequestEndBlock::new());
+        app.commit(&RequestCommit::new());
+        let start_app_hash = app.last_state.as_ref().unwrap().last_apphash;
+        assert_ne!(start_app_hash, env.genesis_app_hash);
+
+        for i in 2..10 {
+            let mut req = env.req_begin_block(i, 0);
+            req.mut_header()
+                .set_time(seconds_to_timestamp(start_block_time));
+            req.set_last_commit_info(env.last_commit_info_signed());
+            app.begin_block(&req);
+            app.end_block(&RequestEndBlock::new());
+            app.commit(&RequestCommit::new());
+            assert_eq!(
+                app.last_state.as_ref().unwrap().last_apphash,
+                start_app_hash
+            );
+        }
+    }
 }

dev-utils/example-dev-conf.json

@@ -35,6 +35,5 @@
                 "value": "pPxJH5wUamXcCvSxgywzmANuO0UNR5x3nCbUzsrCeX8="
             }
         ]
-    },
-    "genesis_time": "2019-10-23T07:26:34.783271158Z"
-}
+    }
+}

dev-utils/src/commands/genesis_command.rs

@@ -9,6 +9,7 @@ use hex::encode_upper;
 use structopt::StructOpt;
 
 use chain_abci::app::init_app_hash;
+use chain_core::common::Timespec;
 use chain_core::init::address::RedeemAddress;
 use chain_core::init::coin::Coin;
 use chain_core::init::config::{InitConfig, InitNetworkParameters};
@@ -112,7 +113,16 @@ fn generate_genesis_command(
             )
         })?;
 
-    let (app_hash, app_state, validators) = generate_genesis(&genesis_dev_config)?;
+    let genesis_time = Time::from_str(
+        tendermint_genesis["genesis_time"]
+            .as_str()
+            .expect("genesis time config should be string"),
+    )
+    .expect("invalid genesis time format")
+    .duration_since(Time::unix_epoch())
+    .expect("invalid genesis time")
+    .as_secs();
+    let (app_hash, app_state, validators) = generate_genesis(&genesis_dev_config, genesis_time)?;
 
     let app_hash = serde_json::to_value(app_hash).chain(|| {
         (
@@ -182,6 +192,7 @@ fn find_tendermint_path_from_home() -> Option<PathBuf> {
 
 pub fn generate_genesis(
     genesis_dev_config: &GenesisDevConfig,
+    genesis_time: Timespec,
 ) -> Result<(String, InitConfig, Vec<TendermintValidator>)> {
     let mut dist: BTreeMap<RedeemAddress, (StakedStateDestination, Coin)> = BTreeMap::new();
 
@@ -212,14 +223,7 @@ pub fn generate_genesis(
         network_params,
         genesis_dev_config.council_nodes.clone(),
     );
-    let genesis_app_hash = init_app_hash(
-        &config,
-        genesis_dev_config
-            .genesis_time
-            .duration_since(Time::unix_epoch())
-            .expect("invalid genesis time")
-            .as_secs(),
-    );
+    let genesis_app_hash = init_app_hash(&config, genesis_time);
 
     let validators = generate_validators(&genesis_dev_config)?;
 

dev-utils/src/commands/genesis_dev_config.rs

@@ -9,7 +9,6 @@ use chain_core::init::{
 };
 use chain_core::state::account::{ValidatorName, ValidatorSecurityContact};
 use chain_core::state::tendermint::TendermintValidatorPubKey;
-use client_common::tendermint::types::Time;
 
 #[derive(Deserialize, Debug)]
 pub struct GenesisDevConfig {
@@ -28,12 +27,10 @@ pub struct GenesisDevConfig {
             TendermintValidatorPubKey,
         ),
     >,
-    pub genesis_time: Time,
 }
 
 impl GenesisDevConfig {
     pub fn new(expansion_cap: Coin) -> Self {
-        let gt = Time::from_str("2019-03-21T02:26:51.366017Z").unwrap();
         GenesisDevConfig {
             distribution: BTreeMap::new(),
             unbonding_period: 60,
@@ -60,7 +57,6 @@ impl GenesisDevConfig {
                 per_byte_fee: "1.25".to_string(),
             },
             council_nodes: BTreeMap::new(),
-            genesis_time: gt,
         }
     }
 }

dev-utils/src/commands/init_command.rs

@@ -32,6 +32,7 @@ pub struct InitCommand {
     remain_coin: Coin,
     tendermint_command: String,
     validators: Vec<TendermintValidator>,
+    genesis_time: Time,
 }
 
 impl InitCommand {
@@ -49,6 +50,7 @@ impl InitCommand {
             remain_coin: Coin::max(),
             tendermint_command: "./tendermint".to_string(),
             validators: Vec::new(),
+            genesis_time: Time::unix_epoch(),
         }
     }
 
@@ -163,19 +165,6 @@ impl InitCommand {
         Ok(())
     }
 
-    fn read_genesis_time(&mut self) -> Result<()> {
-        // change
-        let old_genesis_time = self.genesis_dev_config.genesis_time.to_string();
-
-        let new_genesis_time: String = self.ask_string(
-            format!("genesis_time( {} )=", old_genesis_time).as_str(),
-            old_genesis_time.as_str(),
-        );
-
-        self.genesis_dev_config.genesis_time = Time::from_str(&new_genesis_time).unwrap();
-        Ok(())
-    }
-
     fn read_councils(&mut self) -> Result<()> {
         println!(
             "{} {}",
@@ -203,13 +192,19 @@ impl InitCommand {
         self.read_chain_id()
             .and_then(|_| self.read_staking_address())
             .and_then(|_| self.read_wallets())
-            .and_then(|_| self.read_genesis_time())
             .and_then(|_| self.read_councils())
     }
 
     fn generate_app_info(&mut self) -> Result<()> {
         // app_hash,  app_state
-        let result = generate_genesis(&self.genesis_dev_config).unwrap();
+        let result = generate_genesis(
+            &self.genesis_dev_config,
+            self.genesis_time
+                .duration_since(Time::unix_epoch())
+                .unwrap()
+                .as_secs(),
+        )
+        .unwrap();
         self.app_hash = result.0;
         self.app_state = Some(result.1);
         self.validators = result.2;
@@ -236,6 +231,7 @@ impl InitCommand {
                 let pub_key = &json["validators"][0]["pub_key"]["value"];
                 self.tendermint_pubkey = pub_key.as_str().unwrap().to_string();
                 self.chain_id = json["chain_id"].as_str().unwrap().to_string();
+                self.genesis_time = Time::from_str(json["genesis_time"].as_str().unwrap()).unwrap();
                 Ok(())
             })
             .chain(|| {
@@ -268,7 +264,7 @@ impl InitCommand {
 
         let app_hash = self.app_hash.clone();
         let app_state = self.app_state.clone();
-        let gt = self.genesis_dev_config.genesis_time.to_string();
+        let gt = self.genesis_time.to_string();
         let mut json_string = String::from("");
         fs::read_to_string(&InitCommand::get_tendermint_filename())
             .and_then(|contents| {