Skip to content

Split the verification that needs CommonParams and doesn't need it #1546

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
May 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 8 additions & 9 deletions codechain/run_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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(BlockId::Latest).unwrap().network_id();
let routing_table = RoutingTable::new();
let service = network_start(network_id, timer_loop, &network_config, Arc::clone(&routing_table))?;

Expand Down Expand Up @@ -335,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<ChainNotify>);
let client = client.client();
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<ChainNotify>);
Some(service)
} else {
None
Expand Down
2 changes: 1 addition & 1 deletion codechain/subcommand/account_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion codechain/subcommand/convert_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ fn get_network_id(matches: &ArgMatches) -> Result<NetworkId, String> {
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)
}

Expand Down
29 changes: 21 additions & 8 deletions core/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -215,10 +216,14 @@ impl<'x> OpenBlock<'x> {
}

/// Turn this into a `ClosedBlock`.
pub fn close(mut self, parent_transactions_root: H256) -> Result<ClosedBlock, Error> {
pub fn close(
mut self,
parent_transactions_root: H256,
parent_common_params: &CommonParams,
) -> Result<ClosedBlock, Error> {
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)
}
Expand All @@ -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<LockedBlock, Error> {
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<LockedBlock, Error> {
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)
}
Expand Down Expand Up @@ -428,7 +437,7 @@ impl IsBlock for SealedBlock {
}

/// Enact the block given by block header, transactions and uncles
pub fn enact<C: ChainTimeInfo + FindActionHandler>(
pub fn enact<C: ChainTimeInfo + EngineInfo + FindActionHandler>(
header: &Header,
transactions: &[SignedTransaction],
engine: &CodeChainEngine,
Expand All @@ -441,11 +450,14 @@ pub fn enact<C: ChainTimeInfo + FindActionHandler>(
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((*header.parent_hash()).into()).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;

Expand All @@ -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![]);
}
}
26 changes: 21 additions & 5 deletions core/src/client/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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};
Expand Down Expand Up @@ -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()),
}
}

Expand Down Expand Up @@ -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)
}
}
}
}

Expand Down Expand Up @@ -484,8 +492,16 @@ impl StateInfo for Client {
}

impl EngineInfo for Client {
fn common_params(&self, block_number: Option<BlockNumber>) -> &CommonParams {
self.engine().machine().common_params(block_number)
fn common_params(&self, block_id: BlockId) -> Option<CommonParams> {
self.state_info(block_id.into()).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 {
Expand Down Expand Up @@ -556,7 +572,7 @@ impl BlockChainTrait for Client {

fn genesis_accounts(&self) -> Vec<PlatformAddress> {
// 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(BlockId::Latest).unwrap().network_id();
self.genesis_accounts.iter().map(|addr| PlatformAddress::new_v1(network_id, *addr)).collect()
}

Expand Down
4 changes: 4 additions & 0 deletions core/src/client/importer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -242,6 +243,8 @@ impl Importer {
);
})?;

let common_params = client.common_params(parent.hash().into()).unwrap();

// Verify Block Family
self.verifier
.verify_block_family(
Expand All @@ -255,6 +258,7 @@ impl Importer {
block_provider: &*chain,
client,
}),
&common_params,
)
.map_err(|e| {
cwarn!(
Expand Down
5 changes: 2 additions & 3 deletions core/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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};

Expand Down Expand Up @@ -87,7 +86,7 @@ pub trait BlockChainTrait {
}

pub trait EngineInfo: Send + Sync {
fn common_params(&self, block_number: Option<BlockNumber>) -> &CommonParams;
fn common_params(&self, block_id: BlockId) -> Option<CommonParams>;
fn block_reward(&self, block_number: u64) -> u64;
fn mining_reward(&self, block_number: u64) -> Option<u64>;
fn recommended_confirmation(&self) -> u32;
Expand Down
34 changes: 31 additions & 3 deletions core/src/client/test_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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()
}
}
}
}

Expand Down Expand Up @@ -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,
}
}
Expand Down Expand Up @@ -572,3 +582,21 @@ impl super::EngineClient for TestBlockChainClient {
Arc::new(db)
}
}

impl EngineInfo for TestBlockChainClient {
fn common_params(&self, _block_id: BlockId) -> Option<CommonParams> {
unimplemented!()
}

fn block_reward(&self, _block_number: u64) -> u64 {
unimplemented!()
}

fn mining_reward(&self, _block_number: u64) -> Option<u64> {
unimplemented!()
}

fn recommended_confirmation(&self) -> u32 {
unimplemented!()
}
}
Loading