From 61be1491a14d7ce55642e685d736cdd9504b6ea0 Mon Sep 17 00:00:00 2001 From: pscott <30843220+pscott@users.noreply.github.com> Date: Mon, 16 Dec 2019 23:04:50 +0100 Subject: [PATCH] Add support for gzip (#641) * add support for gzip * Fix clippy warnings * Fix additional clippy warnings and optimized get_deposits function * Fix get_deposits function call * Add simulator to CI * Install ganache before running sim --- beacon_node/beacon_chain/src/eth1_chain.rs | 2 +- beacon_node/eth1/src/block_cache.rs | 13 +++---- beacon_node/eth1/src/deposit_cache.rs | 43 ++++++++++------------ beacon_node/eth1/src/http.rs | 20 ---------- beacon_node/eth1/tests/test.rs | 12 +++--- 5 files changed, 32 insertions(+), 58 deletions(-) diff --git a/beacon_node/beacon_chain/src/eth1_chain.rs b/beacon_node/beacon_chain/src/eth1_chain.rs index 96b2ea6aa..a22e83353 100644 --- a/beacon_node/beacon_chain/src/eth1_chain.rs +++ b/beacon_node/beacon_chain/src/eth1_chain.rs @@ -340,7 +340,7 @@ impl> Eth1ChainBackend for CachingEth1Backend { .deposits() .read() .cache - .get_deposits(next..last, deposit_count, DEPOSIT_TREE_DEPTH) + .get_deposits(next, last, deposit_count, DEPOSIT_TREE_DEPTH) .map_err(|e| Error::BackendError(format!("Failed to get deposits: {:?}", e))) .map(|(_deposit_root, deposits)| deposits) } diff --git a/beacon_node/eth1/src/block_cache.rs b/beacon_node/eth1/src/block_cache.rs index a15ef86a6..bdf4060b9 100644 --- a/beacon_node/eth1/src/block_cache.rs +++ b/beacon_node/eth1/src/block_cache.rs @@ -192,10 +192,7 @@ mod tests { } fn get_blocks(n: usize, interval_secs: u64) -> Vec { - (0..n as u64) - .into_iter() - .map(|i| get_block(i, interval_secs)) - .collect() + (0..n as u64).map(|i| get_block(i, interval_secs)).collect() } fn insert(cache: &mut BlockCache, s: Eth1Block) -> Result<(), Error> { @@ -213,16 +210,16 @@ mod tests { insert(&mut cache, block.clone()).expect("should add consecutive blocks"); } - for len in vec![0, 1, 2, 3, 4, 8, 15, 16] { + for len in &[0, 1, 2, 3, 4, 8, 15, 16] { let mut cache = cache.clone(); - cache.truncate(len); + cache.truncate(*len); assert_eq!( cache.blocks.len(), - len, + *len, "should truncate to length: {}", - len + *len ); } diff --git a/beacon_node/eth1/src/deposit_cache.rs b/beacon_node/eth1/src/deposit_cache.rs index f2b43f55e..1a237a1d9 100644 --- a/beacon_node/eth1/src/deposit_cache.rs +++ b/beacon_node/eth1/src/deposit_cache.rs @@ -1,6 +1,5 @@ use crate::DepositLog; use eth2_hashing::hash; -use std::ops::Range; use tree_hash::TreeHash; use types::{Deposit, Hash256}; @@ -143,24 +142,25 @@ impl DepositCache { /// /// ## Errors /// - /// - If `deposit_count` is larger than `range.end`. + /// - If `deposit_count` is larger than `end`. /// - There are not sufficient deposits in the tree to generate the proof. pub fn get_deposits( &self, - range: Range, + start: u64, + end: u64, deposit_count: u64, tree_depth: usize, ) -> Result<(Hash256, Vec), Error> { - if deposit_count < range.end { + if deposit_count < end { // It's invalid to ask for more deposits than should exist. Err(Error::DepositCountInvalid { deposit_count, - range_end: range.end, + range_end: end, }) - } else if range.end > self.logs.len() as u64 { + } else if end > self.logs.len() as u64 { // The range of requested deposits exceeds the deposits stored locally. Err(Error::InsufficientDeposits { - requested: range.end, + requested: end, known_deposits: self.logs.len(), }) } else if deposit_count > self.roots.len() as u64 { @@ -187,7 +187,7 @@ impl DepositCache { let deposits = self .logs - .get(range.start as usize..range.end as usize) + .get(start as usize..end as usize) .ok_or_else(|| Error::InternalError("Unable to get known log".into()))? .iter() .map(|deposit_log| { @@ -281,31 +281,31 @@ pub mod tests { // Get 0 deposits, with max deposit count. let (_, deposits) = tree - .get_deposits(0..0, n, TREE_DEPTH) + .get_deposits(0, 0, n, TREE_DEPTH) .expect("should get the full tree"); assert_eq!(deposits.len(), 0, "should return no deposits"); // Get 0 deposits, with 0 deposit count. let (_, deposits) = tree - .get_deposits(0..0, 0, TREE_DEPTH) + .get_deposits(0, 0, 0, TREE_DEPTH) .expect("should get the full tree"); assert_eq!(deposits.len(), 0, "should return no deposits"); // Get 0 deposits, with 0 deposit count, tree depth 0. let (_, deposits) = tree - .get_deposits(0..0, 0, 0) + .get_deposits(0, 0, 0, 0) .expect("should get the full tree"); assert_eq!(deposits.len(), 0, "should return no deposits"); // Get all deposits, with max deposit count. let (full_root, deposits) = tree - .get_deposits(0..n, n, TREE_DEPTH) + .get_deposits(0, n, n, TREE_DEPTH) .expect("should get the full tree"); assert_eq!(deposits.len(), n as usize, "should return all deposits"); // Get 4 deposits, with max deposit count. let (root, deposits) = tree - .get_deposits(0..4, n, TREE_DEPTH) + .get_deposits(0, 4, n, TREE_DEPTH) .expect("should get the four from the full tree"); assert_eq!( deposits.len(), @@ -318,18 +318,15 @@ pub mod tests { ); // Get half of the deposits, with half deposit count. + let half = n / 2; let (half_root, deposits) = tree - .get_deposits(0..n / 2, n / 2, TREE_DEPTH) + .get_deposits(0, half, half, TREE_DEPTH) .expect("should get the half tree"); - assert_eq!( - deposits.len(), - n as usize / 2, - "should return half deposits" - ); + assert_eq!(deposits.len(), half as usize, "should return half deposits"); // Get 4 deposits, with half deposit count. let (root, deposits) = tree - .get_deposits(0..4, n / 2, TREE_DEPTH) + .get_deposits(0, 4, n / 2, TREE_DEPTH) .expect("should get the half tree"); assert_eq!( deposits.len(), @@ -360,12 +357,12 @@ pub mod tests { } // Range too high. - assert!(tree.get_deposits(0..n + 1, n, TREE_DEPTH).is_err()); + assert!(tree.get_deposits(0, n + 1, n, TREE_DEPTH).is_err()); // Count too high. - assert!(tree.get_deposits(0..n, n + 1, TREE_DEPTH).is_err()); + assert!(tree.get_deposits(0, n, n + 1, TREE_DEPTH).is_err()); // Range higher than count. - assert!(tree.get_deposits(0..4, 2, TREE_DEPTH).is_err()); + assert!(tree.get_deposits(0, 4, 2, TREE_DEPTH).is_err()); } } diff --git a/beacon_node/eth1/src/http.rs b/beacon_node/eth1/src/http.rs index f28ca74a3..0b2e3a32b 100644 --- a/beacon_node/eth1/src/http.rs +++ b/beacon_node/eth1/src/http.rs @@ -11,10 +11,8 @@ //! There is no ABI parsing here, all function signatures and topics are hard-coded as constants. use futures::{Future, Stream}; -use libflate::gzip::Decoder; use reqwest::{header::CONTENT_TYPE, r#async::ClientBuilder, StatusCode}; use serde_json::{json, Value}; -use std::io::prelude::*; use std::ops::Range; use std::time::Duration; use types::Hash256; @@ -346,24 +344,6 @@ pub fn send_rpc_request( .and_then(move |bytes| match encoding.as_str() { "application/json" => Ok(bytes), "application/json; charset=utf-8" => Ok(bytes), - // Note: gzip is not presently working because we always seem to get an empty - // response from the server. - // - // I expect this is some simple-to-solve issue for someone who is familiar with - // the eth1 JSON RPC. - // - // Some public-facing web3 servers use gzip to compress their traffic, it would - // be good to support this. - "application/x-gzip" => { - let mut decoder = Decoder::new(&bytes[..]) - .map_err(|e| format!("Failed to create gzip decoder: {}", e))?; - let mut decompressed = vec![]; - decoder - .read_to_end(&mut decompressed) - .map_err(|e| format!("Failed to decompress gzip data: {}", e))?; - - Ok(decompressed) - } other => Err(format!("Unsupported encoding: {}", other)), }) .map(|bytes| String::from_utf8_lossy(&bytes).into_owned()) diff --git a/beacon_node/eth1/tests/test.rs b/beacon_node/eth1/tests/test.rs index 31d19cf04..603cfab48 100644 --- a/beacon_node/eth1/tests/test.rs +++ b/beacon_node/eth1/tests/test.rs @@ -425,7 +425,7 @@ mod deposit_tree { ); for round in 0..3 { - let deposits: Vec<_> = (0..n).into_iter().map(|_| random_deposit_data()).collect(); + let deposits: Vec<_> = (0..n).map(|_| random_deposit_data()).collect(); for deposit in &deposits { deposit_contract @@ -448,8 +448,8 @@ mod deposit_tree { .deposits() .read() .cache - .get_deposits(first..last, last, 32) - .expect(&format!("should get deposits in round {}", round)); + .get_deposits(first, last, last, 32) + .unwrap_or_else(|_| panic!("should get deposits in round {}", round)); assert_eq!( local_deposits.len(), @@ -498,7 +498,7 @@ mod deposit_tree { log, ); - let deposits: Vec<_> = (0..n).into_iter().map(|_| random_deposit_data()).collect(); + let deposits: Vec<_> = (0..n).map(|_| random_deposit_data()).collect(); for deposit in &deposits { deposit_contract @@ -524,7 +524,7 @@ mod deposit_tree { let n = 8; - let deposits: Vec<_> = (0..n).into_iter().map(|_| random_deposit_data()).collect(); + let deposits: Vec<_> = (0..n).map(|_| random_deposit_data()).collect(); let eth1 = runtime .block_on(GanacheEth1Instance::new()) @@ -586,7 +586,7 @@ mod deposit_tree { // Ensure that the root from the deposit tree matches what the contract reported. let (root, deposits) = tree - .get_deposits(0..i as u64, deposit_counts[i], DEPOSIT_CONTRACT_TREE_DEPTH) + .get_deposits(0, i as u64, deposit_counts[i], DEPOSIT_CONTRACT_TREE_DEPTH) .expect("should get deposits"); assert_eq!( root, deposit_roots[i],