From 5ed1eb5499a71458a399c2bc7921014e572eadc2 Mon Sep 17 00:00:00 2001 From: Seulgi Kim Date: Wed, 15 May 2019 12:07:41 +0900 Subject: [PATCH 1/9] Move CommonParams to codechain-types --- Cargo.lock | 1 + core/src/client/client.rs | 4 +-- core/src/client/mod.rs | 3 +- core/src/codechain_machine.rs | 11 +++--- core/src/scheme/mod.rs | 2 -- core/src/scheme/scheme.rs | 4 +-- core/src/transaction.rs | 3 +- json/src/scheme/params.rs | 2 +- types/Cargo.toml | 1 + .../src/scheme => types/src}/common_params.rs | 34 +++---------------- types/src/lib.rs | 3 ++ 11 files changed, 21 insertions(+), 47 deletions(-) rename {core/src/scheme => types/src}/common_params.rs (87%) diff --git a/Cargo.lock b/Cargo.lock index b27a2461a9..e14edc2e97 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -571,6 +571,7 @@ name = "codechain-types" version = "0.1.0" dependencies = [ "codechain-crypto 0.1.0", + "codechain-json 0.1.0", "codechain-key 0.1.0", "primitives 0.4.0 (git+https://github.com/CodeChain-io/rust-codechain-primitives.git)", "rlp 0.2.1", diff --git a/core/src/client/client.rs b/core/src/client/client.rs index 0e1dad8284..0ddf929e14 100644 --- a/core/src/client/client.rs +++ b/core/src/client/client.rs @@ -28,7 +28,7 @@ use cstate::{ }; use ctimer::{TimeoutHandler, TimerApi, TimerScheduleError, TimerToken}; use ctypes::transaction::{AssetTransferInput, PartialHashing, ShardTransaction}; -use ctypes::{BlockNumber, ShardId}; +use ctypes::{BlockNumber, CommonParams, ShardId}; use cvm::{decode, execute, ChainTimeInfo, ScriptResult, VMConfig}; use hashdb::AsHashDB; use journaldb; @@ -49,7 +49,7 @@ use crate::consensus::CodeChainEngine; use crate::encoded; use crate::error::{BlockImportError, Error, ImportError, SchemeError}; use crate::miner::{Miner, MinerService}; -use crate::scheme::{CommonParams, Scheme}; +use crate::scheme::Scheme; use crate::service::ClientIoMessage; use crate::transaction::{LocalizedTransaction, PendingSignedTransactions, UnverifiedTransaction}; use crate::types::{BlockId, BlockStatus, TransactionId, VerificationQueueInfo as BlockQueueInfo}; diff --git a/core/src/client/mod.rs b/core/src/client/mod.rs index f402818ada..b8ecc3c94f 100644 --- a/core/src/client/mod.rs +++ b/core/src/client/mod.rs @@ -37,7 +37,7 @@ use cmerkle::Result as TrieResult; use cnetwork::NodeId; use cstate::{AssetScheme, FindActionHandler, OwnedAsset, StateResult, Text, TopLevelState, TopStateView}; use ctypes::transaction::{AssetTransferInput, PartialHashing, ShardTransaction}; -use ctypes::{BlockNumber, ShardId}; +use ctypes::{BlockNumber, CommonParams, ShardId}; use cvm::ChainTimeInfo; use kvdb::KeyValueDB; use primitives::{Bytes, H160, H256, U256}; @@ -46,7 +46,6 @@ use crate::block::{ClosedBlock, OpenBlock, SealedBlock}; use crate::blockchain_info::BlockChainInfo; use crate::encoded; use crate::error::BlockImportError; -use crate::scheme::CommonParams; use crate::transaction::{LocalizedTransaction, PendingSignedTransactions}; use crate::types::{BlockId, BlockStatus, TransactionId, VerificationQueueInfo as BlockQueueInfo}; diff --git a/core/src/codechain_machine.rs b/core/src/codechain_machine.rs index 4641fa04ea..0a2b322ed6 100644 --- a/core/src/codechain_machine.rs +++ b/core/src/codechain_machine.rs @@ -21,13 +21,12 @@ use ckey::Address; use cstate::{StateError, TopState, TopStateView}; use ctypes::errors::{HistoryError, SyntaxError}; use ctypes::transaction::{Action, AssetTransferInput, OrderOnTransfer, Timelock}; -use ctypes::BlockNumber; +use ctypes::{BlockNumber, CommonParams}; use crate::block::{ExecutedBlock, IsBlock}; use crate::client::BlockChainTrait; use crate::error::Error; use crate::header::Header; -use crate::scheme::CommonParams; use crate::transaction::{SignedTransaction, UnverifiedTransaction}; struct Params { @@ -300,7 +299,7 @@ mod tests { #[test] fn common_params_are_not_changed_since_genesis() { - let genesis_params = CommonParams::default(); + let genesis_params = CommonParams::default_for_test(); let machine = CodeChainMachine::new(genesis_params.clone()); assert_eq!(&genesis_params, machine.common_params(Some(0))); assert_eq!(&genesis_params, machine.common_params(Some(1))); @@ -309,7 +308,7 @@ mod tests { #[test] fn common_params_changed_at_1() { - let genesis_params = CommonParams::default(); + let genesis_params = CommonParams::default_for_test(); let params_at_1 = { let mut params = genesis_params.clone(); params.set_min_store_transaction_cost(genesis_params.min_store_transaction_cost() + 10); @@ -335,7 +334,7 @@ mod tests { #[test] fn common_params_changed_at_2() { - let genesis_params = CommonParams::default(); + let genesis_params = CommonParams::default_for_test(); let params_at_2 = { let mut params = genesis_params.clone(); params.set_min_store_transaction_cost(genesis_params.min_store_transaction_cost() + 10); @@ -363,7 +362,7 @@ mod tests { #[test] fn common_params_changed_several_times() { - let genesis_params = CommonParams::default(); + let genesis_params = CommonParams::default_for_test(); let params_at_10 = { let mut params = genesis_params.clone(); params.set_min_store_transaction_cost(genesis_params.min_store_transaction_cost() + 10); diff --git a/core/src/scheme/mod.rs b/core/src/scheme/mod.rs index d34d558730..e5fe849499 100644 --- a/core/src/scheme/mod.rs +++ b/core/src/scheme/mod.rs @@ -14,7 +14,6 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . -mod common_params; mod genesis; mod pod_account; mod pod_shard_metadata; @@ -23,6 +22,5 @@ mod pod_state; mod scheme; mod seal; -pub use self::common_params::CommonParams; pub use self::genesis::Genesis; pub use self::scheme::Scheme; diff --git a/core/src/scheme/scheme.rs b/core/src/scheme/scheme.rs index d945767014..87ed66521d 100644 --- a/core/src/scheme/scheme.rs +++ b/core/src/scheme/scheme.rs @@ -23,7 +23,7 @@ use ckey::Address; use cmerkle::TrieFactory; use cstate::{Metadata, MetadataAddress, Shard, ShardAddress, StateDB, StateResult, StateWithCache, TopLevelState}; use ctypes::errors::SyntaxError; -use ctypes::{BlockNumber, ShardId}; +use ctypes::{BlockNumber, CommonParams, ShardId}; use hashdb::{AsHashDB, HashDB}; use parking_lot::RwLock; use primitives::{Bytes, H256, U256}; @@ -33,7 +33,7 @@ use crate::blockchain::HeaderProvider; use super::pod_state::{PodAccounts, PodShards}; use super::seal::Generic as GenericSeal; -use super::{CommonParams, Genesis}; +use super::Genesis; use crate::codechain_machine::CodeChainMachine; use crate::consensus::{BlakePoW, CodeChainEngine, Cuckoo, NullEngine, SimplePoA, Solo, Tendermint}; use crate::error::{Error, SchemeError}; diff --git a/core/src/transaction.rs b/core/src/transaction.rs index fe6b8e47c7..f26c82afe7 100644 --- a/core/src/transaction.rs +++ b/core/src/transaction.rs @@ -20,12 +20,11 @@ use ccrypto::blake256; use ckey::{self, public_to_address, recover, sign, Private, Public, Signature}; use ctypes::errors::SyntaxError; use ctypes::transaction::Transaction; -use ctypes::BlockNumber; +use ctypes::{BlockNumber, CommonParams}; use primitives::H256; use rlp::{self, DecoderError, Encodable, RlpStream, UntrustedRlp}; use crate::error::Error; -use crate::scheme::CommonParams; /// Signed transaction information without verified signature. #[derive(Debug, Clone, Eq, PartialEq)] diff --git a/json/src/scheme/params.rs b/json/src/scheme/params.rs index ed24cfd304..0ebab6e04d 100644 --- a/json/src/scheme/params.rs +++ b/json/src/scheme/params.rs @@ -19,7 +19,7 @@ use ckey::NetworkId; use crate::uint::Uint; /// Scheme params. -#[derive(Debug, PartialEq, Deserialize)] +#[derive(Debug, Default, PartialEq, Deserialize)] #[serde(rename_all = "camelCase")] pub struct Params { /// Maximum size of extra data. diff --git a/types/Cargo.toml b/types/Cargo.toml index ebf5a32a9d..f180e252b9 100644 --- a/types/Cargo.toml +++ b/types/Cargo.toml @@ -5,6 +5,7 @@ authors = ["CodeChain Team "] [dependencies] codechain-crypto = { path = "../crypto" } +codechain-json = { path = "../json" } codechain-key = { path = "../key" } primitives = { git = "https://github.com/CodeChain-io/rust-codechain-primitives.git", version = "0.4" } rlp = { path = "../util/rlp" } diff --git a/core/src/scheme/common_params.rs b/types/src/common_params.rs similarity index 87% rename from core/src/scheme/common_params.rs rename to types/src/common_params.rs index bc04fff175..e43e844d5b 100644 --- a/core/src/scheme/common_params.rs +++ b/types/src/common_params.rs @@ -252,35 +252,9 @@ impl Decodable for CommonParams { } } -#[cfg(test)] -impl Default for CommonParams { - fn default() -> Self { - CommonParams { - size: 23, - max_extra_data_size: Default::default(), - max_asset_scheme_metadata_size: Default::default(), - max_transfer_metadata_size: Default::default(), - max_text_content_size: Default::default(), - network_id: Default::default(), - min_pay_transaction_cost: Default::default(), - min_set_regular_key_transaction_cost: Default::default(), - min_create_shard_transaction_cost: Default::default(), - min_set_shard_owners_transaction_cost: Default::default(), - min_set_shard_users_transaction_cost: Default::default(), - min_wrap_ccc_transaction_cost: Default::default(), - min_custom_transaction_cost: Default::default(), - min_store_transaction_cost: Default::default(), - min_remove_transaction_cost: Default::default(), - min_asset_mint_cost: Default::default(), - min_asset_transfer_cost: Default::default(), - min_asset_scheme_change_cost: Default::default(), - min_asset_supply_increase_cost: Default::default(), - min_asset_compose_cost: Default::default(), - min_asset_decompose_cost: Default::default(), - min_asset_unwrap_ccc_cost: Default::default(), - max_body_size: Default::default(), - snapshot_period: Default::default(), - } +impl CommonParams { + pub fn default_for_test() -> Self { + Self::from(Params::default()) } } @@ -291,6 +265,6 @@ mod tests { #[test] fn encode_and_decode_default() { - rlp_encode_and_decode_test!(CommonParams::default()); + rlp_encode_and_decode_test!(CommonParams::default_for_test()); } } diff --git a/types/src/lib.rs b/types/src/lib.rs index e8f269aa66..4c3c94516e 100644 --- a/types/src/lib.rs +++ b/types/src/lib.rs @@ -15,6 +15,7 @@ // along with this program. If not, see . extern crate codechain_crypto as ccrypto; +extern crate codechain_json as cjson; extern crate codechain_key as ckey; extern crate primitives; extern crate rlp; @@ -24,9 +25,11 @@ extern crate serde; #[macro_use] extern crate serde_derive; +mod common_params; pub mod errors; pub mod transaction; pub mod util; pub type BlockNumber = u64; pub type ShardId = u16; +pub use common_params::CommonParams; From 39f1c072576e68750822d9981a8e8f1a6c504f6f Mon Sep 17 00:00:00 2001 From: Seulgi Kim Date: Wed, 15 May 2019 17:12:46 +0900 Subject: [PATCH 2/9] Make CommonParams copyable --- codechain/run_node.rs | 3 +- core/src/client/client.rs | 2 +- core/src/client/mod.rs | 2 +- core/src/codechain_machine.rs | 71 ++++++++++++++++++----------------- core/src/scheme/scheme.rs | 4 +- types/src/common_params.rs | 2 +- 6 files changed, 43 insertions(+), 41 deletions(-) diff --git a/codechain/run_node.rs b/codechain/run_node.rs index 2534a4020d..c66823d62e 100644 --- a/codechain/run_node.rs +++ b/codechain/run_node.rs @@ -263,7 +263,8 @@ pub fn run_node(matches: &ArgMatches) -> Result<(), String> { if !config.network.disable.unwrap() { let network_config = config.network_config()?; // XXX: What should we do if the network id has been changed. - let network_id = client.client().common_params(None).network_id(); + let c = client.client(); + let network_id = c.common_params(None).network_id(); let routing_table = RoutingTable::new(); let service = network_start(network_id, timer_loop, &network_config, Arc::clone(&routing_table))?; diff --git a/core/src/client/client.rs b/core/src/client/client.rs index 0ddf929e14..0847ebc4c4 100644 --- a/core/src/client/client.rs +++ b/core/src/client/client.rs @@ -484,7 +484,7 @@ impl StateInfo for Client { } impl EngineInfo for Client { - fn common_params(&self, block_number: Option) -> &CommonParams { + fn common_params(&self, block_number: Option) -> CommonParams { self.engine().machine().common_params(block_number) } diff --git a/core/src/client/mod.rs b/core/src/client/mod.rs index b8ecc3c94f..e498c45051 100644 --- a/core/src/client/mod.rs +++ b/core/src/client/mod.rs @@ -86,7 +86,7 @@ pub trait BlockChainTrait { } pub trait EngineInfo: Send + Sync { - fn common_params(&self, block_number: Option) -> &CommonParams; + fn common_params(&self, block_number: Option) -> CommonParams; fn block_reward(&self, block_number: u64) -> u64; fn mining_reward(&self, block_number: u64) -> Option; fn recommended_confirmation(&self) -> u32; diff --git a/core/src/codechain_machine.rs b/core/src/codechain_machine.rs index 0a2b322ed6..e7c0ee3e51 100644 --- a/core/src/codechain_machine.rs +++ b/core/src/codechain_machine.rs @@ -51,15 +51,16 @@ impl CodeChainMachine { } /// Get the general parameters of the chain. - pub fn common_params(&self, block_number: Option) -> &CommonParams { - assert!(!self.params.is_empty()); + pub fn common_params(&self, block_number: Option) -> CommonParams { + let params = &self.params; + assert!(!params.is_empty()); let block_number = if let Some(block_number) = block_number { block_number } else { - return &self.params.last().unwrap().params // the latest block. + return params.last().unwrap().params // the latest block. }; - &self - .params + + params .iter() .take_while( |Params { @@ -82,7 +83,7 @@ impl CodeChainMachine { } .into()) } - p.verify_basic(self.common_params(Some(header.number())), self.is_order_disabled)?; + p.verify_basic(&self.common_params(Some(header.number())), self.is_order_disabled)?; Ok(()) } @@ -300,17 +301,17 @@ mod tests { #[test] fn common_params_are_not_changed_since_genesis() { let genesis_params = CommonParams::default_for_test(); - let machine = CodeChainMachine::new(genesis_params.clone()); - assert_eq!(&genesis_params, machine.common_params(Some(0))); - assert_eq!(&genesis_params, machine.common_params(Some(1))); - assert_eq!(&genesis_params, machine.common_params(None)); + let machine = CodeChainMachine::new(genesis_params); + assert_eq!(genesis_params, machine.common_params(Some(0))); + assert_eq!(genesis_params, machine.common_params(Some(1))); + assert_eq!(genesis_params, machine.common_params(None)); } #[test] fn common_params_changed_at_1() { let genesis_params = CommonParams::default_for_test(); let params_at_1 = { - let mut params = genesis_params.clone(); + let mut params = genesis_params; params.set_min_store_transaction_cost(genesis_params.min_store_transaction_cost() + 10); params }; @@ -318,25 +319,25 @@ mod tests { params: vec![ Params { changed_block: 0, - params: genesis_params.clone(), + params: genesis_params, }, Params { changed_block: 1, - params: params_at_1.clone(), + params: params_at_1, }, ], is_order_disabled: false, }; - assert_eq!(&genesis_params, machine.common_params(Some(0))); - assert_eq!(¶ms_at_1, machine.common_params(Some(1))); - assert_eq!(¶ms_at_1, machine.common_params(None)); + assert_eq!(genesis_params, machine.common_params(Some(0))); + assert_eq!(params_at_1, machine.common_params(Some(1))); + assert_eq!(params_at_1, machine.common_params(None)); } #[test] fn common_params_changed_at_2() { let genesis_params = CommonParams::default_for_test(); let params_at_2 = { - let mut params = genesis_params.clone(); + let mut params = genesis_params; params.set_min_store_transaction_cost(genesis_params.min_store_transaction_cost() + 10); params }; @@ -344,19 +345,19 @@ mod tests { params: vec![ Params { changed_block: 0, - params: genesis_params.clone(), + params: genesis_params, }, Params { changed_block: 2, - params: params_at_2.clone(), + params: params_at_2, }, ], is_order_disabled: false, }; - assert_eq!(&genesis_params, machine.common_params(Some(0))); - assert_eq!(&genesis_params, machine.common_params(Some(1))); - assert_eq!(¶ms_at_2, machine.common_params(Some(2))); - assert_eq!(¶ms_at_2, machine.common_params(None)); + assert_eq!(genesis_params, machine.common_params(Some(0))); + assert_eq!(genesis_params, machine.common_params(Some(1))); + assert_eq!(params_at_2, machine.common_params(Some(2))); + assert_eq!(params_at_2, machine.common_params(None)); } @@ -364,17 +365,17 @@ mod tests { fn common_params_changed_several_times() { let genesis_params = CommonParams::default_for_test(); let params_at_10 = { - let mut params = genesis_params.clone(); + let mut params = genesis_params; params.set_min_store_transaction_cost(genesis_params.min_store_transaction_cost() + 10); params }; let params_at_20 = { - let mut params = params_at_10.clone(); + let mut params = params_at_10; params.set_min_store_transaction_cost(params_at_10.min_store_transaction_cost() + 10); params }; let params_at_30 = { - let mut params = params_at_20.clone(); + let mut params = params_at_20; params.set_min_store_transaction_cost(params_at_20.min_store_transaction_cost() + 10); params }; @@ -382,35 +383,35 @@ mod tests { params: vec![ Params { changed_block: 0, - params: genesis_params.clone(), + params: genesis_params, }, Params { changed_block: 10, - params: params_at_10.clone(), + params: params_at_10, }, Params { changed_block: 20, - params: params_at_20.clone(), + params: params_at_20, }, Params { changed_block: 30, - params: params_at_30.clone(), + params: params_at_30, }, ], is_order_disabled: false, }; for i in 0..10 { - assert_eq!(&genesis_params, machine.common_params(Some(i)), "unexpected params at block {}", i); + assert_eq!(genesis_params, machine.common_params(Some(i)), "unexpected params at block {}", i); } for i in 10..20 { - assert_eq!(¶ms_at_10, machine.common_params(Some(i)), "unexpected params at block {}", i); + assert_eq!(params_at_10, machine.common_params(Some(i)), "unexpected params at block {}", i); } for i in 20..30 { - assert_eq!(¶ms_at_20, machine.common_params(Some(i)), "unexpected params at block {}", i); + assert_eq!(params_at_20, machine.common_params(Some(i)), "unexpected params at block {}", i); } for i in 30..40 { - assert_eq!(¶ms_at_30, machine.common_params(Some(i)), "unexpected params at block {}", i); + assert_eq!(params_at_30, machine.common_params(Some(i)), "unexpected params at block {}", i); } - assert_eq!(¶ms_at_30, machine.common_params(None)); + assert_eq!(params_at_30, machine.common_params(None)); } } diff --git a/core/src/scheme/scheme.rs b/core/src/scheme/scheme.rs index 87ed66521d..6f14a55682 100644 --- a/core/src/scheme/scheme.rs +++ b/core/src/scheme/scheme.rs @@ -279,8 +279,8 @@ impl Scheme { } /// Get common blockchain parameters. - pub fn params(&self, block_number: Option) -> &CommonParams { - &self.engine.machine().common_params(block_number) + pub fn params(&self, block_number: Option) -> CommonParams { + self.engine.machine().common_params(block_number) } /// Get the header of the genesis block. diff --git a/types/src/common_params.rs b/types/src/common_params.rs index e43e844d5b..0b4be3bdb4 100644 --- a/types/src/common_params.rs +++ b/types/src/common_params.rs @@ -18,7 +18,7 @@ use cjson::scheme::Params; use ckey::NetworkId; use rlp::{Decodable, DecoderError, Encodable, RlpStream, UntrustedRlp}; -#[derive(Clone, Debug, PartialEq)] +#[derive(Copy, Clone, Debug, PartialEq)] pub struct CommonParams { size: usize, /// Maximum size of extra data. From d2bc60ce5cd0986b0d250bea79c86c13a87e3715 Mon Sep 17 00:00:00 2001 From: Seulgi Kim Date: Fri, 17 May 2019 10:12:23 +0900 Subject: [PATCH 3/9] Rename verify_*_unordered to verify_*_seal --- core/src/codechain_machine.rs | 7 ++----- core/src/consensus/blake_pow/mod.rs | 4 ++-- core/src/consensus/cuckoo/mod.rs | 8 ++++---- core/src/consensus/mod.rs | 14 ++++++-------- core/src/consensus/null_engine/mod.rs | 5 ----- core/src/consensus/simple_poa/mod.rs | 4 ---- core/src/consensus/solo/mod.rs | 6 +----- core/src/consensus/tendermint/engine.rs | 4 ---- core/src/miner/miner.rs | 2 +- core/src/verification/queue/kind.rs | 6 +++--- core/src/verification/verification.rs | 8 ++++---- 11 files changed, 23 insertions(+), 45 deletions(-) diff --git a/core/src/codechain_machine.rs b/core/src/codechain_machine.rs index e7c0ee3e51..081915bb78 100644 --- a/core/src/codechain_machine.rs +++ b/core/src/codechain_machine.rs @@ -88,11 +88,8 @@ impl CodeChainMachine { Ok(()) } - /// Verify a particular transaction is valid, regardless of order. - pub fn verify_transaction_unordered( - p: UnverifiedTransaction, - _header: &Header, - ) -> Result { + /// Verify a particular transaction's seal is valid. + pub fn verify_transaction_seal(p: UnverifiedTransaction, _header: &Header) -> Result { p.check_low_s()?; Ok(SignedTransaction::try_new(p)?) } diff --git a/core/src/consensus/blake_pow/mod.rs b/core/src/consensus/blake_pow/mod.rs index 6ca702f206..24da0e5ac2 100644 --- a/core/src/consensus/blake_pow/mod.rs +++ b/core/src/consensus/blake_pow/mod.rs @@ -101,7 +101,7 @@ impl ConsensusEngine for BlakePoW { } fn verify_local_seal(&self, header: &Header) -> Result<(), Error> { - self.verify_block_basic(header).and_then(|_| self.verify_block_unordered(header)) + self.verify_block_basic(header).and_then(|_| self.verify_block_seal(header)) } fn verify_block_basic(&self, header: &Header) -> Result<(), Error> { @@ -116,7 +116,7 @@ impl ConsensusEngine for BlakePoW { Ok(()) } - fn verify_block_unordered(&self, header: &Header) -> Result<(), Error> { + fn verify_block_seal(&self, header: &Header) -> Result<(), Error> { let seal = Seal::parse_seal(header.seal())?; let mut message = header.bare_hash().0; diff --git a/core/src/consensus/cuckoo/mod.rs b/core/src/consensus/cuckoo/mod.rs index d06b3adb71..bb8f6a1d3d 100644 --- a/core/src/consensus/cuckoo/mod.rs +++ b/core/src/consensus/cuckoo/mod.rs @@ -107,7 +107,7 @@ impl ConsensusEngine for Cuckoo { } fn verify_local_seal(&self, header: &Header) -> Result<(), Error> { - self.verify_block_basic(header).and_then(|_| self.verify_block_unordered(header)) + self.verify_block_basic(header).and_then(|_| self.verify_block_seal(header)) } fn verify_block_basic(&self, header: &Header) -> Result<(), Error> { @@ -122,7 +122,7 @@ impl ConsensusEngine for Cuckoo { Ok(()) } - fn verify_block_unordered(&self, header: &Header) -> Result<(), Error> { + fn verify_block_seal(&self, header: &Header) -> Result<(), Error> { let seal = Seal::parse_seal(header.seal())?; let mut message = header.bare_hash().0; @@ -231,11 +231,11 @@ mod tests { } #[test] - fn verify_block_unordered_err() { + fn verify_block_seal_err() { let engine = Scheme::new_test_cuckoo().engine; let default_header = Header::default(); - assert!(engine.verify_block_unordered(&default_header).is_err()); + assert!(engine.verify_block_seal(&default_header).is_err()); } #[test] diff --git a/core/src/consensus/mod.rs b/core/src/consensus/mod.rs index 35137db88b..f5e5411960 100644 --- a/core/src/consensus/mod.rs +++ b/core/src/consensus/mod.rs @@ -176,7 +176,9 @@ pub trait ConsensusEngine: Sync + Send { /// /// It is fine to require access to state or a full client for this function, since /// light clients do not generate seals. - fn verify_local_seal(&self, header: &Header) -> Result<(), Error>; + fn verify_local_seal(&self, _header: &Header) -> Result<(), Error> { + Ok(()) + } /// Phase 1 quick block verification. Only does checks that are cheap. Returns either a null `Ok` or a general error detailing the problem with import. fn verify_block_basic(&self, _header: &Header) -> Result<(), Error> { @@ -184,7 +186,7 @@ pub trait ConsensusEngine: Sync + Send { } /// Phase 2 verification. Perform costly checks such as transaction signatures. Returns either a null `Ok` or a general error detailing the problem with import. - fn verify_block_unordered(&self, _header: &Header) -> Result<(), Error> { + fn verify_block_seal(&self, _header: &Header) -> Result<(), Error> { Ok(()) } @@ -357,12 +359,8 @@ pub trait CodeChainEngine: ConsensusEngine { } /// Verify a particular transaction is valid. - fn verify_transaction_unordered( - &self, - tx: UnverifiedTransaction, - header: &Header, - ) -> Result { - CodeChainMachine::verify_transaction_unordered(tx, header) + fn verify_transaction_seal(&self, tx: UnverifiedTransaction, header: &Header) -> Result { + CodeChainMachine::verify_transaction_seal(tx, header) } } diff --git a/core/src/consensus/null_engine/mod.rs b/core/src/consensus/null_engine/mod.rs index 8ed7805416..6819a12824 100644 --- a/core/src/consensus/null_engine/mod.rs +++ b/core/src/consensus/null_engine/mod.rs @@ -22,7 +22,6 @@ use crate::block::ExecutedBlock; use crate::codechain_machine::CodeChainMachine; use crate::consensus::EngineType; use crate::error::Error; -use crate::header::Header; /// An engine which does not provide any consensus mechanism and does not seal blocks. pub struct NullEngine { @@ -53,10 +52,6 @@ impl ConsensusEngine for NullEngine { EngineType::Solo } - fn verify_local_seal(&self, _header: &Header) -> Result<(), Error> { - Ok(()) - } - fn on_close_block(&self, block: &mut ExecutedBlock) -> Result<(), Error> { let (author, total_reward) = { let header = block.header(); diff --git a/core/src/consensus/simple_poa/mod.rs b/core/src/consensus/simple_poa/mod.rs index 8b3a5d5550..adf6d61469 100644 --- a/core/src/consensus/simple_poa/mod.rs +++ b/core/src/consensus/simple_poa/mod.rs @@ -109,10 +109,6 @@ impl ConsensusEngine for SimplePoA { Seal::None } - fn verify_local_seal(&self, _header: &Header) -> Result<(), Error> { - Ok(()) - } - fn verify_block_external(&self, header: &Header) -> Result<(), Error> { verify_external(header, &*self.validators) } diff --git a/core/src/consensus/solo/mod.rs b/core/src/consensus/solo/mod.rs index 89db756142..fe0855b72e 100644 --- a/core/src/consensus/solo/mod.rs +++ b/core/src/consensus/solo/mod.rs @@ -78,10 +78,6 @@ impl ConsensusEngine for Solo { Seal::Solo } - fn verify_local_seal(&self, _header: &Header) -> Result<(), Error> { - Ok(()) - } - fn on_close_block(&self, block: &mut ExecutedBlock) -> Result<(), Error> { let author = *block.header().author(); let (total_reward, min_fee) = { @@ -150,6 +146,6 @@ mod tests { header.set_seal(vec![::rlp::encode(&H520::default()).into_vec()]); - assert!(engine.verify_block_unordered(&header).is_ok()); + assert!(engine.verify_block_seal(&header).is_ok()); } } diff --git a/core/src/consensus/tendermint/engine.rs b/core/src/consensus/tendermint/engine.rs index df7592c0b6..929a0745b0 100644 --- a/core/src/consensus/tendermint/engine.rs +++ b/core/src/consensus/tendermint/engine.rs @@ -87,10 +87,6 @@ impl ConsensusEngine for Tendermint { self.inner.send(worker::Event::ProposalGenerated(Box::from(sealed_block.clone()))).unwrap(); } - fn verify_local_seal(&self, _header: &Header) -> Result<(), Error> { - Ok(()) - } - fn verify_block_basic(&self, header: &Header) -> Result<(), Error> { let (result, receiver) = crossbeam::bounded(1); self.inner diff --git a/core/src/miner/miner.rs b/core/src/miner/miner.rs index 902ae08d34..624c431eab 100644 --- a/core/src/miner/miner.rs +++ b/core/src/miner/miner.rs @@ -275,7 +275,7 @@ impl Miner { match self .engine .verify_transaction_basic(&tx, &fake_header) - .and_then(|_| self.engine.verify_transaction_unordered(tx, &fake_header)) + .and_then(|_| self.engine.verify_transaction_seal(tx, &fake_header)) { Err(e) => { cdebug!(MINER, "Rejected transaction {:?} with invalid signature: {:?}", hash, e); diff --git a/core/src/verification/queue/kind.rs b/core/src/verification/queue/kind.rs index 7ce6ce1739..d9d5a547b4 100644 --- a/core/src/verification/queue/kind.rs +++ b/core/src/verification/queue/kind.rs @@ -121,7 +121,7 @@ pub mod headers { fn verify(un: Self::Unverified, engine: &CodeChainEngine, check_seal: bool) -> Result { if check_seal { - engine.verify_block_unordered(&un).map(|_| un) + engine.verify_block_seal(&un).map(|_| un) } else { Ok(un) } @@ -137,7 +137,7 @@ pub mod headers { pub mod blocks { use primitives::{Bytes, H256, U256}; - use super::super::super::verification::{verify_block_basic, verify_block_unordered, PreverifiedBlock}; + use super::super::super::verification::{verify_block_basic, verify_block_seal, PreverifiedBlock}; use super::{BlockLike, Kind, MemUsage}; use crate::consensus::CodeChainEngine; use crate::error::Error; @@ -164,7 +164,7 @@ pub mod blocks { fn verify(un: Self::Unverified, engine: &CodeChainEngine, check_seal: bool) -> Result { let hash = un.hash(); - match verify_block_unordered(un.header, un.bytes, engine, check_seal) { + match verify_block_seal(un.header, un.bytes, engine, check_seal) { Ok(verified) => Ok(verified), Err(e) => { cwarn!(CLIENT, "Stage 2 block verification failed for {}: {:?}", hash, e); diff --git a/core/src/verification/verification.rs b/core/src/verification/verification.rs index d20054da01..5c614de88a 100644 --- a/core/src/verification/verification.rs +++ b/core/src/verification/verification.rs @@ -30,7 +30,7 @@ use crate::header::Header; use crate::transaction::{SignedTransaction, UnverifiedTransaction}; use crate::views::BlockView; -/// Preprocessed block data gathered in `verify_block_unordered` call +/// Preprocessed block data gathered in `verify_block_seal` call pub struct PreverifiedBlock { /// Populated block header pub header: Header, @@ -129,21 +129,21 @@ fn verify_transactions_root( /// Phase 2 verification. Perform costly checks such as transaction signatures and block nonce for ethash. /// Still operates on a individual block /// Returns a `PreverifiedBlock` structure populated with transactions -pub fn verify_block_unordered( +pub fn verify_block_seal( header: Header, bytes: Bytes, engine: &CodeChainEngine, check_seal: bool, ) -> Result { if check_seal { - engine.verify_block_unordered(&header)?; + engine.verify_block_seal(&header)?; } // Verify transactions. let mut transactions = Vec::new(); { let v = BlockView::new(&bytes); for t in v.transactions() { - let signed = engine.verify_transaction_unordered(t, &header)?; + let signed = engine.verify_transaction_seal(t, &header)?; transactions.push(signed); } } From f62bfc2546e0b54a82d076e9dac1b37c23cc1e64 Mon Sep 17 00:00:00 2001 From: Seulgi Kim Date: Fri, 17 May 2019 11:03:37 +0900 Subject: [PATCH 4/9] Split the verification that needs params and doesn't need it --- core/src/codechain_machine.rs | 20 +++- core/src/consensus/mod.rs | 4 +- core/src/miner/miner.rs | 7 +- core/src/transaction.rs | 9 +- core/src/verification/queue/kind.rs | 17 ++- core/src/verification/verification.rs | 69 ++++++++---- types/src/transaction/action.rs | 150 +++++++++++++++----------- 7 files changed, 174 insertions(+), 102 deletions(-) diff --git a/core/src/codechain_machine.rs b/core/src/codechain_machine.rs index 081915bb78..c347158632 100644 --- a/core/src/codechain_machine.rs +++ b/core/src/codechain_machine.rs @@ -74,16 +74,26 @@ impl CodeChainMachine { } /// Does basic verification of the transaction. - pub fn verify_transaction_basic(&self, p: &UnverifiedTransaction, header: &Header) -> Result<(), Error> { - let min_cost = self.min_cost(&p.action, Some(header.number())); - if p.fee < min_cost { + pub fn verify_transaction_basic_with_params( + &self, + tx: &UnverifiedTransaction, + header: &Header, + ) -> Result<(), Error> { + let block_number = header.number(); + if block_number == 0 { + return Ok(()) + } + let parent_block_number = block_number - 1; + + let min_cost = self.min_cost(&tx.action, Some(parent_block_number)); + if tx.fee < min_cost { return Err(SyntaxError::InsufficientFee { minimal: min_cost, - got: p.fee, + got: tx.fee, } .into()) } - p.verify_basic(&self.common_params(Some(header.number())), self.is_order_disabled)?; + tx.verify_basic_with_params(&self.common_params(Some(parent_block_number)), self.is_order_disabled)?; Ok(()) } diff --git a/core/src/consensus/mod.rs b/core/src/consensus/mod.rs index f5e5411960..0c0a79b820 100644 --- a/core/src/consensus/mod.rs +++ b/core/src/consensus/mod.rs @@ -344,7 +344,7 @@ impl fmt::Display for EngineError { /// Common type alias for an engine coupled with an CodeChain-like state machine. pub trait CodeChainEngine: ConsensusEngine { /// Additional verification for transactions in blocks. - fn verify_transaction_basic(&self, tx: &UnverifiedTransaction, header: &Header) -> Result<(), Error> { + fn verify_transaction_basic_with_params(&self, tx: &UnverifiedTransaction, header: &Header) -> Result<(), Error> { if let Action::Custom { handler_id, bytes, @@ -355,7 +355,7 @@ pub trait CodeChainEngine: ConsensusEngine { .ok_or_else(|| SyntaxError::InvalidCustomAction(format!("{} is an invalid handler id", handler_id)))?; handler.verify(bytes)?; } - self.machine().verify_transaction_basic(tx, header) + self.machine().verify_transaction_basic_with_params(tx, header) } /// Verify a particular transaction is valid. diff --git a/core/src/miner/miner.rs b/core/src/miner/miner.rs index 624c431eab..135a1371b3 100644 --- a/core/src/miner/miner.rs +++ b/core/src/miner/miner.rs @@ -272,9 +272,10 @@ impl Miner { if !self.is_allowed_transaction(&tx.action) { cdebug!(MINER, "Rejected transaction {:?}: {:?} is not allowed transaction", hash, tx.action); } - match self - .engine - .verify_transaction_basic(&tx, &fake_header) + match tx + .verify_basic() + .map_err(From::from) + .and_then(|_| self.engine.verify_transaction_basic_with_params(&tx, &fake_header)) .and_then(|_| self.engine.verify_transaction_seal(tx, &fake_header)) { Err(e) => { diff --git a/core/src/transaction.rs b/core/src/transaction.rs index f26c82afe7..ea05c5e3a6 100644 --- a/core/src/transaction.rs +++ b/core/src/transaction.rs @@ -132,7 +132,12 @@ impl UnverifiedTransaction { } /// Verify basic signature params. Does not attempt signer recovery. - pub fn verify_basic(&self, params: &CommonParams, is_order_disabled: bool) -> Result<(), SyntaxError> { + pub fn verify_basic(&self) -> Result<(), SyntaxError> { + self.action.verify() + } + + /// Verify basic signature params. Does not attempt signer recovery. + pub fn verify_basic_with_params(&self, params: &CommonParams, is_order_disabled: bool) -> Result<(), SyntaxError> { if self.network_id != params.network_id() { return Err(SyntaxError::InvalidNetworkId(self.network_id)) } @@ -140,7 +145,7 @@ impl UnverifiedTransaction { if byte_size >= params.max_body_size() { return Err(SyntaxError::TransactionIsTooBig) } - self.action.verify( + self.action.verify_with_params( params.network_id(), params.max_asset_scheme_metadata_size(), params.max_transfer_metadata_size(), diff --git a/core/src/verification/queue/kind.rs b/core/src/verification/queue/kind.rs index d9d5a547b4..791ee39365 100644 --- a/core/src/verification/queue/kind.rs +++ b/core/src/verification/queue/kind.rs @@ -85,12 +85,13 @@ pub mod headers { use primitives::{H256, U256}; - use super::super::super::verification::verify_header_params; + use super::super::super::verification::verify_header_basic; use super::{BlockLike, Kind}; use crate::consensus::CodeChainEngine; use crate::error::Error; use crate::header::Header; use crate::service::ClientIoMessage; + use verification::verify_header_basic_with_engine; impl BlockLike for Header { fn hash(&self) -> H256 { @@ -116,7 +117,9 @@ pub mod headers { fn create(input: Self::Input, engine: &CodeChainEngine) -> Result { // FIXME: this doesn't seem to match with full block verification - verify_header_params(&input, engine).map(|_| input) + verify_header_basic(&input)?; + verify_header_basic_with_engine(&input, engine)?; + Ok(input) } fn verify(un: Self::Unverified, engine: &CodeChainEngine, check_seal: bool) -> Result { @@ -137,7 +140,10 @@ pub mod headers { pub mod blocks { use primitives::{Bytes, H256, U256}; - use super::super::super::verification::{verify_block_basic, verify_block_seal, PreverifiedBlock}; + use super::super::super::verification::{ + verify_block_basic, verify_block_basic_with_params, verify_block_seal, verify_header_basic_with_engine, + PreverifiedBlock, + }; use super::{BlockLike, Kind, MemUsage}; use crate::consensus::CodeChainEngine; use crate::error::Error; @@ -153,7 +159,10 @@ pub mod blocks { type Verified = PreverifiedBlock; fn create(input: Self::Input, engine: &CodeChainEngine) -> Result { - match verify_block_basic(&input.header, &input.bytes, engine) { + match verify_block_basic(&input.header, &input.bytes) + .and_then(|_| verify_header_basic_with_engine(&input.header, engine)) + .and_then(|_| verify_block_basic_with_params(&input.header, &input.bytes, engine)) + { Ok(()) => Ok(input), Err(e) => { cwarn!(CLIENT, "Stage 1 block verification failed for {}: {:?}", input.hash(), e); diff --git a/core/src/verification/verification.rs b/core/src/verification/verification.rs index 5c614de88a..65df482dab 100644 --- a/core/src/verification/verification.rs +++ b/core/src/verification/verification.rs @@ -41,31 +41,38 @@ pub struct PreverifiedBlock { } /// Phase 1 quick block verification. Only does checks that are cheap. Operates on a single block -pub fn verify_block_basic(header: &Header, bytes: &[u8], engine: &CodeChainEngine) -> Result<(), Error> { - verify_header_params(&header, engine)?; - engine.verify_block_basic(&header)?; +pub fn verify_block_basic(header: &Header, bytes: &[u8]) -> Result<(), Error> { + verify_header_basic(header)?; let body_rlp = UntrustedRlp::new(bytes).at(1)?; + + for t in body_rlp.iter().map(|rlp| rlp.as_val::()) { + t?.verify_basic()?; + } + Ok(()) +} + +pub fn verify_header_basic_with_engine(header: &Header, engine: &CodeChainEngine) -> Result<(), Error> { + engine.verify_block_basic(&header)?; + Ok(()) +} + +pub fn verify_block_basic_with_params(header: &Header, bytes: &[u8], engine: &CodeChainEngine) -> Result<(), Error> { + verify_header_basic_with_params(&header, engine)?; + + let body_rlp = UntrustedRlp::new(bytes).at(1).expect("verify_block_basic already checked it"); if body_rlp.as_raw().len() > engine.machine().common_params(Some(header.number())).max_body_size() { return Err(BlockError::BodySizeIsTooBig.into()) } - for t in body_rlp.iter().map(|rlp| rlp.as_val::()) { - engine.verify_transaction_basic(&t?, &header)?; + for t in body_rlp.iter().map(|rlp| rlp.as_val().expect("verify_block_basic already checked it")) { + engine.verify_transaction_basic_with_params(&t, &header)?; } Ok(()) } /// Check basic header parameters. -pub fn verify_header_params(header: &Header, engine: &CodeChainEngine) -> Result<(), Error> { - let expected_seal_fields = engine.seal_fields(header); - if header.seal().len() != expected_seal_fields { - return Err(From::from(BlockError::InvalidSealArity(Mismatch { - expected: expected_seal_fields, - found: header.seal().len(), - }))) - } - +pub fn verify_header_basic(header: &Header) -> Result<(), Error> { let block_number = header.number(); if block_number >= BlockNumber::max_value() { return Err(From::from(BlockError::RidiculousNumber(OutOfBounds { @@ -74,14 +81,6 @@ pub fn verify_header_params(header: &Header, engine: &CodeChainEngine) -> Result found: block_number, }))) } - let max_extra_data_size = engine.machine().common_params(Some(block_number)).max_extra_data_size(); - if block_number != 0 && header.extra_data().len() > max_extra_data_size { - return Err(From::from(BlockError::ExtraDataOutOfBounds(OutOfBounds { - min: None, - max: Some(max_extra_data_size), - found: header.extra_data().len(), - }))) - } const ACCEPTABLE_DRIFT_SECS: u64 = 15; let now = SystemTime::now().duration_since(UNIX_EPOCH).unwrap_or_default(); @@ -108,6 +107,32 @@ pub fn verify_header_params(header: &Header, engine: &CodeChainEngine) -> Result Ok(()) } +pub fn verify_header_basic_with_params(header: &Header, engine: &CodeChainEngine) -> Result<(), Error> { + let expected_seal_fields = engine.seal_fields(header); + if header.seal().len() != expected_seal_fields { + return Err(From::from(BlockError::InvalidSealArity(Mismatch { + expected: expected_seal_fields, + found: header.seal().len(), + }))) + } + + let block_number = header.number(); + if block_number == 0 { + return Ok(()) + } + let parent_block_number = block_number - 1; + let max_extra_data_size = engine.machine().common_params(Some(parent_block_number)).max_extra_data_size(); + if header.extra_data().len() > max_extra_data_size { + return Err(From::from(BlockError::ExtraDataOutOfBounds(OutOfBounds { + min: None, + max: Some(max_extra_data_size), + found: header.extra_data().len(), + }))) + } + + Ok(()) +} + /// Verify block data against header: transactions root fn verify_transactions_root( block: &[u8], diff --git a/types/src/transaction/action.rs b/types/src/transaction/action.rs index da9dae30a5..bd69a88879 100644 --- a/types/src/transaction/action.rs +++ b/types/src/transaction/action.rs @@ -184,29 +184,12 @@ impl Action { self.asset_transaction().map(|tx| tx.tracker()) } - pub fn verify( - &self, - system_network_id: NetworkId, - max_asset_scheme_metadata_size: usize, - max_transfer_metadata_size: usize, - max_text_size: usize, - is_order_disabled: bool, - ) -> Result<(), SyntaxError> { - if let Some(network_id) = self.network_id() { - if network_id != system_network_id { - return Err(SyntaxError::InvalidNetworkId(network_id)) - } - } - + pub fn verify(&self) -> Result<(), SyntaxError> { match self { Action::MintAsset { - metadata, output, .. } => { - if metadata.len() > max_asset_scheme_metadata_size { - return Err(SyntaxError::MetadataTooBig) - } if output.supply == 0 { return Err(SyntaxError::ZeroQuantity) } @@ -216,12 +199,8 @@ impl Action { inputs, outputs, orders, - metadata, .. } => { - if metadata.len() > max_transfer_metadata_size { - return Err(SyntaxError::MetadataTooBig) - } if outputs.len() > 512 { return Err(SyntaxError::TooManyOutputs(outputs.len())) } @@ -236,9 +215,6 @@ impl Action { } check_duplication_in_prev_out(burns, inputs)?; - if is_order_disabled && !orders.is_empty() { - return Err(SyntaxError::DisabledTransaction) - } if outputs.iter().any(|output| output.quantity == 0) { return Err(SyntaxError::ZeroQuantity) } @@ -249,13 +225,9 @@ impl Action { verify_input_and_output_consistent_with_order(orders, inputs, outputs)?; } Action::ChangeAssetScheme { - metadata, asset_type, .. } => { - if metadata.len() > max_asset_scheme_metadata_size { - return Err(SyntaxError::MetadataTooBig) - } if asset_type.is_zero() { return Err(SyntaxError::CannotChangeWcccAssetScheme) } @@ -273,7 +245,6 @@ impl Action { } } Action::ComposeAsset { - metadata, inputs, output, .. @@ -294,9 +265,6 @@ impl Action { got: output.supply, }) } - if metadata.len() > max_asset_scheme_metadata_size { - return Err(SyntaxError::MetadataTooBig) - } } Action::DecomposeAsset { input, @@ -340,6 +308,78 @@ impl Action { return Err(SyntaxError::ZeroQuantity) } } + Action::Store { + .. + } => {} + _ => {} + } + Ok(()) + } + + pub fn verify_with_params( + &self, + system_network_id: NetworkId, + max_asset_scheme_metadata_size: usize, + max_transfer_metadata_size: usize, + max_text_size: usize, + is_order_disabled: bool, + ) -> Result<(), SyntaxError> { + if let Some(network_id) = self.network_id() { + if network_id != system_network_id { + return Err(SyntaxError::InvalidNetworkId(network_id)) + } + } + + match self { + Action::MintAsset { + metadata, + .. + } => { + if metadata.len() > max_asset_scheme_metadata_size { + return Err(SyntaxError::MetadataTooBig) + } + } + Action::TransferAsset { + orders, + metadata, + .. + } => { + if metadata.len() > max_transfer_metadata_size { + return Err(SyntaxError::MetadataTooBig) + } + + if is_order_disabled && !orders.is_empty() { + return Err(SyntaxError::DisabledTransaction) + } + } + Action::ChangeAssetScheme { + metadata, + .. + } => { + if metadata.len() > max_asset_scheme_metadata_size { + return Err(SyntaxError::MetadataTooBig) + } + } + Action::IncreaseAssetSupply { + .. + } => {} + Action::ComposeAsset { + metadata, + .. + } => { + if metadata.len() > max_asset_scheme_metadata_size { + return Err(SyntaxError::MetadataTooBig) + } + } + Action::DecomposeAsset { + .. + } => {} + Action::UnwrapCCC { + .. + } => {} + Action::WrapCCC { + .. + } => {} Action::Store { content, .. @@ -1442,7 +1482,8 @@ mod tests { approvals: vec![], expiration: None, }; - assert_eq!(action.verify(NetworkId::default(), 1000, 1000, 1000, false), Ok(())); + assert_eq!(action.verify(), Ok(())); + assert_eq!(action.verify_with_params(NetworkId::default(), 1000, 1000, 1000, false), Ok(())); } #[test] @@ -1564,7 +1605,8 @@ mod tests { expiration: None, }; - assert_eq!(action.verify(NetworkId::default(), 1000, 1000, 1000, false), Ok(())); + assert_eq!(action.verify(), Ok(())); + assert_eq!(action.verify_with_params(NetworkId::default(), 1000, 1000, 1000, false), Ok(())); } #[test] @@ -1651,10 +1693,7 @@ mod tests { approvals: vec![], expiration: None, }; - assert_eq!( - action.verify(NetworkId::default(), 1000, 1000, 1000, false), - Err(SyntaxError::InconsistentTransactionInOutWithOrders) - ); + assert_eq!(action.verify(), Err(SyntaxError::InconsistentTransactionInOutWithOrders)); // Case 2: multiple outputs with same order and asset_type let origin_output_1 = AssetOutPoint { @@ -1773,10 +1812,7 @@ mod tests { approvals: vec![], expiration: None, }; - assert_eq!( - action.verify(NetworkId::default(), 1000, 1000, 1000, false), - Err(SyntaxError::InconsistentTransactionInOutWithOrders) - ); + assert_eq!(action.verify(), Err(SyntaxError::InconsistentTransactionInOutWithOrders)); // Case 2-2: asset_type_to let action = Action::TransferAsset { @@ -1862,10 +1898,7 @@ mod tests { approvals: vec![], expiration: None, }; - assert_eq!( - action.verify(NetworkId::default(), 1000, 1000, 1000, false), - Err(SyntaxError::InconsistentTransactionInOutWithOrders) - ); + assert_eq!(action.verify(), Err(SyntaxError::InconsistentTransactionInOutWithOrders)); // Case 2-3: asset_type_fee let action = Action::TransferAsset { @@ -1951,10 +1984,7 @@ mod tests { approvals: vec![], expiration: None, }; - assert_eq!( - action.verify(NetworkId::default(), 1000, 1000, 1000, false), - Err(SyntaxError::InconsistentTransactionInOutWithOrders) - ); + assert_eq!(action.verify(), Err(SyntaxError::InconsistentTransactionInOutWithOrders)); } #[test] @@ -2071,7 +2101,8 @@ mod tests { approvals: vec![], expiration: None, }; - assert_eq!(action.verify(NetworkId::default(), 1000, 1000, 1000, false), Ok(())); + assert_eq!(action.verify(), Ok(())); + assert_eq!(action.verify_with_params(NetworkId::default(), 1000, 1000, 1000, false), Ok(())); } #[test] @@ -2092,10 +2123,7 @@ mod tests { }, receiver: Address::random(), }; - assert_eq!( - tx_zero_quantity.verify(NetworkId::default(), 1000, 1000, 1000, false), - Err(SyntaxError::ZeroQuantity) - ); + assert_eq!(tx_zero_quantity.verify(), Err(SyntaxError::ZeroQuantity)); let invalid_asset_type = H160::random(); let tx_invalid_asset_type = Action::UnwrapCCC { @@ -2114,10 +2142,7 @@ mod tests { }, receiver: Address::random(), }; - assert_eq!( - tx_invalid_asset_type.verify(NetworkId::default(), 1000, 1000, 1000, false), - Err(SyntaxError::InvalidAssetType(invalid_asset_type)) - ); + assert_eq!(tx_invalid_asset_type.verify(), Err(SyntaxError::InvalidAssetType(invalid_asset_type))); } #[test] @@ -2129,9 +2154,6 @@ mod tests { quantity: 0, payer: Address::random(), }; - assert_eq!( - tx_zero_quantity.verify(NetworkId::default(), 1000, 1000, 1000, false), - Err(SyntaxError::ZeroQuantity) - ); + assert_eq!(tx_zero_quantity.verify(), Err(SyntaxError::ZeroQuantity)); } } From 04d17b3562b9f903659cd1c5ee3d1d2e1070f02b Mon Sep 17 00:00:00 2001 From: Seulgi Kim Date: Fri, 17 May 2019 12:45:46 +0900 Subject: [PATCH 5/9] Postpone verifications that need CommonParams until running the parent block Currently, CommonParams is immutable, but the next patch will make it changeable. So we need to check it on the parent states. --- core/src/client/importer.rs | 4 ++ core/src/codechain_machine.rs | 8 +--- core/src/consensus/blake_pow/mod.rs | 4 +- core/src/consensus/cuckoo/mod.rs | 12 +++--- core/src/consensus/mod.rs | 6 +-- core/src/consensus/solo/mod.rs | 2 +- core/src/consensus/tendermint/engine.rs | 4 +- core/src/consensus/tendermint/mod.rs | 2 +- core/src/consensus/tendermint/worker.rs | 8 ++-- core/src/miner/miner.rs | 2 +- core/src/transaction.rs | 4 +- core/src/verification/canon_verifier.rs | 5 ++- core/src/verification/noop_verifier.rs | 3 ++ core/src/verification/queue/kind.rs | 10 ++--- core/src/verification/verification.rs | 47 +++++++++++++----------- core/src/verification/verifier.rs | 3 ++ test/src/e2e.long/ordersDisabled.test.ts | 42 ++++++++++++++------- 17 files changed, 96 insertions(+), 70 deletions(-) diff --git a/core/src/client/importer.rs b/core/src/client/importer.rs index e562022f24..d08a7a8a85 100644 --- a/core/src/client/importer.rs +++ b/core/src/client/importer.rs @@ -38,6 +38,7 @@ use crate::types::BlockId; use crate::verification::queue::{BlockQueue, HeaderQueue}; use crate::verification::{self, PreverifiedBlock, Verifier}; use crate::views::{BlockView, HeaderView}; +use client::EngineInfo; pub struct Importer { /// Lock used during block import @@ -242,6 +243,8 @@ impl Importer { ); })?; + let common_params = client.common_params(Some(parent.number())); + // Verify Block Family self.verifier .verify_block_family( @@ -255,6 +258,7 @@ impl Importer { block_provider: &*chain, client, }), + &common_params, ) .map_err(|e| { cwarn!( diff --git a/core/src/codechain_machine.rs b/core/src/codechain_machine.rs index c347158632..3adfb38f4e 100644 --- a/core/src/codechain_machine.rs +++ b/core/src/codechain_machine.rs @@ -74,11 +74,7 @@ impl CodeChainMachine { } /// Does basic verification of the transaction. - pub fn verify_transaction_basic_with_params( - &self, - tx: &UnverifiedTransaction, - header: &Header, - ) -> Result<(), Error> { + pub fn verify_transaction_with_params(&self, tx: &UnverifiedTransaction, header: &Header) -> Result<(), Error> { let block_number = header.number(); if block_number == 0 { return Ok(()) @@ -93,7 +89,7 @@ impl CodeChainMachine { } .into()) } - tx.verify_basic_with_params(&self.common_params(Some(parent_block_number)), self.is_order_disabled)?; + tx.verify_with_params(&self.common_params(Some(parent_block_number)), self.is_order_disabled)?; Ok(()) } diff --git a/core/src/consensus/blake_pow/mod.rs b/core/src/consensus/blake_pow/mod.rs index 24da0e5ac2..9daa1395cb 100644 --- a/core/src/consensus/blake_pow/mod.rs +++ b/core/src/consensus/blake_pow/mod.rs @@ -101,10 +101,10 @@ impl ConsensusEngine for BlakePoW { } fn verify_local_seal(&self, header: &Header) -> Result<(), Error> { - self.verify_block_basic(header).and_then(|_| self.verify_block_seal(header)) + self.verify_header_basic(header).and_then(|_| self.verify_block_seal(header)) } - fn verify_block_basic(&self, header: &Header) -> Result<(), Error> { + fn verify_header_basic(&self, header: &Header) -> Result<(), Error> { if *header.score() < self.params.min_score { return Err(From::from(BlockError::ScoreOutOfBounds(OutOfBounds { min: Some(self.params.min_score), diff --git a/core/src/consensus/cuckoo/mod.rs b/core/src/consensus/cuckoo/mod.rs index bb8f6a1d3d..2738e85cfe 100644 --- a/core/src/consensus/cuckoo/mod.rs +++ b/core/src/consensus/cuckoo/mod.rs @@ -107,10 +107,10 @@ impl ConsensusEngine for Cuckoo { } fn verify_local_seal(&self, header: &Header) -> Result<(), Error> { - self.verify_block_basic(header).and_then(|_| self.verify_block_seal(header)) + self.verify_header_basic(header).and_then(|_| self.verify_block_seal(header)) } - fn verify_block_basic(&self, header: &Header) -> Result<(), Error> { + fn verify_header_basic(&self, header: &Header) -> Result<(), Error> { if *header.score() < self.params.min_score { return Err(From::from(BlockError::ScoreOutOfBounds(OutOfBounds { min: Some(self.params.min_score), @@ -214,20 +214,20 @@ mod tests { } #[test] - fn verify_block_basic_err() { + fn verify_header_basic_err() { let engine = Scheme::new_test_cuckoo().engine; let default_header = Header::default(); - assert!(engine.verify_block_basic(&default_header).is_err()); + assert!(engine.verify_header_basic(&default_header).is_err()); } #[test] - fn verify_block_basic_ok() { + fn verify_header_basic_ok() { let scheme = Scheme::new_test_cuckoo(); let engine = &*scheme.engine; let genesis_header = scheme.genesis_header(); - assert!(engine.verify_block_basic(&genesis_header).is_ok()); + assert!(engine.verify_header_basic(&genesis_header).is_ok()); } #[test] diff --git a/core/src/consensus/mod.rs b/core/src/consensus/mod.rs index 0c0a79b820..2432179920 100644 --- a/core/src/consensus/mod.rs +++ b/core/src/consensus/mod.rs @@ -181,7 +181,7 @@ pub trait ConsensusEngine: Sync + Send { } /// Phase 1 quick block verification. Only does checks that are cheap. Returns either a null `Ok` or a general error detailing the problem with import. - fn verify_block_basic(&self, _header: &Header) -> Result<(), Error> { + fn verify_header_basic(&self, _header: &Header) -> Result<(), Error> { Ok(()) } @@ -344,7 +344,7 @@ impl fmt::Display for EngineError { /// Common type alias for an engine coupled with an CodeChain-like state machine. pub trait CodeChainEngine: ConsensusEngine { /// Additional verification for transactions in blocks. - fn verify_transaction_basic_with_params(&self, tx: &UnverifiedTransaction, header: &Header) -> Result<(), Error> { + fn verify_transaction_with_params(&self, tx: &UnverifiedTransaction, header: &Header) -> Result<(), Error> { if let Action::Custom { handler_id, bytes, @@ -355,7 +355,7 @@ pub trait CodeChainEngine: ConsensusEngine { .ok_or_else(|| SyntaxError::InvalidCustomAction(format!("{} is an invalid handler id", handler_id)))?; handler.verify(bytes)?; } - self.machine().verify_transaction_basic_with_params(tx, header) + self.machine().verify_transaction_with_params(tx, header) } /// Verify a particular transaction is valid. diff --git a/core/src/consensus/solo/mod.rs b/core/src/consensus/solo/mod.rs index fe0855b72e..548f04cf32 100644 --- a/core/src/consensus/solo/mod.rs +++ b/core/src/consensus/solo/mod.rs @@ -142,7 +142,7 @@ mod tests { let engine = Scheme::new_test_solo().engine; let mut header: Header = Header::default(); - assert!(engine.verify_block_basic(&header).is_ok()); + assert!(engine.verify_header_basic(&header).is_ok()); header.set_seal(vec![::rlp::encode(&H520::default()).into_vec()]); diff --git a/core/src/consensus/tendermint/engine.rs b/core/src/consensus/tendermint/engine.rs index 929a0745b0..c591144162 100644 --- a/core/src/consensus/tendermint/engine.rs +++ b/core/src/consensus/tendermint/engine.rs @@ -87,10 +87,10 @@ impl ConsensusEngine for Tendermint { self.inner.send(worker::Event::ProposalGenerated(Box::from(sealed_block.clone()))).unwrap(); } - fn verify_block_basic(&self, header: &Header) -> Result<(), Error> { + fn verify_header_basic(&self, header: &Header) -> Result<(), Error> { let (result, receiver) = crossbeam::bounded(1); self.inner - .send(worker::Event::VerifyBlockBasic { + .send(worker::Event::VerifyHeaderBasic { header: Box::from(header.clone()), result, }) diff --git a/core/src/consensus/tendermint/mod.rs b/core/src/consensus/tendermint/mod.rs index 12d87e99ad..7c4ba26587 100644 --- a/core/src/consensus/tendermint/mod.rs +++ b/core/src/consensus/tendermint/mod.rs @@ -189,7 +189,7 @@ mod tests { let engine = Scheme::new_test_tendermint().engine; let header = Header::default(); - let verify_result = engine.verify_block_basic(&header); + let verify_result = engine.verify_header_basic(&header); match verify_result { Err(Error::Block(BlockError::InvalidSealArity(_))) => {} diff --git a/core/src/consensus/tendermint/worker.rs b/core/src/consensus/tendermint/worker.rs index bf0e898328..4ecb7a0ff8 100644 --- a/core/src/consensus/tendermint/worker.rs +++ b/core/src/consensus/tendermint/worker.rs @@ -104,7 +104,7 @@ pub enum Event { result: crossbeam::Sender, }, ProposalGenerated(Box), - VerifyBlockBasic { + VerifyHeaderBasic { header: Box
, result: crossbeam::Sender>, }, @@ -247,8 +247,8 @@ impl Worker { Ok(Event::ProposalGenerated(sealed)) => { inner.proposal_generated(&*sealed); } - Ok(Event::VerifyBlockBasic{header, result}) => { - result.send(inner.verify_block_basic(&*header)).unwrap(); + Ok(Event::VerifyHeaderBasic{header, result}) => { + result.send(inner.verify_header_basic(&*header)).unwrap(); } Ok(Event::VerifyBlockExternal{header, result, }) => { result.send(inner.verify_block_external(&*header)).unwrap(); @@ -1051,7 +1051,7 @@ impl Worker { }; } - fn verify_block_basic(&self, header: &Header) -> Result<(), Error> { + fn verify_header_basic(&self, header: &Header) -> Result<(), Error> { let seal_length = header.seal().len(); let expected_seal_fields = self.seal_fields(); if seal_length != expected_seal_fields { diff --git a/core/src/miner/miner.rs b/core/src/miner/miner.rs index 135a1371b3..05805936f8 100644 --- a/core/src/miner/miner.rs +++ b/core/src/miner/miner.rs @@ -275,7 +275,7 @@ impl Miner { match tx .verify_basic() .map_err(From::from) - .and_then(|_| self.engine.verify_transaction_basic_with_params(&tx, &fake_header)) + .and_then(|_| self.engine.verify_transaction_with_params(&tx, &fake_header)) .and_then(|_| self.engine.verify_transaction_seal(tx, &fake_header)) { Err(e) => { diff --git a/core/src/transaction.rs b/core/src/transaction.rs index ea05c5e3a6..29c47f25a0 100644 --- a/core/src/transaction.rs +++ b/core/src/transaction.rs @@ -136,8 +136,8 @@ impl UnverifiedTransaction { self.action.verify() } - /// Verify basic signature params. Does not attempt signer recovery. - pub fn verify_basic_with_params(&self, params: &CommonParams, is_order_disabled: bool) -> Result<(), SyntaxError> { + /// Verify transactiosn with the common params. Does not attempt signer recovery. + pub fn verify_with_params(&self, params: &CommonParams, is_order_disabled: bool) -> Result<(), SyntaxError> { if self.network_id != params.network_id() { return Err(SyntaxError::InvalidNetworkId(self.network_id)) } diff --git a/core/src/verification/canon_verifier.rs b/core/src/verification/canon_verifier.rs index 156b1174fc..a7cbe4d52a 100644 --- a/core/src/verification/canon_verifier.rs +++ b/core/src/verification/canon_verifier.rs @@ -14,6 +14,8 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . +use ctypes::CommonParams; + use super::verification; use super::Verifier; use crate::client::BlockChainTrait; @@ -32,8 +34,9 @@ impl Verifier for CanonVerifier { parent: &Header, engine: &CodeChainEngine, do_full: Option>, + common_params: &CommonParams, ) -> Result<(), Error> { - verification::verify_block_family(block, header, parent, engine, do_full) + verification::verify_block_family(block, header, parent, engine, do_full, common_params) } fn verify_block_final(&self, expected: &Header, got: &Header) -> Result<(), Error> { diff --git a/core/src/verification/noop_verifier.rs b/core/src/verification/noop_verifier.rs index b51226d07c..368f15af31 100644 --- a/core/src/verification/noop_verifier.rs +++ b/core/src/verification/noop_verifier.rs @@ -14,6 +14,8 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . +use ctypes::CommonParams; + use super::{verification, Verifier}; use crate::client::BlockChainTrait; use crate::consensus::CodeChainEngine; @@ -31,6 +33,7 @@ impl Verifier for NoopVerifier { _t: &Header, _: &CodeChainEngine, _: Option>, + _common_params: &CommonParams, ) -> Result<(), Error> { Ok(()) } diff --git a/core/src/verification/queue/kind.rs b/core/src/verification/queue/kind.rs index 791ee39365..c19572b11f 100644 --- a/core/src/verification/queue/kind.rs +++ b/core/src/verification/queue/kind.rs @@ -91,7 +91,7 @@ pub mod headers { use crate::error::Error; use crate::header::Header; use crate::service::ClientIoMessage; - use verification::verify_header_basic_with_engine; + use verification::verify_header_with_engine; impl BlockLike for Header { fn hash(&self) -> H256 { @@ -118,7 +118,7 @@ pub mod headers { fn create(input: Self::Input, engine: &CodeChainEngine) -> Result { // FIXME: this doesn't seem to match with full block verification verify_header_basic(&input)?; - verify_header_basic_with_engine(&input, engine)?; + verify_header_with_engine(&input, engine)?; Ok(input) } @@ -141,8 +141,7 @@ pub mod blocks { use primitives::{Bytes, H256, U256}; use super::super::super::verification::{ - verify_block_basic, verify_block_basic_with_params, verify_block_seal, verify_header_basic_with_engine, - PreverifiedBlock, + verify_block_basic, verify_block_seal, verify_header_with_engine, PreverifiedBlock, }; use super::{BlockLike, Kind, MemUsage}; use crate::consensus::CodeChainEngine; @@ -160,8 +159,7 @@ pub mod blocks { fn create(input: Self::Input, engine: &CodeChainEngine) -> Result { match verify_block_basic(&input.header, &input.bytes) - .and_then(|_| verify_header_basic_with_engine(&input.header, engine)) - .and_then(|_| verify_block_basic_with_params(&input.header, &input.bytes, engine)) + .and_then(|_| verify_header_with_engine(&input.header, engine)) { Ok(()) => Ok(input), Err(e) => { diff --git a/core/src/verification/verification.rs b/core/src/verification/verification.rs index 65df482dab..d37fe3eef2 100644 --- a/core/src/verification/verification.rs +++ b/core/src/verification/verification.rs @@ -18,7 +18,7 @@ use std::time::{SystemTime, UNIX_EPOCH}; use cmerkle::skewed_merkle_root; use ctypes::util::unexpected::{Mismatch, OutOfBounds}; -use ctypes::BlockNumber; +use ctypes::{BlockNumber, CommonParams}; use primitives::{Bytes, H256}; use rlp::UntrustedRlp; @@ -52,21 +52,34 @@ pub fn verify_block_basic(header: &Header, bytes: &[u8]) -> Result<(), Error> { Ok(()) } -pub fn verify_header_basic_with_engine(header: &Header, engine: &CodeChainEngine) -> Result<(), Error> { - engine.verify_block_basic(&header)?; +pub fn verify_header_with_engine(header: &Header, engine: &CodeChainEngine) -> Result<(), Error> { + engine.verify_header_basic(&header)?; + + let expected_seal_fields = engine.seal_fields(header); + if header.seal().len() != expected_seal_fields { + return Err(From::from(BlockError::InvalidSealArity(Mismatch { + expected: expected_seal_fields, + found: header.seal().len(), + }))) + } Ok(()) } -pub fn verify_block_basic_with_params(header: &Header, bytes: &[u8], engine: &CodeChainEngine) -> Result<(), Error> { - verify_header_basic_with_params(&header, engine)?; +pub fn verify_block_with_params( + header: &Header, + bytes: &[u8], + engine: &CodeChainEngine, + common_params: &CommonParams, +) -> Result<(), Error> { + verify_header_with_params(&header, common_params)?; let body_rlp = UntrustedRlp::new(bytes).at(1).expect("verify_block_basic already checked it"); - if body_rlp.as_raw().len() > engine.machine().common_params(Some(header.number())).max_body_size() { + if body_rlp.as_raw().len() > common_params.max_body_size() { return Err(BlockError::BodySizeIsTooBig.into()) } for t in body_rlp.iter().map(|rlp| rlp.as_val().expect("verify_block_basic already checked it")) { - engine.verify_transaction_basic_with_params(&t, &header)?; + engine.verify_transaction_with_params(&t, header)?; } Ok(()) } @@ -107,21 +120,8 @@ pub fn verify_header_basic(header: &Header) -> Result<(), Error> { Ok(()) } -pub fn verify_header_basic_with_params(header: &Header, engine: &CodeChainEngine) -> Result<(), Error> { - let expected_seal_fields = engine.seal_fields(header); - if header.seal().len() != expected_seal_fields { - return Err(From::from(BlockError::InvalidSealArity(Mismatch { - expected: expected_seal_fields, - found: header.seal().len(), - }))) - } - - let block_number = header.number(); - if block_number == 0 { - return Ok(()) - } - let parent_block_number = block_number - 1; - let max_extra_data_size = engine.machine().common_params(Some(parent_block_number)).max_extra_data_size(); +pub fn verify_header_with_params(header: &Header, common_params: &CommonParams) -> Result<(), Error> { + let max_extra_data_size = common_params.max_extra_data_size(); if header.extra_data().len() > max_extra_data_size { return Err(From::from(BlockError::ExtraDataOutOfBounds(OutOfBounds { min: None, @@ -201,7 +201,10 @@ pub fn verify_block_family( parent: &Header, engine: &CodeChainEngine, do_full: Option>, + common_params: &CommonParams, ) -> Result<(), Error> { + verify_block_with_params(header, block, engine, common_params)?; + // TODO: verify timestamp verify_parent(&header, &parent)?; verify_transactions_root(block, header.transactions_root(), *parent.transactions_root())?; diff --git a/core/src/verification/verifier.rs b/core/src/verification/verifier.rs index 6cce995639..c0f2379f79 100644 --- a/core/src/verification/verifier.rs +++ b/core/src/verification/verifier.rs @@ -14,6 +14,8 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . +use ctypes::CommonParams; + use super::verification; use crate::client::BlockChainTrait; use crate::consensus::CodeChainEngine; @@ -32,6 +34,7 @@ where parent: &Header, engine: &CodeChainEngine, do_full: Option>, + common_params: &CommonParams, ) -> Result<(), Error>; /// Do a final verification check for an enacted header vs its expected counterpart. diff --git a/test/src/e2e.long/ordersDisabled.test.ts b/test/src/e2e.long/ordersDisabled.test.ts index 7ba69a7506..bf5c2119cc 100644 --- a/test/src/e2e.long/ordersDisabled.test.ts +++ b/test/src/e2e.long/ordersDisabled.test.ts @@ -119,7 +119,7 @@ describe("order is disabled", function() { await node.sdk.rpc.chain.sendSignedTransaction(signed); expect.fail(); } catch (e) { - expect(e).is.similarTo(ERROR.DISABLED_TRANSACTION); + expect(e).is.similarTo(ERROR.INVALID_ORDER_ASSET_TYPES); } }); }); @@ -760,7 +760,9 @@ describe("order is disabled", function() { await node.sdk.rpc.chain.sendSignedTransaction(signed); expect.fail(); } catch (e) { - expect(e).is.similarTo(ERROR.DISABLED_TRANSACTION); + expect(e).is.similarTo( + ERROR.INCONSISTENT_TRANSACTION_IN_OUT_WITH_ORDERS + ); } }); @@ -828,7 +830,7 @@ describe("order is disabled", function() { await node.sdk.rpc.chain.sendSignedTransaction(signed); expect.fail(); } catch (e) { - expect(e).is.similarTo(ERROR.DISABLED_TRANSACTION); + expect(e).is.similarTo(ERROR.INVALID_SPENT_QUANTITY); } }); @@ -896,7 +898,9 @@ describe("order is disabled", function() { await node.sdk.rpc.chain.sendSignedTransaction(signed); expect.fail(); } catch (e) { - expect(e).is.similarTo(ERROR.DISABLED_TRANSACTION); + expect(e).is.similarTo( + ERROR.INCONSISTENT_TRANSACTION_IN_OUT_WITH_ORDERS + ); } }); @@ -971,7 +975,9 @@ describe("order is disabled", function() { await node.sdk.rpc.chain.sendSignedTransaction(signed); expect.fail(); } catch (e) { - expect(e).is.similarTo(ERROR.DISABLED_TRANSACTION); + expect(e).is.similarTo( + ERROR.INVALID_ORDER_LOCK_SCRIPT_HASH + ); } }); @@ -1042,7 +1048,7 @@ describe("order is disabled", function() { await node.sdk.rpc.chain.sendSignedTransaction(signed); expect.fail(); } catch (e) { - expect(e).is.similarTo(ERROR.DISABLED_TRANSACTION); + expect(e).is.similarTo(ERROR.INVALID_ORDER_PARAMETERS); } }); @@ -1116,7 +1122,9 @@ describe("order is disabled", function() { await node.sdk.rpc.chain.sendSignedTransaction(signed); expect.fail(); } catch (e) { - expect(e).is.similarTo(ERROR.DISABLED_TRANSACTION); + expect(e).is.similarTo( + ERROR.INCONSISTENT_TRANSACTION_IN_OUT_WITH_ORDERS + ); } }); @@ -1190,7 +1198,9 @@ describe("order is disabled", function() { await node.sdk.rpc.chain.sendSignedTransaction(signed); expect.fail(); } catch (e) { - expect(e).is.similarTo(ERROR.DISABLED_TRANSACTION); + expect(e).is.similarTo( + ERROR.INCONSISTENT_TRANSACTION_IN_OUT_WITH_ORDERS + ); } }); @@ -1270,7 +1280,9 @@ describe("order is disabled", function() { await node.sdk.rpc.chain.sendSignedTransaction(signed); expect.fail(); } catch (e) { - expect(e).is.similarTo(ERROR.DISABLED_TRANSACTION); + expect(e).is.similarTo( + ERROR.INCONSISTENT_TRANSACTION_IN_OUT_WITH_ORDERS + ); } }); @@ -1340,7 +1352,7 @@ describe("order is disabled", function() { await node.sdk.rpc.chain.sendSignedTransaction(signed); expect.fail(); } catch (e) { - expect(e).is.similarTo(ERROR.DISABLED_TRANSACTION); + expect(e).is.similarTo(ERROR.INVALID_ORIGIN_OUTPUTS); } }); @@ -1409,7 +1421,7 @@ describe("order is disabled", function() { await node.sdk.rpc.chain.sendSignedTransaction(signed); expect.fail(); } catch (e) { - expect(e).is.similarTo(ERROR.DISABLED_TRANSACTION); + expect(e).is.similarTo(ERROR.INVALID_ORIGIN_OUTPUTS); } }); @@ -1757,7 +1769,9 @@ describe("order is disabled", function() { await node.sdk.rpc.chain.sendSignedTransaction(signed); expect.fail(); } catch (e) { - expect(e).is.similarTo(ERROR.DISABLED_TRANSACTION); + expect(e).is.similarTo( + ERROR.INVALID_ORDER_ASSET_QUANTITIES + ); } }); @@ -1827,7 +1841,9 @@ describe("order is disabled", function() { await node.sdk.rpc.chain.sendSignedTransaction(signed); expect.fail(); } catch (e) { - expect(e).is.similarTo(ERROR.DISABLED_TRANSACTION); + expect(e).is.similarTo( + ERROR.INVALID_ORDER_ASSET_QUANTITIES + ); } }); From 54a0e4fe52b587c1ccbac581fd8ca83d3b835fdf Mon Sep 17 00:00:00 2001 From: Seulgi Kim Date: Mon, 20 May 2019 10:15:35 +0900 Subject: [PATCH 6/9] common_params may return None After introducing ChangeParams, common_params should return None, when the block is not executed yet. --- codechain/run_node.rs | 12 ++- codechain/subcommand/account_command.rs | 2 +- codechain/subcommand/convert_command.rs | 2 +- core/src/client/client.rs | 4 +- core/src/client/importer.rs | 2 +- core/src/client/mod.rs | 2 +- core/src/codechain_machine.rs | 77 +++++++++--------- core/src/consensus/mod.rs | 9 ++- core/src/consensus/solo/mod.rs | 7 +- core/src/consensus/tendermint/engine.rs | 5 +- core/src/miner/miner.rs | 12 ++- core/src/scheme/scheme.rs | 12 +-- core/src/transaction.rs | 8 +- core/src/verification/verification.rs | 2 +- rpc/src/v1/impls/account.rs | 2 +- rpc/src/v1/impls/chain.rs | 103 ++++++++++++------------ rpc/src/v1/impls/devel.rs | 9 ++- rpc/src/v1/impls/engine.rs | 2 +- types/src/common_params.rs | 15 ++++ types/src/transaction/action.rs | 35 +++++--- 20 files changed, 179 insertions(+), 143 deletions(-) diff --git a/codechain/run_node.rs b/codechain/run_node.rs index c66823d62e..5122711da5 100644 --- a/codechain/run_node.rs +++ b/codechain/run_node.rs @@ -264,7 +264,7 @@ pub fn run_node(matches: &ArgMatches) -> Result<(), String> { let network_config = config.network_config()?; // XXX: What should we do if the network id has been changed. let c = client.client(); - let network_id = c.common_params(None).network_id(); + let network_id = c.common_params(None).unwrap().network_id(); let routing_table = RoutingTable::new(); let service = network_start(network_id, timer_loop, &network_config, Arc::clone(&routing_table))?; @@ -336,12 +336,10 @@ pub fn run_node(matches: &ArgMatches) -> Result<(), String> { let _snapshot_service = { if !config.snapshot.disable.unwrap() { // FIXME: Let's make it load snapshot period dynamically to support changing the period. - let service = SnapshotService::new( - client.client(), - config.snapshot.path.unwrap(), - scheme.params(None).snapshot_period(), - ); - client.client().add_notify(Arc::downgrade(&service) as Weak); + let client = client.client(); + let snapshot_period = client.common_params(None).unwrap().snapshot_period(); + let service = SnapshotService::new(Arc::clone(&client), config.snapshot.path.unwrap(), snapshot_period); + client.add_notify(Arc::downgrade(&service) as Weak); Some(service) } else { None diff --git a/codechain/subcommand/account_command.rs b/codechain/subcommand/account_command.rs index 2446de8684..64a4c96374 100644 --- a/codechain/subcommand/account_command.rs +++ b/codechain/subcommand/account_command.rs @@ -44,7 +44,7 @@ pub fn run_account_command(matches: &ArgMatches) -> Result<(), String> { let ap = AccountProvider::new(keystore); let chain = get_global_argument(matches, "chain").unwrap_or_else(|| "mainnet".into()); let chain_type: ChainType = chain.parse().unwrap(); - let network_id: NetworkId = chain_type.scheme().map(|scheme| scheme.params(None).network_id())?; + let network_id: NetworkId = chain_type.scheme().map(|scheme| scheme.genesis_params().network_id())?; match matches.subcommand() { ("create", _) => create(&ap, network_id), diff --git a/codechain/subcommand/convert_command.rs b/codechain/subcommand/convert_command.rs index 5a32d3f893..d7f5a2c419 100644 --- a/codechain/subcommand/convert_command.rs +++ b/codechain/subcommand/convert_command.rs @@ -134,7 +134,7 @@ fn get_network_id(matches: &ArgMatches) -> Result { let chain = matches.value_of("chain").unwrap_or_else(|| "solo"); let chain_type: ChainType = chain.parse().unwrap(); // XXX: What should we do if the network id has been changed - let network_id: NetworkId = chain_type.scheme().map(|scheme| scheme.params(None).network_id())?; + let network_id: NetworkId = chain_type.scheme().map(|scheme| scheme.genesis_params().network_id())?; Ok(network_id) } diff --git a/core/src/client/client.rs b/core/src/client/client.rs index 0847ebc4c4..7399bc2530 100644 --- a/core/src/client/client.rs +++ b/core/src/client/client.rs @@ -484,7 +484,7 @@ impl StateInfo for Client { } impl EngineInfo for Client { - fn common_params(&self, block_number: Option) -> CommonParams { + fn common_params(&self, block_number: Option) -> Option { self.engine().machine().common_params(block_number) } @@ -556,7 +556,7 @@ impl BlockChainTrait for Client { fn genesis_accounts(&self) -> Vec { // XXX: What should we do if the network id has been changed - let network_id = self.common_params(None).network_id(); + let network_id = self.common_params(None).unwrap().network_id(); self.genesis_accounts.iter().map(|addr| PlatformAddress::new_v1(network_id, *addr)).collect() } diff --git a/core/src/client/importer.rs b/core/src/client/importer.rs index d08a7a8a85..9db03da603 100644 --- a/core/src/client/importer.rs +++ b/core/src/client/importer.rs @@ -243,7 +243,7 @@ impl Importer { ); })?; - let common_params = client.common_params(Some(parent.number())); + let common_params = client.common_params(Some(parent.number())).unwrap(); // Verify Block Family self.verifier diff --git a/core/src/client/mod.rs b/core/src/client/mod.rs index e498c45051..eff7eeeb09 100644 --- a/core/src/client/mod.rs +++ b/core/src/client/mod.rs @@ -86,7 +86,7 @@ pub trait BlockChainTrait { } pub trait EngineInfo: Send + Sync { - fn common_params(&self, block_number: Option) -> CommonParams; + fn common_params(&self, block_number: Option) -> Option; fn block_reward(&self, block_number: u64) -> u64; fn mining_reward(&self, block_number: u64) -> Option; fn recommended_confirmation(&self) -> u32; diff --git a/core/src/codechain_machine.rs b/core/src/codechain_machine.rs index 3adfb38f4e..ee309a9767 100644 --- a/core/src/codechain_machine.rs +++ b/core/src/codechain_machine.rs @@ -51,37 +51,37 @@ impl CodeChainMachine { } /// Get the general parameters of the chain. - pub fn common_params(&self, block_number: Option) -> CommonParams { + pub fn common_params(&self, block_number: Option) -> Option { let params = &self.params; assert!(!params.is_empty()); let block_number = if let Some(block_number) = block_number { block_number } else { - return params.last().unwrap().params // the latest block. + return Some(params.last().unwrap().params) // the latest block. }; - params - .iter() - .take_while( - |Params { - changed_block, - .. - }| *changed_block <= block_number, - ) - .last() - .unwrap() - .params + Some( + params + .iter() + .take_while( + |Params { + changed_block, + .. + }| *changed_block <= block_number, + ) + .last() + .unwrap() + .params, + ) } /// Does basic verification of the transaction. - pub fn verify_transaction_with_params(&self, tx: &UnverifiedTransaction, header: &Header) -> Result<(), Error> { - let block_number = header.number(); - if block_number == 0 { - return Ok(()) - } - let parent_block_number = block_number - 1; - - let min_cost = self.min_cost(&tx.action, Some(parent_block_number)); + pub fn verify_transaction_with_params( + &self, + tx: &UnverifiedTransaction, + common_params: &CommonParams, + ) -> Result<(), Error> { + let min_cost = Self::min_cost(common_params, &tx.action); if tx.fee < min_cost { return Err(SyntaxError::InsufficientFee { minimal: min_cost, @@ -89,7 +89,7 @@ impl CodeChainMachine { } .into()) } - tx.verify_with_params(&self.common_params(Some(parent_block_number)), self.is_order_disabled)?; + tx.verify_with_params(common_params, self.is_order_disabled)?; Ok(()) } @@ -220,8 +220,7 @@ impl CodeChainMachine { Ok(()) } - pub fn min_cost(&self, action: &Action, block_number: Option) -> u64 { - let params = self.common_params(block_number); + pub fn min_cost(params: &CommonParams, action: &Action) -> u64 { match action { Action::MintAsset { .. @@ -305,9 +304,9 @@ mod tests { fn common_params_are_not_changed_since_genesis() { let genesis_params = CommonParams::default_for_test(); let machine = CodeChainMachine::new(genesis_params); - assert_eq!(genesis_params, machine.common_params(Some(0))); - assert_eq!(genesis_params, machine.common_params(Some(1))); - assert_eq!(genesis_params, machine.common_params(None)); + assert_eq!(Some(genesis_params), machine.common_params(Some(0))); + assert_eq!(Some(genesis_params), machine.common_params(Some(1))); + assert_eq!(Some(genesis_params), machine.common_params(None)); } #[test] @@ -331,9 +330,9 @@ mod tests { ], is_order_disabled: false, }; - assert_eq!(genesis_params, machine.common_params(Some(0))); - assert_eq!(params_at_1, machine.common_params(Some(1))); - assert_eq!(params_at_1, machine.common_params(None)); + assert_eq!(Some(genesis_params), machine.common_params(Some(0))); + assert_eq!(Some(params_at_1), machine.common_params(Some(1))); + assert_eq!(Some(params_at_1), machine.common_params(None)); } #[test] @@ -357,10 +356,10 @@ mod tests { ], is_order_disabled: false, }; - assert_eq!(genesis_params, machine.common_params(Some(0))); - assert_eq!(genesis_params, machine.common_params(Some(1))); - assert_eq!(params_at_2, machine.common_params(Some(2))); - assert_eq!(params_at_2, machine.common_params(None)); + assert_eq!(Some(genesis_params), machine.common_params(Some(0))); + assert_eq!(Some(genesis_params), machine.common_params(Some(1))); + assert_eq!(Some(params_at_2), machine.common_params(Some(2))); + assert_eq!(Some(params_at_2), machine.common_params(None)); } @@ -404,17 +403,17 @@ mod tests { is_order_disabled: false, }; for i in 0..10 { - assert_eq!(genesis_params, machine.common_params(Some(i)), "unexpected params at block {}", i); + assert_eq!(Some(genesis_params), machine.common_params(Some(i)), "unexpected params at block {}", i); } for i in 10..20 { - assert_eq!(params_at_10, machine.common_params(Some(i)), "unexpected params at block {}", i); + assert_eq!(Some(params_at_10), machine.common_params(Some(i)), "unexpected params at block {}", i); } for i in 20..30 { - assert_eq!(params_at_20, machine.common_params(Some(i)), "unexpected params at block {}", i); + assert_eq!(Some(params_at_20), machine.common_params(Some(i)), "unexpected params at block {}", i); } for i in 30..40 { - assert_eq!(params_at_30, machine.common_params(Some(i)), "unexpected params at block {}", i); + assert_eq!(Some(params_at_30), machine.common_params(Some(i)), "unexpected params at block {}", i); } - assert_eq!(params_at_30, machine.common_params(None)); + assert_eq!(Some(params_at_30), machine.common_params(None)); } } diff --git a/core/src/consensus/mod.rs b/core/src/consensus/mod.rs index 2432179920..233c664f89 100644 --- a/core/src/consensus/mod.rs +++ b/core/src/consensus/mod.rs @@ -43,6 +43,7 @@ use cstate::ActionHandler; use ctypes::errors::SyntaxError; use ctypes::transaction::Action; use ctypes::util::unexpected::{Mismatch, OutOfBounds}; +use ctypes::CommonParams; use primitives::{Bytes, H256, U256}; use self::tendermint::types::{BitSet, View}; @@ -344,7 +345,11 @@ impl fmt::Display for EngineError { /// Common type alias for an engine coupled with an CodeChain-like state machine. pub trait CodeChainEngine: ConsensusEngine { /// Additional verification for transactions in blocks. - fn verify_transaction_with_params(&self, tx: &UnverifiedTransaction, header: &Header) -> Result<(), Error> { + fn verify_transaction_with_params( + &self, + tx: &UnverifiedTransaction, + common_params: &CommonParams, + ) -> Result<(), Error> { if let Action::Custom { handler_id, bytes, @@ -355,7 +360,7 @@ pub trait CodeChainEngine: ConsensusEngine { .ok_or_else(|| SyntaxError::InvalidCustomAction(format!("{} is an invalid handler id", handler_id)))?; handler.verify(bytes)?; } - self.machine().verify_transaction_with_params(tx, header) + self.machine().verify_transaction_with_params(tx, common_params) } /// Verify a particular transaction is valid. diff --git a/core/src/consensus/solo/mod.rs b/core/src/consensus/solo/mod.rs index 548f04cf32..6dd2099c35 100644 --- a/core/src/consensus/solo/mod.rs +++ b/core/src/consensus/solo/mod.rs @@ -81,12 +81,15 @@ impl ConsensusEngine for Solo { fn on_close_block(&self, block: &mut ExecutedBlock) -> Result<(), Error> { let author = *block.header().author(); let (total_reward, min_fee) = { - let block_number = block.header().number(); let transactions = block.transactions(); let block_reward = self.block_reward(block.header().number()); let total_fee: u64 = transactions.iter().map(|tx| tx.fee).sum(); + let block_number = block.header().number(); + assert_ne!(0, block_number); + let parent_block_number = block.header().number() - 1; + let common_params = self.machine().common_params(Some(parent_block_number)).unwrap(); let min_fee: u64 = - transactions.iter().map(|tx| self.machine().min_cost(&tx.action, Some(block_number))).sum(); + transactions.iter().map(|tx| CodeChainMachine::min_cost(&common_params, &tx.action)).sum(); (block_reward + total_fee, min_fee) }; diff --git a/core/src/consensus/tendermint/engine.rs b/core/src/consensus/tendermint/engine.rs index c591144162..edf68442e4 100644 --- a/core/src/consensus/tendermint/engine.rs +++ b/core/src/consensus/tendermint/engine.rs @@ -134,7 +134,10 @@ impl ConsensusEngine for Tendermint { let transactions = block.transactions(); let total_fee: u64 = transactions.iter().map(|tx| tx.fee).sum(); let block_number = block.header().number(); - let min_fee = transactions.iter().map(|tx| self.machine.min_cost(&tx.action, Some(block_number))).sum(); + assert_ne!(0, block_number); + let parent_block_number = block.header().number() - 1; + let common_params = self.machine().common_params(Some(parent_block_number)).unwrap(); + let min_fee = transactions.iter().map(|tx| CodeChainMachine::min_cost(&common_params, &tx.action)).sum(); (total_fee, min_fee) }; assert!(total_fee >= min_fee, "{} >= {}", total_fee, min_fee); diff --git a/core/src/miner/miner.rs b/core/src/miner/miner.rs index 05805936f8..9ce63d5fc5 100644 --- a/core/src/miner/miner.rs +++ b/core/src/miner/miner.rs @@ -254,7 +254,8 @@ impl Miner { default_origin: TxOrigin, mem_pool: &mut MemPool, ) -> Vec> { - let fake_header = client.best_block_header().decode().generate_child(); + let best_header = client.best_block_header().decode(); + let fake_header = best_header.generate_child(); let current_block_number = client.chain_info().best_block_number; let current_timestamp = client.chain_info().best_block_timestamp; let mut inserted = Vec::with_capacity(transactions.len()); @@ -275,7 +276,10 @@ impl Miner { match tx .verify_basic() .map_err(From::from) - .and_then(|_| self.engine.verify_transaction_with_params(&tx, &fake_header)) + .and_then(|_| { + let common_params = self.engine.machine().common_params(Some(best_header.number())).unwrap(); + self.engine.verify_transaction_with_params(&tx, &common_params) + }) .and_then(|_| self.engine.verify_transaction_seal(tx, &fake_header)) { Err(e) => { @@ -452,7 +456,7 @@ impl Miner { let params = self.params.read().clone(); let open_block = chain.prepare_open_block(parent_block_id, params.author, params.extra_data); let block_number = open_block.block().header().number(); - let max_body_size = self.engine.machine().common_params(Some(block_number)).max_body_size(); + let max_body_size = self.engine.machine().common_params(Some(block_number)).unwrap().max_body_size(); const DEFAULT_RANGE: Range = 0..::std::u64::MAX; let transactions = mem_pool .top_transactions(max_body_size, Some(open_block.header().timestamp()), DEFAULT_RANGE) @@ -990,7 +994,7 @@ impl MinerService for Miner { } fn ready_transactions(&self, range: Range) -> PendingSignedTransactions { - let max_body_size = self.engine.machine().common_params(None).max_body_size(); + let max_body_size = self.engine.machine().common_params(None).unwrap().max_body_size(); self.mem_pool.read().top_transactions(max_body_size, None, range) } diff --git a/core/src/scheme/scheme.rs b/core/src/scheme/scheme.rs index 6f14a55682..d979ae2267 100644 --- a/core/src/scheme/scheme.rs +++ b/core/src/scheme/scheme.rs @@ -23,7 +23,7 @@ use ckey::Address; use cmerkle::TrieFactory; use cstate::{Metadata, MetadataAddress, Shard, ShardAddress, StateDB, StateResult, StateWithCache, TopLevelState}; use ctypes::errors::SyntaxError; -use ctypes::{BlockNumber, CommonParams, ShardId}; +use ctypes::{CommonParams, ShardId}; use hashdb::{AsHashDB, HashDB}; use parking_lot::RwLock; use primitives::{Bytes, H256, U256}; @@ -209,7 +209,7 @@ impl Scheme { let header = chain.block_header(&genesis_header_hash).ok_or_else(|| Error::Scheme(SchemeError::InvalidCommonParams))?; let extra_data = header.extra_data(); - let common_params_hash = blake256(&self.params(Some(0)).rlp_bytes()).to_vec(); + let common_params_hash = blake256(&self.genesis_params().rlp_bytes()).to_vec(); if extra_data != &common_params_hash { return Err(Error::Scheme(SchemeError::InvalidCommonParams)) } @@ -279,8 +279,8 @@ impl Scheme { } /// Get common blockchain parameters. - pub fn params(&self, block_number: Option) -> CommonParams { - self.engine.machine().common_params(block_number) + pub fn genesis_params(&self) -> CommonParams { + self.engine.machine().common_params(Some(0)).unwrap() } /// Get the header of the genesis block. @@ -291,7 +291,7 @@ impl Scheme { header.set_number(0); header.set_author(self.author); header.set_transactions_root(self.transactions_root); - header.set_extra_data(blake256(&self.params(Some(0)).rlp_bytes()).to_vec()); + header.set_extra_data(blake256(&self.genesis_params().rlp_bytes()).to_vec()); header.set_state_root(self.state_root()); header.set_score(self.score); header.set_seal({ @@ -363,7 +363,7 @@ mod tests { #[test] fn extra_data_of_genesis_header_is_hash_of_common_params() { let scheme = Scheme::new_test(); - let common_params = scheme.params(Some(0)); + let common_params = scheme.genesis_params(); let hash_of_common_params = H256::blake(&common_params.rlp_bytes()).to_vec(); let genesis_header = scheme.genesis_header(); diff --git a/core/src/transaction.rs b/core/src/transaction.rs index 29c47f25a0..0521e7ac5a 100644 --- a/core/src/transaction.rs +++ b/core/src/transaction.rs @@ -145,13 +145,7 @@ impl UnverifiedTransaction { if byte_size >= params.max_body_size() { return Err(SyntaxError::TransactionIsTooBig) } - self.action.verify_with_params( - params.network_id(), - params.max_asset_scheme_metadata_size(), - params.max_transfer_metadata_size(), - params.max_text_content_size(), - is_order_disabled, - ) + self.action.verify_with_params(params, is_order_disabled) } } diff --git a/core/src/verification/verification.rs b/core/src/verification/verification.rs index d37fe3eef2..ac62bda7ec 100644 --- a/core/src/verification/verification.rs +++ b/core/src/verification/verification.rs @@ -79,7 +79,7 @@ pub fn verify_block_with_params( } for t in body_rlp.iter().map(|rlp| rlp.as_val().expect("verify_block_basic already checked it")) { - engine.verify_transaction_with_params(&t, header)?; + engine.verify_transaction_with_params(&t, common_params)?; } Ok(()) } diff --git a/rpc/src/v1/impls/account.rs b/rpc/src/v1/impls/account.rs index ba6bbcfc2f..cccde5b392 100644 --- a/rpc/src/v1/impls/account.rs +++ b/rpc/src/v1/impls/account.rs @@ -53,7 +53,7 @@ where fn network_id(&self) -> NetworkId { // XXX: What should we do if the network id has been changed - self.client.common_params(None).network_id() + self.client.common_params(None).unwrap().network_id() } } diff --git a/rpc/src/v1/impls/chain.rs b/rpc/src/v1/impls/chain.rs index d4a72e5036..8cadf8e2c8 100644 --- a/rpc/src/v1/impls/chain.rs +++ b/rpc/src/v1/impls/chain.rs @@ -94,13 +94,17 @@ where shard_id: ShardId, block_number: Option, ) -> Result> { - let network_id = self.client.common_params(block_number).network_id(); - let block_id = block_number.map(BlockId::Number).unwrap_or(BlockId::Latest); - Ok(self - .client - .get_asset_scheme(asset_type, shard_id, block_id) - .map_err(errors::transaction_state)? - .map(|asset_scheme| AssetScheme::from_core(asset_scheme, network_id))) + if let Some(common_params) = self.client.common_params(block_number) { + let network_id = common_params.network_id(); + let block_id = block_number.map(BlockId::Number).unwrap_or(BlockId::Latest); + Ok(self + .client + .get_asset_scheme(asset_type, shard_id, block_id) + .map_err(errors::transaction_state)? + .map(|asset_scheme| AssetScheme::from_core(asset_scheme, network_id))) + } else { + Ok(None) + } } fn get_text(&self, transaction_hash: H256, block_number: Option) -> Result> { @@ -109,7 +113,7 @@ where .client .get_text(transaction_hash, block_id) .map_err(errors::transaction_state)? - .map(|text| Text::from_core(text, self.client.common_params(block_number).network_id()))) + .map(|text| Text::from_core(text, self.client.common_params(block_number).unwrap().network_id()))) } fn get_asset( @@ -155,11 +159,10 @@ where fn get_regular_key_owner(&self, public: Public, block_number: Option) -> Result> { let block_id = block_number.map(BlockId::Number).unwrap_or(BlockId::Latest); - let network_id = self.client.common_params(block_number).network_id(); - Ok(self - .client - .regular_key_owner(&public_to_address(&public), block_id.into()) - .and_then(|address| Some(PlatformAddress::new_v1(network_id, address)))) + Ok(self.client.regular_key_owner(&public_to_address(&public), block_id.into()).and_then(|address| { + let network_id = self.client.common_params(block_number).unwrap().network_id(); + Some(PlatformAddress::new_v1(network_id, address)) + })) } fn get_genesis_accounts(&self) -> Result> { @@ -183,20 +186,18 @@ where fn get_shard_owners(&self, shard_id: ShardId, block_number: Option) -> Result>> { let block_id = block_number.map(BlockId::Number).unwrap_or(BlockId::Latest); - let network_id = self.client.common_params(block_number).network_id(); - Ok(self - .client - .shard_owners(shard_id, block_id.into()) - .map(|owners| owners.into_iter().map(|owner| PlatformAddress::new_v1(network_id, owner)).collect())) + Ok(self.client.shard_owners(shard_id, block_id.into()).map(|owners| { + let network_id = self.client.common_params(block_number).unwrap().network_id(); + owners.into_iter().map(|owner| PlatformAddress::new_v1(network_id, owner)).collect() + })) } fn get_shard_users(&self, shard_id: ShardId, block_number: Option) -> Result>> { let block_id = block_number.map(BlockId::Number).unwrap_or(BlockId::Latest); - let network_id = self.client.common_params(block_number).network_id(); - Ok(self - .client - .shard_users(shard_id, block_id.into()) - .map(|users| users.into_iter().map(|user| PlatformAddress::new_v1(network_id, user)).collect())) + Ok(self.client.shard_users(shard_id, block_id.into()).map(|users| { + let network_id = self.client.common_params(block_number).unwrap().network_id(); + users.into_iter().map(|user| PlatformAddress::new_v1(network_id, user)).collect() + })) } fn get_best_block_number(&self) -> Result { @@ -217,10 +218,9 @@ where fn get_block_by_number(&self, block_number: u64) -> Result> { let id = BlockId::Number(block_number); - Ok(self - .client - .block(&id) - .map(|block| Block::from_core(block.decode(), self.client.common_params(Some(block_number)).network_id()))) + Ok(self.client.block(&id).map(|block| { + Block::from_core(block.decode(), self.client.common_params(Some(block_number)).unwrap().network_id()) + })) } fn get_block_by_hash(&self, block_hash: H256) -> Result> { @@ -228,7 +228,7 @@ where Ok(self.client.block(&id).map(|block| { let block = block.decode(); let block_number = block.header.number(); - Block::from_core(block, self.client.common_params(Some(block_number)).network_id()) + Block::from_core(block, self.client.common_params(Some(block_number)).unwrap().network_id()) })) } @@ -237,27 +237,30 @@ where } fn get_min_transaction_fee(&self, action_type: String, block_number: u64) -> Result> { - let common_parameters = self.client.common_params(Some(block_number)); - Ok(match action_type.as_str() { - "mintAsset" => Some(common_parameters.min_asset_mint_cost()), - "transferAsset" => Some(common_parameters.min_asset_transfer_cost()), - "changeAssetScheme" => Some(common_parameters.min_asset_scheme_change_cost()), - "increaseAssetSupply" => Some(common_parameters.min_asset_supply_increase_cost()), - "unwrapCCC" => Some(common_parameters.min_asset_unwrap_ccc_cost()), - "pay" => Some(common_parameters.min_pay_transaction_cost()), - "setRegularKey" => Some(common_parameters.min_set_regular_key_transaction_cost()), - "createShard" => Some(common_parameters.min_create_shard_transaction_cost()), - "setShardOwners" => Some(common_parameters.min_set_shard_owners_transaction_cost()), - "setShardUsers" => Some(common_parameters.min_set_shard_users_transaction_cost()), - "wrapCCC" => Some(common_parameters.min_wrap_ccc_transaction_cost()), - "store" => Some(common_parameters.min_store_transaction_cost()), - "remove" => Some(common_parameters.min_remove_transaction_cost()), - "custom" => Some(common_parameters.min_custom_transaction_cost()), - "composeAsset" => Some(common_parameters.min_asset_compose_cost()), - "decomposeAsset" => Some(common_parameters.min_asset_decompose_cost()), - - _ => None, - }) + if let Some(common_parameters) = self.client.common_params(Some(block_number)) { + Ok(match action_type.as_str() { + "mintAsset" => Some(common_parameters.min_asset_mint_cost()), + "transferAsset" => Some(common_parameters.min_asset_transfer_cost()), + "changeAssetScheme" => Some(common_parameters.min_asset_scheme_change_cost()), + "increaseAssetSupply" => Some(common_parameters.min_asset_supply_increase_cost()), + "unwrapCCC" => Some(common_parameters.min_asset_unwrap_ccc_cost()), + "pay" => Some(common_parameters.min_pay_transaction_cost()), + "setRegularKey" => Some(common_parameters.min_set_regular_key_transaction_cost()), + "createShard" => Some(common_parameters.min_create_shard_transaction_cost()), + "setShardOwners" => Some(common_parameters.min_set_shard_owners_transaction_cost()), + "setShardUsers" => Some(common_parameters.min_set_shard_users_transaction_cost()), + "wrapCCC" => Some(common_parameters.min_wrap_ccc_transaction_cost()), + "store" => Some(common_parameters.min_store_transaction_cost()), + "remove" => Some(common_parameters.min_remove_transaction_cost()), + "custom" => Some(common_parameters.min_custom_transaction_cost()), + "composeAsset" => Some(common_parameters.min_asset_compose_cost()), + "decomposeAsset" => Some(common_parameters.min_asset_decompose_cost()), + + _ => None, + }) + } else { + Ok(None) + } } fn get_mining_reward(&self, block_number: u64) -> Result> { @@ -265,7 +268,7 @@ where } fn get_network_id(&self) -> Result { - Ok(self.client.common_params(None).network_id()) + Ok(self.client.common_params(None).unwrap().network_id()) } fn execute_transaction(&self, tx: UnsignedTransaction, sender: PlatformAddress) -> Result> { diff --git a/rpc/src/v1/impls/devel.rs b/rpc/src/v1/impls/devel.rs index b2c894a1d4..f829b48dcd 100644 --- a/rpc/src/v1/impls/devel.rs +++ b/rpc/src/v1/impls/devel.rs @@ -112,10 +112,11 @@ where } fn test_tps(&self, setting: TPSTestSetting) -> Result { - let mint_fee = self.client.common_params(None).min_asset_mint_cost(); - let transfer_fee = self.client.common_params(None).min_asset_transfer_cost(); - let pay_fee = self.client.common_params(None).min_pay_transaction_cost(); - let network_id = self.client.common_params(None).network_id(); + let common_params = self.client.common_params(None).unwrap(); + let mint_fee = common_params.min_asset_mint_cost(); + let transfer_fee = common_params.min_asset_transfer_cost(); + let pay_fee = common_params.min_pay_transaction_cost(); + let network_id = common_params.network_id(); // NOTE: Assuming solo network let genesis_secret: Private = "ede1d4ccb4ec9a8bbbae9a13db3f4a7b56ea04189be86ac3a6a439d9a0a1addd".into(); diff --git a/rpc/src/v1/impls/engine.rs b/rpc/src/v1/impls/engine.rs index 087e51a2b7..ce22e477f5 100644 --- a/rpc/src/v1/impls/engine.rs +++ b/rpc/src/v1/impls/engine.rs @@ -62,7 +62,7 @@ where Ok(None) } else { // XXX: What should we do if the network id has been changed - let network_id = self.client.common_params(None).network_id(); + let network_id = self.client.common_params(None).unwrap().network_id(); Ok(Some(PlatformAddress::new_v1(network_id, author))) } } diff --git a/types/src/common_params.rs b/types/src/common_params.rs index 0b4be3bdb4..53f78e7a48 100644 --- a/types/src/common_params.rs +++ b/types/src/common_params.rs @@ -256,6 +256,21 @@ impl CommonParams { pub fn default_for_test() -> Self { Self::from(Params::default()) } + + #[cfg(test)] + pub fn set_max_asset_scheme_metadata_size(&mut self, max_asset_scheme_metadata_size: usize) { + self.max_asset_scheme_metadata_size = max_asset_scheme_metadata_size; + } + + #[cfg(test)] + pub fn set_max_transfer_metadata_size(&mut self, max_transfer_metadata_size: usize) { + self.max_transfer_metadata_size = max_transfer_metadata_size; + } + + #[cfg(test)] + pub fn set_max_text_content_size(&mut self, max_text_content_size: usize) { + self.max_text_content_size = max_text_content_size; + } } #[cfg(test)] diff --git a/types/src/transaction/action.rs b/types/src/transaction/action.rs index bd69a88879..fe8b20db6b 100644 --- a/types/src/transaction/action.rs +++ b/types/src/transaction/action.rs @@ -23,7 +23,7 @@ use rlp::{Decodable, DecoderError, Encodable, RlpStream, UntrustedRlp}; use crate::errors::SyntaxError; use crate::transaction::{AssetMintOutput, AssetTransferInput, AssetTransferOutput, OrderOnTransfer, ShardTransaction}; -use crate::ShardId; +use crate::{CommonParams, ShardId}; const PAY: u8 = 0x02; const SET_REGULAR_KEY: u8 = 0x03; @@ -316,15 +316,9 @@ impl Action { Ok(()) } - pub fn verify_with_params( - &self, - system_network_id: NetworkId, - max_asset_scheme_metadata_size: usize, - max_transfer_metadata_size: usize, - max_text_size: usize, - is_order_disabled: bool, - ) -> Result<(), SyntaxError> { + pub fn verify_with_params(&self, common_params: &CommonParams, is_order_disabled: bool) -> Result<(), SyntaxError> { if let Some(network_id) = self.network_id() { + let system_network_id = common_params.network_id(); if network_id != system_network_id { return Err(SyntaxError::InvalidNetworkId(network_id)) } @@ -335,6 +329,7 @@ impl Action { metadata, .. } => { + let max_asset_scheme_metadata_size = common_params.max_asset_scheme_metadata_size(); if metadata.len() > max_asset_scheme_metadata_size { return Err(SyntaxError::MetadataTooBig) } @@ -344,6 +339,7 @@ impl Action { metadata, .. } => { + let max_transfer_metadata_size = common_params.max_transfer_metadata_size(); if metadata.len() > max_transfer_metadata_size { return Err(SyntaxError::MetadataTooBig) } @@ -356,6 +352,7 @@ impl Action { metadata, .. } => { + let max_asset_scheme_metadata_size = common_params.max_asset_scheme_metadata_size(); if metadata.len() > max_asset_scheme_metadata_size { return Err(SyntaxError::MetadataTooBig) } @@ -367,6 +364,7 @@ impl Action { metadata, .. } => { + let max_asset_scheme_metadata_size = common_params.max_asset_scheme_metadata_size(); if metadata.len() > max_asset_scheme_metadata_size { return Err(SyntaxError::MetadataTooBig) } @@ -384,6 +382,7 @@ impl Action { content, .. } => { + let max_text_size = common_params.max_text_content_size(); if content.len() > max_text_size { return Err(SyntaxError::TextContentTooBig) } @@ -1483,7 +1482,11 @@ mod tests { expiration: None, }; assert_eq!(action.verify(), Ok(())); - assert_eq!(action.verify_with_params(NetworkId::default(), 1000, 1000, 1000, false), Ok(())); + let mut common_params = CommonParams::default_for_test(); + common_params.set_max_asset_scheme_metadata_size(1000); + common_params.set_max_transfer_metadata_size(1000); + common_params.set_max_text_content_size(1000); + assert_eq!(action.verify_with_params(&common_params, false), Ok(())); } #[test] @@ -1606,7 +1609,11 @@ mod tests { }; assert_eq!(action.verify(), Ok(())); - assert_eq!(action.verify_with_params(NetworkId::default(), 1000, 1000, 1000, false), Ok(())); + let mut common_params = CommonParams::default_for_test(); + common_params.set_max_asset_scheme_metadata_size(1000); + common_params.set_max_transfer_metadata_size(1000); + common_params.set_max_text_content_size(1000); + assert_eq!(action.verify_with_params(&common_params, false), Ok(())); } #[test] @@ -2102,7 +2109,11 @@ mod tests { expiration: None, }; assert_eq!(action.verify(), Ok(())); - assert_eq!(action.verify_with_params(NetworkId::default(), 1000, 1000, 1000, false), Ok(())); + let mut common_params = CommonParams::default_for_test(); + common_params.set_max_asset_scheme_metadata_size(1000); + common_params.set_max_transfer_metadata_size(1000); + common_params.set_max_text_content_size(1000); + assert_eq!(action.verify_with_params(&common_params, false), Ok(())); } #[test] From 70249cbca1b1fc7c2e83acc918e7fe7746265261 Mon Sep 17 00:00:00 2001 From: Seulgi Kim Date: Mon, 20 May 2019 11:57:33 +0900 Subject: [PATCH 7/9] Read CommonParams from states Currently, CodeChainMachine manages the CommonParams, but it's fragile on reogranization. This patch makes CodeChainMachine has only the genesis parameters and the engine read the parameters from states. --- core/src/block.rs | 29 +++-- core/src/client/client.rs | 11 +- core/src/client/test_client.rs | 24 +++- core/src/codechain_machine.rs | 162 +----------------------- core/src/consensus/blake_pow/mod.rs | 3 +- core/src/consensus/cuckoo/mod.rs | 7 +- core/src/consensus/mod.rs | 2 +- core/src/consensus/null_engine/mod.rs | 4 +- core/src/consensus/simple_poa/mod.rs | 6 +- core/src/consensus/solo/mod.rs | 13 +- core/src/consensus/tendermint/engine.rs | 10 +- core/src/consensus/tendermint/mod.rs | 4 +- core/src/miner/miner.rs | 43 ++++--- core/src/miner/mod.rs | 18 +-- core/src/miner/sealing_queue.rs | 4 +- core/src/scheme/scheme.rs | 2 +- rpc/src/v1/impls/devel.rs | 2 +- rpc/src/v1/impls/mempool.rs | 8 +- rpc/src/v1/impls/miner.rs | 8 +- state/src/item/metadata.rs | 13 +- 20 files changed, 142 insertions(+), 231 deletions(-) diff --git a/core/src/block.rs b/core/src/block.rs index f6ab94bedf..620f008262 100644 --- a/core/src/block.rs +++ b/core/src/block.rs @@ -22,12 +22,13 @@ use cmerkle::skewed_merkle_root; use cstate::{FindActionHandler, StateDB, StateError, StateWithCache, TopLevelState}; use ctypes::errors::HistoryError; use ctypes::util::unexpected::Mismatch; -use ctypes::BlockNumber; +use ctypes::{BlockNumber, CommonParams}; use cvm::ChainTimeInfo; use primitives::{Bytes, H256}; use rlp::{Decodable, DecoderError, Encodable, RlpStream, UntrustedRlp}; use super::invoice::Invoice; +use crate::client::EngineInfo; use crate::consensus::CodeChainEngine; use crate::error::{BlockError, Error}; use crate::header::{Header, Seal}; @@ -215,10 +216,14 @@ impl<'x> OpenBlock<'x> { } /// Turn this into a `ClosedBlock`. - pub fn close(mut self, parent_transactions_root: H256) -> Result { + pub fn close( + mut self, + parent_transactions_root: H256, + parent_common_params: &CommonParams, + ) -> Result { let unclosed_state = self.block.state.clone(); - if let Err(e) = self.engine.on_close_block(&mut self.block) { + if let Err(e) = self.engine.on_close_block(&mut self.block, parent_common_params) { warn!("Encountered error on closing the block: {}", e); return Err(e) } @@ -239,8 +244,12 @@ impl<'x> OpenBlock<'x> { } /// Turn this into a `LockedBlock`. - pub fn close_and_lock(mut self, parent_transactions_root: H256) -> Result { - if let Err(e) = self.engine.on_close_block(&mut self.block) { + pub fn close_and_lock( + mut self, + parent_transactions_root: H256, + parent_common_params: &CommonParams, + ) -> Result { + if let Err(e) = self.engine.on_close_block(&mut self.block, parent_common_params) { warn!("Encountered error on closing the block: {}", e); return Err(e) } @@ -428,7 +437,7 @@ impl IsBlock for SealedBlock { } /// Enact the block given by block header, transactions and uncles -pub fn enact( +pub fn enact( header: &Header, transactions: &[SignedTransaction], engine: &CodeChainEngine, @@ -441,11 +450,14 @@ pub fn enact( b.populate_from(header); b.push_transactions(transactions, client, parent.number(), parent.timestamp())?; - b.close_and_lock(*parent.transactions_root()) + let parent_common_params = client.common_params(Some(header.number() - 1)).unwrap(); + b.close_and_lock(*parent.transactions_root(), &parent_common_params) } #[cfg(test)] mod tests { + use ctypes::CommonParams; + use crate::scheme::Scheme; use crate::tests::helpers::get_temp_state_db; @@ -458,7 +470,8 @@ mod tests { let db = scheme.ensure_genesis_state(get_temp_state_db()).unwrap(); let b = OpenBlock::try_new(&*scheme.engine, db, &genesis_header, Address::default(), vec![]).unwrap(); let parent_transactions_root = *genesis_header.transactions_root(); - let b = b.close_and_lock(parent_transactions_root).unwrap(); + let parent_common_params = CommonParams::default_for_test(); + let b = b.close_and_lock(parent_transactions_root, &parent_common_params).unwrap(); let _ = b.seal(&*scheme.engine, vec![]); } } diff --git a/core/src/client/client.rs b/core/src/client/client.rs index 7399bc2530..ddc091a7ab 100644 --- a/core/src/client/client.rs +++ b/core/src/client/client.rs @@ -485,7 +485,16 @@ impl StateInfo for Client { impl EngineInfo for Client { fn common_params(&self, block_number: Option) -> Option { - self.engine().machine().common_params(block_number) + let block_id = block_number.map(BlockId::Number).unwrap_or(BlockId::Latest); + self.state_info(StateOrBlock::Block(block_id)).map(|state| { + state + .metadata() + .unwrap_or_else(|err| unreachable!("Unexpected failure. Maybe DB was corrupted: {:?}", err)) + .unwrap() + .params() + .map(Clone::clone) + .unwrap_or_else(|| *self.engine().machine().genesis_common_params()) + }) } fn block_reward(&self, block_number: u64) -> u64 { diff --git a/core/src/client/test_client.rs b/core/src/client/test_client.rs index 2a0ace5f41..685e9fa68e 100644 --- a/core/src/client/test_client.rs +++ b/core/src/client/test_client.rs @@ -42,7 +42,7 @@ use cnetwork::NodeId; use cstate::{FindActionHandler, StateDB}; use ctimer::{TimeoutHandler, TimerToken}; use ctypes::transaction::{Action, Transaction}; -use ctypes::BlockNumber; +use ctypes::{BlockNumber, CommonParams}; use cvm::ChainTimeInfo; use journaldb; use kvdb::KeyValueDB; @@ -55,8 +55,8 @@ use crate::block::{ClosedBlock, OpenBlock, SealedBlock}; use crate::blockchain_info::BlockChainInfo; use crate::client::ImportResult; use crate::client::{ - AccountData, BlockChainClient, BlockChainTrait, BlockProducer, BlockStatus, ImportBlock, MiningBlockChainClient, - StateOrBlock, + AccountData, BlockChainClient, BlockChainTrait, BlockProducer, BlockStatus, EngineInfo, ImportBlock, + MiningBlockChainClient, StateOrBlock, }; use crate::db::{COL_STATE, NUM_COLUMNS}; use crate::encoded; @@ -572,3 +572,21 @@ impl super::EngineClient for TestBlockChainClient { Arc::new(db) } } + +impl EngineInfo for TestBlockChainClient { + fn common_params(&self, _block_number: Option) -> Option { + unimplemented!() + } + + fn block_reward(&self, _block_number: u64) -> u64 { + unimplemented!() + } + + fn mining_reward(&self, _block_number: u64) -> Option { + unimplemented!() + } + + fn recommended_confirmation(&self) -> u32 { + unimplemented!() + } +} diff --git a/core/src/codechain_machine.rs b/core/src/codechain_machine.rs index ee309a9767..7c4e76b120 100644 --- a/core/src/codechain_machine.rs +++ b/core/src/codechain_machine.rs @@ -15,13 +15,11 @@ // along with this program. If not, see . // A state machine. -use std::iter::Iterator; - use ckey::Address; use cstate::{StateError, TopState, TopStateView}; use ctypes::errors::{HistoryError, SyntaxError}; use ctypes::transaction::{Action, AssetTransferInput, OrderOnTransfer, Timelock}; -use ctypes::{BlockNumber, CommonParams}; +use ctypes::CommonParams; use crate::block::{ExecutedBlock, IsBlock}; use crate::client::BlockChainTrait; @@ -29,50 +27,22 @@ use crate::error::Error; use crate::header::Header; use crate::transaction::{SignedTransaction, UnverifiedTransaction}; -struct Params { - changed_block: BlockNumber, - params: CommonParams, -} - pub struct CodeChainMachine { - params: Vec, + params: CommonParams, is_order_disabled: bool, } impl CodeChainMachine { pub fn new(params: CommonParams) -> Self { CodeChainMachine { - params: vec![Params { - changed_block: 0, - params, - }], + params, is_order_disabled: is_order_disabled(), } } /// Get the general parameters of the chain. - pub fn common_params(&self, block_number: Option) -> Option { - let params = &self.params; - assert!(!params.is_empty()); - let block_number = if let Some(block_number) = block_number { - block_number - } else { - return Some(params.last().unwrap().params) // the latest block. - }; - - Some( - params - .iter() - .take_while( - |Params { - changed_block, - .. - }| *changed_block <= block_number, - ) - .last() - .unwrap() - .params, - ) + pub fn genesis_common_params(&self) -> &CommonParams { + &self.params } /// Does basic verification of the transaction. @@ -295,125 +265,3 @@ fn is_order_disabled() -> bool { Err(err) => unreachable!("{:?}", err), } } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn common_params_are_not_changed_since_genesis() { - let genesis_params = CommonParams::default_for_test(); - let machine = CodeChainMachine::new(genesis_params); - assert_eq!(Some(genesis_params), machine.common_params(Some(0))); - assert_eq!(Some(genesis_params), machine.common_params(Some(1))); - assert_eq!(Some(genesis_params), machine.common_params(None)); - } - - #[test] - fn common_params_changed_at_1() { - let genesis_params = CommonParams::default_for_test(); - let params_at_1 = { - let mut params = genesis_params; - params.set_min_store_transaction_cost(genesis_params.min_store_transaction_cost() + 10); - params - }; - let machine = CodeChainMachine { - params: vec![ - Params { - changed_block: 0, - params: genesis_params, - }, - Params { - changed_block: 1, - params: params_at_1, - }, - ], - is_order_disabled: false, - }; - assert_eq!(Some(genesis_params), machine.common_params(Some(0))); - assert_eq!(Some(params_at_1), machine.common_params(Some(1))); - assert_eq!(Some(params_at_1), machine.common_params(None)); - } - - #[test] - fn common_params_changed_at_2() { - let genesis_params = CommonParams::default_for_test(); - let params_at_2 = { - let mut params = genesis_params; - params.set_min_store_transaction_cost(genesis_params.min_store_transaction_cost() + 10); - params - }; - let machine = CodeChainMachine { - params: vec![ - Params { - changed_block: 0, - params: genesis_params, - }, - Params { - changed_block: 2, - params: params_at_2, - }, - ], - is_order_disabled: false, - }; - assert_eq!(Some(genesis_params), machine.common_params(Some(0))); - assert_eq!(Some(genesis_params), machine.common_params(Some(1))); - assert_eq!(Some(params_at_2), machine.common_params(Some(2))); - assert_eq!(Some(params_at_2), machine.common_params(None)); - } - - - #[test] - fn common_params_changed_several_times() { - let genesis_params = CommonParams::default_for_test(); - let params_at_10 = { - let mut params = genesis_params; - params.set_min_store_transaction_cost(genesis_params.min_store_transaction_cost() + 10); - params - }; - let params_at_20 = { - let mut params = params_at_10; - params.set_min_store_transaction_cost(params_at_10.min_store_transaction_cost() + 10); - params - }; - let params_at_30 = { - let mut params = params_at_20; - params.set_min_store_transaction_cost(params_at_20.min_store_transaction_cost() + 10); - params - }; - let machine = CodeChainMachine { - params: vec![ - Params { - changed_block: 0, - params: genesis_params, - }, - Params { - changed_block: 10, - params: params_at_10, - }, - Params { - changed_block: 20, - params: params_at_20, - }, - Params { - changed_block: 30, - params: params_at_30, - }, - ], - is_order_disabled: false, - }; - for i in 0..10 { - assert_eq!(Some(genesis_params), machine.common_params(Some(i)), "unexpected params at block {}", i); - } - for i in 10..20 { - assert_eq!(Some(params_at_10), machine.common_params(Some(i)), "unexpected params at block {}", i); - } - for i in 20..30 { - assert_eq!(Some(params_at_20), machine.common_params(Some(i)), "unexpected params at block {}", i); - } - for i in 30..40 { - assert_eq!(Some(params_at_30), machine.common_params(Some(i)), "unexpected params at block {}", i); - } - assert_eq!(Some(params_at_30), machine.common_params(None)); - } -} diff --git a/core/src/consensus/blake_pow/mod.rs b/core/src/consensus/blake_pow/mod.rs index 9daa1395cb..bd4bac8030 100644 --- a/core/src/consensus/blake_pow/mod.rs +++ b/core/src/consensus/blake_pow/mod.rs @@ -20,6 +20,7 @@ use std::cmp::{max, min}; use ccrypto::blake256; use ctypes::util::unexpected::{Mismatch, OutOfBounds}; +use ctypes::CommonParams; use primitives::U256; use rlp::UntrustedRlp; @@ -159,7 +160,7 @@ impl ConsensusEngine for BlakePoW { header.set_score(score); } - fn on_close_block(&self, block: &mut ExecutedBlock) -> Result<(), Error> { + fn on_close_block(&self, block: &mut ExecutedBlock, _parent_common_params: &CommonParams) -> Result<(), Error> { let author = *block.header().author(); let total_reward = self.block_reward(block.header().number()) + self.block_fee(Box::new(block.transactions().to_owned().into_iter().map(Into::into))); diff --git a/core/src/consensus/cuckoo/mod.rs b/core/src/consensus/cuckoo/mod.rs index 2738e85cfe..7cd47faa17 100644 --- a/core/src/consensus/cuckoo/mod.rs +++ b/core/src/consensus/cuckoo/mod.rs @@ -20,6 +20,7 @@ use std::cmp::{max, min}; use ccrypto::blake256; use ctypes::util::unexpected::{Mismatch, OutOfBounds}; +use ctypes::CommonParams; use cuckoo::Cuckoo as CuckooVerifier; use primitives::U256; use rlp::UntrustedRlp; @@ -169,7 +170,7 @@ impl ConsensusEngine for Cuckoo { header.set_score(score); } - fn on_close_block(&self, block: &mut ExecutedBlock) -> Result<(), Error> { + fn on_close_block(&self, block: &mut ExecutedBlock, _parent_common_params: &CommonParams) -> Result<(), Error> { let author = *block.header().author(); let total_reward = self.block_reward(block.header().number()) + self.block_fee(Box::new(block.transactions().to_owned().into_iter().map(Into::into))); @@ -191,6 +192,8 @@ impl ConsensusEngine for Cuckoo { #[cfg(test)] mod tests { + use ctypes::CommonParams; + use crate::block::{IsBlock, OpenBlock}; use crate::scheme::Scheme; use crate::tests::helpers::get_temp_state_db; @@ -254,7 +257,7 @@ mod tests { let block = OpenBlock::try_new(engine, db, &header, Default::default(), vec![]).unwrap(); let mut executed_block = block.block().clone(); - assert!(engine.on_close_block(&mut executed_block).is_ok()); + assert!(engine.on_close_block(&mut executed_block, &CommonParams::default_for_test()).is_ok()); assert_eq!(0xd, engine.machine().balance(&executed_block, header.author()).unwrap()); } diff --git a/core/src/consensus/mod.rs b/core/src/consensus/mod.rs index 233c664f89..222f3d48bb 100644 --- a/core/src/consensus/mod.rs +++ b/core/src/consensus/mod.rs @@ -213,7 +213,7 @@ pub trait ConsensusEngine: Sync + Send { fn stop(&self) {} /// Block transformation functions, after the transactions. - fn on_close_block(&self, _block: &mut ExecutedBlock) -> Result<(), Error> { + fn on_close_block(&self, _block: &mut ExecutedBlock, _parent_common_params: &CommonParams) -> Result<(), Error> { Ok(()) } diff --git a/core/src/consensus/null_engine/mod.rs b/core/src/consensus/null_engine/mod.rs index 6819a12824..c6ad5c2708 100644 --- a/core/src/consensus/null_engine/mod.rs +++ b/core/src/consensus/null_engine/mod.rs @@ -16,6 +16,8 @@ mod params; +use ctypes::CommonParams; + use self::params::NullEngineParams; use super::ConsensusEngine; use crate::block::ExecutedBlock; @@ -52,7 +54,7 @@ impl ConsensusEngine for NullEngine { EngineType::Solo } - fn on_close_block(&self, block: &mut ExecutedBlock) -> Result<(), Error> { + fn on_close_block(&self, block: &mut ExecutedBlock, _parent_common_params: &CommonParams) -> Result<(), Error> { let (author, total_reward) = { let header = block.header(); let author = *header.author(); diff --git a/core/src/consensus/simple_poa/mod.rs b/core/src/consensus/simple_poa/mod.rs index adf6d61469..50a9705f86 100644 --- a/core/src/consensus/simple_poa/mod.rs +++ b/core/src/consensus/simple_poa/mod.rs @@ -19,6 +19,7 @@ mod params; use std::sync::{Arc, Weak}; use ckey::{public_to_address, recover, Address, Signature}; +use ctypes::CommonParams; use parking_lot::RwLock; use self::params::SimplePoAParams; @@ -113,7 +114,7 @@ impl ConsensusEngine for SimplePoA { verify_external(header, &*self.validators) } - fn on_close_block(&self, block: &mut ExecutedBlock) -> Result<(), Error> { + fn on_close_block(&self, block: &mut ExecutedBlock, _parent_common_params: &CommonParams) -> Result<(), Error> { let author = *block.header().author(); let total_reward = self.block_reward(block.header().number()) + self.block_fee(Box::new(block.transactions().to_owned().into_iter().map(Into::into))); @@ -170,7 +171,8 @@ mod tests { let genesis_header = scheme.genesis_header(); let b = OpenBlock::try_new(engine, db, &genesis_header, Default::default(), vec![]).unwrap(); let parent_transactions_root = *genesis_header.transactions_root(); - let b = b.close_and_lock(parent_transactions_root).unwrap(); + let parent_common_params = CommonParams::default_for_test(); + let b = b.close_and_lock(parent_transactions_root, &parent_common_params).unwrap(); if let Some(seal) = engine.generate_seal(b.block(), &genesis_header).seal_fields() { assert!(b.try_seal(engine, seal).is_ok()); } diff --git a/core/src/consensus/solo/mod.rs b/core/src/consensus/solo/mod.rs index 6dd2099c35..ff070ee329 100644 --- a/core/src/consensus/solo/mod.rs +++ b/core/src/consensus/solo/mod.rs @@ -19,6 +19,7 @@ mod params; use std::sync::Arc; use cstate::{ActionHandler, HitHandler}; +use ctypes::CommonParams; use self::params::SoloParams; use super::stake; @@ -78,18 +79,14 @@ impl ConsensusEngine for Solo { Seal::Solo } - fn on_close_block(&self, block: &mut ExecutedBlock) -> Result<(), Error> { + fn on_close_block(&self, block: &mut ExecutedBlock, parent_common_params: &CommonParams) -> Result<(), Error> { let author = *block.header().author(); let (total_reward, min_fee) = { let transactions = block.transactions(); let block_reward = self.block_reward(block.header().number()); let total_fee: u64 = transactions.iter().map(|tx| tx.fee).sum(); - let block_number = block.header().number(); - assert_ne!(0, block_number); - let parent_block_number = block.header().number() - 1; - let common_params = self.machine().common_params(Some(parent_block_number)).unwrap(); let min_fee: u64 = - transactions.iter().map(|tx| CodeChainMachine::min_cost(&common_params, &tx.action)).sum(); + transactions.iter().map(|tx| CodeChainMachine::min_cost(&parent_common_params, &tx.action)).sum(); (block_reward + total_fee, min_fee) }; @@ -119,6 +116,7 @@ impl ConsensusEngine for Solo { #[cfg(test)] mod tests { + use ctypes::CommonParams; use primitives::H520; use crate::block::{IsBlock, OpenBlock}; @@ -134,7 +132,8 @@ mod tests { let genesis_header = scheme.genesis_header(); let b = OpenBlock::try_new(engine, db, &genesis_header, Default::default(), vec![]).unwrap(); let parent_transactions_root = *genesis_header.transactions_root(); - let b = b.close_and_lock(parent_transactions_root).unwrap(); + let parent_common_params = CommonParams::default_for_test(); + let b = b.close_and_lock(parent_transactions_root, &parent_common_params).unwrap(); if let Some(seal) = engine.generate_seal(b.block(), &genesis_header).seal_fields() { assert!(b.try_seal(engine, seal).is_ok()); } diff --git a/core/src/consensus/tendermint/engine.rs b/core/src/consensus/tendermint/engine.rs index edf68442e4..702cc694f1 100644 --- a/core/src/consensus/tendermint/engine.rs +++ b/core/src/consensus/tendermint/engine.rs @@ -22,6 +22,7 @@ use ckey::Address; use cnetwork::NetworkService; use crossbeam_channel as crossbeam; use cstate::ActionHandler; +use ctypes::CommonParams; use primitives::H256; use super::super::stake; @@ -128,16 +129,13 @@ impl ConsensusEngine for Tendermint { fn stop(&self) {} - fn on_close_block(&self, block: &mut ExecutedBlock) -> Result<(), Error> { + fn on_close_block(&self, block: &mut ExecutedBlock, parent_common_params: &CommonParams) -> Result<(), Error> { let author = *block.header().author(); let (total_fee, min_fee) = { let transactions = block.transactions(); let total_fee: u64 = transactions.iter().map(|tx| tx.fee).sum(); - let block_number = block.header().number(); - assert_ne!(0, block_number); - let parent_block_number = block.header().number() - 1; - let common_params = self.machine().common_params(Some(parent_block_number)).unwrap(); - let min_fee = transactions.iter().map(|tx| CodeChainMachine::min_cost(&common_params, &tx.action)).sum(); + let min_fee = + transactions.iter().map(|tx| CodeChainMachine::min_cost(&parent_common_params, &tx.action)).sum(); (total_fee, min_fee) }; assert!(total_fee >= min_fee, "{} >= {}", total_fee, min_fee); diff --git a/core/src/consensus/tendermint/mod.rs b/core/src/consensus/tendermint/mod.rs index 7c4ba26587..65b33b88b2 100644 --- a/core/src/consensus/tendermint/mod.rs +++ b/core/src/consensus/tendermint/mod.rs @@ -118,6 +118,7 @@ const SEAL_FIELDS: usize = 4; mod tests { use ccrypto::blake256; use ckey::Address; + use ctypes::CommonParams; use primitives::Bytes; use super::message::{message_info_rlp, VoteStep}; @@ -151,7 +152,8 @@ mod tests { let db = scheme.ensure_genesis_state(db).unwrap(); let genesis_header = scheme.genesis_header(); let b = OpenBlock::try_new(scheme.engine.as_ref(), db, &genesis_header, proposer, vec![]).unwrap(); - let b = b.close(*genesis_header.transactions_root()).unwrap(); + let common_params = CommonParams::default_for_test(); + let b = b.close(*genesis_header.transactions_root(), &common_params).unwrap(); if let Some(seal) = scheme.engine.generate_seal(b.block(), &genesis_header).seal_fields() { (b, seal) } else { diff --git a/core/src/miner/miner.rs b/core/src/miner/miner.rs index 9ce63d5fc5..3ee8230944 100644 --- a/core/src/miner/miner.rs +++ b/core/src/miner/miner.rs @@ -38,7 +38,9 @@ use super::work_notify::{NotifyWork, WorkPoster}; use super::{MinerService, MinerStatus, TransactionImportResult}; use crate::account_provider::{AccountProvider, Error as AccountProviderError}; use crate::block::{Block, ClosedBlock, IsBlock}; -use crate::client::{AccountData, BlockChainTrait, BlockProducer, Client, ImportBlock, MiningBlockChainClient}; +use crate::client::{ + AccountData, BlockChainTrait, BlockProducer, Client, EngineInfo, ImportBlock, MiningBlockChainClient, +}; use crate::consensus::{CodeChainEngine, EngineType}; use crate::error::Error; use crate::header::Header; @@ -247,7 +249,7 @@ impl Miner { } } - fn add_transactions_to_pool( + fn add_transactions_to_pool( &self, client: &C, transactions: Vec, @@ -277,7 +279,7 @@ impl Miner { .verify_basic() .map_err(From::from) .and_then(|_| { - let common_params = self.engine.machine().common_params(Some(best_header.number())).unwrap(); + let common_params = client.common_params(Some(best_header.number())).unwrap(); self.engine.verify_transaction_with_params(&tx, &common_params) }) .and_then(|_| self.engine.verify_transaction_seal(tx, &fake_header)) @@ -442,7 +444,9 @@ impl Miner { } /// Prepares new block for sealing including top transactions from queue. - fn prepare_block( + fn prepare_block< + C: AccountData + BlockChainTrait + BlockProducer + ChainTimeInfo + EngineInfo + FindActionHandler, + >( &self, parent_block_id: BlockId, chain: &C, @@ -456,7 +460,7 @@ impl Miner { let params = self.params.read().clone(); let open_block = chain.prepare_open_block(parent_block_id, params.author, params.extra_data); let block_number = open_block.block().header().number(); - let max_body_size = self.engine.machine().common_params(Some(block_number)).unwrap().max_body_size(); + let max_body_size = chain.common_params(Some(block_number - 1)).unwrap().max_body_size(); const DEFAULT_RANGE: Range = 0..::std::u64::MAX; let transactions = mem_pool .top_transactions(max_body_size, Some(open_block.header().timestamp()), DEFAULT_RANGE) @@ -517,13 +521,14 @@ impl Miner { } cdebug!(MINER, "Pushed {}/{} transactions", tx_count, tx_total); - let transactions_root = { + let (transactions_root, parent_number) = { let parent_hash = open_block.header().parent_hash(); let parent_header = chain.block_header(&BlockId::Hash(*parent_hash)).expect("Parent header MUST exist"); let parent_view = parent_header.view(); - parent_view.transactions_root() + (parent_view.transactions_root(), parent_view.number()) }; - let block = open_block.close(transactions_root)?; + let parent_common_params = chain.common_params(Some(parent_number)).unwrap(); + let block = open_block.close(transactions_root, &parent_common_params)?; let fetch_seq = |p: &Public| { let address = public_to_address(p); @@ -696,7 +701,7 @@ impl MinerService for Miner { _enacted: &[H256], retracted: &[H256], ) where - C: AccountData + BlockChainTrait + BlockProducer + ImportBlock, { + C: AccountData + BlockChainTrait + BlockProducer + EngineInfo + ImportBlock, { ctrace!(MINER, "chain_new_blocks"); // Then import all transactions... @@ -741,7 +746,9 @@ impl MinerService for Miner { self.engine.engine_type() } - fn prepare_work_sealing( + fn prepare_work_sealing< + C: AccountData + BlockChainTrait + BlockProducer + ChainTimeInfo + EngineInfo + FindActionHandler, + >( &self, client: &C, ) -> bool { @@ -789,7 +796,8 @@ impl MinerService for Miner { fn update_sealing(&self, chain: &C, parent_block: BlockId, allow_empty_block: bool) where - C: AccountData + BlockChainTrait + BlockProducer + ImportBlock + ChainTimeInfo + FindActionHandler, { + C: AccountData + BlockChainTrait + BlockProducer + EngineInfo + ImportBlock + ChainTimeInfo + FindActionHandler, + { ctrace!(MINER, "update_sealing: preparing a block"); let parent_block_number = chain.block_header(&parent_block).expect("Parent is always exist").number(); @@ -867,7 +875,7 @@ impl MinerService for Miner { fn map_sealing_work(&self, client: &C, f: F) -> Option where - C: AccountData + BlockChainTrait + BlockProducer + ChainTimeInfo + FindActionHandler, + C: AccountData + BlockChainTrait + BlockProducer + ChainTimeInfo + EngineInfo + FindActionHandler, F: FnOnce(&ClosedBlock) -> T, { ctrace!(MINER, "map_sealing_work: entering"); self.prepare_work_sealing(client); @@ -878,7 +886,7 @@ impl MinerService for Miner { ret.map(f) } - fn import_external_transactions( + fn import_external_transactions( &self, client: &C, transactions: Vec, @@ -903,7 +911,7 @@ impl MinerService for Miner { results } - fn import_own_transaction( + fn import_own_transaction( &self, chain: &C, tx: SignedTransaction, @@ -948,7 +956,7 @@ impl MinerService for Miner { imported } - fn import_incomplete_transaction( + fn import_incomplete_transaction( &self, client: &C, account_provider: &AccountProvider, @@ -994,7 +1002,8 @@ impl MinerService for Miner { } fn ready_transactions(&self, range: Range) -> PendingSignedTransactions { - let max_body_size = self.engine.machine().common_params(None).unwrap().max_body_size(); + // FIXME: Update the body size when the common params are updated + let max_body_size = self.engine.machine().genesis_common_params().max_body_size(); self.mem_pool.read().top_transactions(max_body_size, None, range) } @@ -1007,7 +1016,7 @@ impl MinerService for Miner { self.mem_pool.read().future_transactions() } - fn start_sealing(&self, client: &C) { + fn start_sealing(&self, client: &C) { cdebug!(MINER, "Start sealing"); self.sealing_enabled.store(true, Ordering::Relaxed); // ------------------------------------------------------------------ diff --git a/core/src/miner/mod.rs b/core/src/miner/mod.rs index 882b148543..2550f7cf1b 100644 --- a/core/src/miner/mod.rs +++ b/core/src/miner/mod.rs @@ -35,7 +35,7 @@ pub use self::miner::{AuthoringParams, Miner, MinerOptions}; pub use self::stratum::{Config as StratumConfig, Error as StratumError, Stratum}; use crate::account_provider::{AccountProvider, Error as AccountProviderError}; use crate::block::ClosedBlock; -use crate::client::{AccountData, BlockChainTrait, BlockProducer, ImportBlock, MiningBlockChainClient}; +use crate::client::{AccountData, BlockChainTrait, BlockProducer, EngineInfo, ImportBlock, MiningBlockChainClient}; use crate::consensus::EngineType; use crate::error::Error; use crate::transaction::{PendingSignedTransactions, SignedTransaction, UnverifiedTransaction}; @@ -73,7 +73,7 @@ pub trait MinerService: Send + Sync { /// Called when blocks are imported to chain, updates transactions queue. fn chain_new_blocks(&self, chain: &C, imported: &[H256], invalid: &[H256], enacted: &[H256], retracted: &[H256]) where - C: AccountData + BlockChainTrait + BlockProducer + ImportBlock; + C: AccountData + BlockChainTrait + BlockProducer + EngineInfo + ImportBlock; /// PoW chain - can produce work package fn can_produce_work_package(&self) -> bool; @@ -84,12 +84,12 @@ pub trait MinerService: Send + Sync { /// Returns true if we had to prepare new pending block. fn prepare_work_sealing(&self, &C) -> bool where - C: AccountData + BlockChainTrait + BlockProducer + ChainTimeInfo + FindActionHandler; + C: AccountData + BlockChainTrait + BlockProducer + ChainTimeInfo + EngineInfo + FindActionHandler; /// New chain head event. Restart mining operation. fn update_sealing(&self, chain: &C, parent_block: BlockId, allow_empty_block: bool) where - C: AccountData + BlockChainTrait + BlockProducer + ImportBlock + ChainTimeInfo + FindActionHandler; + C: AccountData + BlockChainTrait + BlockProducer + ImportBlock + ChainTimeInfo + EngineInfo + FindActionHandler; /// Submit `seal` as a valid solution for the header of `pow_hash`. /// Will check the seal, but not actually insert the block into the chain. @@ -98,26 +98,26 @@ pub trait MinerService: Send + Sync { /// Get the sealing work package and if `Some`, apply some transform. fn map_sealing_work(&self, client: &C, f: F) -> Option where - C: AccountData + BlockChainTrait + BlockProducer + ChainTimeInfo + FindActionHandler, + C: AccountData + BlockChainTrait + BlockProducer + ChainTimeInfo + EngineInfo + FindActionHandler, F: FnOnce(&ClosedBlock) -> T, Self: Sized; /// Imports transactions to mem pool. - fn import_external_transactions( + fn import_external_transactions( &self, client: &C, transactions: Vec, ) -> Vec>; /// Imports own (node owner) transaction to mem pool. - fn import_own_transaction( + fn import_own_transaction( &self, chain: &C, tx: SignedTransaction, ) -> Result; /// Imports incomplete (node owner) transaction to mem pool. - fn import_incomplete_transaction( + fn import_incomplete_transaction( &self, chain: &C, account_provider: &AccountProvider, @@ -137,7 +137,7 @@ pub trait MinerService: Send + Sync { fn future_transactions(&self) -> Vec; /// Start sealing. - fn start_sealing(&self, client: &C); + fn start_sealing(&self, client: &C); /// Stop sealing. fn stop_sealing(&self); diff --git a/core/src/miner/sealing_queue.rs b/core/src/miner/sealing_queue.rs index d488c64cc4..53a1f3968a 100644 --- a/core/src/miner/sealing_queue.rs +++ b/core/src/miner/sealing_queue.rs @@ -73,6 +73,7 @@ impl SealingQueue { #[cfg(test)] mod tests { use ckey::Address; + use ctypes::CommonParams; use super::SealingQueue; use crate::block::{ClosedBlock, OpenBlock}; @@ -87,7 +88,8 @@ mod tests { let db = scheme.ensure_genesis_state(get_temp_state_db()).unwrap(); let b = OpenBlock::try_new(&*scheme.engine, db, &genesis_header, address, vec![]).unwrap(); let parent_transactions_root = *genesis_header.transactions_root(); - b.close(parent_transactions_root).unwrap() + let common_params = CommonParams::default_for_test(); + b.close(parent_transactions_root, &common_params).unwrap() } #[test] diff --git a/core/src/scheme/scheme.rs b/core/src/scheme/scheme.rs index d979ae2267..b7bb618c28 100644 --- a/core/src/scheme/scheme.rs +++ b/core/src/scheme/scheme.rs @@ -280,7 +280,7 @@ impl Scheme { /// Get common blockchain parameters. pub fn genesis_params(&self) -> CommonParams { - self.engine.machine().common_params(Some(0)).unwrap() + *self.engine.machine().genesis_common_params() } /// Get the header of the genesis block. diff --git a/rpc/src/v1/impls/devel.rs b/rpc/src/v1/impls/devel.rs index f829b48dcd..4246564cc0 100644 --- a/rpc/src/v1/impls/devel.rs +++ b/rpc/src/v1/impls/devel.rs @@ -223,7 +223,7 @@ where fn send_tx(tx: Transaction, client: &C, key_pair: &KeyPair, miner: &M) -> Result where - C: MiningBlockChainClient, + C: MiningBlockChainClient + EngineInfo, M: MinerService, { let signed = SignedTransaction::new_with_sign(tx, key_pair.private()); let hash = signed.hash(); diff --git a/rpc/src/v1/impls/mempool.rs b/rpc/src/v1/impls/mempool.rs index df80845cd4..8f8e7d81c0 100644 --- a/rpc/src/v1/impls/mempool.rs +++ b/rpc/src/v1/impls/mempool.rs @@ -16,7 +16,7 @@ use std::sync::Arc; -use ccore::{MinerService, MiningBlockChainClient, SignedTransaction}; +use ccore::{EngineInfo, MinerService, MiningBlockChainClient, SignedTransaction}; use cjson::bytes::Bytes; use primitives::H256; use rlp::UntrustedRlp; @@ -29,7 +29,7 @@ use super::super::types::PendingTransactions; pub struct MempoolClient where - C: MiningBlockChainClient, + C: MiningBlockChainClient + EngineInfo, M: MinerService, { client: Arc, miner: Arc, @@ -37,7 +37,7 @@ where impl MempoolClient where - C: MiningBlockChainClient, + C: MiningBlockChainClient + EngineInfo, M: MinerService, { pub fn new(client: Arc, miner: Arc) -> Self { @@ -50,7 +50,7 @@ where impl Mempool for MempoolClient where - C: MiningBlockChainClient + 'static, + C: MiningBlockChainClient + EngineInfo + 'static, M: MinerService + 'static, { fn send_signed_transaction(&self, raw: Bytes) -> Result { diff --git a/rpc/src/v1/impls/miner.rs b/rpc/src/v1/impls/miner.rs index 2ece1757f0..cf6ef1cf42 100644 --- a/rpc/src/v1/impls/miner.rs +++ b/rpc/src/v1/impls/miner.rs @@ -17,7 +17,7 @@ use std::sync::Arc; use ccore::block::IsBlock; -use ccore::{EngineClient, MinerService, MiningBlockChainClient}; +use ccore::{EngineClient, EngineInfo, MinerService, MiningBlockChainClient}; use cjson::bytes::Bytes; use jsonrpc_core::Result; use primitives::H256; @@ -28,7 +28,7 @@ use super::super::types::Work; pub struct MinerClient where - C: MiningBlockChainClient + EngineClient, + C: MiningBlockChainClient + EngineClient + EngineInfo, M: MinerService, { client: Arc, miner: Arc, @@ -36,7 +36,7 @@ where impl MinerClient where - C: MiningBlockChainClient + EngineClient, + C: MiningBlockChainClient + EngineClient + EngineInfo, M: MinerService, { pub fn new(client: Arc, miner: Arc) -> Self { @@ -49,7 +49,7 @@ where impl Miner for MinerClient where - C: MiningBlockChainClient + EngineClient + 'static, + C: MiningBlockChainClient + EngineClient + EngineInfo + 'static, M: MinerService + 'static, { fn get_work(&self) -> Result { diff --git a/state/src/item/metadata.rs b/state/src/item/metadata.rs index 9bcadc9814..1a54ce0b2d 100644 --- a/state/src/item/metadata.rs +++ b/state/src/item/metadata.rs @@ -14,10 +14,10 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . -use primitives::{Bytes, H256}; +use primitives::H256; use rlp::{Decodable, DecoderError, Encodable, RlpStream, UntrustedRlp}; -use ctypes::ShardId; +use ctypes::{CommonParams, ShardId}; use crate::CacheableItem; @@ -27,7 +27,7 @@ pub struct Metadata { number_of_initial_shards: ShardId, hashes: Vec, seq: usize, - params: Option, + params: Option, } impl Metadata { @@ -69,6 +69,10 @@ impl Metadata { index }) } + + pub fn params(&self) -> Option<&CommonParams> { + self.params.as_ref() + } } impl Default for Metadata { @@ -146,6 +150,7 @@ impl MetadataAddress { #[cfg(test)] mod tests { + use ctypes::CommonParams; use rlp::rlp_encode_and_decode_test; use super::*; @@ -191,7 +196,7 @@ mod tests { number_of_initial_shards: 1, hashes: vec![], seq: 3, - params: Some(vec![0, 1, 2, 3]), + params: Some(CommonParams::default_for_test()), }; rlp_encode_and_decode_test!(metadata); } From 7afbe00f0664e83ae34b585d116215502e56fb2e Mon Sep 17 00:00:00 2001 From: Seulgi Kim Date: Mon, 20 May 2019 13:46:49 +0900 Subject: [PATCH 8/9] Make common_params receive BlockId instead of BlockNumber --- codechain/run_node.rs | 8 ++++---- core/src/block.rs | 2 +- core/src/client/client.rs | 7 +++---- core/src/client/importer.rs | 2 +- core/src/client/mod.rs | 2 +- core/src/client/test_client.rs | 2 +- core/src/miner/miner.rs | 21 +++++++++++++-------- rpc/src/v1/impls/account.rs | 4 ++-- rpc/src/v1/impls/chain.rs | 26 ++++++++++++++------------ rpc/src/v1/impls/devel.rs | 2 +- rpc/src/v1/impls/engine.rs | 2 +- 11 files changed, 42 insertions(+), 36 deletions(-) diff --git a/codechain/run_node.rs b/codechain/run_node.rs index 5122711da5..cd7d40c687 100644 --- a/codechain/run_node.rs +++ b/codechain/run_node.rs @@ -20,8 +20,8 @@ use std::sync::{Arc, Weak}; use std::time::{SystemTime, UNIX_EPOCH}; use ccore::{ - AccountProvider, AccountProviderError, ChainNotify, Client, ClientConfig, ClientService, EngineInfo, EngineType, - Miner, MinerService, Scheme, Stratum, StratumConfig, StratumError, NUM_COLUMNS, + AccountProvider, AccountProviderError, BlockId, ChainNotify, Client, ClientConfig, ClientService, EngineInfo, + EngineType, Miner, MinerService, Scheme, Stratum, StratumConfig, StratumError, NUM_COLUMNS, }; use cdiscovery::{Config, Discovery}; use ckey::{Address, NetworkId, PlatformAddress}; @@ -264,7 +264,7 @@ pub fn run_node(matches: &ArgMatches) -> Result<(), String> { let network_config = config.network_config()?; // XXX: What should we do if the network id has been changed. let c = client.client(); - let network_id = c.common_params(None).unwrap().network_id(); + let network_id = c.common_params(BlockId::Latest).unwrap().network_id(); let routing_table = RoutingTable::new(); let service = network_start(network_id, timer_loop, &network_config, Arc::clone(&routing_table))?; @@ -337,7 +337,7 @@ pub fn run_node(matches: &ArgMatches) -> Result<(), String> { if !config.snapshot.disable.unwrap() { // FIXME: Let's make it load snapshot period dynamically to support changing the period. let client = client.client(); - let snapshot_period = client.common_params(None).unwrap().snapshot_period(); + let snapshot_period = client.common_params(BlockId::Latest).unwrap().snapshot_period(); let service = SnapshotService::new(Arc::clone(&client), config.snapshot.path.unwrap(), snapshot_period); client.add_notify(Arc::downgrade(&service) as Weak); Some(service) diff --git a/core/src/block.rs b/core/src/block.rs index 620f008262..d8eb377176 100644 --- a/core/src/block.rs +++ b/core/src/block.rs @@ -450,7 +450,7 @@ pub fn enact( b.populate_from(header); b.push_transactions(transactions, client, parent.number(), parent.timestamp())?; - let parent_common_params = client.common_params(Some(header.number() - 1)).unwrap(); + let parent_common_params = client.common_params((*header.parent_hash()).into()).unwrap(); b.close_and_lock(*parent.transactions_root(), &parent_common_params) } diff --git a/core/src/client/client.rs b/core/src/client/client.rs index ddc091a7ab..bf60cb3b2b 100644 --- a/core/src/client/client.rs +++ b/core/src/client/client.rs @@ -484,9 +484,8 @@ impl StateInfo for Client { } impl EngineInfo for Client { - fn common_params(&self, block_number: Option) -> Option { - let block_id = block_number.map(BlockId::Number).unwrap_or(BlockId::Latest); - self.state_info(StateOrBlock::Block(block_id)).map(|state| { + fn common_params(&self, block_id: BlockId) -> Option { + self.state_info(block_id.into()).map(|state| { state .metadata() .unwrap_or_else(|err| unreachable!("Unexpected failure. Maybe DB was corrupted: {:?}", err)) @@ -565,7 +564,7 @@ impl BlockChainTrait for Client { fn genesis_accounts(&self) -> Vec { // XXX: What should we do if the network id has been changed - let network_id = self.common_params(None).unwrap().network_id(); + let network_id = self.common_params(BlockId::Latest).unwrap().network_id(); self.genesis_accounts.iter().map(|addr| PlatformAddress::new_v1(network_id, *addr)).collect() } diff --git a/core/src/client/importer.rs b/core/src/client/importer.rs index 9db03da603..3dc1b950d1 100644 --- a/core/src/client/importer.rs +++ b/core/src/client/importer.rs @@ -243,7 +243,7 @@ impl Importer { ); })?; - let common_params = client.common_params(Some(parent.number())).unwrap(); + let common_params = client.common_params(parent.hash().into()).unwrap(); // Verify Block Family self.verifier diff --git a/core/src/client/mod.rs b/core/src/client/mod.rs index eff7eeeb09..655d6cb380 100644 --- a/core/src/client/mod.rs +++ b/core/src/client/mod.rs @@ -86,7 +86,7 @@ pub trait BlockChainTrait { } pub trait EngineInfo: Send + Sync { - fn common_params(&self, block_number: Option) -> Option; + fn common_params(&self, block_id: BlockId) -> Option; fn block_reward(&self, block_number: u64) -> u64; fn mining_reward(&self, block_number: u64) -> Option; fn recommended_confirmation(&self) -> u32; diff --git a/core/src/client/test_client.rs b/core/src/client/test_client.rs index 685e9fa68e..464602c21a 100644 --- a/core/src/client/test_client.rs +++ b/core/src/client/test_client.rs @@ -574,7 +574,7 @@ impl super::EngineClient for TestBlockChainClient { } impl EngineInfo for TestBlockChainClient { - fn common_params(&self, _block_number: Option) -> Option { + fn common_params(&self, _block_id: BlockId) -> Option { unimplemented!() } diff --git a/core/src/miner/miner.rs b/core/src/miner/miner.rs index 3ee8230944..f8dc3a708f 100644 --- a/core/src/miner/miner.rs +++ b/core/src/miner/miner.rs @@ -279,7 +279,7 @@ impl Miner { .verify_basic() .map_err(From::from) .and_then(|_| { - let common_params = client.common_params(Some(best_header.number())).unwrap(); + let common_params = client.common_params(best_header.hash().into()).unwrap(); self.engine.verify_transaction_with_params(&tx, &common_params) }) .and_then(|_| self.engine.verify_transaction_seal(tx, &fake_header)) @@ -459,8 +459,13 @@ impl Miner { ctrace!(MINER, "prepare_block: No existing work - making new block"); let params = self.params.read().clone(); let open_block = chain.prepare_open_block(parent_block_id, params.author, params.extra_data); - let block_number = open_block.block().header().number(); - let max_body_size = chain.common_params(Some(block_number - 1)).unwrap().max_body_size(); + let (block_number, parent_hash) = { + let header = open_block.block().header(); + let block_number = header.number(); + let parent_hash = *header.parent_hash(); + (block_number, parent_hash) + }; + let max_body_size = chain.common_params(parent_hash.into()).unwrap().max_body_size(); const DEFAULT_RANGE: Range = 0..::std::u64::MAX; let transactions = mem_pool .top_transactions(max_body_size, Some(open_block.header().timestamp()), DEFAULT_RANGE) @@ -521,13 +526,13 @@ impl Miner { } cdebug!(MINER, "Pushed {}/{} transactions", tx_count, tx_total); - let (transactions_root, parent_number) = { - let parent_hash = open_block.header().parent_hash(); - let parent_header = chain.block_header(&BlockId::Hash(*parent_hash)).expect("Parent header MUST exist"); + let (transactions_root, parent_hash) = { + let parent_hash = *open_block.header().parent_hash(); + let parent_header = chain.block_header(&parent_hash.into()).expect("Parent header MUST exist"); let parent_view = parent_header.view(); - (parent_view.transactions_root(), parent_view.number()) + (parent_view.transactions_root(), parent_hash) }; - let parent_common_params = chain.common_params(Some(parent_number)).unwrap(); + let parent_common_params = chain.common_params(parent_hash.into()).unwrap(); let block = open_block.close(transactions_root, &parent_common_params)?; let fetch_seq = |p: &Public| { diff --git a/rpc/src/v1/impls/account.rs b/rpc/src/v1/impls/account.rs index cccde5b392..61ae477750 100644 --- a/rpc/src/v1/impls/account.rs +++ b/rpc/src/v1/impls/account.rs @@ -18,7 +18,7 @@ use std::convert::TryInto; use std::sync::Arc; use std::time::Duration; -use ccore::{AccountData, AccountProvider, EngineInfo, MinerService, MiningBlockChainClient}; +use ccore::{AccountData, AccountProvider, BlockId, EngineInfo, MinerService, MiningBlockChainClient}; use ckey::{NetworkId, Password, PlatformAddress, Signature}; use ctypes::transaction::IncompleteTransaction; use jsonrpc_core::Result; @@ -53,7 +53,7 @@ where fn network_id(&self) -> NetworkId { // XXX: What should we do if the network id has been changed - self.client.common_params(None).unwrap().network_id() + self.client.common_params(BlockId::Latest).unwrap().network_id() } } diff --git a/rpc/src/v1/impls/chain.rs b/rpc/src/v1/impls/chain.rs index 8cadf8e2c8..9beab92f42 100644 --- a/rpc/src/v1/impls/chain.rs +++ b/rpc/src/v1/impls/chain.rs @@ -94,7 +94,8 @@ where shard_id: ShardId, block_number: Option, ) -> Result> { - if let Some(common_params) = self.client.common_params(block_number) { + let block_id = block_number.map(BlockId::from).unwrap_or(BlockId::Latest); + if let Some(common_params) = self.client.common_params(block_id) { let network_id = common_params.network_id(); let block_id = block_number.map(BlockId::Number).unwrap_or(BlockId::Latest); Ok(self @@ -113,7 +114,7 @@ where .client .get_text(transaction_hash, block_id) .map_err(errors::transaction_state)? - .map(|text| Text::from_core(text, self.client.common_params(block_number).unwrap().network_id()))) + .map(|text| Text::from_core(text, self.client.common_params(block_id).unwrap().network_id()))) } fn get_asset( @@ -160,7 +161,7 @@ where fn get_regular_key_owner(&self, public: Public, block_number: Option) -> Result> { let block_id = block_number.map(BlockId::Number).unwrap_or(BlockId::Latest); Ok(self.client.regular_key_owner(&public_to_address(&public), block_id.into()).and_then(|address| { - let network_id = self.client.common_params(block_number).unwrap().network_id(); + let network_id = self.client.common_params(block_id).unwrap().network_id(); Some(PlatformAddress::new_v1(network_id, address)) })) } @@ -187,7 +188,7 @@ where fn get_shard_owners(&self, shard_id: ShardId, block_number: Option) -> Result>> { let block_id = block_number.map(BlockId::Number).unwrap_or(BlockId::Latest); Ok(self.client.shard_owners(shard_id, block_id.into()).map(|owners| { - let network_id = self.client.common_params(block_number).unwrap().network_id(); + let network_id = self.client.common_params(block_id).unwrap().network_id(); owners.into_iter().map(|owner| PlatformAddress::new_v1(network_id, owner)).collect() })) } @@ -195,7 +196,7 @@ where fn get_shard_users(&self, shard_id: ShardId, block_number: Option) -> Result>> { let block_id = block_number.map(BlockId::Number).unwrap_or(BlockId::Latest); Ok(self.client.shard_users(shard_id, block_id.into()).map(|users| { - let network_id = self.client.common_params(block_number).unwrap().network_id(); + let network_id = self.client.common_params(block_id).unwrap().network_id(); users.into_iter().map(|user| PlatformAddress::new_v1(network_id, user)).collect() })) } @@ -218,17 +219,18 @@ where fn get_block_by_number(&self, block_number: u64) -> Result> { let id = BlockId::Number(block_number); - Ok(self.client.block(&id).map(|block| { - Block::from_core(block.decode(), self.client.common_params(Some(block_number)).unwrap().network_id()) - })) + Ok(self + .client + .block(&id) + .map(|block| Block::from_core(block.decode(), self.client.common_params(id).unwrap().network_id()))) } fn get_block_by_hash(&self, block_hash: H256) -> Result> { let id = BlockId::Hash(block_hash); Ok(self.client.block(&id).map(|block| { let block = block.decode(); - let block_number = block.header.number(); - Block::from_core(block, self.client.common_params(Some(block_number)).unwrap().network_id()) + let block_hash = block.header.hash(); + Block::from_core(block, self.client.common_params(id).unwrap().network_id()) })) } @@ -237,7 +239,7 @@ where } fn get_min_transaction_fee(&self, action_type: String, block_number: u64) -> Result> { - if let Some(common_parameters) = self.client.common_params(Some(block_number)) { + if let Some(common_parameters) = self.client.common_params(block_number.into()) { Ok(match action_type.as_str() { "mintAsset" => Some(common_parameters.min_asset_mint_cost()), "transferAsset" => Some(common_parameters.min_asset_transfer_cost()), @@ -268,7 +270,7 @@ where } fn get_network_id(&self) -> Result { - Ok(self.client.common_params(None).unwrap().network_id()) + Ok(self.client.common_params(BlockId::Latest).unwrap().network_id()) } fn execute_transaction(&self, tx: UnsignedTransaction, sender: PlatformAddress) -> Result> { diff --git a/rpc/src/v1/impls/devel.rs b/rpc/src/v1/impls/devel.rs index 4246564cc0..9c36dc3278 100644 --- a/rpc/src/v1/impls/devel.rs +++ b/rpc/src/v1/impls/devel.rs @@ -112,7 +112,7 @@ where } fn test_tps(&self, setting: TPSTestSetting) -> Result { - let common_params = self.client.common_params(None).unwrap(); + let common_params = self.client.common_params(BlockId::Latest).unwrap(); let mint_fee = common_params.min_asset_mint_cost(); let transfer_fee = common_params.min_asset_transfer_cost(); let pay_fee = common_params.min_pay_transaction_cost(); diff --git a/rpc/src/v1/impls/engine.rs b/rpc/src/v1/impls/engine.rs index ce22e477f5..f106f29346 100644 --- a/rpc/src/v1/impls/engine.rs +++ b/rpc/src/v1/impls/engine.rs @@ -62,7 +62,7 @@ where Ok(None) } else { // XXX: What should we do if the network id has been changed - let network_id = self.client.common_params(None).unwrap().network_id(); + let network_id = self.client.common_params(BlockId::Latest).unwrap().network_id(); Ok(Some(PlatformAddress::new_v1(network_id, author))) } } From 50c2a0dfb9c8e7228800fe1212e64b30a2e78681 Mon Sep 17 00:00:00 2001 From: Seulgi Kim Date: Mon, 20 May 2019 20:02:54 +0900 Subject: [PATCH 9/9] Fix chain RPCs to read the parameters of the parent block --- core/src/client/client.rs | 8 +++++ core/src/client/test_client.rs | 10 ++++++ core/src/types/ids.rs | 2 ++ rpc/src/v1/impls/chain.rs | 57 ++++++++++++++++++++++------------ 4 files changed, 58 insertions(+), 19 deletions(-) diff --git a/core/src/client/client.rs b/core/src/client/client.rs index bf60cb3b2b..e5b0782679 100644 --- a/core/src/client/client.rs +++ b/core/src/client/client.rs @@ -222,6 +222,7 @@ impl Client { BlockId::Number(number) => chain.block_hash(*number), BlockId::Earliest => chain.block_hash(0), BlockId::Latest => Some(chain.best_block_hash()), + BlockId::ParentOfLatest => Some(chain.best_block_header().parent_hash()), } } @@ -293,6 +294,13 @@ impl Client { BlockId::Hash(hash) => self.block_chain().block_number(hash), BlockId::Earliest => Some(0), BlockId::Latest => Some(self.block_chain().best_block_detail().number), + BlockId::ParentOfLatest => { + if self.block_chain().best_block_detail().number == 0 { + None + } else { + Some(self.block_chain().best_block_detail().number - 1) + } + } } } diff --git a/core/src/client/test_client.rs b/core/src/client/test_client.rs index 464602c21a..b46387c771 100644 --- a/core/src/client/test_client.rs +++ b/core/src/client/test_client.rs @@ -266,6 +266,15 @@ impl TestBlockChainClient { BlockId::Number(n) => self.numbers.read().get(&(*n as usize)).cloned(), BlockId::Earliest => self.numbers.read().get(&0).cloned(), BlockId::Latest => self.numbers.read().get(&(self.numbers.read().len() - 1)).cloned(), + BlockId::ParentOfLatest => { + let numbers = self.numbers.read(); + let len = numbers.len(); + if len < 2 { + None + } else { + self.numbers.read().get(&(len - 2)).cloned() + } + } } } @@ -505,6 +514,7 @@ impl BlockChainClient for TestBlockChainClient { BlockId::Number(number) if (*number as usize) < self.blocks.read().len() => BlockStatus::InChain, BlockId::Hash(ref hash) if self.blocks.read().get(hash).is_some() => BlockStatus::InChain, BlockId::Latest | BlockId::Earliest => BlockStatus::InChain, + BlockId::ParentOfLatest => BlockStatus::InChain, _ => BlockStatus::Unknown, } } diff --git a/core/src/types/ids.rs b/core/src/types/ids.rs index 1a88ee9aac..66b4e5b22f 100644 --- a/core/src/types/ids.rs +++ b/core/src/types/ids.rs @@ -29,6 +29,8 @@ pub enum BlockId { Earliest, /// Latest mined block. Latest, + /// Parent of latest mined block. + ParentOfLatest, } impl From for BlockId { diff --git a/rpc/src/v1/impls/chain.rs b/rpc/src/v1/impls/chain.rs index 9beab92f42..ac5948d330 100644 --- a/rpc/src/v1/impls/chain.rs +++ b/rpc/src/v1/impls/chain.rs @@ -94,10 +94,13 @@ where shard_id: ShardId, block_number: Option, ) -> Result> { - let block_id = block_number.map(BlockId::from).unwrap_or(BlockId::Latest); - if let Some(common_params) = self.client.common_params(block_id) { + if block_number == Some(0) { + return Ok(None) + } + let parent_block_id = block_number.map(|n| (n - 1).into()).unwrap_or(BlockId::ParentOfLatest); + if let Some(common_params) = self.client.common_params(parent_block_id) { let network_id = common_params.network_id(); - let block_id = block_number.map(BlockId::Number).unwrap_or(BlockId::Latest); + let block_id = block_number.map(BlockId::from).unwrap_or(BlockId::Latest); Ok(self .client .get_asset_scheme(asset_type, shard_id, block_id) @@ -109,12 +112,14 @@ where } fn get_text(&self, transaction_hash: H256, block_number: Option) -> Result> { - let block_id = block_number.map(BlockId::Number).unwrap_or(BlockId::Latest); - Ok(self - .client - .get_text(transaction_hash, block_id) - .map_err(errors::transaction_state)? - .map(|text| Text::from_core(text, self.client.common_params(block_id).unwrap().network_id()))) + if block_number == Some(0) { + return Ok(None) + } + let block_id = block_number.map(BlockId::from).unwrap_or(BlockId::Latest); + Ok(self.client.get_text(transaction_hash, block_id).map_err(errors::transaction_state)?.map(|text| { + let parent_block_id = block_number.map(|n| (n - 1).into()).unwrap_or(BlockId::ParentOfLatest); + Text::from_core(text, self.client.common_params(parent_block_id).unwrap().network_id()) + })) } fn get_asset( @@ -161,7 +166,8 @@ where fn get_regular_key_owner(&self, public: Public, block_number: Option) -> Result> { let block_id = block_number.map(BlockId::Number).unwrap_or(BlockId::Latest); Ok(self.client.regular_key_owner(&public_to_address(&public), block_id.into()).and_then(|address| { - let network_id = self.client.common_params(block_id).unwrap().network_id(); + let parent_block_id = block_number.map(|n| (n - 1).into()).unwrap_or(BlockId::ParentOfLatest); + let network_id = self.client.common_params(parent_block_id).unwrap().network_id(); Some(PlatformAddress::new_v1(network_id, address)) })) } @@ -188,7 +194,8 @@ where fn get_shard_owners(&self, shard_id: ShardId, block_number: Option) -> Result>> { let block_id = block_number.map(BlockId::Number).unwrap_or(BlockId::Latest); Ok(self.client.shard_owners(shard_id, block_id.into()).map(|owners| { - let network_id = self.client.common_params(block_id).unwrap().network_id(); + let parent_block_id = block_number.map(|n| (n - 1).into()).unwrap_or(BlockId::ParentOfLatest); + let network_id = self.client.common_params(parent_block_id).unwrap().network_id(); owners.into_iter().map(|owner| PlatformAddress::new_v1(network_id, owner)).collect() })) } @@ -196,7 +203,8 @@ where fn get_shard_users(&self, shard_id: ShardId, block_number: Option) -> Result>> { let block_id = block_number.map(BlockId::Number).unwrap_or(BlockId::Latest); Ok(self.client.shard_users(shard_id, block_id.into()).map(|users| { - let network_id = self.client.common_params(block_id).unwrap().network_id(); + let parent_block_id = block_number.map(|n| (n - 1).into()).unwrap_or(BlockId::ParentOfLatest); + let network_id = self.client.common_params(parent_block_id).unwrap().network_id(); users.into_iter().map(|user| PlatformAddress::new_v1(network_id, user)).collect() })) } @@ -219,18 +227,26 @@ where fn get_block_by_number(&self, block_number: u64) -> Result> { let id = BlockId::Number(block_number); - Ok(self - .client - .block(&id) - .map(|block| Block::from_core(block.decode(), self.client.common_params(id).unwrap().network_id()))) + Ok(self.client.block(&id).map(|block| { + let block_id_to_read_params = if block_number == 0 { + 0.into() + } else { + (block_number - 1).into() + }; + Block::from_core(block.decode(), self.client.common_params(block_id_to_read_params).unwrap().network_id()) + })) } fn get_block_by_hash(&self, block_hash: H256) -> Result> { let id = BlockId::Hash(block_hash); Ok(self.client.block(&id).map(|block| { let block = block.decode(); - let block_hash = block.header.hash(); - Block::from_core(block, self.client.common_params(id).unwrap().network_id()) + let block_id_to_read_params = if block.header.number() == 0 { + 0.into() + } else { + (*block.header.parent_hash()).into() + }; + Block::from_core(block, self.client.common_params(block_id_to_read_params).unwrap().network_id()) })) } @@ -239,7 +255,10 @@ where } fn get_min_transaction_fee(&self, action_type: String, block_number: u64) -> Result> { - if let Some(common_parameters) = self.client.common_params(block_number.into()) { + if block_number == 0 { + return Ok(None) + } + if let Some(common_parameters) = self.client.common_params((block_number - 1).into()) { Ok(match action_type.as_str() { "mintAsset" => Some(common_parameters.min_asset_mint_cost()), "transferAsset" => Some(common_parameters.min_asset_transfer_cost()),