From 387e3ec3fd38df75a23997b2190cf138ccb0ad84 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 26 Jan 2016 15:00:22 +0100 Subject: [PATCH 1/8] Ensure Spec::ensure_db_good() places DB entries for code & storage. --- src/account.rs | 6 +++++- src/blockchain.rs | 5 +++++ src/client.rs | 17 ++++++++++++++++- src/evm/interpreter.rs | 3 +++ src/executive.rs | 11 +++++------ src/externalities.rs | 1 + src/pod_account.rs | 14 +++++++++++++- src/spec.rs | 5 +++-- src/state.rs | 1 + src/tests/chain.rs | 5 +++-- util/src/trie/sectriedbmut.rs | 6 ++++++ util/src/trie/triedbmut.rs | 5 +++++ 12 files changed, 66 insertions(+), 13 deletions(-) diff --git a/src/account.rs b/src/account.rs index b0fbf3f85..409637d6f 100644 --- a/src/account.rs +++ b/src/account.rs @@ -149,11 +149,15 @@ impl Account { /// Provide a database to lookup `code_hash`. Should not be called if it is a contract without code. pub fn cache_code(&mut self, db: &HashDB) -> bool { // TODO: fill out self.code_cache; + trace!("Account::cache_code: ic={}; self.code_hash={:?}, self.code_cache={}", self.is_cached(), self.code_hash, self.code_cache.pretty()); self.is_cached() || match self.code_hash { Some(ref h) => match db.lookup(h) { Some(x) => { self.code_cache = x.to_vec(); true }, - _ => false, + _ => { + warn!("Failed reverse lookup of {}", h); + false + }, }, _ => false, } diff --git a/src/blockchain.rs b/src/blockchain.rs index 3bd31688a..da9ee04c2 100644 --- a/src/blockchain.rs +++ b/src/blockchain.rs @@ -107,6 +107,11 @@ pub trait BlockProvider { fn genesis_hash(&self) -> H256 { self.block_hash(0).expect("Genesis hash should always exist") } + + /// Returns the header of the genesis block. + fn genesis_header(&self) -> Header { + self.block_header(&self.genesis_hash()).unwrap() + } } #[derive(Debug, Hash, Eq, PartialEq, Clone)] diff --git a/src/client.rs b/src/client.rs index 2cc6c150d..91ccf4b95 100644 --- a/src/client.rs +++ b/src/client.rs @@ -4,8 +4,10 @@ use blockchain::{BlockChain, BlockProvider, CacheSize}; use views::BlockView; use error::*; use header::BlockNumber; +use state::State; use spec::Spec; use engine::Engine; +use views::HeaderView; use block_queue::{BlockQueue, BlockQueueInfo}; use service::NetSyncMessage; use env_info::LastHashes; @@ -98,6 +100,11 @@ pub trait BlockChainClient : Sync + Send { /// Get blockchain information. fn chain_info(&self) -> BlockChainInfo; + + /// Get the best block header. + fn best_block_header(&self) -> Bytes { + self.block_header(&self.chain_info().best_block_hash).unwrap() + } } #[derive(Default, Clone, Debug, Eq, PartialEq)] @@ -137,7 +144,9 @@ const HISTORY: u64 = 1000; impl Client { /// Create a new client with given spec and DB path. pub fn new(spec: Spec, path: &Path, message_channel: IoChannel ) -> Result, Error> { - let chain = Arc::new(RwLock::new(BlockChain::new(&spec.genesis_block(), path))); + let gb = spec.genesis_block(); + flushln!("Spec says genesis block is {}", gb.pretty()); + let chain = Arc::new(RwLock::new(BlockChain::new(&gb, path))); let mut opts = Options::new(); opts.set_max_open_files(256); opts.create_if_missing(true); @@ -168,6 +177,7 @@ impl Client { if engine.spec().ensure_db_good(&mut state_db) { state_db.commit(0, &engine.spec().genesis_header().hash(), None).expect("Error commiting genesis state to state DB"); } + flushln!("Client::new: commiting. Best root now: {}. contains: {}", chain.read().unwrap().genesis_header().state_root, state_db.contains(&chain.read().unwrap().genesis_header().state_root)); Ok(Arc::new(Client { chain: chain, engine: engine.clone(), @@ -261,6 +271,11 @@ impl Client { self.uncommited_states.write().unwrap().remove(hash); } + /// Get a copy of the best block's state. + pub fn state(&self) -> State { + State::from_existing(self.state_db.clone(), HeaderView::new(&self.best_block_header()).state_root(), self.engine.account_start_nonce()) + } + /// Get info on the cache. pub fn cache_info(&self) -> CacheSize { self.chain.read().unwrap().cache_size() diff --git a/src/evm/interpreter.rs b/src/evm/interpreter.rs index 7657b1bbe..2f43b5199 100644 --- a/src/evm/interpreter.rs +++ b/src/evm/interpreter.rs @@ -275,6 +275,8 @@ impl evm::Evm for Interpreter { code: &code }; + flushln!("Executing: {:?}", params); + while reader.position < code.len() { let instruction = code[reader.position]; reader.position += 1; @@ -640,6 +642,7 @@ impl Interpreter { return Ok(InstructionResult::StopExecution); }, instructions::SUICIDE => { + flushln!("SUICIDE!"); let address = stack.pop_back(); ext.suicide(&u256_to_address(&address)); return Ok(InstructionResult::StopExecution); diff --git a/src/executive.rs b/src/executive.rs index f5952e530..9c3a669cf 100644 --- a/src/executive.rs +++ b/src/executive.rs @@ -166,7 +166,6 @@ impl<'a> Executive<'a> { /// Modifies the substate and the output. /// Returns either gas_left or `evm::Error`. pub fn call(&mut self, params: ActionParams, substate: &mut Substate, mut output: BytesRef) -> evm::Result { - println!("Calling executive. Sender: {}", params.sender); // backup used in case of running out of gas let backup = self.state.clone(); @@ -174,7 +173,7 @@ impl<'a> Executive<'a> { if let ActionValue::Transfer(val) = params.value { self.state.transfer_balance(¶ms.sender, ¶ms.address, &val); } - trace!("Executive::call(params={:?}) self.env_info={:?}", params, self.info); + flushln!("Executive::call(params={:?}) self.env_info={:?}", params, self.info); if self.engine.is_builtin(¶ms.code_address) { // if destination is builtin, try to execute it @@ -261,17 +260,17 @@ impl<'a> Executive<'a> { let refund_value = gas_left * t.gas_price; let fees_value = gas_used * t.gas_price; - trace!("exec::finalize: t.gas={}, sstore_refunds={}, suicide_refunds={}, refunds_bound={}, gas_left_prerefund={}, refunded={}, gas_left={}, gas_used={}, refund_value={}, fees_value={}\n", + flushln!("exec::finalize: t.gas={}, sstore_refunds={}, suicide_refunds={}, refunds_bound={}, gas_left_prerefund={}, refunded={}, gas_left={}, gas_used={}, refund_value={}, fees_value={}\n", t.gas, sstore_refunds, suicide_refunds, refunds_bound, gas_left_prerefund, refunded, gas_left, gas_used, refund_value, fees_value); - trace!("exec::finalize: Refunding refund_value={}, sender={}\n", refund_value, t.sender().unwrap()); + flushln!("exec::finalize: Refunding refund_value={}, sender={}\n", refund_value, t.sender().unwrap()); self.state.add_balance(&t.sender().unwrap(), &refund_value); - trace!("exec::finalize: Compensating author: fees_value={}, author={}\n", fees_value, &self.info.author); + flushln!("exec::finalize: Compensating author: fees_value={}, author={}\n", fees_value, &self.info.author); self.state.add_balance(&self.info.author, &fees_value); // perform suicides for address in &substate.suicides { - trace!("Killing {}", address); + flushln!("Killing {}", address); self.state.kill_account(address); } diff --git a/src/externalities.rs b/src/externalities.rs index f9a79c3c0..e7a73d553 100644 --- a/src/externalities.rs +++ b/src/externalities.rs @@ -215,6 +215,7 @@ impl<'a> Ext for Externalities<'a> { fn suicide(&mut self, refund_address: &Address) { let address = self.origin_info.address.clone(); let balance = self.balance(&address); + flushln!("Suiciding {} -> {} (xfer: {})", address, refund_address, balance); self.state.transfer_balance(&address, refund_address, &balance); self.substate.suicides.insert(address); } diff --git a/src/pod_account.rs b/src/pod_account.rs index 81b8b1c44..979658202 100644 --- a/src/pod_account.rs +++ b/src/pod_account.rs @@ -31,7 +31,7 @@ impl PodAccount { } } - /// TODO [Gav Wood] Please document me + /// Returns the RLP for this account. pub fn rlp(&self) -> Bytes { let mut stream = RlpStream::new_list(4); stream.append(&self.nonce); @@ -40,6 +40,18 @@ impl PodAccount { stream.append(&self.code.sha3()); stream.out() } + + /// Place additional data into given hash DB. + pub fn insert_additional(&self, db: &mut HashDB) { + if !self.code.is_empty() { + db.insert(&self.code); + } + let mut r = H256::new(); + let mut t = SecTrieDBMut::new(db, &mut r); + for (k, v) in &self.storage { + t.insert(k, &encode(&U256::from(v.as_slice()))); + } + } } impl fmt::Display for PodAccount { diff --git a/src/spec.rs b/src/spec.rs index 326552524..f4166eab2 100644 --- a/src/spec.rs +++ b/src/spec.rs @@ -261,7 +261,6 @@ impl Spec { /// Ensure that the given state DB has the trie nodes in for the genesis state. pub fn ensure_db_good(&self, db: &mut HashDB) -> bool { if !db.contains(&self.state_root()) { - info!("Populating genesis state..."); let mut root = H256::new(); { let mut t = SecTrieDBMut::new(db, &mut root); @@ -269,8 +268,10 @@ impl Spec { t.insert(address.as_slice(), &account.rlp()); } } + for (_, account) in self.genesis_state.get().iter() { + account.insert_additional(db); + } assert!(db.contains(&self.state_root())); - info!("Genesis state is ready"); true } else { false } } diff --git a/src/state.rs b/src/state.rs index 7310f63a7..238638617 100644 --- a/src/state.rs +++ b/src/state.rs @@ -103,6 +103,7 @@ impl State { /// Mutate storage of account `a` so that it is `value` for `key`. pub fn code(&self, a: &Address) -> Option { + flushln!("Getting code at {}", a); self.get(a, true).as_ref().map_or(None, |a|a.code().map(|x|x.to_vec())) } diff --git a/src/tests/chain.rs b/src/tests/chain.rs index ba42ae935..046c16052 100644 --- a/src/tests/chain.rs +++ b/src/tests/chain.rs @@ -22,7 +22,8 @@ fn do_json_test(json_data: &[u8]) -> Vec { let blocks: Vec = test["blocks"].as_array().unwrap().iter().map(|e| xjson!(&e["rlp"])).collect(); let mut spec = ethereum::new_frontier_like_test(); - spec.set_genesis_state(PodState::from_json(test.find("pre").unwrap())); + let s = PodState::from_json(test.find("pre").unwrap()); + spec.set_genesis_state(s); spec.overwrite_genesis(test.find("genesisBlockHeader").unwrap()); assert!(spec.is_state_root_valid()); @@ -56,7 +57,7 @@ declare_test!{BlockchainTests_bcInvalidHeaderTest, "BlockchainTests/bcInvalidHea declare_test!{ignore => BlockchainTests_bcInvalidRLPTest, "BlockchainTests/bcInvalidRLPTest"} // FAILS declare_test!{ignore => BlockchainTests_bcMultiChainTest, "BlockchainTests/bcMultiChainTest"} // FAILS declare_test!{BlockchainTests_bcRPC_API_Test, "BlockchainTests/bcRPC_API_Test"} -declare_test!{ignore => BlockchainTests_bcStateTest, "BlockchainTests/bcStateTest"} // FAILS (Suicides, GasUsed) +declare_test!{BlockchainTests_bcStateTest, "BlockchainTests/bcStateTest"} // FAILS (Suicides, GasUsed) declare_test!{BlockchainTests_bcTotalDifficultyTest, "BlockchainTests/bcTotalDifficultyTest"} declare_test!{ignore => BlockchainTests_bcUncleHeaderValiditiy, "BlockchainTests/bcUncleHeaderValiditiy"} // FAILS declare_test!{ignore => BlockchainTests_bcUncleTest, "BlockchainTests/bcUncleTest"} // FAILS diff --git a/util/src/trie/sectriedbmut.rs b/util/src/trie/sectriedbmut.rs index 99c2e2bb1..7f8e292f0 100644 --- a/util/src/trie/sectriedbmut.rs +++ b/util/src/trie/sectriedbmut.rs @@ -25,6 +25,12 @@ impl<'db> SecTrieDBMut<'db> { pub fn from_existing(db: &'db mut HashDB, root: &'db mut H256) -> Self { SecTrieDBMut { raw: TrieDBMut::from_existing(db, root) } } + + /// Get the backing database. + pub fn db(&'db self) -> &'db HashDB { self.raw.db() } + + /// Get the backing database. + pub fn db_mut(&'db mut self) -> &'db mut HashDB { self.raw.db_mut() } } impl<'db> Trie for SecTrieDBMut<'db> { diff --git a/util/src/trie/triedbmut.rs b/util/src/trie/triedbmut.rs index 48749bf0d..17d3f5866 100644 --- a/util/src/trie/triedbmut.rs +++ b/util/src/trie/triedbmut.rs @@ -87,6 +87,11 @@ impl<'db> TrieDBMut<'db> { self.db } + /// Get the backing database. + pub fn db_mut(&'db mut self) -> &'db mut HashDB { + self.db + } + /// Determine all the keys in the backing database that belong to the trie. pub fn keys(&self) -> Vec { let mut ret: Vec = Vec::new(); From 50d1038cc58db35417fe10284e57f73bb5db0a75 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 26 Jan 2016 15:02:14 +0100 Subject: [PATCH 2/8] Remove flushln!s. --- src/client.rs | 3 +-- src/evm/interpreter.rs | 3 --- src/executive.rs | 9 ++++----- src/externalities.rs | 2 +- src/state.rs | 1 - 5 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/client.rs b/src/client.rs index 91ccf4b95..59568584b 100644 --- a/src/client.rs +++ b/src/client.rs @@ -145,7 +145,6 @@ impl Client { /// Create a new client with given spec and DB path. pub fn new(spec: Spec, path: &Path, message_channel: IoChannel ) -> Result, Error> { let gb = spec.genesis_block(); - flushln!("Spec says genesis block is {}", gb.pretty()); let chain = Arc::new(RwLock::new(BlockChain::new(&gb, path))); let mut opts = Options::new(); opts.set_max_open_files(256); @@ -177,7 +176,7 @@ impl Client { if engine.spec().ensure_db_good(&mut state_db) { state_db.commit(0, &engine.spec().genesis_header().hash(), None).expect("Error commiting genesis state to state DB"); } - flushln!("Client::new: commiting. Best root now: {}. contains: {}", chain.read().unwrap().genesis_header().state_root, state_db.contains(&chain.read().unwrap().genesis_header().state_root)); + trace!("Client::new: commiting. Best root now: {}. contains: {}", chain.read().unwrap().genesis_header().state_root, state_db.contains(&chain.read().unwrap().genesis_header().state_root)); Ok(Arc::new(Client { chain: chain, engine: engine.clone(), diff --git a/src/evm/interpreter.rs b/src/evm/interpreter.rs index 2f43b5199..7657b1bbe 100644 --- a/src/evm/interpreter.rs +++ b/src/evm/interpreter.rs @@ -275,8 +275,6 @@ impl evm::Evm for Interpreter { code: &code }; - flushln!("Executing: {:?}", params); - while reader.position < code.len() { let instruction = code[reader.position]; reader.position += 1; @@ -642,7 +640,6 @@ impl Interpreter { return Ok(InstructionResult::StopExecution); }, instructions::SUICIDE => { - flushln!("SUICIDE!"); let address = stack.pop_back(); ext.suicide(&u256_to_address(&address)); return Ok(InstructionResult::StopExecution); diff --git a/src/executive.rs b/src/executive.rs index 9c3a669cf..07ab4c4c5 100644 --- a/src/executive.rs +++ b/src/executive.rs @@ -173,7 +173,7 @@ impl<'a> Executive<'a> { if let ActionValue::Transfer(val) = params.value { self.state.transfer_balance(¶ms.sender, ¶ms.address, &val); } - flushln!("Executive::call(params={:?}) self.env_info={:?}", params, self.info); + trace!("Executive::call(params={:?}) self.env_info={:?}", params, self.info); if self.engine.is_builtin(¶ms.code_address) { // if destination is builtin, try to execute it @@ -260,17 +260,16 @@ impl<'a> Executive<'a> { let refund_value = gas_left * t.gas_price; let fees_value = gas_used * t.gas_price; - flushln!("exec::finalize: t.gas={}, sstore_refunds={}, suicide_refunds={}, refunds_bound={}, gas_left_prerefund={}, refunded={}, gas_left={}, gas_used={}, refund_value={}, fees_value={}\n", + trace!("exec::finalize: t.gas={}, sstore_refunds={}, suicide_refunds={}, refunds_bound={}, gas_left_prerefund={}, refunded={}, gas_left={}, gas_used={}, refund_value={}, fees_value={}\n", t.gas, sstore_refunds, suicide_refunds, refunds_bound, gas_left_prerefund, refunded, gas_left, gas_used, refund_value, fees_value); - flushln!("exec::finalize: Refunding refund_value={}, sender={}\n", refund_value, t.sender().unwrap()); + trace!("exec::finalize: Refunding refund_value={}, sender={}\n", refund_value, t.sender().unwrap()); self.state.add_balance(&t.sender().unwrap(), &refund_value); - flushln!("exec::finalize: Compensating author: fees_value={}, author={}\n", fees_value, &self.info.author); + trace!("exec::finalize: Compensating author: fees_value={}, author={}\n", fees_value, &self.info.author); self.state.add_balance(&self.info.author, &fees_value); // perform suicides for address in &substate.suicides { - flushln!("Killing {}", address); self.state.kill_account(address); } diff --git a/src/externalities.rs b/src/externalities.rs index e7a73d553..f1b8c1958 100644 --- a/src/externalities.rs +++ b/src/externalities.rs @@ -215,7 +215,7 @@ impl<'a> Ext for Externalities<'a> { fn suicide(&mut self, refund_address: &Address) { let address = self.origin_info.address.clone(); let balance = self.balance(&address); - flushln!("Suiciding {} -> {} (xfer: {})", address, refund_address, balance); + trace!("Suiciding {} -> {} (xfer: {})", address, refund_address, balance); self.state.transfer_balance(&address, refund_address, &balance); self.substate.suicides.insert(address); } diff --git a/src/state.rs b/src/state.rs index 238638617..7310f63a7 100644 --- a/src/state.rs +++ b/src/state.rs @@ -103,7 +103,6 @@ impl State { /// Mutate storage of account `a` so that it is `value` for `key`. pub fn code(&self, a: &Address) -> Option { - flushln!("Getting code at {}", a); self.get(a, true).as_ref().map_or(None, |a|a.code().map(|x|x.to_vec())) } From f0da7bde2bcc82bc4b0ffd5310d2249e68f2262a Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 26 Jan 2016 15:33:24 +0100 Subject: [PATCH 3/8] Fix StateTests. Closes #214 --- src/block_queue.rs | 4 +++- src/client.rs | 6 +++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/block_queue.rs b/src/block_queue.rs index fa091d0c4..e0868c011 100644 --- a/src/block_queue.rs +++ b/src/block_queue.rs @@ -158,6 +158,7 @@ impl BlockQueue { }, Err(err) => { let mut v = verification.lock().unwrap(); + flushln!("Stage 2 block verification failed for {}\nError: {:?}", block_hash, err); warn!(target: "client", "Stage 2 block verification failed for {}\nError: {:?}", block_hash, err); v.bad.insert(block_hash.clone()); v.verifying.retain(|e| e.hash != block_hash); @@ -191,7 +192,7 @@ impl BlockQueue { /// Wait for queue to be empty pub fn flush(&mut self) { let mut verification = self.verification.lock().unwrap(); - while !verification.unverified.is_empty() && !verification.verifying.is_empty() { + while !verification.unverified.is_empty() || !verification.verifying.is_empty() { verification = self.empty.wait(verification).unwrap(); } } @@ -221,6 +222,7 @@ impl BlockQueue { self.more_to_verify.notify_all(); }, Err(err) => { + flushln!("Stage 1 block verification failed for {}\nError: {:?}", BlockView::new(&bytes).header_view().sha3(), err); warn!(target: "client", "Stage 1 block verification failed for {}\nError: {:?}", BlockView::new(&bytes).header_view().sha3(), err); self.verification.lock().unwrap().bad.insert(header.hash()); } diff --git a/src/client.rs b/src/client.rs index 59568584b..453f4ab64 100644 --- a/src/client.rs +++ b/src/client.rs @@ -199,6 +199,7 @@ impl Client { let _import_lock = self.import_lock.lock(); let blocks = self.block_queue.write().unwrap().drain(128); for block in blocks { + flushln!("Importing {}...", block.header.hash()); if bad.contains(&block.header.parent_hash) { self.block_queue.write().unwrap().mark_as_bad(&block.header.hash()); bad.insert(block.header.hash()); @@ -207,6 +208,7 @@ impl Client { let header = &block.header; if let Err(e) = verify_block_family(&header, &block.bytes, self.engine.deref().deref(), self.chain.read().unwrap().deref()) { + flushln!("Stage 3 block verification failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e); warn!(target: "client", "Stage 3 block verification failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e); self.block_queue.write().unwrap().mark_as_bad(&header.hash()); bad.insert(block.header.hash()); @@ -215,6 +217,7 @@ impl Client { let parent = match self.chain.read().unwrap().block_header(&header.parent_hash) { Some(p) => p, None => { + flushln!("Block import failed for #{} ({}): Parent not found ({}) ", header.number(), header.hash(), header.parent_hash); warn!(target: "client", "Block import failed for #{} ({}): Parent not found ({}) ", header.number(), header.hash(), header.parent_hash); self.block_queue.write().unwrap().mark_as_bad(&header.hash()); bad.insert(block.header.hash()); @@ -238,8 +241,9 @@ impl Client { let result = match enact_verified(&block, self.engine.deref().deref(), db, &parent, &last_hashes) { Ok(b) => b, Err(e) => { + flushln!("Block import failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e); warn!(target: "client", "Block import failed for #{} ({})\nError: {:?}", header.number(), header.hash(), e); - bad.insert(block.header.hash()); + bad.insert(block.header.hash()); self.block_queue.write().unwrap().mark_as_bad(&header.hash()); return; } From 9da88e99f84c6d3d73213f302496f43af5bec971 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 26 Jan 2016 15:37:14 +0100 Subject: [PATCH 4/8] Disable flushln for now. --- src/client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client.rs b/src/client.rs index 453f4ab64..9529e356c 100644 --- a/src/client.rs +++ b/src/client.rs @@ -199,7 +199,7 @@ impl Client { let _import_lock = self.import_lock.lock(); let blocks = self.block_queue.write().unwrap().drain(128); for block in blocks { - flushln!("Importing {}...", block.header.hash()); +// flushln!("Importing {}...", block.header.hash()); if bad.contains(&block.header.parent_hash) { self.block_queue.write().unwrap().mark_as_bad(&block.header.hash()); bad.insert(block.header.hash()); From 0e5d9ee2a00a2afd20e9a41086078ef36cd3ffdf Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 26 Jan 2016 15:39:49 +0100 Subject: [PATCH 5/8] Closes #215 --- src/tests/chain.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/chain.rs b/src/tests/chain.rs index 046c16052..e7b1759f2 100644 --- a/src/tests/chain.rs +++ b/src/tests/chain.rs @@ -48,7 +48,7 @@ fn do_json_test(json_data: &[u8]) -> Vec { failed } -declare_test!{ignore => BlockchainTests_bcBlockGasLimitTest, "BlockchainTests/bcBlockGasLimitTest"} // FAILS +declare_test!{BlockchainTests_bcBlockGasLimitTest, "BlockchainTests/bcBlockGasLimitTest"} declare_test!{BlockchainTests_bcForkBlockTest, "BlockchainTests/bcForkBlockTest"} declare_test!{ignore => BlockchainTests_bcForkStressTest, "BlockchainTests/bcForkStressTest"} // FAILS declare_test!{ignore => BlockchainTests_bcForkUncle, "BlockchainTests/bcForkUncle"} // FAILS @@ -57,7 +57,7 @@ declare_test!{BlockchainTests_bcInvalidHeaderTest, "BlockchainTests/bcInvalidHea declare_test!{ignore => BlockchainTests_bcInvalidRLPTest, "BlockchainTests/bcInvalidRLPTest"} // FAILS declare_test!{ignore => BlockchainTests_bcMultiChainTest, "BlockchainTests/bcMultiChainTest"} // FAILS declare_test!{BlockchainTests_bcRPC_API_Test, "BlockchainTests/bcRPC_API_Test"} -declare_test!{BlockchainTests_bcStateTest, "BlockchainTests/bcStateTest"} // FAILS (Suicides, GasUsed) +declare_test!{BlockchainTests_bcStateTest, "BlockchainTests/bcStateTest"} declare_test!{BlockchainTests_bcTotalDifficultyTest, "BlockchainTests/bcTotalDifficultyTest"} declare_test!{ignore => BlockchainTests_bcUncleHeaderValiditiy, "BlockchainTests/bcUncleHeaderValiditiy"} // FAILS declare_test!{ignore => BlockchainTests_bcUncleTest, "BlockchainTests/bcUncleTest"} // FAILS From 18f1d5e8514f38c337b2d6d3771147234172cf79 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 26 Jan 2016 17:16:06 +0100 Subject: [PATCH 6/8] Address issue on PR. --- src/client.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/client.rs b/src/client.rs index 9529e356c..a83ff554e 100644 --- a/src/client.rs +++ b/src/client.rs @@ -176,7 +176,6 @@ impl Client { if engine.spec().ensure_db_good(&mut state_db) { state_db.commit(0, &engine.spec().genesis_header().hash(), None).expect("Error commiting genesis state to state DB"); } - trace!("Client::new: commiting. Best root now: {}. contains: {}", chain.read().unwrap().genesis_header().state_root, state_db.contains(&chain.read().unwrap().genesis_header().state_root)); Ok(Arc::new(Client { chain: chain, engine: engine.clone(), From 8a665fe313870015efd484e57d909730ebfc333e Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 26 Jan 2016 18:02:49 +0100 Subject: [PATCH 7/8] Tests check block format. --- src/ethereum/ethash.rs | 4 ++++ src/tests/chain.rs | 33 +++++++++++++++++++++++---------- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/src/ethereum/ethash.rs b/src/ethereum/ethash.rs index aebee1e92..057a67d20 100644 --- a/src/ethereum/ethash.rs +++ b/src/ethereum/ethash.rs @@ -99,6 +99,10 @@ impl Engine for Ethash { } fn verify_block_basic(&self, header: &Header, _block: Option<&[u8]>) -> result::Result<(), Error> { + // check the seal fields. + try!(UntrustedRlp::new(&header.seal[0]).as_val::()); + try!(UntrustedRlp::new(&header.seal[1]).as_val::()); + let min_difficulty = decode(self.spec().engine_params.get("minimumDifficulty").unwrap()); if header.difficulty < min_difficulty { return Err(From::from(BlockError::InvalidDifficulty(Mismatch { expected: min_difficulty, found: header.difficulty }))) diff --git a/src/tests/chain.rs b/src/tests/chain.rs index e7b1759f2..8be1b1c92 100644 --- a/src/tests/chain.rs +++ b/src/tests/chain.rs @@ -31,9 +31,22 @@ fn do_json_test(json_data: &[u8]) -> Vec { dir.push(H32::random().hex()); { let client = Client::new(spec, &dir, IoChannel::disconnected()).unwrap(); - blocks.into_iter().foreach(|b| { + for b in blocks.into_iter() { + { + let urlp = UntrustedRlp::new(&b); + if !urlp.is_list() || urlp.item_count() != 3 || urlp.size() != b.len() { continue; } + if urlp.val_at::
(0).is_err() { continue; } + if !urlp.at(1).unwrap().is_list() { continue; } + if urlp.at(1).unwrap().iter().find(|i| i.as_val::().is_err()).is_some() { + continue; + } + if !urlp.at(2).unwrap().is_list() { continue; } + if urlp.at(2).unwrap().iter().find(|i| i.as_val::
().is_err()).is_some() { + continue; + } + } client.import_block(b).unwrap(); - }); + } client.flush_queue(); client.import_verified_blocks(&IoChannel::disconnected()); fail_unless(client.chain_info().best_block_hash == H256::from_json(&test["lastblockhash"])); @@ -50,16 +63,16 @@ fn do_json_test(json_data: &[u8]) -> Vec { declare_test!{BlockchainTests_bcBlockGasLimitTest, "BlockchainTests/bcBlockGasLimitTest"} declare_test!{BlockchainTests_bcForkBlockTest, "BlockchainTests/bcForkBlockTest"} -declare_test!{ignore => BlockchainTests_bcForkStressTest, "BlockchainTests/bcForkStressTest"} // FAILS -declare_test!{ignore => BlockchainTests_bcForkUncle, "BlockchainTests/bcForkUncle"} // FAILS +declare_test!{BlockchainTests_bcForkStressTest, "BlockchainTests/bcForkStressTest"} // STILL FAILS +declare_test!{BlockchainTests_bcForkUncle, "BlockchainTests/bcForkUncle"} // STILL FAILS declare_test!{BlockchainTests_bcGasPricerTest, "BlockchainTests/bcGasPricerTest"} declare_test!{BlockchainTests_bcInvalidHeaderTest, "BlockchainTests/bcInvalidHeaderTest"} -declare_test!{ignore => BlockchainTests_bcInvalidRLPTest, "BlockchainTests/bcInvalidRLPTest"} // FAILS -declare_test!{ignore => BlockchainTests_bcMultiChainTest, "BlockchainTests/bcMultiChainTest"} // FAILS +declare_test!{BlockchainTests_bcInvalidRLPTest, "BlockchainTests/bcInvalidRLPTest"} // FAILS +declare_test!{BlockchainTests_bcMultiChainTest, "BlockchainTests/bcMultiChainTest"} // FAILS declare_test!{BlockchainTests_bcRPC_API_Test, "BlockchainTests/bcRPC_API_Test"} declare_test!{BlockchainTests_bcStateTest, "BlockchainTests/bcStateTest"} declare_test!{BlockchainTests_bcTotalDifficultyTest, "BlockchainTests/bcTotalDifficultyTest"} -declare_test!{ignore => BlockchainTests_bcUncleHeaderValiditiy, "BlockchainTests/bcUncleHeaderValiditiy"} // FAILS -declare_test!{ignore => BlockchainTests_bcUncleTest, "BlockchainTests/bcUncleTest"} // FAILS -declare_test!{ignore => BlockchainTests_bcValidBlockTest, "BlockchainTests/bcValidBlockTest"} // FAILS -declare_test!{ignore => BlockchainTests_bcWalletTest, "BlockchainTests/bcWalletTest"} // FAILS +declare_test!{BlockchainTests_bcUncleHeaderValiditiy, "BlockchainTests/bcUncleHeaderValiditiy"} // FAILS +declare_test!{BlockchainTests_bcUncleTest, "BlockchainTests/bcUncleTest"} // FAILS +declare_test!{BlockchainTests_bcValidBlockTest, "BlockchainTests/bcValidBlockTest"} // FAILS +declare_test!{BlockchainTests_bcWalletTest, "BlockchainTests/bcWalletTest"} // FAILS From e904d2145f05d8ca56ee9fd7c6b3411b5f6f2218 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 26 Jan 2016 19:18:22 +0100 Subject: [PATCH 8/8] Block refactoring, stricter RLP. Fixed #234. Partially fixes #233 for Blocks. Fixed #222. --- src/block.rs | 199 +++++++++++++++++++++------------- src/engine.rs | 6 +- src/ethereum/ethash.rs | 2 +- src/header.rs | 2 +- src/log_entry.rs | 2 +- src/receipt.rs | 2 +- src/tests/chain.rs | 16 +-- src/transaction.rs | 8 +- util/src/bytes.rs | 11 +- util/src/rlp/rlperrors.rs | 2 + util/src/rlp/untrusted_rlp.rs | 3 + 11 files changed, 151 insertions(+), 102 deletions(-) diff --git a/src/block.rs b/src/block.rs index 1ff326430..83780ab43 100644 --- a/src/block.rs +++ b/src/block.rs @@ -5,80 +5,124 @@ use engine::*; use state::*; use verification::PreVerifiedBlock; -/// A transaction/receipt execution entry. -pub struct Entry { - transaction: Transaction, - receipt: Receipt, +/// A block, encoded as it is on the block chain. +// TODO: rename to Block +#[derive(Default, Debug, Clone)] +pub struct Block { + /// The header of this block. + pub header: Header, + /// The transactions in this block. + pub transactions: Vec, + /// The uncles of this block. + pub uncles: Vec
, +} + +impl Block { + /// Returns true iff the given bytes form a valid encoding of a block in RLP. + // TODO: implement Decoder for this and have this use that. + pub fn is_good(b: &[u8]) -> bool { + /* + let urlp = UntrustedRlp::new(&b); + if !urlp.is_list() || urlp.item_count() != 3 || urlp.size() != b.len() { return false; } + if urlp.val_at::
(0).is_err() { return false; } + + if !urlp.at(1).unwrap().is_list() { return false; } + if urlp.at(1).unwrap().iter().find(|i| i.as_val::().is_err()).is_some() { + return false; + } + + if !urlp.at(2).unwrap().is_list() { return false; } + if urlp.at(2).unwrap().iter().find(|i| i.as_val::
().is_err()).is_some() { + return false; + } + true*/ + UntrustedRlp::new(b).as_val::().is_ok() + } +} + +impl Decodable for Block { + fn decode(decoder: &D) -> Result where D: Decoder { + if decoder.as_raw().len() != try!(decoder.as_rlp().payload_info()).total() { + return Err(DecoderError::RlpIsTooBig); + } + let d = try!(decoder.as_list()); + if d.len() != 3 { + return Err(DecoderError::RlpIncorrectListLen); + } + Ok(Block { + header: try!(Decodable::decode(&d[0])), + transactions: try!(Decodable::decode(&d[1])), + uncles: try!(Decodable::decode(&d[2])), + }) + } } /// Internal type for a block's common elements. -pub struct Block { - header: Header, +// TODO: rename to ExecutedBlock +// TODO: use BareBlock +#[derive(Debug, Clone)] +pub struct ExecutedBlock { + base: Block, - /// State is the most final state in the block. + receipts: Vec, + transactions_set: HashSet, state: State, - - archive: Vec, - archive_set: HashSet, - - uncles: Vec
, } -/// A set of references to `Block` fields that are publicly accessible. +/// A set of references to `ExecutedBlock` fields that are publicly accessible. pub struct BlockRefMut<'a> { /// TODO [Gav Wood] Please document me pub header: &'a Header, /// TODO [Gav Wood] Please document me - pub state: &'a mut State, - /// TODO [Gav Wood] Please document me - pub archive: &'a Vec, + pub transactions: &'a Vec, /// TODO [Gav Wood] Please document me pub uncles: &'a Vec
, + + /// TODO [Gav Wood] Please document me + pub receipts: &'a Vec, + /// TODO [Gav Wood] Please document me + pub state: &'a mut State, } -impl Block { +impl ExecutedBlock { /// Create a new block from the given `state`. - fn new(state: State) -> Block { - Block { - header: Header::new(), - state: state, - archive: Vec::new(), - archive_set: HashSet::new(), - uncles: Vec::new(), - } - } + fn new(state: State) -> ExecutedBlock { ExecutedBlock { base: Default::default(), receipts: Default::default(), transactions_set: Default::default(), state: state } } /// Get a structure containing individual references to all public fields. pub fn fields(&mut self) -> BlockRefMut { BlockRefMut { - header: &self.header, + header: &self.base.header, + transactions: &self.base.transactions, + uncles: &self.base.uncles, state: &mut self.state, - archive: &self.archive, - uncles: &self.uncles, + receipts: &self.receipts, } } } -/// Trait for a object that is_a `Block`. +/// Trait for a object that is_a `ExecutedBlock`. pub trait IsBlock { /// Get the block associated with this object. - fn block(&self) -> &Block; + fn block(&self) -> &ExecutedBlock; /// Get the header associated with this object's block. - fn header(&self) -> &Header { &self.block().header } + fn header(&self) -> &Header { &self.block().base.header } /// Get the final state associated with this object's block. fn state(&self) -> &State { &self.block().state } /// Get all information on transactions in this block. - fn archive(&self) -> &Vec { &self.block().archive } + fn transactions(&self) -> &Vec { &self.block().base.transactions } + + /// Get all information on receipts in this block. + fn receipts(&self) -> &Vec { &self.block().receipts } /// Get all uncles in this block. - fn uncles(&self) -> &Vec
{ &self.block().uncles } + fn uncles(&self) -> &Vec
{ &self.block().base.uncles } } -impl IsBlock for Block { - fn block(&self) -> &Block { self } +impl IsBlock for ExecutedBlock { + fn block(&self) -> &ExecutedBlock { self } } /// Block that is ready for transactions to be added. @@ -86,7 +130,7 @@ impl IsBlock for Block { /// It's a bit like a Vec, eccept that whenever a transaction is pushed, we execute it and /// maintain the system `state()`. We also archive execution receipts in preparation for later block creation. pub struct OpenBlock<'x, 'y> { - block: Block, + block: ExecutedBlock, engine: &'x Engine, last_hashes: &'y LastHashes, } @@ -104,7 +148,7 @@ pub struct ClosedBlock<'x, 'y> { /// /// The block's header has valid seal arguments. The block cannot be reversed into a ClosedBlock or OpenBlock. pub struct SealedBlock { - block: Block, + block: ExecutedBlock, uncle_bytes: Bytes, } @@ -112,42 +156,42 @@ impl<'x, 'y> OpenBlock<'x, 'y> { /// Create a new OpenBlock ready for transaction pushing. pub fn new<'a, 'b>(engine: &'a Engine, db: JournalDB, parent: &Header, last_hashes: &'b LastHashes, author: Address, extra_data: Bytes) -> OpenBlock<'a, 'b> { let mut r = OpenBlock { - block: Block::new(State::from_existing(db, parent.state_root().clone(), engine.account_start_nonce())), + block: ExecutedBlock::new(State::from_existing(db, parent.state_root().clone(), engine.account_start_nonce())), engine: engine, last_hashes: last_hashes, }; - r.block.header.set_number(parent.number() + 1); - r.block.header.set_author(author); - r.block.header.set_extra_data(extra_data); - r.block.header.set_timestamp_now(); + r.block.base.header.set_number(parent.number() + 1); + r.block.base.header.set_author(author); + r.block.base.header.set_extra_data(extra_data); + r.block.base.header.set_timestamp_now(); - engine.populate_from_parent(&mut r.block.header, parent); + engine.populate_from_parent(&mut r.block.base.header, parent); engine.on_new_block(&mut r.block); r } /// Alter the author for the block. - pub fn set_author(&mut self, author: Address) { self.block.header.set_author(author); } + pub fn set_author(&mut self, author: Address) { self.block.base.header.set_author(author); } /// Alter the timestamp of the block. - pub fn set_timestamp(&mut self, timestamp: u64) { self.block.header.set_timestamp(timestamp); } + pub fn set_timestamp(&mut self, timestamp: u64) { self.block.base.header.set_timestamp(timestamp); } /// Alter the difficulty for the block. - pub fn set_difficulty(&mut self, a: U256) { self.block.header.set_difficulty(a); } + pub fn set_difficulty(&mut self, a: U256) { self.block.base.header.set_difficulty(a); } /// Alter the gas limit for the block. - pub fn set_gas_limit(&mut self, a: U256) { self.block.header.set_gas_limit(a); } + pub fn set_gas_limit(&mut self, a: U256) { self.block.base.header.set_gas_limit(a); } /// Alter the gas limit for the block. - pub fn set_gas_used(&mut self, a: U256) { self.block.header.set_gas_used(a); } + pub fn set_gas_used(&mut self, a: U256) { self.block.base.header.set_gas_used(a); } /// Alter the extra_data for the block. pub fn set_extra_data(&mut self, extra_data: Bytes) -> Result<(), BlockError> { if extra_data.len() > self.engine.maximum_extra_data_size() { Err(BlockError::ExtraDataOutOfBounds(OutOfBounds{min: None, max: Some(self.engine.maximum_extra_data_size()), found: extra_data.len()})) } else { - self.block.header.set_extra_data(extra_data); + self.block.base.header.set_extra_data(extra_data); Ok(()) } } @@ -157,12 +201,12 @@ impl<'x, 'y> OpenBlock<'x, 'y> { /// NOTE Will check chain constraints and the uncle number but will NOT check /// that the header itself is actually valid. pub fn push_uncle(&mut self, valid_uncle_header: Header) -> Result<(), BlockError> { - if self.block.uncles.len() >= self.engine.maximum_uncle_count() { - return Err(BlockError::TooManyUncles(OutOfBounds{min: None, max: Some(self.engine.maximum_uncle_count()), found: self.block.uncles.len()})); + if self.block.base.uncles.len() >= self.engine.maximum_uncle_count() { + return Err(BlockError::TooManyUncles(OutOfBounds{min: None, max: Some(self.engine.maximum_uncle_count()), found: self.block.base.uncles.len()})); } // TODO: check number // TODO: check not a direct ancestor (use last_hashes for that) - self.block.uncles.push(valid_uncle_header); + self.block.base.uncles.push(valid_uncle_header); Ok(()) } @@ -170,13 +214,13 @@ impl<'x, 'y> OpenBlock<'x, 'y> { pub fn env_info(&self) -> EnvInfo { // TODO: memoise. EnvInfo { - number: self.block.header.number, - author: self.block.header.author.clone(), - timestamp: self.block.header.timestamp, - difficulty: self.block.header.difficulty.clone(), + number: self.block.base.header.number, + author: self.block.base.header.author.clone(), + timestamp: self.block.base.header.timestamp, + difficulty: self.block.base.header.difficulty.clone(), last_hashes: self.last_hashes.clone(), // TODO: should be a reference. - gas_used: self.block.archive.last().map_or(U256::zero(), |t| t.receipt.gas_used), - gas_limit: self.block.header.gas_limit.clone(), + gas_used: self.block.receipts.last().map_or(U256::zero(), |r| r.gas_used), + gas_limit: self.block.base.header.gas_limit.clone(), } } @@ -188,9 +232,10 @@ impl<'x, 'y> OpenBlock<'x, 'y> { // info!("env_info says gas_used={}", env_info.gas_used); match self.block.state.apply(&env_info, self.engine, &t) { Ok(receipt) => { - self.block.archive_set.insert(h.unwrap_or_else(||t.hash())); - self.block.archive.push(Entry { transaction: t, receipt: receipt }); - Ok(&self.block.archive.last().unwrap().receipt) + self.block.transactions_set.insert(h.unwrap_or_else(||t.hash())); + self.block.base.transactions.push(t); + self.block.receipts.push(receipt); + Ok(&self.block.receipts.last().unwrap()) } Err(x) => Err(From::from(x)) } @@ -200,25 +245,25 @@ impl<'x, 'y> OpenBlock<'x, 'y> { pub fn close(self) -> ClosedBlock<'x, 'y> { let mut s = self; s.engine.on_close_block(&mut s.block); - s.block.header.transactions_root = ordered_trie_root(s.block.archive.iter().map(|ref e| e.transaction.rlp_bytes()).collect()); - let uncle_bytes = s.block.uncles.iter().fold(RlpStream::new_list(s.block.uncles.len()), |mut s, u| {s.append(&u.rlp(Seal::With)); s} ).out(); - s.block.header.uncles_hash = uncle_bytes.sha3(); - s.block.header.state_root = s.block.state.root().clone(); - s.block.header.receipts_root = ordered_trie_root(s.block.archive.iter().map(|ref e| e.receipt.rlp_bytes()).collect()); - s.block.header.log_bloom = s.block.archive.iter().fold(LogBloom::zero(), |mut b, e| {b |= &e.receipt.log_bloom; b}); - s.block.header.gas_used = s.block.archive.last().map_or(U256::zero(), |t| t.receipt.gas_used); - s.block.header.note_dirty(); + s.block.base.header.transactions_root = ordered_trie_root(s.block.base.transactions.iter().map(|ref e| e.rlp_bytes()).collect()); + let uncle_bytes = s.block.base.uncles.iter().fold(RlpStream::new_list(s.block.base.uncles.len()), |mut s, u| {s.append(&u.rlp(Seal::With)); s} ).out(); + s.block.base.header.uncles_hash = uncle_bytes.sha3(); + s.block.base.header.state_root = s.block.state.root().clone(); + s.block.base.header.receipts_root = ordered_trie_root(s.block.receipts.iter().map(|ref r| r.rlp_bytes()).collect()); + s.block.base.header.log_bloom = s.block.receipts.iter().fold(LogBloom::zero(), |mut b, r| {b |= &r.log_bloom; b}); + s.block.base.header.gas_used = s.block.receipts.last().map_or(U256::zero(), |r| r.gas_used); + s.block.base.header.note_dirty(); ClosedBlock::new(s, uncle_bytes) } } impl<'x, 'y> IsBlock for OpenBlock<'x, 'y> { - fn block(&self) -> &Block { &self.block } + fn block(&self) -> &ExecutedBlock { &self.block } } impl<'x, 'y> IsBlock for ClosedBlock<'x, 'y> { - fn block(&self) -> &Block { &self.open_block.block } + fn block(&self) -> &ExecutedBlock { &self.open_block.block } } impl<'x, 'y> ClosedBlock<'x, 'y> { @@ -240,7 +285,7 @@ impl<'x, 'y> ClosedBlock<'x, 'y> { if seal.len() != s.open_block.engine.seal_fields() { return Err(BlockError::InvalidSealArity(Mismatch{expected: s.open_block.engine.seal_fields(), found: seal.len()})); } - s.open_block.block.header.set_seal(seal); + s.open_block.block.base.header.set_seal(seal); Ok(SealedBlock { block: s.open_block.block, uncle_bytes: s.uncle_bytes }) } @@ -255,9 +300,9 @@ impl SealedBlock { /// Get the RLP-encoding of the block. pub fn rlp_bytes(&self) -> Bytes { let mut block_rlp = RlpStream::new_list(3); - self.block.header.stream_rlp(&mut block_rlp, Seal::With); - block_rlp.append_list(self.block.archive.len()); - for e in &self.block.archive { e.transaction.rlp_append(&mut block_rlp); } + self.block.base.header.stream_rlp(&mut block_rlp, Seal::With); + block_rlp.append_list(self.block.receipts.len()); + for t in &self.block.base.transactions { t.rlp_append(&mut block_rlp); } block_rlp.append_raw(&self.uncle_bytes, 1); block_rlp.out() } @@ -267,7 +312,7 @@ impl SealedBlock { } impl IsBlock for SealedBlock { - fn block(&self) -> &Block { &self.block } + fn block(&self) -> &ExecutedBlock { &self.block } } /// Enact the block given by block header, transactions and uncles diff --git a/src/engine.rs b/src/engine.rs index d94797290..1fb6ef0ca 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1,5 +1,5 @@ use common::*; -use block::Block; +use block::ExecutedBlock; use spec::Spec; use evm::Schedule; use evm::Factory; @@ -37,9 +37,9 @@ pub trait Engine : Sync + Send { fn account_start_nonce(&self) -> U256 { decode(&self.spec().engine_params.get("accountStartNonce").unwrap()) } /// Block transformation functions, before and after the transactions. - fn on_new_block(&self, _block: &mut Block) {} + fn on_new_block(&self, _block: &mut ExecutedBlock) {} /// TODO [Gav Wood] Please document me - fn on_close_block(&self, _block: &mut Block) {} + fn on_close_block(&self, _block: &mut ExecutedBlock) {} // TODO: consider including State in the params for verification functions. /// Phase 1 quick block verification. Only does checks that are cheap. `block` (the header's full block) diff --git a/src/ethereum/ethash.rs b/src/ethereum/ethash.rs index 057a67d20..a677a86cc 100644 --- a/src/ethereum/ethash.rs +++ b/src/ethereum/ethash.rs @@ -83,7 +83,7 @@ impl Engine for Ethash { /// Apply the block reward on finalisation of the block. /// This assumes that all uncles are valid uncles (i.e. of at least one generation before the current). - fn on_close_block(&self, block: &mut Block) { + fn on_close_block(&self, block: &mut ExecutedBlock) { let reward = self.spec().engine_params.get("blockReward").map_or(U256::from(0u64), |a| decode(&a)); let fields = block.fields(); diff --git a/src/header.rs b/src/header.rs index 68526d8c6..a57d4166d 100644 --- a/src/header.rs +++ b/src/header.rs @@ -11,7 +11,7 @@ pub type BlockNumber = u64; /// which is non-specific. /// /// Doesn't do all that much on its own. -#[derive(Debug, Clone)] +#[derive(Default, Debug, Clone)] pub struct Header { // TODO: make all private. /// TODO [Gav Wood] Please document me diff --git a/src/log_entry.rs b/src/log_entry.rs index a791b38a6..be39a72f2 100644 --- a/src/log_entry.rs +++ b/src/log_entry.rs @@ -2,7 +2,7 @@ use util::*; use basic_types::LogBloom; /// A single log's entry. -#[derive(Debug,PartialEq,Eq)] +#[derive(Default, Debug, Clone, PartialEq, Eq)] pub struct LogEntry { /// TODO [Gav Wood] Please document me pub address: Address, diff --git a/src/receipt.rs b/src/receipt.rs index 403915fdc..4389de962 100644 --- a/src/receipt.rs +++ b/src/receipt.rs @@ -3,7 +3,7 @@ use basic_types::LogBloom; use log_entry::LogEntry; /// Information describing execution of a transaction. -#[derive(Debug)] +#[derive(Default, Debug, Clone)] pub struct Receipt { /// TODO [Gav Wood] Please document me pub state_root: H256, diff --git a/src/tests/chain.rs b/src/tests/chain.rs index 8be1b1c92..ec899c73b 100644 --- a/src/tests/chain.rs +++ b/src/tests/chain.rs @@ -2,6 +2,7 @@ use std::env; use super::test_common::*; use client::{BlockChainClient,Client}; use pod_state::*; +use block::Block; use ethereum; fn do_json_test(json_data: &[u8]) -> Vec { @@ -31,20 +32,7 @@ fn do_json_test(json_data: &[u8]) -> Vec { dir.push(H32::random().hex()); { let client = Client::new(spec, &dir, IoChannel::disconnected()).unwrap(); - for b in blocks.into_iter() { - { - let urlp = UntrustedRlp::new(&b); - if !urlp.is_list() || urlp.item_count() != 3 || urlp.size() != b.len() { continue; } - if urlp.val_at::
(0).is_err() { continue; } - if !urlp.at(1).unwrap().is_list() { continue; } - if urlp.at(1).unwrap().iter().find(|i| i.as_val::().is_err()).is_some() { - continue; - } - if !urlp.at(2).unwrap().is_list() { continue; } - if urlp.at(2).unwrap().iter().find(|i| i.as_val::
().is_err()).is_some() { - continue; - } - } + for b in blocks.into_iter().filter(|ref b| Block::is_good(b)) { client.import_block(b).unwrap(); } client.flush_queue(); diff --git a/src/transaction.rs b/src/transaction.rs index 47b8fa91f..cacdf2a25 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -3,7 +3,7 @@ use basic_types::*; use error::*; use evm::Schedule; -#[derive(Debug,Clone)] +#[derive(Debug, Clone)] /// TODO [Gav Wood] Please document me pub enum Action { /// TODO [Gav Wood] Please document me @@ -12,9 +12,13 @@ pub enum Action { Call(Address), } +impl Default for Action { + fn default() -> Action { Action::Create } +} + /// A set of information describing an externally-originating message call /// or contract creation operation. -#[derive(Debug,Clone)] +#[derive(Default, Debug, Clone)] pub struct Transaction { /// TODO [debris] Please document me pub nonce: U256, diff --git a/util/src/bytes.rs b/util/src/bytes.rs index a30581a1f..3144dd482 100644 --- a/util/src/bytes.rs +++ b/util/src/bytes.rs @@ -273,7 +273,9 @@ pub enum FromBytesError { /// TODO [debris] Please document me DataIsTooShort, /// TODO [debris] Please document me - DataIsTooLong + DataIsTooLong, + /// Integer-representation is non-canonically prefixed with zero byte(s). + ZeroPrefixedInt, } impl StdError for FromBytesError { @@ -310,6 +312,9 @@ macro_rules! impl_uint_from_bytes { match bytes.len() { 0 => Ok(0), l if l <= mem::size_of::<$to>() => { + if bytes[0] == 0 { + return Err(FromBytesError::ZeroPrefixedInt) + } let mut res = 0 as $to; for i in 0..l { let shift = (l - 1 - i) * 8; @@ -344,7 +349,9 @@ macro_rules! impl_uint_from_bytes { ($name: ident) => { impl FromBytes for $name { fn from_bytes(bytes: &[u8]) -> FromBytesResult<$name> { - if bytes.len() <= $name::SIZE { + if !bytes.is_empty() && bytes[0] == 0 { + Err(FromBytesError::ZeroPrefixedInt) + } else if bytes.len() <= $name::SIZE { Ok($name::from(bytes)) } else { Err(FromBytesError::DataIsTooLong) diff --git a/util/src/rlp/rlperrors.rs b/util/src/rlp/rlperrors.rs index 97adbced1..8946098bd 100644 --- a/util/src/rlp/rlperrors.rs +++ b/util/src/rlp/rlperrors.rs @@ -7,6 +7,8 @@ use bytes::FromBytesError; pub enum DecoderError { /// TODO [debris] Please document me FromBytesError(FromBytesError), + /// Given data has additional bytes at the end of the valid RLP fragment. + RlpIsTooBig, /// TODO [debris] Please document me RlpIsTooShort, /// TODO [debris] Please document me diff --git a/util/src/rlp/untrusted_rlp.rs b/util/src/rlp/untrusted_rlp.rs index 768d058c1..d7921a25b 100644 --- a/util/src/rlp/untrusted_rlp.rs +++ b/util/src/rlp/untrusted_rlp.rs @@ -46,6 +46,9 @@ impl PayloadInfo { value_len: value_len, } } + + /// Total size of the RLP. + pub fn total(&self) -> usize { self.header_len + self.value_len } } /// Data-oriented view onto rlp-slice.