From 6c24d9897ae63d2fb8f9d132b95fda2871e27ff2 Mon Sep 17 00:00:00 2001 From: David Date: Thu, 24 May 2018 17:29:28 +0200 Subject: [PATCH 1/3] Set the request index to that of the current request (#8683) * Set the request index to that of the current request When setting up the chain of (two) requests to look up a block by hash, the second need to refer to the first. This fixes an issue where the back ref was set to the subsequent request, not the current one. When the requests are executed we loop through them in order and ensure the requests that should produce headers all match up. We do this by index so they better be right. In other words: off by one. --- rpc/src/v1/helpers/light_fetch.rs | 3 +-- rpc/src/v1/tests/eth.rs | 12 ++++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/rpc/src/v1/helpers/light_fetch.rs b/rpc/src/v1/helpers/light_fetch.rs index 413670d5210..1baf9a76471 100644 --- a/rpc/src/v1/helpers/light_fetch.rs +++ b/rpc/src/v1/helpers/light_fetch.rs @@ -129,9 +129,8 @@ impl LightFetch { } } BlockId::Hash(h) => { - reqs.push(request::HeaderByHash(h.into()).into()); - let idx = reqs.len(); + reqs.push(request::HeaderByHash(h.into()).into()); Ok(HeaderRef::Unresolved(idx, h.into())) } _ => Err(errors::unknown_block()) // latest, earliest, and pending will have all already returned. diff --git a/rpc/src/v1/tests/eth.rs b/rpc/src/v1/tests/eth.rs index 4b83710bc02..26117471cee 100644 --- a/rpc/src/v1/tests/eth.rs +++ b/rpc/src/v1/tests/eth.rs @@ -204,6 +204,18 @@ fn eth_get_block() { assert_eq!(tester.handler.handle_request_sync(req_block).unwrap(), res_block); } +#[test] +fn eth_get_block_by_hash() { + let chain = extract_chain!("BlockchainTests/bcGasPricerTest/RPC_API_Test"); + let tester = EthTester::from_chain(&chain); + + // We're looking for block number 4 from "RPC_API_Test_Frontier" + let req_block = r#"{"method":"eth_getBlockByHash","params":["0x9c9bdab4cb53fd834e790b13545597f026494d42112e84c0aca9dd6bcc545295",false],"id":1,"jsonrpc":"2.0"}"#; + + let res_block = r#"{"jsonrpc":"2.0","result":{"author":"0x8888f1f195afa192cfee860698584c030f4c9db1","difficulty":"0x200c0","extraData":"0x","gasLimit":"0x1dd8112","gasUsed":"0x5458","hash":"0x9c9bdab4cb53fd834e790b13545597f026494d42112e84c0aca9dd6bcc545295","logsBloom":"0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000","miner":"0x8888f1f195afa192cfee860698584c030f4c9db1","mixHash":"0xaddea8d25bb0f955fa6c1d58d74ab8a3fec99d37943e2a261e3b12f97d6bff7c","nonce":"0x8e18bed16d5a88da","number":"0x4","parentHash":"0x2cbf4fc930c5b4c87598f43fc8eb26dccdab2f58a7d0d3ca92ec60a5444a330e","receiptsRoot":"0x7ed8026cf72ed0e98e6fd53ab406e51ffd34397d9da0052494ff41376fda7b5f","sealFields":["0xa0addea8d25bb0f955fa6c1d58d74ab8a3fec99d37943e2a261e3b12f97d6bff7c","0x888e18bed16d5a88da"],"sha3Uncles":"0x75cc08a7cb2cf8081446659fecb2633fb6b922d26edd59bd2272b1f5cae1c78b","size":"0x661","stateRoot":"0x68805721294e365020aca15ed56c360d9dc2cf03cbeff84c9b84b8aed023bfb5","timestamp":"0x59d662ff","totalDifficulty":"0xa0180","transactions":["0xb094b9dc356dbb8b256402c6d5709288066ad6a372c90c9c516f14277545fd58"],"transactionsRoot":"0x97a593d8d7e15b57f5c6bb25bc6c325463ef99f874bc08a78656c3ab5cb23262","uncles":["0xa1e9c9ecd2af999e0723aae1dc55dd9789ca618e0b34badcc8ac7d9a3dad3af2","0x81d429b6b6635214a2b0f976cc4b2ed49808140d6bede50129bc10d22ac9249e"]},"id":1}"#; + assert_eq!(tester.handler.handle_request_sync(req_block).unwrap(), res_block); +} + // a frontier-like test with an expanded gas limit and balance on known account. const TRANSACTION_COUNT_SPEC: &'static [u8] = br#"{ "name": "Frontier (Test)", From 80528c53445ed958ad6164a49ac1cf28e8f52c56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Thu, 24 May 2018 19:43:18 +0200 Subject: [PATCH 2/3] parity: trim whitespace when parsing duration strings (#8692) --- parity/helpers.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/parity/helpers.rs b/parity/helpers.rs index 80ade509834..a5ec3c99d4b 100644 --- a/parity/helpers.rs +++ b/parity/helpers.rs @@ -47,11 +47,11 @@ fn to_seconds(s: &str) -> Result { "1minute" | "1 minute" | "minute" => Ok(60), "hourly" | "1hour" | "1 hour" | "hour" => Ok(60 * 60), "daily" | "1day" | "1 day" | "day" => Ok(24 * 60 * 60), - x if x.ends_with("seconds") => x[0..x.len() - 7].parse().map_err(bad), - x if x.ends_with("minutes") => x[0..x.len() - 7].parse::().map_err(bad).map(|x| x * 60), - x if x.ends_with("hours") => x[0..x.len() - 5].parse::().map_err(bad).map(|x| x * 60 * 60), - x if x.ends_with("days") => x[0..x.len() - 4].parse::().map_err(bad).map(|x| x * 24 * 60 * 60), - x => x.parse().map_err(bad), + x if x.ends_with("seconds") => x[0..x.len() - 7].trim().parse().map_err(bad), + x if x.ends_with("minutes") => x[0..x.len() - 7].trim().parse::().map_err(bad).map(|x| x * 60), + x if x.ends_with("hours") => x[0..x.len() - 5].trim().parse::().map_err(bad).map(|x| x * 60 * 60), + x if x.ends_with("days") => x[0..x.len() - 4].trim().parse::().map_err(bad).map(|x| x * 24 * 60 * 60), + x => x.trim().parse().map_err(bad), } } @@ -350,6 +350,8 @@ mod tests { assert_eq!(to_duration("1day").unwrap(), Duration::from_secs(1 * 24 * 60 * 60)); assert_eq!(to_duration("2days").unwrap(), Duration::from_secs(2 * 24 *60 * 60)); assert_eq!(to_duration("15days").unwrap(), Duration::from_secs(15 * 24 * 60 * 60)); + assert_eq!(to_duration("15 days").unwrap(), Duration::from_secs(15 * 24 * 60 * 60)); + assert_eq!(to_duration("2 seconds").unwrap(), Duration::from_secs(2)); } #[test] From 7d7d4822a5087b24b49f2a1ddb111f2daa51a017 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Fri, 25 May 2018 13:19:32 +0200 Subject: [PATCH 3/3] Implement recursive Debug for Nodes in patrica_trie::TrieDB (#8697) fixes #8184 --- util/patricia_trie/src/triedb.rs | 228 ++++++++++++++++++++++--------- 1 file changed, 164 insertions(+), 64 deletions(-) diff --git a/util/patricia_trie/src/triedb.rs b/util/patricia_trie/src/triedb.rs index aed683117d3..259525e2ee2 100644 --- a/util/patricia_trie/src/triedb.rs +++ b/util/patricia_trie/src/triedb.rs @@ -78,64 +78,9 @@ impl<'db> TrieDB<'db> { /// Get the data of the root node. fn root_data(&self) -> super::Result { - self.db.get(self.root).ok_or_else(|| Box::new(TrieError::InvalidStateRoot(*self.root))) - } - - /// Indentation helper for `format_all`. - fn fmt_indent(&self, f: &mut fmt::Formatter, size: usize) -> fmt::Result { - for _ in 0..size { - write!(f, " ")?; - } - Ok(()) - } - - /// Recursion helper for implementation of formatting trait. - fn fmt_all(&self, node: Node, f: &mut fmt::Formatter, deepness: usize) -> fmt::Result { - match node { - Node::Leaf(slice, value) => writeln!(f, "'{:?}: {:?}.", slice, value.pretty())?, - Node::Extension(ref slice, ref item) => { - write!(f, "'{:?} ", slice)?; - if let Ok(node) = self.get_raw_or_lookup(&*item) { - match Node::decoded(&node) { - Ok(n) => self.fmt_all(n, f, deepness)?, - Err(err) => writeln!(f, "ERROR decoding node extension Rlp: {}", err)?, - } - } - }, - Node::Branch(ref nodes, ref value) => { - writeln!(f, "")?; - if let Some(ref v) = *value { - self.fmt_indent(f, deepness + 1)?; - writeln!(f, "=: {:?}", v.pretty())? - } - for i in 0..16 { - let node = self.get_raw_or_lookup(&*nodes[i]); - match node.as_ref() { - Ok(n) => { - match Node::decoded(&*n) { - Ok(Node::Empty) => {}, - Ok(n) => { - self.fmt_indent(f, deepness + 1)?; - write!(f, "'{:x} ", i)?; - self.fmt_all(n, f, deepness + 1)?; - } - Err(e) => { - write!(f, "ERROR decoding node branch Rlp: {}", e)? - } - } - } - Err(e) => { - write!(f, "ERROR: {}", e)?; - } - } - } - }, - // empty - Node::Empty => { - writeln!(f, "")?; - } - }; - Ok(()) + self.db + .get(self.root) + .ok_or_else(|| Box::new(TrieError::InvalidStateRoot(*self.root))) } /// Given some node-describing data `node`, return the actual node RLP. @@ -174,15 +119,58 @@ impl<'db> Trie for TrieDB<'db> { } } +// This is for pretty debug output only +struct TrieAwareDebugNode<'db, 'a> { + trie: &'db TrieDB<'db>, + key: &'a[u8] +} + +impl<'db, 'a> fmt::Debug for TrieAwareDebugNode<'db, 'a> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + if let Ok(node) = self.trie.get_raw_or_lookup(self.key) { + match Node::decoded(&node) { + Ok(Node::Leaf(slice, value)) => f.debug_struct("Node::Leaf") + .field("slice", &slice) + .field("value", &value) + .finish(), + Ok(Node::Extension(ref slice, ref item)) => f.debug_struct("Node::Extension") + .field("slice", &slice) + .field("item", &TrieAwareDebugNode{trie: self.trie, key: item}) + .finish(), + Ok(Node::Branch(ref nodes, ref value)) => { + let nodes: Vec = nodes.into_iter().map(|n| TrieAwareDebugNode{trie: self.trie, key: n} ).collect(); + f.debug_struct("Node::Branch") + .field("nodes", &nodes) + .field("value", &value) + .finish() + }, + Ok(Node::Empty) => f.debug_struct("Node::Empty").finish(), + + Err(e) => f.debug_struct("BROKEN_NODE") + .field("key", &self.key) + .field("error", &format!("ERROR decoding node branch Rlp: {}", e)) + .finish() + } + } else { + f.debug_struct("BROKEN_NODE") + .field("key", &self.key) + .field("error", &"Not found") + .finish() + } + } +} + + impl<'db> fmt::Debug for TrieDB<'db> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - writeln!(f, "c={:?} [", self.hash_count)?; let root_rlp = self.db.get(self.root).expect("Trie root not found!"); - match Node::decoded(&root_rlp) { - Ok(node) => self.fmt_all(node, f, 0)?, - Err(e) => writeln!(f, "ERROR decoding node rlp: {}", e)?, - } - writeln!(f, "]") + f.debug_struct("TrieDB") + .field("hash_count", &self.hash_count) + .field("root", &TrieAwareDebugNode { + trie: self, + key: &root_rlp + }) + .finish() } } @@ -494,6 +482,118 @@ fn get_len() { assert_eq!(t.get_with(b"C", |x: &[u8]| x.len()), Ok(None)); } + +#[test] +fn debug_output_supports_pretty_print() { + use memorydb::*; + use super::TrieMut; + use super::triedbmut::*; + + let d = vec![ DBValue::from_slice(b"A"), DBValue::from_slice(b"AA"), DBValue::from_slice(b"AB"), DBValue::from_slice(b"B") ]; + + let mut memdb = MemoryDB::new(); + let mut root = H256::new(); + let root = { + let mut t = TrieDBMut::new(&mut memdb, &mut root); + for x in &d { + t.insert(x, x).unwrap(); + } + t.root().clone() + }; + let t = TrieDB::new(&memdb, &root).unwrap(); + + assert_eq!(format!("{:?}", t), "TrieDB { hash_count: 0, root: Node::Extension { slice: 4, item: Node::Branch { nodes: [Node::Empty, Node::Branch { nodes: [Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Branch { nodes: [Node::Empty, Node::Leaf { slice: , value: [65, 65] }, Node::Leaf { slice: , value: [65, 66] }, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty], value: None }, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty], value: Some([65]) }, Node::Leaf { slice: , value: [66] }, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty, Node::Empty], value: None } } }"); + assert_eq!(format!("{:#?}", t), +"TrieDB { + hash_count: 0, + root: Node::Extension { + slice: 4, + item: Node::Branch { + nodes: [ + Node::Empty, + Node::Branch { + nodes: [ + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Branch { + nodes: [ + Node::Empty, + Node::Leaf { + slice: , + value: [ + 65, + 65 + ] + }, + Node::Leaf { + slice: , + value: [ + 65, + 66 + ] + }, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty + ], + value: None + }, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty + ], + value: Some( + [ + 65 + ] + ) + }, + Node::Leaf { + slice: , + value: [ + 66 + ] + }, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty, + Node::Empty + ], + value: None + } + } +}"); +} + // Test will work once https://github.com/paritytech/parity/pull/8527 is merged and rlp::decode returns Result instead of panicking //#[test] //fn test_lookup_with_corrupt_data_returns_decoder_error() {