Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Fix block detail updating #11015

Merged
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
29 changes: 24 additions & 5 deletions ethcore/blockchain/src/blockchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,12 @@ pub trait BlockProvider {
where F: Fn(&LogEntry) -> bool + Send + Sync, Self: Sized;
}

/// Interface for querying blocks with pending db transaction by hash and by number.
trait InTransactionBlockProvider {
/// Get the familial details concerning a block.
dvdplm marked this conversation as resolved.
Show resolved Hide resolved
fn uncommitted_block_details(&self, hash: &H256) -> Option<BlockDetails>;
}

#[derive(Debug, Hash, Eq, PartialEq, Clone)]
enum CacheId {
BlockHeader(H256),
Expand Down Expand Up @@ -424,6 +430,19 @@ impl BlockProvider for BlockChain {
}
}

impl InTransactionBlockProvider for BlockChain {
fn uncommitted_block_details(&self, hash: &H256) -> Option<BlockDetails> {
let result = self.db.key_value().read_with_two_layer_cache(
db::COL_EXTRA,
&self.pending_block_details,
&self.block_details,
hash
)?;
self.cache_man.lock().note_used(CacheId::BlockDetails(*hash));
Some(result)
}
}

/// An iterator which walks the blockchain towards the genesis.
#[derive(Clone)]
pub struct AncestryIter<'a> {
Expand Down Expand Up @@ -792,7 +811,7 @@ impl BlockChain {
batch.put(db::COL_HEADERS, hash.as_bytes(), &compressed_header);
batch.put(db::COL_BODIES, hash.as_bytes(), &compressed_body);

let maybe_parent = self.block_details(&block_parent_hash);
let maybe_parent = self.uncommitted_block_details(&block_parent_hash);

if let Some(parent_details) = maybe_parent {
// parent known to be in chain.
Expand Down Expand Up @@ -1044,7 +1063,7 @@ impl BlockChain {
///
/// Used in snapshots to glue the chunks together at the end.
pub fn add_child(&self, batch: &mut DBTransaction, block_hash: H256, child_hash: H256) {
let mut parent_details = self.block_details(&block_hash)
let mut parent_details = self.uncommitted_block_details(&block_hash)
.unwrap_or_else(|| panic!("Invalid block hash: {:?}", block_hash));

parent_details.children.push(child_hash);
Expand Down Expand Up @@ -1151,7 +1170,7 @@ impl BlockChain {
/// Mark a block to be considered finalized. Returns `Some(())` if the operation succeeds, and `None` if the block
/// hash is not found.
pub fn mark_finalized(&self, batch: &mut DBTransaction, block_hash: H256) -> Option<()> {
let mut block_details = self.block_details(&block_hash)?;
let mut block_details = self.uncommitted_block_details(&block_hash)?;
block_details.is_finalized = true;

self.update_block_details(batch, block_hash, block_details);
Expand Down Expand Up @@ -1344,7 +1363,7 @@ impl BlockChain {
/// Uses the given parent details or attempts to load them from the database.
fn prepare_block_details_update(&self, parent_hash: H256, info: &BlockInfo, is_finalized: bool) -> HashMap<H256, BlockDetails> {
// update parent
let mut parent_details = self.block_details(&parent_hash).unwrap_or_else(|| panic!("Invalid parent hash: {:?}", parent_hash));
let mut parent_details = self.uncommitted_block_details(&parent_hash).unwrap_or_else(|| panic!("Invalid parent hash: {:?}", parent_hash));
parent_details.children.push(info.hash);

// create current block details.
Expand Down Expand Up @@ -1650,7 +1669,7 @@ mod tests {
let fork_choice = {
let header = block.header_view();
let parent_hash = header.parent_hash();
let parent_details = bc.block_details(&parent_hash).unwrap_or_else(|| panic!("Invalid parent hash: {:?}", parent_hash));
let parent_details = bc.uncommitted_block_details(&parent_hash).unwrap_or_else(|| panic!("Invalid parent hash: {:?}", parent_hash));
let block_total_difficulty = parent_details.total_difficulty + header.difficulty();
if block_total_difficulty > bc.best_block_total_difficulty() {
common_types::engines::ForkChoice::New
Expand Down
15 changes: 15 additions & 0 deletions ethcore/db/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,21 @@ pub trait Readable {
})
}

/// Returns value for given key either in two-layered cache or in database.
fn read_with_two_layer_cache<K, T, C>(&self, col: Option<u32>, l1_cache: &RwLock<C>, l2_cache: &RwLock<C>, key: &K) -> Option<T> where
K: Key<T> + Eq + Hash + Clone,
T: Clone + rlp::Decodable,
C: Cache<K, T> {
{
let read = l1_cache.read();
if let Some(v) = read.get(key) {
return Some(v.clone());
}
}

self.read_with_cache(col, l2_cache, key)
}

/// Returns true if given value exists.
fn exists<T, R>(&self, col: Option<u32>, key: &dyn Key<T, Target = R>) -> bool where R: AsRef<[u8]>;

Expand Down
18 changes: 17 additions & 1 deletion ethcore/engines/null-engine/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,26 @@ use machine::{
ExecutedBlock,
Machine,
};
use common_types::snapshot::Snapshotting;
use common_types::{
ancestry_action::AncestryAction,
header::ExtendedHeader,
snapshot::Snapshotting
};

/// Params for a null engine.
#[derive(Clone, Default)]
pub struct NullEngineParams {
/// base reward for a block.
pub block_reward: U256,
/// Immediate finalization.
pub immediate_finalization: bool
}

impl From<ethjson::spec::NullEngineParams> for NullEngineParams {
fn from(p: ethjson::spec::NullEngineParams) -> Self {
NullEngineParams {
block_reward: p.block_reward.map_or_else(Default::default, Into::into),
immediate_finalization: p.immediate_finalization.unwrap_or(false)
}
}
}
Expand Down Expand Up @@ -108,4 +115,13 @@ impl Engine for NullEngine {
fn params(&self) -> &CommonParams {
self.machine.params()
}

fn ancestry_actions(&self, _header: &Header, ancestry: &mut dyn Iterator<Item=ExtendedHeader>) -> Vec<AncestryAction> {
if self.params.immediate_finalization {
// always mark parent finalized
ancestry.take(1).map(|e| AncestryAction::MarkFinalized(e.header.hash())).collect()
} else {
Vec::new()
}
}
}
38 changes: 38 additions & 0 deletions ethcore/res/null_morden_with_finality.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
{
"name": "Morden",
"engine": {
"null": {
"params": {
"immediateFinalization": true
}
}
},
"params": {
"gasLimitBoundDivisor": "0x0400",
"accountStartNonce": "0x0",
"maximumExtraDataSize": "0x20",
"minGasLimit": "0x1388",
"networkID" : "0x2"
},
"genesis": {
"seal": {
"ethereum": {
"nonce": "0x00006d6f7264656e",
"mixHash": "0x00000000000000000000000000000000000000647572616c65787365646c6578"
}
},
"difficulty": "0x20000",
"author": "0x0000000000000000000000000000000000000000",
"timestamp": "0x00",
"parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
"extraData": "0x",
"gasLimit": "0x2fefd8"
},
"accounts": {
"0000000000000000000000000000000000000001": { "balance": "1", "nonce": "1048576", "builtin": { "name": "ecrecover", "pricing": { "linear": { "base": 3000, "word": 0 } } } },
"0000000000000000000000000000000000000002": { "balance": "1", "nonce": "1048576", "builtin": { "name": "sha256", "pricing": { "linear": { "base": 60, "word": 12 } } } },
"0000000000000000000000000000000000000003": { "balance": "1", "nonce": "1048576", "builtin": { "name": "ripemd160", "pricing": { "linear": { "base": 600, "word": 120 } } } },
"0000000000000000000000000000000000000004": { "balance": "1", "nonce": "1048576", "builtin": { "name": "identity", "pricing": { "linear": { "base": 15, "word": 3 } } } },
"102e61f5d8f9bc71d0ad4a084df4e65e05ce0e1c": { "balance": "1606938044258990275541962092341162602522202993782792835301376", "nonce": "1048576" }
}
}
1 change: 1 addition & 0 deletions ethcore/spec/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ bundle_test_spec! {
"null" => new_null,
"null_morden" => new_test,
"null_morden_with_reward" => new_test_with_reward,
"null_morden_with_finality" => new_test_with_finality,
"validator_contract" => new_validator_contract,
"validator_multi" => new_validator_multi,
"validator_safe_contract" => new_validator_safe_contract
Expand Down
22 changes: 20 additions & 2 deletions ethcore/src/client/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2640,13 +2640,13 @@ mod tests {
receipt::{Receipt, LocalizedReceipt, TransactionOutcome},
transaction::{Transaction, LocalizedTransaction, Action},
};
use test_helpers::{generate_dummy_client, get_good_dummy_block_hash, generate_dummy_client_with_data};
use test_helpers::{generate_dummy_client, generate_dummy_client_with_data, generate_dummy_client_with_spec_and_data, get_good_dummy_block_hash};
use std::thread;
use std::time::Duration;
use std::sync::Arc;
use std::sync::atomic::{AtomicBool, Ordering};
use kvdb::DBTransaction;
use blockchain::ExtrasInsert;
use blockchain::{BlockProvider, ExtrasInsert};
use hash::keccak;
use super::transaction_receipt;
use ethkey::KeyPair;
Expand Down Expand Up @@ -2781,4 +2781,22 @@ mod tests {
outcome: TransactionOutcome::StateRoot(state_root),
});
}

#[test]
fn should_mark_finalization_correctly_for_parent() {
let client = generate_dummy_client_with_spec_and_data(spec::new_test_with_finality, 2, 0, &[]);
let chain = client.chain();

let block1_details = chain.block_hash(1).and_then(|h| chain.block_details(&h));
assert!(block1_details.is_some());
let block1_details = block1_details.unwrap();
assert_eq!(block1_details.children.len(), 1);
assert!(block1_details.is_finalized);

let block2_details = chain.block_hash(2).and_then(|h| chain.block_details(&h));
assert!(block2_details.is_some());
let block2_details = block2_details.unwrap();
assert_eq!(block2_details.children.len(), 0);
assert!(!block2_details.is_finalized);
}
}
2 changes: 2 additions & 0 deletions json/src/spec/null_engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ use uint::Uint;
pub struct NullEngineParams {
/// Block reward.
pub block_reward: Option<Uint>,
/// Immediate finalization.
pub immediate_finalization: Option<bool>
}

/// Null engine descriptor
Expand Down