Commit to storage based on actual change instead of dirty flag. (#1721)

* fix missing from_key_bytes impl for DepositList and VoteList.

* fix is_builtin check, do not include null address.

* Commit to storage based on actual change instead of dirty flag.

- Refactored storage_layout, StateDb.
- Commit staking state in genesis block.

* optimize lock for get

* sample chain

Author: yangzhe1990

accounts/cfxstore/src/account/safe_account.rs

@@ -141,7 +141,7 @@ impl SafeAccount {
         let meta_plain = json::VaultKeyMeta::load(&meta_plain)
             .map_err(|e| Error::Custom(format!("{:?}", e)))?;
 
-        if !meta_plain.address.is_user_account_address() {
+        if !Address::from_slice(&meta_plain.address).is_user_account_address() {
             warn!("Trying to import a non-user type account address. Are you trying to import an Ethkey for Conflux? Note that the address scheme between Ethereum and Conflux are different.");
             return Err(Error::Custom(format!(
                 "Import non-user type address. Address: {:?}",

accounts/cfxstore/src/json/hash.rs

@@ -15,7 +15,6 @@
 // along with Parity Ethereum.  If not, see <http://www.gnu.org/licenses/>.
 
 use super::Error;
-use cfx_types::address_util::AddressUtil;
 use rustc_hex::{FromHex, ToHex};
 use serde::{
     de::{Error as SerdeError, Visitor},
@@ -146,11 +145,3 @@ impl Eq for H160 {}
 impl Ord for H160 {
     fn cmp(&self, other: &Self) -> Ordering { self.0.cmp(&other.0) }
 }
-
-impl AddressUtil for H160 {
-    #[inline]
-    fn type_byte(&self) -> &u8 { &self.0[0] }
-
-    #[inline]
-    fn type_byte_mut(&mut self) -> &mut u8 { &mut self.0[0] }
-}

cfx_types/src/lib.rs

@@ -39,6 +39,8 @@ pub mod address_util {
 
         fn type_byte_mut(&mut self) -> &mut u8;
 
+        fn is_null_address(&self) -> bool;
+
         #[inline]
         fn address_type_bits(&self) -> u8 { self.type_byte() & 0xf0 }
 
@@ -54,6 +56,7 @@ pub mod address_util {
             self.is_contract_address()
                 || self.is_user_account_address()
                 || self.is_builtin_address()
+                || self.is_null_address()
         }
 
         #[inline]
@@ -69,6 +72,7 @@ pub mod address_util {
         #[inline]
         fn is_builtin_address(&self) -> bool {
             self.address_type_bits() == TYPE_BITS_BUILTIN
+                && !self.is_null_address()
         }
 
         #[inline]
@@ -90,6 +94,22 @@ pub mod address_util {
         fn type_byte_mut(&mut self) -> &mut u8 {
             &mut self.as_fixed_bytes_mut()[0]
         }
+
+        #[inline]
+        fn is_null_address(&self) -> bool { self.is_zero() }
+    }
+
+    impl AddressUtil for &[u8] {
+        #[inline]
+        fn type_byte(&self) -> &u8 { &self[0] }
+
+        #[inline]
+        fn type_byte_mut(&mut self) -> &mut u8 { unreachable!() }
+
+        #[inline]
+        fn is_null_address(&self) -> bool {
+            self.iter().all(|&byte| byte == 0u8)
+        }
     }
 
     #[cfg(test)]

changelogs/CHANGELOG-1.0.x.md

@@ -6,6 +6,7 @@
 - Fix code() return value for uninitialized contract.
 - Fix bug in kill_account after which the contract account is revived by simple transaction.
 - Fix the place of collateral refund for suicide contracts.
+- Fix missing StorageKey conversion from bytes of DepositList and VoteList.
 
 ## Incompatible changes
 

core/benchmark/storage/src/main.rs

@@ -2155,7 +2155,7 @@ fn main() -> errors::Result<()> {
 use cfx_types::hexstr_to_h256;
 use cfxcore::{
     block_data_manager::StateAvailabilityBoundary,
-    statedb::StateDb,
+    statedb::{StateDb, StateDbExt},
     storage::{
         state::StateTrait,
         storage_db::key_value_db::{KeyValueDbTrait, KeyValueDbTraitRead},

core/examples/snapshot_merge_test.rs

@@ -4,7 +4,7 @@
 
 use cfx_types::{Address, H256};
 use cfxcore::{
-    statedb::StateDb,
+    statedb::{StateDb, StateDbExt},
     storage::{
         state::StateTrait,
         state_manager::{StateManager, StateManagerTrait},
@@ -426,7 +426,7 @@ where
             .unwrap();
     }
     let epoch = H256::random();
-    state.commit(epoch).unwrap();
+    state.commit(epoch, /* debug_record = */ None).unwrap();
     epoch
 }
 

core/src/consensus/mod.rs

@@ -31,7 +31,7 @@ use crate::{
     pow::{PowComputer, ProofOfWorkConfig},
     rpc_errors::Result as RpcResult,
     state::State,
-    statedb::StateDb,
+    statedb::{StateDb, StateDbExt, StateDbGetOriginalMethods},
     statistics::SharedStatistics,
     storage::state_manager::StateManagerTrait,
     transaction_pool::SharedTransactionPool,
@@ -552,7 +552,7 @@ impl ConsensusGraph {
     ) -> Result<Option<StorageRoot>, String> {
         let state_db = self.get_state_db_by_epoch_number(epoch_number)?;
 
-        match state_db.get_storage_root(&address) {
+        match state_db.get_original_storage_root(&address) {
             Ok(maybe_storage_root) => Ok(maybe_storage_root),
             Err(e) => {
                 error!("db error occurred: {:?}", e);

core/src/executive/context.rs

@@ -17,7 +17,7 @@ use crate::{
     },
 };
 use cfx_types::{address_util::AddressUtil, Address, H256, U256};
-use primitives::{transaction::UNSIGNED_SENDER, StorageLayout};
+use primitives::transaction::UNSIGNED_SENDER;
 use std::sync::Arc;
 
 /// Policy for handling output data on `RETURN` opcode.
@@ -327,11 +327,6 @@ impl<'a> ContextTrait for Context<'a> {
                     self.origin.storage_owner,
                 )?;
 
-                self.state.set_storage_layout(
-                    &self.origin.address,
-                    StorageLayout::Regular(0),
-                )?;
-
                 Ok(*gas - return_cost)
             }
             OutputPolicy::InitContract => Ok(*gas),

core/src/executive/executive.rs

@@ -27,7 +27,8 @@ use crate::{
 };
 use cfx_types::{address_util::AddressUtil, Address, H256, U256, U512};
 use primitives::{
-    receipt::StorageChange, transaction::Action, SignedTransaction,
+    receipt::StorageChange, storage::STORAGE_LAYOUT_REGULAR_V0,
+    transaction::Action, SignedTransaction, StorageLayout,
 };
 use std::{
     cell::RefCell,
@@ -336,7 +337,7 @@ impl<'a> CallCreateExecutive<'a> {
 
     fn transfer_exec_balance_and_init_contract(
         params: &ActionParams, spec: &Spec, state: &mut State,
-        substate: &mut Substate,
+        substate: &mut Substate, storage_layout: Option<StorageLayout>,
     ) -> vm::Result<()>
     {
         if let ActionValue::Transfer(val) = params.value {
@@ -353,6 +354,7 @@ impl<'a> CallCreateExecutive<'a> {
                 &params.original_sender,
                 val.saturating_add(prev_balance),
                 state.contract_start_nonce(),
+                storage_layout,
             )?;
         } else {
             // In contract creation, the `params.value` should never be
@@ -704,7 +706,11 @@ impl<'a> CallCreateExecutive<'a> {
                         )?;
                         state.checkpoint();
                         Self::transfer_exec_balance_and_init_contract(
-                            &params, spec, state, substate,
+                            &params,
+                            spec,
+                            state,
+                            substate,
+                            Some(STORAGE_LAYOUT_REGULAR_V0),
                         )?;
                         Ok(())
                     };

core/src/executive/executive_tests.rs

@@ -21,7 +21,9 @@ use cfx_types::{
     address_util::AddressUtil, Address, BigEndianHash, U256, U512,
 };
 use keylib::{Generator, Random};
-use primitives::{transaction::Action, Transaction};
+use primitives::{
+    storage::STORAGE_LAYOUT_REGULAR_V0, transaction::Action, Transaction,
+};
 use rustc_hex::FromHex;
 use std::{
     cmp::{self, min},
@@ -611,7 +613,6 @@ fn test_deposit_withdraw_lock() {
         *state.total_issued_tokens(),
         U256::from(2_000_000_000_000_000_000u64)
     );
-    assert_eq!(state.block_number(), 0);
 
     let mut params = ActionParams::default();
     params.code_address = STORAGE_INTEREST_STAKING_CONTRACT_ADDRESS.clone();
@@ -646,7 +647,6 @@ fn test_deposit_withdraw_lock() {
         U256::from(2_000_000_000_000_000_000u64)
     );
     assert_eq!(*state.total_staking_tokens(), U256::zero());
-    assert_eq!(state.block_number(), 0);
 
     // deposit 10^18 - 1, not enough
     params.call_type = CallType::Call;
@@ -692,7 +692,6 @@ fn test_deposit_withdraw_lock() {
         *state.total_staking_tokens(),
         U256::from(1_000_000_000_000_000_000u64)
     );
-    assert_eq!(state.block_number(), 0);
 
     // empty data
     params.data = None;
@@ -725,7 +724,6 @@ fn test_deposit_withdraw_lock() {
         *state.total_staking_tokens(),
         U256::from(1_000_000_000_000_000_000u64)
     );
-    assert_eq!(state.block_number(), 0);
 
     // less data
     params.data = Some("b6b55f25000000000000000000000000000000000000000000000000000000174876e8".from_hex().unwrap());
@@ -758,7 +756,6 @@ fn test_deposit_withdraw_lock() {
         *state.total_staking_tokens(),
         U256::from(1_000_000_000_000_000_000u64)
     );
-    assert_eq!(state.block_number(), 0);
 
     // more data
     params.data = Some("b6b55f25000000000000000000000000000000000000000000000000000000174876e80000".from_hex().unwrap());
@@ -791,7 +788,6 @@ fn test_deposit_withdraw_lock() {
         *state.total_staking_tokens(),
         U256::from(1_000_000_000_000_000_000u64)
     );
-    assert_eq!(state.block_number(), 0);
 
     // withdraw
     params.data = Some("2e1a7d4d0000000000000000000000000000000000000000000000000000000ba43b7400".from_hex().unwrap());
@@ -820,7 +816,6 @@ fn test_deposit_withdraw_lock() {
         *state.total_staking_tokens(),
         U256::from(999_999_950_000_000_000u64)
     );
-    assert_eq!(state.block_number(), 0);
     // withdraw more than staking balance
     params.data = Some("2e1a7d4d0000000000000000000000000000000000000000000000000de0b6a803288c01".from_hex().unwrap());
     let result = Executive::new(
@@ -854,7 +849,6 @@ fn test_deposit_withdraw_lock() {
         *state.total_staking_tokens(),
         U256::from(999_999_950_000_000_000u64)
     );
-    assert_eq!(state.block_number(), 0);
 
     // lock until block_number = 0
     params.data = Some("5547dedb00000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000000".from_hex().unwrap());
@@ -1062,6 +1056,7 @@ fn test_commission_privilege() {
             &sender.address(),
             U256::zero(),
             U256::one(),
+            Some(STORAGE_LAYOUT_REGULAR_V0),
         )
         .expect(&concat!(file!(), ":", line!(), ":", column!()));
     state.init_code(&address, code, sender.address()).unwrap();
@@ -1429,6 +1424,7 @@ fn test_storage_commission_privilege() {
             &sender.address(),
             U256::zero(),
             U256::one(),
+            Some(STORAGE_LAYOUT_REGULAR_V0),
         )
         .expect(&concat!(file!(), ":", line!(), ":", column!()));
     state.init_code(&address, code, sender.address()).unwrap();