From ab0407c3454b813aaf81ce54e4789c9f3ad38fbb Mon Sep 17 00:00:00 2001 From: Peter van Nostrand Date: Thu, 24 May 2018 19:28:23 -0400 Subject: [PATCH 01/15] Problem: the readme and example config TOML files include obsolete gas-price config options Solution: remove the old config options, document the new gas-price config options --- README.md | 14 ++++++++------ examples/config.toml | 10 +++++++--- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index e887a9d..3041ced 100644 --- a/README.md +++ b/README.md @@ -34,6 +34,8 @@ and they can convert them back as well. Requires `rust` and `cargo`: [installation instructions.](https://www.rust-lang.org/en-US/install.html) +Requires `rustc` >= 1.26. + Requires `solc` to be in `$PATH`: [installation instructions.](https://solidity.readthedocs.io/en/develop/installing-solidity.html) Assuming you've cloned the bridge (`git clone git@github.com:poanetwork/poa-bridge.git`), run @@ -82,6 +84,9 @@ rpc_host = "http://localhost" rpc_port = 8545 required_confirmations = 0 password = "home_password.txt" +gas_price_oracle_url = "https://gasprice.poa.network" +gas_price_speed = "instant" +default_gas_price = 10000000000 # 10 GWEI [foreign] account = "0x006e27b6a72e1f34c626762f3c4761547aff1421" @@ -99,9 +104,9 @@ accounts = [ required_signatures = 2 [transactions] -deposit_relay = { gas = 3000000, gas_price = 1000000000 } -withdraw_relay = { gas = 3000000, gas_price = 1000000000 } -withdraw_confirm = { gas = 3000000, gas_price = 1000000000 } +deposit_relay = { gas = 3000000 } +withdraw_relay = { gas = 3000000 } +withdraw_confirm = { gas = 3000000 } ``` #### Options @@ -130,13 +135,10 @@ withdraw_confirm = { gas = 3000000, gas_price = 1000000000 } #### transaction options - `transaction.deposit_relay.gas` - specify how much gas should be consumed by deposit relay -- `transaction.deposit_relay.gas_price` - specify gas price for deposit relay - `transaction.deposit_relay.concurrency` - how many concurrent transactions can be sent (default: **100**) - `transaction.withdraw_confirm.gas` - specify how much gas should be consumed by withdraw confirm -- `transaction.withdraw_confirm.gas_price` - specify gas price for withdraw confirm - `transaction.withdraw_confirm.concurrency` - how many concurrent transactions can be sent (default: **100**) - `transaction.withdraw_relay.gas` - specify how much gas should be consumed by withdraw relay -- `transaction.withdraw_relay.gas_price` - specify gas price for withdraw relay - `transaction.withdraw_relay.concurrency` - how many concurrent transactions can be sent (default: **100**) ### Database file format diff --git a/examples/config.toml b/examples/config.toml index 40e6755..878082e 100644 --- a/examples/config.toml +++ b/examples/config.toml @@ -6,12 +6,16 @@ required_confirmations = 0 rpc_host = "http://rpc.host.for.home" rpc_port = 8545 password = "home_password.txt" +gas_price_oracle_url = "https://gasprice.poa.network" +gas_price_speed = "instant" +default_gas_price = 10000000000 # 10 GWEI [foreign] account = "0x006e27b6a72e1f34c626762f3c4761547aff1421" required_confirmations = 0 rpc_host = "https://rpc.host.for.foreign" rpc_port = 443 +default_gas_price = 5000000000 # 5 GWEI [authorities] accounts = [ @@ -20,6 +24,6 @@ accounts = [ required_signatures = 1 [transactions] -home_deploy = { gas = 1000000, gas_price = 1000000000 } -foreign_deploy = { gas = 3000000, gas_price = 1000000000 } -deposit_relay = { gas = 100000, gas_price = 1000000000 } +home_deploy = { gas = 1000000 } +foreign_deploy = { gas = 3000000 } +deposit_relay = { gas = 100000 } From d480d57a846f2e30af06fd8fb08ec02124e7c7e3 Mon Sep 17 00:00:00 2001 From: Peter van Nostrand Date: Thu, 24 May 2018 20:13:05 -0400 Subject: [PATCH 02/15] Problem: not enforcing a minimum required rustc version could lead to build-time errors Solution: add a mechanism to enforce a minimum required rustc version in the bridge's build script --- Cargo.lock | 1 + bridge/Cargo.toml | 3 +++ bridge/build.rs | 21 +++++++++++++++++++++ 3 files changed, 25 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index d02dfe2..c7d5714 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -167,6 +167,7 @@ dependencies = [ "quickcheck 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", "rlp 0.2.1 (git+http://github.com/paritytech/parity?rev=991f0ca)", "rustc-hex 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", + "rustc_version 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", "serde 1.0.43 (registry+https://github.com/rust-lang/crates.io-index)", "serde_derive 1.0.43 (registry+https://github.com/rust-lang/crates.io-index)", "serde_json 1.0.16 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/bridge/Cargo.toml b/bridge/Cargo.toml index 9b94044..2d87e07 100644 --- a/bridge/Cargo.toml +++ b/bridge/Cargo.toml @@ -32,6 +32,9 @@ hyper-tls = "0.1.3" tempdir = "0.3" quickcheck = "0.6.1" +[build-dependencies] +rustc_version = "0.2.2" + [features] default = [] deploy = [] diff --git a/bridge/build.rs b/bridge/build.rs index 30168db..3f00990 100644 --- a/bridge/build.rs +++ b/bridge/build.rs @@ -1,6 +1,27 @@ +extern crate rustc_version; + use std::process::Command; +use rustc_version::{version as get_rustc_version, Version}; + +fn check_rustc_version() { + let minimum_required_version = Version::new(1, 26, 0); + + if let Ok(version) = get_rustc_version() { + if version < minimum_required_version { + panic!( + "Invalid rustc version, `poa-bridge` requires \ + rustc >= {}, found version: {}", + minimum_required_version, + version + ); + } + } +} + fn main() { + check_rustc_version(); + // rerun build script if bridge contract has changed. // without this cargo doesn't since the bridge contract // is outside the crate directories From dc5bba0c3c48a56dca473202e12340a9dd773f2d Mon Sep 17 00:00:00 2001 From: Yurii Rashkovskii Date: Fri, 25 May 2018 00:52:49 -0700 Subject: [PATCH 03/15] Problem: bridge panics if there's an error retrieving gas price Solution: instead, log an error and use previous price --- bridge/src/bridge/gas_price.rs | 43 ++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/bridge/src/bridge/gas_price.rs b/bridge/src/bridge/gas_price.rs index d6efc3d..3e8a2ff 100644 --- a/bridge/src/bridge/gas_price.rs +++ b/bridge/src/bridge/gas_price.rs @@ -12,7 +12,6 @@ use config::{GasPriceSpeed, Node}; use error::Error; const CACHE_TIMEOUT_DURATION: Duration = Duration::from_secs(5 * 60); -const REQUEST_TIMEOUT_DURATION: Duration = Duration::from_secs(30); enum State { Initial, @@ -27,6 +26,8 @@ pub struct GasPriceStream { speed: GasPriceSpeed, request_timer: Timer, interval: Interval, + last_price: u64, + request_timeout: Duration, } impl GasPriceStream { @@ -44,6 +45,8 @@ impl GasPriceStream { speed: node.gas_price_speed, request_timer: timer.clone(), interval: timer.interval_at(Instant::now(), CACHE_TIMEOUT_DURATION), + last_price: node.default_gas_price, + request_timeout: node.request_timeout, } } } @@ -66,7 +69,7 @@ impl Stream for GasPriceStream { ); let request_future = self.request_timer - .timeout(request, REQUEST_TIMEOUT_DURATION); + .timeout(request, self.request_timeout); State::WaitingForResponse(request_future) }, @@ -74,21 +77,37 @@ impl Stream for GasPriceStream { match request_future.poll() { Ok(Async::NotReady) => return Ok(Async::NotReady), Ok(Async::Ready(chunk)) => { - let json_obj: HashMap = json::from_slice(&chunk)?; - - let gas_price = match json_obj.get(self.speed.as_str()) { - Some(json::Value::Number(price)) => (price.as_f64().unwrap() * 1_000_000_000.0).trunc() as u64, - _ => unreachable!(), - }; - - State::Yield(Some(gas_price)) + match json::from_slice::>(&chunk) { + Ok(json_obj) => { + match json_obj.get(self.speed.as_str()) { + Some(json::Value::Number(price)) => State::Yield(Some((price.as_f64().unwrap() * 1_000_000_000.0).trunc() as u64)), + _ => { + error!("Invalid or missing gas price ({}) in the gas price oracle response: {}", self.speed.as_str(), String::from_utf8_lossy(&*chunk)); + State::Yield(Some(self.last_price)) + }, + } + }, + Err(e) => { + error!("Error while parsing response from gas price oracle: {:?} {}", e, String::from_utf8_lossy(&*chunk)); + State::Yield(Some(self.last_price)) + } + } + }, + Err(e) => { + error!("Error while fetching gas price: {:?}", e); + State::Yield(Some(self.last_price)) }, - Err(e) => panic!(e), } }, State::Yield(ref mut opt) => match opt.take() { None => State::Initial, - price => return Ok(Async::Ready(price)), + Some(price) => { + if price != self.last_price { + info!("Gas price: {} gwei", (price as f64) / 1_000_000_000.0); + self.last_price = price; + } + return Ok(Async::Ready(Some(price))) + }, } }; From cd43c2dfcbef2e71b2dc3c899f8d161d13420cbb Mon Sep 17 00:00:00 2001 From: Peter van Nostrand Date: Fri, 25 May 2018 08:25:08 -0400 Subject: [PATCH 04/15] Added underscores to example gas-prices in docs to improve readability. --- README.md | 4 +--- examples/config.toml | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 3041ced..9838cd6 100644 --- a/README.md +++ b/README.md @@ -34,8 +34,6 @@ and they can convert them back as well. Requires `rust` and `cargo`: [installation instructions.](https://www.rust-lang.org/en-US/install.html) -Requires `rustc` >= 1.26. - Requires `solc` to be in `$PATH`: [installation instructions.](https://solidity.readthedocs.io/en/develop/installing-solidity.html) Assuming you've cloned the bridge (`git clone git@github.com:poanetwork/poa-bridge.git`), run @@ -86,7 +84,7 @@ required_confirmations = 0 password = "home_password.txt" gas_price_oracle_url = "https://gasprice.poa.network" gas_price_speed = "instant" -default_gas_price = 10000000000 # 10 GWEI +default_gas_price = 10_000_000_000 # 10 GWEI [foreign] account = "0x006e27b6a72e1f34c626762f3c4761547aff1421" diff --git a/examples/config.toml b/examples/config.toml index 878082e..fb5b3f0 100644 --- a/examples/config.toml +++ b/examples/config.toml @@ -8,14 +8,14 @@ rpc_port = 8545 password = "home_password.txt" gas_price_oracle_url = "https://gasprice.poa.network" gas_price_speed = "instant" -default_gas_price = 10000000000 # 10 GWEI +default_gas_price = 10_000_000_000 # 10 GWEI [foreign] account = "0x006e27b6a72e1f34c626762f3c4761547aff1421" required_confirmations = 0 rpc_host = "https://rpc.host.for.foreign" rpc_port = 443 -default_gas_price = 5000000000 # 5 GWEI +default_gas_price = 5_000_000_000 # 5 GWEI [authorities] accounts = [ From af81eb0d5751621ce4d8c616b7405bcaeef9b3ef Mon Sep 17 00:00:00 2001 From: Yurii Rashkovskii Date: Fri, 25 May 2018 00:24:16 -0700 Subject: [PATCH 05/15] Problem: potential loss of database updates chance of loss of database updates that would cause it to redo transactions it already did. Let's say we've got some database updates through deposit relaying: https://github.com/poanetwork/poa-bridge/blob/master/bridge/src/bridge/mod.rs#L164-L165 Then, during relaying withdrawals, an error happened: https://github.com/poanetwork/poa-bridge/blob/master/bridge/src/bridge/mod.rs#L171-L172 This means that we won't reach https://github.com/poanetwork/poa-bridge/blob/master/bridge/src/bridge/mod.rs#L185-L193 to save the result. Also, in a similar vein, if one of these streams was to end (`Ready(None)`) we'd experience a similar loss of updates: https://github.com/poanetwork/poa-bridge/blob/master/bridge/src/macros.rs#L5 Solution: refactor bridge into two streams, splitting responsibilities One stream (`BridgeEventStream`) returns `BridgeChecked` and the other (`Bridge`) writes those checks down to the database. This way we're not accumulating chcecks before we serialize them, risking not serializing them at all in the event of an unrelated error. Fixes #84 --- bridge/src/bridge/deposit_relay.rs | 5 +- bridge/src/bridge/mod.rs | 200 ++++++++++++-------------- bridge/src/bridge/withdraw_confirm.rs | 5 +- bridge/src/bridge/withdraw_relay.rs | 5 +- 4 files changed, 101 insertions(+), 114 deletions(-) diff --git a/bridge/src/bridge/deposit_relay.rs b/bridge/src/bridge/deposit_relay.rs index 66feab4..23a06f8 100644 --- a/bridge/src/bridge/deposit_relay.rs +++ b/bridge/src/bridge/deposit_relay.rs @@ -11,6 +11,7 @@ use util::web3_filter; use app::App; use ethcore_transaction::{Transaction, Action}; use super::nonce::{NonceCheck, SendRawTransaction}; +use super::BridgeChecked; use itertools::Itertools; fn deposits_filter(home: &home::HomeBridge, address: Address) -> FilterBuilder { @@ -72,7 +73,7 @@ pub struct DepositRelay { } impl Stream for DepositRelay { - type Item = u64; + type Item = BridgeChecked; type Error = Error; fn poll(&mut self) -> Poll, Self::Error> { @@ -126,7 +127,7 @@ impl Stream for DepositRelay { }, DepositRelayState::Yield(ref mut block) => match block.take() { None => DepositRelayState::Wait, - some => return Ok(some.into()), + Some(v) => return Ok(Some(BridgeChecked::DepositRelay(v)).into()), } }; self.state = next_state; diff --git a/bridge/src/bridge/mod.rs b/bridge/src/bridge/mod.rs index eda5179..243e4d7 100644 --- a/bridge/src/bridge/mod.rs +++ b/bridge/src/bridge/mod.rs @@ -15,7 +15,7 @@ use web3::Transport; use web3::types::U256; use app::App; use database::Database; -use error::{Error, ErrorKind, Result}; +use error::{Error, ErrorKind}; use tokio_core::reactor::Handle; pub use self::deploy::{Deploy, Deployed, create_deploy}; @@ -34,57 +34,51 @@ pub enum BridgeChecked { WithdrawConfirm(u64), } -pub trait BridgeBackend { - fn save(&mut self, checks: Vec) -> Result<()>; -} - -pub struct FileBackend { +pub struct Bridge> { path: PathBuf, database: Database, + event_stream: ES, } -impl BridgeBackend for FileBackend { - fn save(&mut self, checks: Vec) -> Result<()> { - for check in checks { - match check { - BridgeChecked::DepositRelay(n) => { - self.database.checked_deposit_relay = n; - }, - BridgeChecked::WithdrawRelay(n) => { - self.database.checked_withdraw_relay = n; - }, - BridgeChecked::WithdrawConfirm(n) => { - self.database.checked_withdraw_confirm = n; - }, - } - } +impl> Stream for Bridge { + type Item = (); + type Error = Error; + fn poll(&mut self) -> Poll, Self::Error> { + let check = try_stream!(self.event_stream.poll()); + match check { + BridgeChecked::DepositRelay(n) => { + self.database.checked_deposit_relay = n; + }, + BridgeChecked::WithdrawRelay(n) => { + self.database.checked_withdraw_relay = n; + }, + BridgeChecked::WithdrawConfirm(n) => { + self.database.checked_withdraw_confirm = n; + }, + } let file = fs::OpenOptions::new() .write(true) .create(true) .open(&self.path)?; - self.database.save(file) + self.database.save(file)?; + Ok(Async::Ready(Some(()))) } - } - -enum BridgeStatus { - Wait, - NextItem(Option<()>), } + /// Creates new bridge. -pub fn create_bridge(app: Arc>, init: &Database, handle: &Handle, home_chain_id: u64, foreign_chain_id: u64) -> Bridge { - let backend = FileBackend { +pub fn create_bridge<'a, T: Transport + 'a + Clone>(app: Arc>, init: &Database, handle: &Handle, home_chain_id: u64, foreign_chain_id: u64) -> Bridge> { + Bridge { path: app.database_path.clone(), database: init.clone(), - }; - - create_bridge_backed_by(app, init, backend, handle, home_chain_id, foreign_chain_id) + event_stream: create_bridge_event_stream(app, init, handle, home_chain_id, foreign_chain_id), + } } /// Creates new bridge writing to custom backend. -pub fn create_bridge_backed_by(app: Arc>, init: &Database, backend: F, handle: &Handle, home_chain_id: u64, foreign_chain_id: u64) -> Bridge { +pub fn create_bridge_event_stream<'a, T: Transport + 'a + Clone>(app: Arc>, init: &Database, handle: &Handle, home_chain_id: u64, foreign_chain_id: u64) -> BridgeEventStream<'a, T> { let home_balance = Arc::new(RwLock::new(None)); let foreign_balance = Arc::new(RwLock::new(None)); @@ -105,16 +99,22 @@ pub fn create_bridge_backed_by(app: Arc< let home_gas_price = Arc::new(RwLock::new(app.config.home.default_gas_price)); let foreign_gas_price = Arc::new(RwLock::new(app.config.foreign.default_gas_price)); - Bridge { + let deposit_relay = create_deposit_relay(app.clone(), init, foreign_balance.clone(), foreign_chain_id, foreign_gas_price.clone()) + .map_err(|e| ErrorKind::ContextualizedError(Box::new(e), "deposit_relay").into()); + let withdraw_relay = create_withdraw_relay(app.clone(), init, home_balance.clone(), home_chain_id, home_gas_price.clone()) + .map_err(|e| ErrorKind::ContextualizedError(Box::new(e), "withdraw_relay").into()); + let withdraw_confirm = create_withdraw_confirm(app.clone(), init, foreign_balance.clone(), foreign_chain_id, foreign_gas_price.clone()) + .map_err(|e| ErrorKind::ContextualizedError(Box::new(e), "withdraw_confirm").into()); + + let bridge = Box::new(deposit_relay.select(withdraw_relay).select(withdraw_confirm)); + + BridgeEventStream { foreign_balance_check: create_balance_check(app.clone(), app.connections.foreign.clone(), app.config.foreign.clone()), home_balance_check: create_balance_check(app.clone(), app.connections.home.clone(), app.config.home.clone()), foreign_balance: foreign_balance.clone(), home_balance: home_balance.clone(), - deposit_relay: create_deposit_relay(app.clone(), init, foreign_balance.clone(), foreign_chain_id, foreign_gas_price.clone()), - withdraw_relay: create_withdraw_relay(app.clone(), init, home_balance.clone(), home_chain_id, home_gas_price.clone()), - withdraw_confirm: create_withdraw_confirm(app.clone(), init, foreign_balance.clone(), foreign_chain_id, foreign_gas_price.clone()), - state: BridgeStatus::Wait, - backend, + bridge, + state: BridgeStatus::Init, running: app.running.clone(), home_gas_stream, foreign_gas_stream, @@ -123,16 +123,19 @@ pub fn create_bridge_backed_by(app: Arc< } } -pub struct Bridge { +enum BridgeStatus { + Init, + Wait, + NextItem(Option), +} + +pub struct BridgeEventStream<'a, T: Transport + 'a> { home_balance_check: BalanceCheck, foreign_balance_check: BalanceCheck, home_balance: Arc>>, foreign_balance: Arc>>, - deposit_relay: DepositRelay, - withdraw_relay: WithdrawRelay, - withdraw_confirm: WithdrawConfirm, + bridge: Box + 'a>, state: BridgeStatus, - backend: F, running: Arc, home_gas_stream: Option, foreign_gas_stream: Option, @@ -142,7 +145,7 @@ pub struct Bridge { use std::sync::atomic::{AtomicBool, Ordering}; -impl Bridge { +impl<'a, T: Transport + 'a> BridgeEventStream<'a, T> { fn check_balances(&mut self) -> Poll, Error> { let mut home_balance = self.home_balance.write().unwrap(); let mut foreign_balance = self.foreign_balance.write().unwrap(); @@ -178,70 +181,36 @@ impl Bridge { } } -impl Stream for Bridge { - type Item = (); +impl<'a, T: Transport + 'a> Stream for BridgeEventStream<'a, T> { + type Item = BridgeChecked; type Error = Error; fn poll(&mut self) -> Poll, Self::Error> { loop { let next_state = match self.state { + BridgeStatus::Init => { + match self.check_balances()? { + Async::NotReady => return Ok(Async::NotReady), + _ => (), + } + BridgeStatus::Wait + }, BridgeStatus::Wait => { if !self.running.load(Ordering::SeqCst) { return Err(ErrorKind::ShutdownRequested.into()) } - // Intended to be used upon startup - let balance_is_absent = { - let mut home_balance = self.home_balance.read().unwrap(); - let mut foreign_balance = self.foreign_balance.read().unwrap(); - home_balance.is_none() || foreign_balance.is_none() - }; - if balance_is_absent { - match self.check_balances()? { - Async::NotReady => return Ok(Async::NotReady), - _ => (), - } - } - let _ = self.get_gas_prices(); - let d_relay = try_bridge!(self.deposit_relay.poll().map_err(|e| ErrorKind::ContextualizedError(Box::new(e), "deposit_relay"))) - .map(BridgeChecked::DepositRelay); - - if d_relay.is_some() { - self.check_balances()?; - } - - let w_relay = try_bridge!(self.withdraw_relay.poll().map_err(|e| ErrorKind::ContextualizedError(Box::new(e), "withdraw_relay"))). - map(BridgeChecked::WithdrawRelay); - - if w_relay.is_some() { - self.check_balances()?; - } - - let w_confirm = try_bridge!(self.withdraw_confirm.poll().map_err(|e| ErrorKind::ContextualizedError(Box::new(e), "withdraw_confirm"))). - map(BridgeChecked::WithdrawConfirm); - - if w_confirm.is_some() { - self.check_balances()?; - } - - let result: Vec<_> = [d_relay, w_relay, w_confirm] - .into_iter() - .filter_map(|c| *c) - .collect(); - - if result.is_empty() { - return Ok(Async::NotReady); - } else { - self.backend.save(result)?; - BridgeStatus::NextItem(Some(())) - } + let item = try_stream!(self.bridge.poll()); + BridgeStatus::NextItem(Some(item)) }, BridgeStatus::NextItem(ref mut v) => match v.take() { - None => BridgeStatus::Wait, - some => return Ok(some.into()), - }, + None => BridgeStatus::Init, + some => { + return Ok(some.into()); + } + } }; self.state = next_state; @@ -254,28 +223,43 @@ mod tests { extern crate tempdir; use self::tempdir::TempDir; use database::Database; - use super::{BridgeBackend, FileBackend, BridgeChecked}; + use super::{Bridge, BridgeChecked}; + use error::Error; + use tokio_core::reactor::Core; + use futures::{Stream, stream}; #[test] - fn test_file_backend() { + fn test_database_updates() { let tempdir = TempDir::new("test_file_backend").unwrap(); let mut path = tempdir.path().to_owned(); path.push("db"); - let mut backend = FileBackend { + + let bridge = Bridge { path: path.clone(), database: Database::default(), + event_stream: stream::iter_ok::<_, Error>(vec![BridgeChecked::DepositRelay(1)]), }; - backend.save(vec![BridgeChecked::DepositRelay(1)]).unwrap(); - assert_eq!(1, backend.database.checked_deposit_relay); - assert_eq!(0, backend.database.checked_withdraw_confirm); - assert_eq!(0, backend.database.checked_withdraw_relay); - backend.save(vec![BridgeChecked::DepositRelay(2), BridgeChecked::WithdrawConfirm(3), BridgeChecked::WithdrawRelay(2)]).unwrap(); - assert_eq!(2, backend.database.checked_deposit_relay); - assert_eq!(3, backend.database.checked_withdraw_confirm); - assert_eq!(2, backend.database.checked_withdraw_relay); + let mut event_loop = Core::new().unwrap(); + let _ = event_loop.run(bridge.collect()); - let loaded = Database::load(path).unwrap(); - assert_eq!(backend.database, loaded); + let db = Database::load(&path).unwrap(); + assert_eq!(1, db.checked_deposit_relay); + assert_eq!(0, db.checked_withdraw_confirm); + assert_eq!(0, db.checked_withdraw_relay); + + let bridge = Bridge { + path: path.clone(), + database: Database::default(), + event_stream: stream::iter_ok::<_, Error>(vec![BridgeChecked::DepositRelay(2), BridgeChecked::WithdrawConfirm(3), BridgeChecked::WithdrawRelay(2)]), + }; + + let mut event_loop = Core::new().unwrap(); + let _ = event_loop.run(bridge.collect()); + + let db = Database::load(&path).unwrap(); + assert_eq!(2, db.checked_deposit_relay); + assert_eq!(3, db.checked_withdraw_confirm); + assert_eq!(2, db.checked_withdraw_relay); } } diff --git a/bridge/src/bridge/withdraw_confirm.rs b/bridge/src/bridge/withdraw_confirm.rs index 9f12b6b..01eeed1 100644 --- a/bridge/src/bridge/withdraw_confirm.rs +++ b/bridge/src/bridge/withdraw_confirm.rs @@ -13,6 +13,7 @@ use message_to_mainnet::{MessageToMainnet, MESSAGE_LENGTH}; use ethcore_transaction::{Transaction, Action}; use itertools::Itertools; use super::nonce::{NonceCheck, SendRawTransaction}; +use super::BridgeChecked; fn withdraws_filter(foreign: &foreign::ForeignBridge, address: Address) -> FilterBuilder { let filter = foreign.events().withdraw().create_filter(); @@ -68,7 +69,7 @@ pub struct WithdrawConfirm { } impl Stream for WithdrawConfirm { - type Item = u64; + type Item = BridgeChecked; type Error = Error; fn poll(&mut self) -> Poll, Self::Error> { @@ -153,7 +154,7 @@ impl Stream for WithdrawConfirm { info!("waiting for new withdraws that should get signed"); WithdrawConfirmState::Wait }, - some => return Ok(some.into()), + Some(v) => return Ok(Some(BridgeChecked::WithdrawConfirm(v)).into()), } }; self.state = next_state; diff --git a/bridge/src/bridge/withdraw_relay.rs b/bridge/src/bridge/withdraw_relay.rs index a9bc5b4..9233133 100644 --- a/bridge/src/bridge/withdraw_relay.rs +++ b/bridge/src/bridge/withdraw_relay.rs @@ -15,6 +15,7 @@ use message_to_mainnet::MessageToMainnet; use signature::Signature; use ethcore_transaction::{Transaction, Action}; use super::nonce::{NonceCheck, SendRawTransaction}; +use super::BridgeChecked; use itertools::Itertools; /// returns a filter for `ForeignBridge.CollectedSignatures` events @@ -109,7 +110,7 @@ pub struct WithdrawRelay { } impl Stream for WithdrawRelay { - type Item = u64; + type Item = BridgeChecked; type Error = Error; fn poll(&mut self) -> Poll, Self::Error> { @@ -250,7 +251,7 @@ impl Stream for WithdrawRelay { info!("waiting for signed withdraws to relay"); WithdrawRelayState::Wait }, - some => return Ok(some.into()), + Some(v) => return Ok(Some(BridgeChecked::WithdrawRelay(v)).into()), } }; self.state = next_state; From 8082daa3924d0dc9b79055f1a82d48379ff433ce Mon Sep 17 00:00:00 2001 From: Yurii Rashkovskii Date: Mon, 28 May 2018 10:00:39 -0700 Subject: [PATCH 06/15] Problem: gas price retrieving is not tested well Solution: extract price retrieving from GasPriceStream and test it. --- bridge/src/bridge/gas_price.rs | 288 ++++++++++++++++++++++++++++++--- bridge/src/bridge/mod.rs | 10 +- bridge/src/error.rs | 4 + 3 files changed, 275 insertions(+), 27 deletions(-) diff --git a/bridge/src/bridge/gas_price.rs b/bridge/src/bridge/gas_price.rs index 3e8a2ff..c67002e 100644 --- a/bridge/src/bridge/gas_price.rs +++ b/bridge/src/bridge/gas_price.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use std::time::{Duration, Instant}; use futures::{Async, Future, Poll, Stream}; -use hyper::{Chunk, client::HttpConnector, Client, Uri}; +use hyper::{Chunk, client::{HttpConnector, Connect}, Client, Uri, Error as HyperError}; use hyper_tls::HttpsConnector; use serde_json as json; use tokio_core::reactor::Handle; @@ -13,15 +13,36 @@ use error::Error; const CACHE_TIMEOUT_DURATION: Duration = Duration::from_secs(5 * 60); -enum State { +enum State { Initial, - WaitingForResponse(Timeout>>), + WaitingForResponse(Timeout), Yield(Option), } -pub struct GasPriceStream { - state: State, - client: Client>, +pub trait Retriever { + type Item: AsRef<[u8]>; + type Future: Future; + fn retrieve(&self, uri: &Uri) -> Self::Future; +} + +impl Retriever for Client where C: Connect, B: Stream + 'static { + type Item = Chunk; + type Future = Box>; + + fn retrieve(&self, uri: &Uri) -> Self::Future { + Box::new( + self.get(uri.clone()) + .and_then(|resp| resp.body().concat2()) + .map_err(|e| e.into()) + ) + } +} + +pub type StandardGasPriceStream = GasPriceStream>, Client>, Chunk>; + +pub struct GasPriceStream where I: AsRef<[u8]>, F: Future, R: Retriever { + state: State, + retriever: R, uri: Uri, speed: GasPriceSpeed, request_timer: Timer, @@ -30,17 +51,22 @@ pub struct GasPriceStream { request_timeout: Duration, } -impl GasPriceStream { - pub fn new(node: &Node, handle: &Handle, timer: &Timer) -> Self { - let client = Client::configure() - .connector(HttpsConnector::new(4, handle).unwrap()) - .build(handle); +impl StandardGasPriceStream { + pub fn new(node: &Node, handle: &Handle, timer: &Timer) -> Self { + let client = Client::configure() + .connector(HttpsConnector::new(4, handle).unwrap()) + .build(handle); + GasPriceStream::new_with_retriever(node, client, timer) + } +} +impl GasPriceStream where I: AsRef<[u8]>, F: Future, R: Retriever { + pub fn new_with_retriever(node: &Node, retriever: R, timer: &Timer) -> Self { let uri: Uri = node.gas_price_oracle_url.clone().unwrap().parse().unwrap(); GasPriceStream { state: State::Initial, - client, + retriever, uri, speed: node.gas_price_speed, request_timer: timer.clone(), @@ -51,7 +77,7 @@ impl GasPriceStream { } } -impl Stream for GasPriceStream { +impl Stream for GasPriceStream where I: AsRef<[u8]>, F: Future, R: Retriever { type Item = u64; type Error = Error; @@ -61,12 +87,7 @@ impl Stream for GasPriceStream { State::Initial => { let _ = try_stream!(self.interval.poll()); - let request: Box> = - Box::new( - self.client.get(self.uri.clone()) - .and_then(|resp| resp.body().concat2()) - .map_err(|e| e.into()) - ); + let request = self.retriever.retrieve(&self.uri); let request_future = self.request_timer .timeout(request, self.request_timeout); @@ -77,18 +98,18 @@ impl Stream for GasPriceStream { match request_future.poll() { Ok(Async::NotReady) => return Ok(Async::NotReady), Ok(Async::Ready(chunk)) => { - match json::from_slice::>(&chunk) { + match json::from_slice::>(chunk.as_ref()) { Ok(json_obj) => { match json_obj.get(self.speed.as_str()) { Some(json::Value::Number(price)) => State::Yield(Some((price.as_f64().unwrap() * 1_000_000_000.0).trunc() as u64)), _ => { - error!("Invalid or missing gas price ({}) in the gas price oracle response: {}", self.speed.as_str(), String::from_utf8_lossy(&*chunk)); + error!("Invalid or missing gas price ({}) in the gas price oracle response: {}", self.speed.as_str(), String::from_utf8_lossy(&*chunk.as_ref())); State::Yield(Some(self.last_price)) }, } }, Err(e) => { - error!("Error while parsing response from gas price oracle: {:?} {}", e, String::from_utf8_lossy(&*chunk)); + error!("Error while parsing response from gas price oracle: {:?} {}", e, String::from_utf8_lossy(&*chunk.as_ref())); State::Yield(Some(self.last_price)) } } @@ -115,3 +136,226 @@ impl Stream for GasPriceStream { } } } + +#[cfg(test)] +mod tests { + + use super::*; + use error::{Error, ErrorKind}; + use futures::{Async, future::{err, ok, FutureResult}}; + use config::{Node, NodeInfo}; + use tokio_timer::Timer; + use std::time::Duration; + use std::path::PathBuf; + use web3::types::Address; + use std::str::FromStr; + + struct ErroredRequest; + + impl Retriever for ErroredRequest { + type Item = Vec; + type Future = FutureResult; + + fn retrieve(&self, _uri: &Uri) -> ::Future { + err(ErrorKind::OtherError("something went wrong".into()).into()) + } + } + + #[test] + fn errored_request() { + let node = Node { + account: Address::new(), + request_timeout: Duration::from_secs(5), + poll_interval: Duration::from_secs(1), + required_confirmations: 0, + rpc_host: "https://rpc".into(), + rpc_port: 443, + password: PathBuf::from("password"), + info: NodeInfo::default(), + gas_price_oracle_url: Some("https://gas.price".into()), + gas_price_speed: GasPriceSpeed::from_str("fast").unwrap(), + gas_price_timeout: Duration::from_secs(5), + default_gas_price: 15_000_000_000, + }; + let timer = Timer::default(); + let mut stream = GasPriceStream::new_with_retriever(&node, ErroredRequest, &timer); + loop { + match stream.poll() { + Ok(Async::Ready(Some(v))) => { + assert_eq!(v, node.default_gas_price); + break; + }, + Err(_) => panic!("should not error out"), + _ => (), + } + } + } + + + struct BadJson; + + impl Retriever for BadJson { + type Item = String; + type Future = FutureResult; + + fn retrieve(&self, _uri: &Uri) -> ::Future { + ok("bad json".into()) + } + } + + #[test] + fn bad_json() { + let node = Node { + account: Address::new(), + request_timeout: Duration::from_secs(5), + poll_interval: Duration::from_secs(1), + required_confirmations: 0, + rpc_host: "https://rpc".into(), + rpc_port: 443, + password: PathBuf::from("password"), + info: NodeInfo::default(), + gas_price_oracle_url: Some("https://gas.price".into()), + gas_price_speed: GasPriceSpeed::from_str("fast").unwrap(), + gas_price_timeout: Duration::from_secs(5), + default_gas_price: 15_000_000_000, + }; + let timer = Timer::default(); + let mut stream = GasPriceStream::new_with_retriever(&node, BadJson, &timer); + loop { + match stream.poll() { + Ok(Async::Ready(Some(v))) => { + assert_eq!(v, node.default_gas_price); + break; + }, + Err(_) => panic!("should not error out"), + _ => (), + } + } + } + + + struct UnexpectedJson; + + impl Retriever for UnexpectedJson { + type Item = String; + type Future = FutureResult; + + fn retrieve(&self, _uri: &Uri) -> ::Future { + ok(r#"{"cow": "moo"}"#.into()) + } + } + + #[test] + fn unexpected_json() { + let node = Node { + account: Address::new(), + request_timeout: Duration::from_secs(5), + poll_interval: Duration::from_secs(1), + required_confirmations: 0, + rpc_host: "https://rpc".into(), + rpc_port: 443, + password: PathBuf::from("password"), + info: NodeInfo::default(), + gas_price_oracle_url: Some("https://gas.price".into()), + gas_price_speed: GasPriceSpeed::from_str("fast").unwrap(), + gas_price_timeout: Duration::from_secs(5), + default_gas_price: 15_000_000_000, + }; + let timer = Timer::default(); + let mut stream = GasPriceStream::new_with_retriever(&node, UnexpectedJson, &timer); + loop { + match stream.poll() { + Ok(Async::Ready(Some(v))) => { + assert_eq!(v, node.default_gas_price); + break; + }, + Err(_) => panic!("should not error out"), + _ => (), + } + } + } + + struct NonObjectJson; + + impl Retriever for NonObjectJson { + type Item = String; + type Future = FutureResult; + + fn retrieve(&self, _uri: &Uri) -> ::Future { + ok("3".into()) + } + } + + #[test] + fn non_object_json() { + let node = Node { + account: Address::new(), + request_timeout: Duration::from_secs(5), + poll_interval: Duration::from_secs(1), + required_confirmations: 0, + rpc_host: "https://rpc".into(), + rpc_port: 443, + password: PathBuf::from("password"), + info: NodeInfo::default(), + gas_price_oracle_url: Some("https://gas.price".into()), + gas_price_speed: GasPriceSpeed::from_str("fast").unwrap(), + gas_price_timeout: Duration::from_secs(5), + default_gas_price: 15_000_000_000, + }; + let timer = Timer::default(); + let mut stream = GasPriceStream::new_with_retriever(&node, NonObjectJson, &timer); + loop { + match stream.poll() { + Ok(Async::Ready(Some(v))) => { + assert_eq!(v, node.default_gas_price); + break; + }, + Err(_) => panic!("should not error out"), + _ => (), + } + } + } + + struct CorrectJson; + + impl Retriever for CorrectJson { + type Item = String; + type Future = FutureResult; + + fn retrieve(&self, _uri: &Uri) -> ::Future { + ok(r#"{"fast": 12.0}"#.into()) + } + } + + #[test] + fn correct_json() { + let node = Node { + account: Address::new(), + request_timeout: Duration::from_secs(5), + poll_interval: Duration::from_secs(1), + required_confirmations: 0, + rpc_host: "https://rpc".into(), + rpc_port: 443, + password: PathBuf::from("password"), + info: NodeInfo::default(), + gas_price_oracle_url: Some("https://gas.price".into()), + gas_price_speed: GasPriceSpeed::from_str("fast").unwrap(), + gas_price_timeout: Duration::from_secs(5), + default_gas_price: 15_000_000_000, + }; + let timer = Timer::default(); + let mut stream = GasPriceStream::new_with_retriever(&node, CorrectJson, &timer); + loop { + match stream.poll() { + Ok(Async::Ready(Some(v))) => { + assert_eq!(v, 12_000_000_000); + break; + }, + Err(_) => panic!("should not error out"), + _ => (), + } + } + } + +} + diff --git a/bridge/src/bridge/mod.rs b/bridge/src/bridge/mod.rs index eda5179..528599a 100644 --- a/bridge/src/bridge/mod.rs +++ b/bridge/src/bridge/mod.rs @@ -24,7 +24,7 @@ pub use self::chain_id::{ChainIdRetrieval, create_chain_id_retrieval}; pub use self::deposit_relay::{DepositRelay, create_deposit_relay}; pub use self::withdraw_relay::{WithdrawRelay, create_withdraw_relay}; pub use self::withdraw_confirm::{WithdrawConfirm, create_withdraw_confirm}; -pub use self::gas_price::GasPriceStream; +pub use self::gas_price::StandardGasPriceStream; /// Last block checked by the bridge components. #[derive(Clone, Copy)] @@ -89,14 +89,14 @@ pub fn create_bridge_backed_by(app: Arc< let foreign_balance = Arc::new(RwLock::new(None)); let home_gas_stream = if app.config.home.gas_price_oracle_url.is_some() { - let stream = GasPriceStream::new(&app.config.home, handle, &app.timer); + let stream = StandardGasPriceStream::new(&app.config.home, handle, &app.timer); Some(stream) } else { None }; let foreign_gas_stream = if app.config.foreign.gas_price_oracle_url.is_some() { - let stream = GasPriceStream::new(&app.config.foreign, handle, &app.timer); + let stream = StandardGasPriceStream::new(&app.config.foreign, handle, &app.timer); Some(stream) } else { None @@ -134,8 +134,8 @@ pub struct Bridge { state: BridgeStatus, backend: F, running: Arc, - home_gas_stream: Option, - foreign_gas_stream: Option, + home_gas_stream: Option, + foreign_gas_stream: Option, home_gas_price: Arc>, foreign_gas_price: Arc>, } diff --git a/bridge/src/error.rs b/bridge/src/error.rs index 6413872..63209c0 100644 --- a/bridge/src/error.rs +++ b/bridge/src/error.rs @@ -58,6 +58,10 @@ error_chain! { description("contextualized error") display("{:?} in {}", err, context) } + OtherError(error: String) { + description("other error") + display("{}", error) + } } } From 4404af9a5faaeca95476db1c40dc1a4672fadec6 Mon Sep 17 00:00:00 2001 From: Yurii Rashkovskii Date: Mon, 28 May 2018 10:06:38 -0700 Subject: [PATCH 07/15] Problem: GasPriceStream using web3 timeout However, this is different infrastructure and might have different requirements. Solution: use gas_price_timeout --- bridge/src/bridge/gas_price.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bridge/src/bridge/gas_price.rs b/bridge/src/bridge/gas_price.rs index c67002e..f5ebbe3 100644 --- a/bridge/src/bridge/gas_price.rs +++ b/bridge/src/bridge/gas_price.rs @@ -72,7 +72,7 @@ impl GasPriceStream where I: AsRef<[u8]>, F: Future Date: Fri, 25 May 2018 00:23:16 -0700 Subject: [PATCH 08/15] Problem: authorities.accounts config setting is obsolete Solution: make it only accessible when `deploy` feature is on (for integration tests) Closes #73 --- README.md | 5 ----- bridge/src/config.rs | 23 ++++++----------------- examples/config.toml | 3 --- 3 files changed, 6 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index e887a9d..115a355 100644 --- a/README.md +++ b/README.md @@ -91,11 +91,6 @@ required_confirmations = 0 password = "foreign_password.txt" [authorities] -accounts = [ - "0x006e27b6a72e1f34c626762f3c4761547aff1421", - "0x006e27b6a72e1f34c626762f3c4761547aff1421", - "0x006e27b6a72e1f34c626762f3c4761547aff1421" -] required_signatures = 2 [transactions] diff --git a/bridge/src/config.rs b/bridge/src/config.rs index 2a845fe..a8fac26 100644 --- a/bridge/src/config.rs +++ b/bridge/src/config.rs @@ -50,6 +50,7 @@ impl Config { home: Node::from_load_struct(config.home)?, foreign: Node::from_load_struct(config.foreign)?, authorities: Authorities { + #[cfg(feature = "deploy")] accounts: config.authorities.accounts, required_signatures: config.authorities.required_signatures, }, @@ -206,6 +207,7 @@ pub struct ContractConfig { #[derive(Debug, PartialEq, Clone)] pub struct Authorities { + #[cfg(feature = "deploy")] pub accounts: Vec
, pub required_signatures: u32, } @@ -305,8 +307,9 @@ mod load { } #[derive(Deserialize)] - #[serde(deny_unknown_fields)] pub struct Authorities { + #[cfg(feature = "deploy")] + #[serde(default)] pub accounts: Vec
, pub required_signatures: u32, } @@ -351,11 +354,6 @@ password = "password" bin = "../compiled_contracts/ForeignBridge.bin" [authorities] -accounts = [ - "0x0000000000000000000000000000000000000001", - "0x0000000000000000000000000000000000000002", - "0x0000000000000000000000000000000000000003" -] required_signatures = 2 [transactions] @@ -402,10 +400,8 @@ home_deploy = { gas = 20 } default_gas_price: DEFAULT_GAS_PRICE_WEI, }, authorities: Authorities { + #[cfg(feature = "deploy")] accounts: vec![ - "0000000000000000000000000000000000000001".into(), - "0000000000000000000000000000000000000002".into(), - "0000000000000000000000000000000000000003".into(), ], required_signatures: 2, }, @@ -449,11 +445,6 @@ password = "password" bin = "../compiled_contracts/ForeignBridge.bin" [authorities] -accounts = [ - "0x0000000000000000000000000000000000000001", - "0x0000000000000000000000000000000000000002", - "0x0000000000000000000000000000000000000003" -] required_signatures = 2 "#; let expected = Config { @@ -495,10 +486,8 @@ required_signatures = 2 default_gas_price: DEFAULT_GAS_PRICE_WEI, }, authorities: Authorities { + #[cfg(feature = "deploy")] accounts: vec![ - "0000000000000000000000000000000000000001".into(), - "0000000000000000000000000000000000000002".into(), - "0000000000000000000000000000000000000003".into(), ], required_signatures: 2, }, diff --git a/examples/config.toml b/examples/config.toml index 40e6755..68e6509 100644 --- a/examples/config.toml +++ b/examples/config.toml @@ -14,9 +14,6 @@ rpc_host = "https://rpc.host.for.foreign" rpc_port = 443 [authorities] -accounts = [ - "0x006e27b6a72e1f34c626762f3c4761547aff1421", -] required_signatures = 1 [transactions] From 3ea15931fbfa6568d60a2ed2022eecfc9eb122c5 Mon Sep 17 00:00:00 2001 From: Yurii Rashkovskii Date: Thu, 31 May 2018 11:05:46 -0700 Subject: [PATCH 09/15] Prepare 0.2.1 --- Cargo.lock | 10 +++++----- bridge/Cargo.toml | 2 +- cli/Cargo.toml | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c7d5714..f1db351 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -147,7 +147,7 @@ dependencies = [ [[package]] name = "bridge" -version = "0.2.0" +version = "0.2.1" dependencies = [ "error-chain 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)", "ethabi 5.1.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -180,9 +180,9 @@ dependencies = [ [[package]] name = "bridge-cli" -version = "0.2.0" +version = "0.2.1" dependencies = [ - "bridge 0.2.0", + "bridge 0.2.1", "ctrlc 3.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "docopt 0.8.3 (registry+https://github.com/rust-lang/crates.io-index)", "env_logger 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1069,7 +1069,7 @@ dependencies = [ name = "integration-tests" version = "0.1.0" dependencies = [ - "bridge 0.2.0", + "bridge 0.2.1", "ethabi 5.1.1 (registry+https://github.com/rust-lang/crates.io-index)", "ethabi-contract 5.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "ethabi-derive 5.1.2 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2335,7 +2335,7 @@ dependencies = [ name = "tests" version = "0.1.0" dependencies = [ - "bridge 0.2.0", + "bridge 0.2.1", "ethabi 5.1.1 (registry+https://github.com/rust-lang/crates.io-index)", "ethcore 1.11.0 (git+http://github.com/paritytech/parity?rev=991f0ca)", "ethereum-types 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/bridge/Cargo.toml b/bridge/Cargo.toml index 2d87e07..a649272 100644 --- a/bridge/Cargo.toml +++ b/bridge/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "bridge" -version = "0.2.0" +version = "0.2.1" [dependencies] futures = "0.1" diff --git a/cli/Cargo.toml b/cli/Cargo.toml index c866912..a26b900 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "bridge-cli" -version = "0.2.0" +version = "0.2.1" [[bin]] name = "bridge" From b7d7dd3a97d262f5e450507f2abcc47cbf8737e5 Mon Sep 17 00:00:00 2001 From: Yurii Rashkovskii Date: Thu, 31 May 2018 13:45:34 -0700 Subject: [PATCH 10/15] Bump version to 0.3.0 --- Cargo.lock | 10 +++++----- bridge/Cargo.toml | 2 +- cli/Cargo.toml | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f1db351..407b4da 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -147,7 +147,7 @@ dependencies = [ [[package]] name = "bridge" -version = "0.2.1" +version = "0.3.0" dependencies = [ "error-chain 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)", "ethabi 5.1.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -180,9 +180,9 @@ dependencies = [ [[package]] name = "bridge-cli" -version = "0.2.1" +version = "0.3.0" dependencies = [ - "bridge 0.2.1", + "bridge 0.3.0", "ctrlc 3.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "docopt 0.8.3 (registry+https://github.com/rust-lang/crates.io-index)", "env_logger 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1069,7 +1069,7 @@ dependencies = [ name = "integration-tests" version = "0.1.0" dependencies = [ - "bridge 0.2.1", + "bridge 0.3.0", "ethabi 5.1.1 (registry+https://github.com/rust-lang/crates.io-index)", "ethabi-contract 5.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "ethabi-derive 5.1.2 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2335,7 +2335,7 @@ dependencies = [ name = "tests" version = "0.1.0" dependencies = [ - "bridge 0.2.1", + "bridge 0.3.0", "ethabi 5.1.1 (registry+https://github.com/rust-lang/crates.io-index)", "ethcore 1.11.0 (git+http://github.com/paritytech/parity?rev=991f0ca)", "ethereum-types 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/bridge/Cargo.toml b/bridge/Cargo.toml index a649272..9bbfbc1 100644 --- a/bridge/Cargo.toml +++ b/bridge/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "bridge" -version = "0.2.1" +version = "0.3.0" [dependencies] futures = "0.1" diff --git a/cli/Cargo.toml b/cli/Cargo.toml index a26b900..ce7f6f4 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "bridge-cli" -version = "0.2.1" +version = "0.3.0" [[bin]] name = "bridge" From 813a8715f99d1957090d84767e13f64ff019c507 Mon Sep 17 00:00:00 2001 From: Yurii Rashkovskii Date: Thu, 31 May 2018 14:05:19 -0700 Subject: [PATCH 11/15] Problem: hard to check which version the bridge is Solution: introduce `bridge -v|--version` flag to print the version The version is built off `git describe`, however, as a backup, if `git describe` fails (stripped .git, etc.), this will fall back to whatever is specified in Cargo.toml Fixes #87 --- Cargo.lock | 7 +++++++ cli/Cargo.toml | 1 + cli/build.rs | 12 ++++++++++++ cli/src/main.rs | 9 +++++++++ 4 files changed, 29 insertions(+) create mode 100644 cli/build.rs diff --git a/Cargo.lock b/Cargo.lock index 407b4da..d486cd8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -192,6 +192,7 @@ dependencies = [ "serde 1.0.43 (registry+https://github.com/rust-lang/crates.io-index)", "serde_derive 1.0.43 (registry+https://github.com/rust-lang/crates.io-index)", "tokio-core 0.1.17 (registry+https://github.com/rust-lang/crates.io-index)", + "version 3.0.0 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -2751,6 +2752,11 @@ dependencies = [ "ws2_32-sys 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "version" +version = "3.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "version_check" version = "0.1.3" @@ -3221,6 +3227,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum vcpkg 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)" = "7ed0f6789c8a85ca41bbc1c9d175422116a9869bd1cf31bb08e1493ecce60380" "checksum vec_map 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)" = "887b5b631c2ad01628bbbaa7dd4c869f80d3186688f8d0b6f58774fbe324988c" "checksum vecio 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "0795a11576d29ae80525a3fda315bf7b534f8feb9d34101e5fe63fb95bb2fd24" +"checksum version 3.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "3a449064fee414fcc201356a3e6c1510f6c8829ed28bb06b91c54ebe208ce065" "checksum version_check 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)" = "6b772017e347561807c1aa192438c5fd74242a670a6cffacc40f2defd1dc069d" "checksum vm 0.1.0 (git+http://github.com/paritytech/parity?rev=991f0ca)" = "" "checksum void 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)" = "6a02e4885ed3bc0f2de90ea6dd45ebcbb66dacffe03547fadbb0eeae2770887d" diff --git a/cli/Cargo.toml b/cli/Cargo.toml index ce7f6f4..159ab9c 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -17,6 +17,7 @@ env_logger = "0.4" futures = "0.1.14" jsonrpc-core = "8.0" ctrlc = { version = "3.1", features = ["termination"] } +version = "3" [features] default = [] diff --git a/cli/build.rs b/cli/build.rs new file mode 100644 index 0000000..c363065 --- /dev/null +++ b/cli/build.rs @@ -0,0 +1,12 @@ +use std::process::Command; + +fn main() { + let cmd = Command::new("git").args(&["describe", "--long", "--tags", "--always", "--dirty=-modified"]).output().unwrap(); + if cmd.status.success() { + // if we're successful, use this as a version + let ver = std::str::from_utf8(&cmd.stdout[1..]).unwrap().trim(); // drop "v" in the front + println!("cargo:rustc-env={}={}", "CARGO_PKG_VERSION", ver); + } + // otherwise, whatever is specified in Cargo manifest + println!("cargo:rerun-if-changed=nonexistentfile"); // always rerun build.rs +} diff --git a/cli/src/main.rs b/cli/src/main.rs index 38d48d6..a24b24c 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -10,6 +10,8 @@ extern crate env_logger; extern crate bridge; extern crate ctrlc; extern crate jsonrpc_core as rpc; +#[macro_use] +extern crate version; use std::{env, fs, io}; use std::sync::Arc; @@ -73,15 +75,18 @@ POA-Ethereum bridge. Usage: bridge --config --database bridge -h | --help + bridge -v | --version Options: -h, --help Display help message and exit. + -v, --version Print version and exit. "#; #[derive(Debug, Deserialize)] pub struct Args { arg_config: PathBuf, arg_database: PathBuf, + flag_version: bool, } use std::sync::atomic::{AtomicBool, Ordering}; @@ -118,6 +123,10 @@ fn execute(command: I, running: Arc) -> Result Date: Thu, 31 May 2018 14:17:48 -0700 Subject: [PATCH 12/15] Problem: hard to understand what changed recently Solution: write up release notes --- RELEASE_NOTES.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 RELEASE_NOTES.md diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md new file mode 100644 index 0000000..f82e14e --- /dev/null +++ b/RELEASE_NOTES.md @@ -0,0 +1,20 @@ +# 0.2.1 + +This release contains a number of bugfixes and a change in handling gas price. +It is no longer set statically but rather dynamically using an external oracle +(see [config example](examples/config.toml)) + +# 0.2.0 + +This release, most notably, fixes a condition in which not all logs might be +retrieved from an RPC endpoint, resulting in the bridge not being able to +see all relevant events. + +It also improves the performance by introducing concurrent transaction batching. + +On the operations side, it'll now print the context of an occurring error +before exiting to help investigating that error. + +# 0.1.0 + +Initial release From 297388a4c01a2427f5f3c9c3d4d3f950b2bf955c Mon Sep 17 00:00:00 2001 From: phahulin Date: Fri, 1 Jun 2018 11:46:59 +0300 Subject: [PATCH 13/15] Fix parameter name in README Change `gas_price_speed_type` to `gas_price_speed` --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 9838cd6..1e18730 100644 --- a/README.md +++ b/README.md @@ -122,7 +122,7 @@ withdraw_confirm = { gas = 3000000 } - `home/foreign.password` - path to the file containing a password for the validator's account (to decrypt the key from the keystore) - `home/foreign.gas_price_oracle_url` - the URL used to query the current gas-price for the home and foreign nodes, this service is known as the gas-price Oracle. This config option defaults to `None` if not supplied in the User's config TOML file. If this config value is `None`, no Oracle gas-price querying will occur, resulting in the config value for `home/foreign.default_gas_price` being used for all gas-prices. - `home/foreign.gas_price_timeout` - the number of seconds to wait for an HTTP response from the gas price oracle before using the default gas price. Defaults to `10 seconds`. -- `home/foreign.gas_price_speed_type` - retrieve the gas-price corresponding to this speed when querying from an Oracle. Defaults to `fast`. The available values are: "instant", "fast", "standard", and "slow". +- `home/foreign.gas_price_speed` - retrieve the gas-price corresponding to this speed when querying from an Oracle. Defaults to `fast`. The available values are: "instant", "fast", "standard", and "slow". - `home/foreign.default_gas_price` - the default gas price (in WEI) used in transactions with the home or foreign nodes. The `default_gas_price` is used when the Oracle cannot be reached. The default value is `15_000_000_000` WEI (ie. 15 GWEI). #### authorities options From ae8cc1552f27ee9d8859016425458612f95efa0b Mon Sep 17 00:00:00 2001 From: Yurii Rashkovskii Date: Fri, 1 Jun 2018 18:31:47 -0700 Subject: [PATCH 14/15] Problem: slow performance and regular timeouts sending transactions Solution: fix the maximum number of concurrent HTTP request at a transport level. It is set by default to 64 and there's now a new configuration parameter (`concurrent_http_requests`) in `home` and `foreign` sections. Previously used `concurrency` parameter from transactions configuration has been removed. --- README.md | 4 +--- bridge/src/app.rs | 8 ++++---- bridge/src/bridge/deposit_relay.rs | 6 +++--- bridge/src/bridge/gas_price.rs | 7 ++++++- bridge/src/bridge/withdraw_confirm.rs | 6 +++--- bridge/src/bridge/withdraw_relay.rs | 6 +++--- bridge/src/config.rs | 14 +++++++++----- 7 files changed, 29 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 9838cd6..98b89b2 100644 --- a/README.md +++ b/README.md @@ -124,6 +124,7 @@ withdraw_confirm = { gas = 3000000 } - `home/foreign.gas_price_timeout` - the number of seconds to wait for an HTTP response from the gas price oracle before using the default gas price. Defaults to `10 seconds`. - `home/foreign.gas_price_speed_type` - retrieve the gas-price corresponding to this speed when querying from an Oracle. Defaults to `fast`. The available values are: "instant", "fast", "standard", and "slow". - `home/foreign.default_gas_price` - the default gas price (in WEI) used in transactions with the home or foreign nodes. The `default_gas_price` is used when the Oracle cannot be reached. The default value is `15_000_000_000` WEI (ie. 15 GWEI). +- `home/foreign.concurrent_http_requests` - the number of concurrent HTTP requests allowed in-flight (default: **64**) #### authorities options @@ -133,11 +134,8 @@ withdraw_confirm = { gas = 3000000 } #### transaction options - `transaction.deposit_relay.gas` - specify how much gas should be consumed by deposit relay -- `transaction.deposit_relay.concurrency` - how many concurrent transactions can be sent (default: **100**) - `transaction.withdraw_confirm.gas` - specify how much gas should be consumed by withdraw confirm -- `transaction.withdraw_confirm.concurrency` - how many concurrent transactions can be sent (default: **100**) - `transaction.withdraw_relay.gas` - specify how much gas should be consumed by withdraw relay -- `transaction.withdraw_relay.concurrency` - how many concurrent transactions can be sent (default: **100**) ### Database file format diff --git a/bridge/src/app.rs b/bridge/src/app.rs index a999cde..70f6980 100644 --- a/bridge/src/app.rs +++ b/bridge/src/app.rs @@ -31,13 +31,13 @@ pub struct Connections where T: Transport { } impl Connections { - pub fn new_http(handle: &Handle, home: &str, foreign: &str) -> Result { + pub fn new_http(handle: &Handle, home: &str, home_concurrent_connections: usize, foreign: &str, foreign_concurrent_connections: usize) -> Result { - let home = Http::with_event_loop(home, handle,1) + let home = Http::with_event_loop(home, handle,home_concurrent_connections) .map_err(ErrorKind::Web3) .map_err(Error::from) .chain_err(||"Cannot connect to home node rpc")?; - let foreign = Http::with_event_loop(foreign, handle, 1) + let foreign = Http::with_event_loop(foreign, handle, foreign_concurrent_connections) .map_err(ErrorKind::Web3) .map_err(Error::from) .chain_err(||"Cannot connect to foreign node rpc")?; @@ -64,7 +64,7 @@ impl App { let home_url:String = format!("{}:{}", config.home.rpc_host, config.home.rpc_port); let foreign_url:String = format!("{}:{}", config.foreign.rpc_host, config.foreign.rpc_port); - let connections = Connections::new_http(handle, home_url.as_ref(), foreign_url.as_ref())?; + let connections = Connections::new_http(handle, home_url.as_ref(), config.home.concurrent_http_requests, foreign_url.as_ref(), config.foreign.concurrent_http_requests)?; let keystore = EthStore::open(Box::new(RootDiskDirectory::at(&config.keystore))).map_err(|e| ErrorKind::KeyStore(e))?; let keystore = AccountProvider::new(Box::new(keystore), AccountProviderSettings { diff --git a/bridge/src/bridge/deposit_relay.rs b/bridge/src/bridge/deposit_relay.rs index 66feab4..ba1ccae 100644 --- a/bridge/src/bridge/deposit_relay.rs +++ b/bridge/src/bridge/deposit_relay.rs @@ -1,5 +1,5 @@ use std::sync::{Arc, RwLock}; -use futures::{self, Future, Stream, stream::{Collect, iter_ok, IterOk, Buffered}, Poll}; +use futures::{self, Future, Stream, stream::{Collect, FuturesUnordered, futures_unordered}, Poll}; use web3::Transport; use web3::types::{U256, Address, Bytes, Log, FilterBuilder}; use ethabi::RawLog; @@ -35,7 +35,7 @@ enum DepositRelayState { Wait, /// Relaying deposits in progress. RelayDeposits { - future: Collect>>, Error>>>, + future: Collect>>>, block: u64, }, /// All deposits till given block has been relayed. @@ -115,7 +115,7 @@ impl Stream for DepositRelay { info!("relaying {} deposits", len); DepositRelayState::RelayDeposits { - future: iter_ok(deposits).buffered(self.app.config.txs.deposit_relay.concurrency).collect(), + future: futures_unordered(deposits).collect(), block: item.to, } }, diff --git a/bridge/src/bridge/gas_price.rs b/bridge/src/bridge/gas_price.rs index f5ebbe3..7988635 100644 --- a/bridge/src/bridge/gas_price.rs +++ b/bridge/src/bridge/gas_price.rs @@ -143,7 +143,7 @@ mod tests { use super::*; use error::{Error, ErrorKind}; use futures::{Async, future::{err, ok, FutureResult}}; - use config::{Node, NodeInfo}; + use config::{Node, NodeInfo, DEFAULT_CONCURRENCY}; use tokio_timer::Timer; use std::time::Duration; use std::path::PathBuf; @@ -176,6 +176,7 @@ mod tests { gas_price_speed: GasPriceSpeed::from_str("fast").unwrap(), gas_price_timeout: Duration::from_secs(5), default_gas_price: 15_000_000_000, + concurrent_http_requests: DEFAULT_CONCURRENCY, }; let timer = Timer::default(); let mut stream = GasPriceStream::new_with_retriever(&node, ErroredRequest, &timer); @@ -218,6 +219,7 @@ mod tests { gas_price_speed: GasPriceSpeed::from_str("fast").unwrap(), gas_price_timeout: Duration::from_secs(5), default_gas_price: 15_000_000_000, + concurrent_http_requests: DEFAULT_CONCURRENCY, }; let timer = Timer::default(); let mut stream = GasPriceStream::new_with_retriever(&node, BadJson, &timer); @@ -260,6 +262,7 @@ mod tests { gas_price_speed: GasPriceSpeed::from_str("fast").unwrap(), gas_price_timeout: Duration::from_secs(5), default_gas_price: 15_000_000_000, + concurrent_http_requests: DEFAULT_CONCURRENCY, }; let timer = Timer::default(); let mut stream = GasPriceStream::new_with_retriever(&node, UnexpectedJson, &timer); @@ -301,6 +304,7 @@ mod tests { gas_price_speed: GasPriceSpeed::from_str("fast").unwrap(), gas_price_timeout: Duration::from_secs(5), default_gas_price: 15_000_000_000, + concurrent_http_requests: DEFAULT_CONCURRENCY, }; let timer = Timer::default(); let mut stream = GasPriceStream::new_with_retriever(&node, NonObjectJson, &timer); @@ -342,6 +346,7 @@ mod tests { gas_price_speed: GasPriceSpeed::from_str("fast").unwrap(), gas_price_timeout: Duration::from_secs(5), default_gas_price: 15_000_000_000, + concurrent_http_requests: DEFAULT_CONCURRENCY, }; let timer = Timer::default(); let mut stream = GasPriceStream::new_with_retriever(&node, CorrectJson, &timer); diff --git a/bridge/src/bridge/withdraw_confirm.rs b/bridge/src/bridge/withdraw_confirm.rs index 9f12b6b..4dc97c7 100644 --- a/bridge/src/bridge/withdraw_confirm.rs +++ b/bridge/src/bridge/withdraw_confirm.rs @@ -1,6 +1,6 @@ use std::sync::{Arc, RwLock}; use std::ops; -use futures::{self, Future, Stream, stream::{Collect, IterOk, iter_ok, Buffered}, Poll}; +use futures::{self, Future, Stream, stream::{Collect, FuturesUnordered, futures_unordered}, Poll}; use web3::Transport; use web3::types::{U256, H520, Address, Bytes, FilterBuilder}; use api::{self, LogStream}; @@ -30,7 +30,7 @@ enum WithdrawConfirmState { Wait, /// Confirming withdraws. ConfirmWithdraws { - future: Collect>>, Error>>>, + future: Collect>>>, block: u64, }, /// All withdraws till given block has been confirmed. @@ -139,7 +139,7 @@ impl Stream for WithdrawConfirm { info!("submitting {} signatures", len); WithdrawConfirmState::ConfirmWithdraws { - future: iter_ok(confirmations).buffered(self.app.config.txs.withdraw_confirm.concurrency).collect(), + future: futures_unordered(confirmations).collect(), block, } }, diff --git a/bridge/src/bridge/withdraw_relay.rs b/bridge/src/bridge/withdraw_relay.rs index a9bc5b4..96a8ecc 100644 --- a/bridge/src/bridge/withdraw_relay.rs +++ b/bridge/src/bridge/withdraw_relay.rs @@ -1,5 +1,5 @@ use std::sync::{Arc, RwLock}; -use futures::{self, Future, Stream, stream::{Collect, iter_ok, IterOk, Buffered}, Poll}; +use futures::{self, Future, Stream, stream::{Collect, FuturesUnordered, futures_unordered}, Poll}; use futures::future::{JoinAll, join_all, Join}; use tokio_timer::Timeout; use web3::Transport; @@ -70,7 +70,7 @@ pub enum WithdrawRelayState { block: u64, }, RelayWithdraws { - future: Collect>>, Error>>>, + future: Collect>>>, block: u64, }, Yield(Option), @@ -236,7 +236,7 @@ impl Stream for WithdrawRelay { info!("relaying {} withdraws", len); WithdrawRelayState::RelayWithdraws { - future: iter_ok(relays).buffered(self.app.config.txs.withdraw_relay.concurrency).collect(), + future: futures_unordered(relays).collect(), block, } }, diff --git a/bridge/src/config.rs b/bridge/src/config.rs index 2a845fe..d580c29 100644 --- a/bridge/src/config.rs +++ b/bridge/src/config.rs @@ -15,7 +15,7 @@ const DEFAULT_POLL_INTERVAL: u64 = 1; const DEFAULT_CONFIRMATIONS: usize = 12; const DEFAULT_TIMEOUT: u64 = 3600; const DEFAULT_RPC_PORT: u16 = 8545; -const DEFAULT_CONCURRENCY: usize = 100; +pub(crate) const DEFAULT_CONCURRENCY: usize = 64; const DEFAULT_GAS_PRICE_SPEED: GasPriceSpeed = GasPriceSpeed::Fast; const DEFAULT_GAS_PRICE_TIMEOUT_SECS: u64 = 10; const DEFAULT_GAS_PRICE_WEI: u64 = 15_000_000_000; @@ -79,6 +79,7 @@ pub struct Node { pub gas_price_speed: GasPriceSpeed, pub gas_price_timeout: Duration, pub default_gas_price: u64, + pub concurrent_http_requests: usize, } use std::sync::{Arc, RwLock}; @@ -118,6 +119,7 @@ impl Node { }; let default_gas_price = node.default_gas_price.unwrap_or(DEFAULT_GAS_PRICE_WEI); + let concurrent_http_requests = node.concurrent_http_requests.unwrap_or(DEFAULT_CONCURRENCY); let result = Node { account: node.account, @@ -141,6 +143,7 @@ impl Node { gas_price_speed, gas_price_timeout, default_gas_price, + concurrent_http_requests, }; Ok(result) @@ -185,7 +188,6 @@ impl Transactions { pub struct TransactionConfig { pub gas: u64, pub gas_price: u64, - pub concurrency: usize, } impl TransactionConfig { @@ -193,7 +195,6 @@ impl TransactionConfig { TransactionConfig { gas: cfg.gas.unwrap_or_default(), gas_price: cfg.gas_price.unwrap_or_default(), - concurrency: cfg.concurrency.unwrap_or(DEFAULT_CONCURRENCY), } } } @@ -277,6 +278,7 @@ mod load { pub gas_price_speed: Option, pub gas_price_timeout: Option, pub default_gas_price: Option, + pub concurrent_http_requests: Option, } #[derive(Deserialize)] @@ -295,7 +297,6 @@ mod load { pub struct TransactionConfig { pub gas: Option, pub gas_price: Option, - pub concurrency: Option, } #[derive(Deserialize)] @@ -382,6 +383,7 @@ home_deploy = { gas = 20 } gas_price_speed: DEFAULT_GAS_PRICE_SPEED, gas_price_timeout: Duration::from_secs(DEFAULT_GAS_PRICE_TIMEOUT_SECS), default_gas_price: DEFAULT_GAS_PRICE_WEI, + concurrent_http_requests: DEFAULT_CONCURRENCY, }, foreign: Node { account: "0000000000000000000000000000000000000001".into(), @@ -400,6 +402,7 @@ home_deploy = { gas = 20 } gas_price_speed: DEFAULT_GAS_PRICE_SPEED, gas_price_timeout: Duration::from_secs(DEFAULT_GAS_PRICE_TIMEOUT_SECS), default_gas_price: DEFAULT_GAS_PRICE_WEI, + concurrent_http_requests: DEFAULT_CONCURRENCY, }, authorities: Authorities { accounts: vec![ @@ -418,7 +421,6 @@ home_deploy = { gas = 20 } expected.txs.home_deploy = TransactionConfig { gas: 20, gas_price: 0, - concurrency: DEFAULT_CONCURRENCY, }; } @@ -475,6 +477,7 @@ required_signatures = 2 gas_price_speed: DEFAULT_GAS_PRICE_SPEED, gas_price_timeout: Duration::from_secs(DEFAULT_GAS_PRICE_TIMEOUT_SECS), default_gas_price: DEFAULT_GAS_PRICE_WEI, + concurrent_http_requests: DEFAULT_CONCURRENCY, }, foreign: Node { account: "0000000000000000000000000000000000000001".into(), @@ -493,6 +496,7 @@ required_signatures = 2 gas_price_speed: DEFAULT_GAS_PRICE_SPEED, gas_price_timeout: Duration::from_secs(DEFAULT_GAS_PRICE_TIMEOUT_SECS), default_gas_price: DEFAULT_GAS_PRICE_WEI, + concurrent_http_requests: DEFAULT_CONCURRENCY, }, authorities: Authorities { accounts: vec![ From b890ada627a781ba1160710447045eca1ba4af6f Mon Sep 17 00:00:00 2001 From: Yurii Rashkovskii Date: Thu, 31 May 2018 22:00:15 -0700 Subject: [PATCH 15/15] Problem: config allows arbitrary keys to be passed This means that a misspelled config field can cause a lot of confusion -- things won't work as expected. Solution: restore more restrictive config deserialization where unknown keys would trigger an error --- bridge/src/config.rs | 49 +++++--------------------------------------- 1 file changed, 5 insertions(+), 44 deletions(-) diff --git a/bridge/src/config.rs b/bridge/src/config.rs index 171b9d3..3a75f8c 100644 --- a/bridge/src/config.rs +++ b/bridge/src/config.rs @@ -108,7 +108,7 @@ impl PartialEq for NodeInfo { impl Node { fn from_load_struct(node: load::Node) -> Result { let gas_price_oracle_url = node.gas_price_oracle_url.clone(); - + let gas_price_speed = match node.gas_price_speed { Some(ref s) => GasPriceSpeed::from_str(s).unwrap(), None => DEFAULT_GAS_PRICE_SPEED @@ -223,7 +223,7 @@ pub enum GasPriceSpeed { impl FromStr for GasPriceSpeed { type Err = (); - + fn from_str(s: &str) -> Result { let speed = match s { "instant" => GasPriceSpeed::Instant, @@ -255,6 +255,7 @@ mod load { use web3::types::Address; #[derive(Deserialize)] + #[serde(deny_unknown_fields)] pub struct Config { pub home: Node, pub foreign: Node, @@ -266,6 +267,7 @@ mod load { } #[derive(Deserialize)] + #[serde(deny_unknown_fields)] pub struct Node { pub account: Address, #[cfg(feature = "deploy")] @@ -284,6 +286,7 @@ mod load { } #[derive(Deserialize)] + #[serde(deny_unknown_fields)] pub struct Transactions { #[cfg(feature = "deploy")] pub home_deploy: Option, @@ -332,7 +335,6 @@ mod tests { fn load_full_setup_from_str() { let toml = r#" keystore = "/keys" -estimated_gas_cost_of_withdraw = 100000 [home] account = "0x1B68Cb0B50181FC4006Ce572cF346e596E51818b" @@ -342,23 +344,16 @@ rpc_host = "127.0.0.1" rpc_port = 8545 password = "password" -[home.contract] -bin = "../compiled_contracts/HomeBridge.bin" - [foreign] account = "0x0000000000000000000000000000000000000001" rpc_host = "127.0.0.1" rpc_port = 8545 password = "password" -[foreign.contract] -bin = "../compiled_contracts/ForeignBridge.bin" - [authorities] required_signatures = 2 [transactions] -home_deploy = { gas = 20 } "#; #[allow(unused_mut)] @@ -366,10 +361,6 @@ home_deploy = { gas = 20 } txs: Transactions::default(), home: Node { account: "1B68Cb0B50181FC4006Ce572cF346e596E51818b".into(), - #[cfg(feature = "deploy")] - contract: ContractConfig { - bin: include_str!("../../compiled_contracts/HomeBridge.bin").from_hex().unwrap().into(), - }, poll_interval: Duration::from_secs(2), request_timeout: Duration::from_secs(DEFAULT_TIMEOUT), required_confirmations: 100, @@ -385,10 +376,6 @@ home_deploy = { gas = 20 } }, foreign: Node { account: "0000000000000000000000000000000000000001".into(), - #[cfg(feature = "deploy")] - contract: ContractConfig { - bin: include_str!("../../compiled_contracts/ForeignBridge.bin").from_hex().unwrap().into(), - }, poll_interval: Duration::from_secs(1), request_timeout: Duration::from_secs(DEFAULT_TIMEOUT), required_confirmations: 12, @@ -408,18 +395,9 @@ home_deploy = { gas = 20 } ], required_signatures: 2, }, - #[cfg(feature = "deploy")] - estimated_gas_cost_of_withdraw: 100_000, keystore: "/keys/".into(), }; - #[cfg(feature = "deploy")] { - expected.txs.home_deploy = TransactionConfig { - gas: 20, - gas_price: 0, - }; - } - let config = Config::load_from_str(toml).unwrap(); assert_eq!(expected, config); } @@ -428,24 +406,17 @@ home_deploy = { gas = 20 } fn load_minimal_setup_from_str() { let toml = r#" keystore = "/keys/" -estimated_gas_cost_of_withdraw = 200000000 [home] account = "0x1B68Cb0B50181FC4006Ce572cF346e596E51818b" rpc_host = "" password = "password" -[home.contract] -bin = "../compiled_contracts/HomeBridge.bin" - [foreign] account = "0x0000000000000000000000000000000000000001" rpc_host = "" password = "password" -[foreign.contract] -bin = "../compiled_contracts/ForeignBridge.bin" - [authorities] required_signatures = 2 "#; @@ -453,10 +424,6 @@ required_signatures = 2 txs: Transactions::default(), home: Node { account: "1B68Cb0B50181FC4006Ce572cF346e596E51818b".into(), - #[cfg(feature = "deploy")] - contract: ContractConfig { - bin: include_str!("../../compiled_contracts/HomeBridge.bin").from_hex().unwrap().into(), - }, poll_interval: Duration::from_secs(1), request_timeout: Duration::from_secs(DEFAULT_TIMEOUT), required_confirmations: 12, @@ -472,10 +439,6 @@ required_signatures = 2 }, foreign: Node { account: "0000000000000000000000000000000000000001".into(), - #[cfg(feature = "deploy")] - contract: ContractConfig { - bin: include_str!("../../compiled_contracts/ForeignBridge.bin").from_hex().unwrap().into(), - }, poll_interval: Duration::from_secs(1), request_timeout: Duration::from_secs(DEFAULT_TIMEOUT), required_confirmations: 12, @@ -495,8 +458,6 @@ required_signatures = 2 ], required_signatures: 2, }, - #[cfg(feature = "deploy")] - estimated_gas_cost_of_withdraw: 200_000_000, keystore: "/keys/".into(), };