From 9a192c1e072f3f8d520df9b07e1ade14a1767200 Mon Sep 17 00:00:00 2001 From: Yurii Rashkovskii Date: Tue, 1 May 2018 09:43:34 -0700 Subject: [PATCH] Problem: bridge should not deploy its contracts anymore Bridge's contracts are now developed in a separate repository and have their own deployment procedure: https://github.com/poanetwork/poa-parity-bridge-contracts However, our integration tests are not yet updated to use this deployment procedure. Solution: disable deployment compile-time by default and only use it in integration tests as a stopgap measure until the new deployment procedure (or any other viable alternative) has been used. --- Makefile | 6 + README.md | 20 +--- bridge/Cargo.toml | 4 + bridge/src/bridge/deploy.rs | 108 +++++++++++------- bridge/src/config.rs | 21 +++- cli/Cargo.toml | 4 + cli/src/main.rs | 4 + integration-tests/Cargo.toml | 2 +- .../tests/basic_deposit_then_withdraw.rs | 2 +- integration-tests/tests/insufficient_funds.rs | 2 +- 10 files changed, 107 insertions(+), 66 deletions(-) create mode 100644 Makefile diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..4e427a7 --- /dev/null +++ b/Makefile @@ -0,0 +1,6 @@ +all: target/release/bridge + +.PHONY: target/release/bridge + +target/release/bridge: + cd cli && cargo build --release diff --git a/README.md b/README.md index 826c0e6..ce0354b 100644 --- a/README.md +++ b/README.md @@ -103,14 +103,14 @@ requires `rust` and `cargo`: [installation instructions.](https://www.rust-lang. 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:paritytech/parity-bridge.git`) +assuming you've cloned the bridge (`git clone git@github.com:poanetwork/parity-bridge.git`) and are in the project directory (`cd parity-bridge`) run: ``` -cargo build -p bridge-cli --release +make ``` -to install copy `../target/release/bridge` into a folder that's in your `$PATH`. +and install `../target/release/bridge` in your `$PATH`. ### run @@ -120,8 +120,6 @@ bridge --config config.toml --database db.toml - `--config` - location of the configuration file. configuration file must exist - `--database` - location of the database file. - if there is no file at specified location, new bridge contracts will be deployed - and new database will be created ### configuration [file example](./examples/config.toml) @@ -151,10 +149,6 @@ accounts = [ "0x006e27b6a72e1f34c626762f3c4761547aff1421" ] required_signatures = 2 - -[transactions] -home_deploy = { gas = 500000 } -foreign_deploy = { gas = 500000 } ``` #### options @@ -190,10 +184,6 @@ foreign_deploy = { gas = 500000 } #### transaction options -- `transaction.home_deploy.gas` - specify how much gas should be consumed by home contract deploy -- `transaction.home_deploy.gas_price` - specify gas price for home contract deploy -- `transaction.foreign_deploy.gas` - specify how much gas should be consumed by foreign contract deploy -- `transaction.foreign_deploy.gas_price` - specify gas price for foreign contract deploy - `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.withdraw_confirm.gas` - specify how much gas should be consumed by withdraw confirm @@ -206,8 +196,6 @@ foreign_deploy = { gas = 500000 } ```toml home_contract_address = "0x49edf201c1e139282643d5e7c6fb0c7219ad1db7" foreign_contract_address = "0x49edf201c1e139282643d5e7c6fb0c7219ad1db8" -home_deploy = 100 -foreign_deploy = 101 checked_deposit_relay = 120 checked_withdraw_relay = 121 checked_withdraw_confirm = 121 @@ -217,8 +205,6 @@ checked_withdraw_confirm = 121 - `home_contract_address` - address of the bridge contract on home chain - `foreign_contract_address` - address of the bridge contract on foreign chain -- `home_deploy` - block number at which home contract has been deployed -- `foreign_deploy` - block number at which foreign contract has been deployed - `checked_deposit_relay` - number of the last block for which an authority has relayed deposits to the foreign - `checked_withdraw_relay` - number of the last block for which an authority has relayed withdraws to the home - `checked_withdraw_confirm` - number of the last block for which an authority has confirmed withdraw diff --git a/bridge/Cargo.toml b/bridge/Cargo.toml index 1074f14..b75783e 100644 --- a/bridge/Cargo.toml +++ b/bridge/Cargo.toml @@ -30,3 +30,7 @@ jsonrpc-core = "8.0" [dev-dependencies] tempdir = "0.3" quickcheck = "0.6.1" + +[features] +default = [] +deploy = [] diff --git a/bridge/src/bridge/deploy.rs b/bridge/src/bridge/deploy.rs index 9d211f2..9c737a8 100644 --- a/bridge/src/bridge/deploy.rs +++ b/bridge/src/bridge/deploy.rs @@ -1,12 +1,18 @@ use std::sync::Arc; -use futures::{Future, Poll, future}; +use futures::{Future, Poll}; +#[cfg(feature = "deploy")] +use futures::future; use web3::Transport; +#[cfg(feature = "deploy")] use web3::types::U256; use app::App; use database::Database; use error::{Error, ErrorKind}; +#[cfg(feature = "deploy")] use api; +#[cfg(feature = "deploy")] use ethcore_transaction::{Transaction, Action}; +#[cfg(feature = "deploy")] use super::nonce::{NonceCheck,TransactionWithConfirmation}; pub enum Deployed { @@ -16,24 +22,38 @@ pub enum Deployed { Existing(Database), } +#[cfg(feature = "deploy")] enum DeployState { CheckIfNeeded, Deploying(future::Join>, NonceCheck>>), } +#[cfg(not(feature = "deploy"))] +enum DeployState { + CheckIfNeeded, +} + +#[allow(unused_variables)] pub fn create_deploy(app: Arc>, home_chain_id: u64, foreign_chain_id: u64) -> Deploy { Deploy { app, state: DeployState::CheckIfNeeded, + #[cfg(feature = "deploy")] home_chain_id, + #[cfg(feature = "deploy")] foreign_chain_id, } } pub struct Deploy { app: Arc>, + #[cfg(feature = "deploy")] state: DeployState, + #[cfg(not(feature = "deploy"))] + state: DeployState, + #[cfg(feature = "deploy")] home_chain_id: u64, + #[cfg(feature = "deploy")] foreign_chain_id: u64, } @@ -43,53 +63,60 @@ impl Future for Deploy { fn poll(&mut self) -> Poll { loop { - let next_state = match self.state { + let _next_state = match self.state { DeployState::CheckIfNeeded => match Database::load(&self.app.database_path).map_err(ErrorKind::from) { Ok(database) => return Ok(Deployed::Existing(database).into()), - Err(ErrorKind::MissingFile(_)) => { - let main_data = self.app.home_bridge.constructor( - self.app.config.home.contract.bin.clone().0, - self.app.config.authorities.required_signatures, - self.app.config.authorities.accounts.clone(), - self.app.config.estimated_gas_cost_of_withdraw - ); - let test_data = self.app.foreign_bridge.constructor( - self.app.config.foreign.contract.bin.clone().0, - self.app.config.authorities.required_signatures, - self.app.config.authorities.accounts.clone(), - self.app.config.estimated_gas_cost_of_withdraw - ); + Err(ErrorKind::MissingFile(_e)) => { + #[cfg(feature = "deploy")] { + println!("deploy"); + let main_data = self.app.home_bridge.constructor( + self.app.config.home.contract.bin.clone().0, + self.app.config.authorities.required_signatures, + self.app.config.authorities.accounts.clone(), + self.app.config.estimated_gas_cost_of_withdraw + ); + let test_data = self.app.foreign_bridge.constructor( + self.app.config.foreign.contract.bin.clone().0, + self.app.config.authorities.required_signatures, + self.app.config.authorities.accounts.clone(), + self.app.config.estimated_gas_cost_of_withdraw + ); - let main_tx = Transaction { - nonce: U256::zero(), - gas_price: self.app.config.txs.home_deploy.gas_price.into(), - gas: self.app.config.txs.home_deploy.gas.into(), - action: Action::Create, - value: U256::zero(), - data: main_data.into(), - }; + let main_tx = Transaction { + nonce: U256::zero(), + gas_price: self.app.config.txs.home_deploy.gas_price.into(), + gas: self.app.config.txs.home_deploy.gas.into(), + action: Action::Create, + value: U256::zero(), + data: main_data.into(), + }; - let test_tx = Transaction { - nonce: U256::zero(), - gas_price: self.app.config.txs.foreign_deploy.gas_price.into(), - gas: self.app.config.txs.foreign_deploy.gas.into(), - action: Action::Create, - value: U256::zero(), - data: test_data.into(), - }; + let test_tx = Transaction { + nonce: U256::zero(), + gas_price: self.app.config.txs.foreign_deploy.gas_price.into(), + gas: self.app.config.txs.foreign_deploy.gas.into(), + action: Action::Create, + value: U256::zero(), + data: test_data.into(), + }; - let main_future = api::send_transaction_with_nonce(self.app.connections.home.clone(), self.app.clone(), - self.app.config.home.clone(), main_tx, self.home_chain_id, - TransactionWithConfirmation(self.app.connections.home.clone(), self.app.config.home.poll_interval, self.app.config.home.required_confirmations)); + let main_future = api::send_transaction_with_nonce(self.app.connections.home.clone(), self.app.clone(), + self.app.config.home.clone(), main_tx, self.home_chain_id, + TransactionWithConfirmation(self.app.connections.home.clone(), self.app.config.home.poll_interval, self.app.config.home.required_confirmations)); - let test_future = api::send_transaction_with_nonce(self.app.connections.foreign.clone(), self.app.clone(), - self.app.config.foreign.clone(), test_tx, self.foreign_chain_id, - TransactionWithConfirmation(self.app.connections.foreign.clone(), self.app.config.foreign.poll_interval, self.app.config.foreign.required_confirmations)); + let test_future = api::send_transaction_with_nonce(self.app.connections.foreign.clone(), self.app.clone(), + self.app.config.foreign.clone(), test_tx, self.foreign_chain_id, + TransactionWithConfirmation(self.app.connections.foreign.clone(), self.app.config.foreign.poll_interval, self.app.config.foreign.required_confirmations)); - DeployState::Deploying(main_future.join(test_future)) + DeployState::Deploying(main_future.join(test_future)) + } + #[cfg(not(feature = "deploy"))] { + return Err(ErrorKind::MissingFile(_e).into()) + } }, Err(err) => return Err(err.into()), }, + #[cfg(feature = "deploy")] DeployState::Deploying(ref mut future) => { let (main_receipt, test_receipt) = try_ready!(future.poll()); let database = Database { @@ -104,8 +131,9 @@ impl Future for Deploy { return Ok(Deployed::New(database).into()) }, }; - - self.state = next_state; + #[allow(unreachable_code)] { + self.state = _next_state; + } } } } diff --git a/bridge/src/config.rs b/bridge/src/config.rs index 0339b16..76cbad1 100644 --- a/bridge/src/config.rs +++ b/bridge/src/config.rs @@ -124,7 +124,9 @@ impl Node { #[derive(Debug, PartialEq, Default, Clone)] pub struct Transactions { + #[cfg(feature = "deploy")] pub home_deploy: TransactionConfig, + #[cfg(feature = "deploy")] pub foreign_deploy: TransactionConfig, pub deposit_relay: TransactionConfig, pub withdraw_confirm: TransactionConfig, @@ -134,7 +136,9 @@ pub struct Transactions { impl Transactions { fn from_load_struct(cfg: load::Transactions) -> Self { Transactions { + #[cfg(feature = "deploy")] home_deploy: cfg.home_deploy.map(TransactionConfig::from_load_struct).unwrap_or_default(), + #[cfg(feature = "deploy")] foreign_deploy: cfg.foreign_deploy.map(TransactionConfig::from_load_struct).unwrap_or_default(), deposit_relay: cfg.deposit_relay.map(TransactionConfig::from_load_struct).unwrap_or_default(), withdraw_confirm: cfg.withdraw_confirm.map(TransactionConfig::from_load_struct).unwrap_or_default(), @@ -201,9 +205,10 @@ mod load { } #[derive(Deserialize)] - #[serde(deny_unknown_fields)] pub struct Transactions { + #[cfg(feature = "deploy")] pub home_deploy: Option, + #[cfg(feature = "deploy")] pub foreign_deploy: Option, pub deposit_relay: Option, pub withdraw_confirm: Option, @@ -235,7 +240,9 @@ mod load { mod tests { use std::time::Duration; use rustc_hex::FromHex; - use super::{Config, Node, ContractConfig, Transactions, Authorities, TransactionConfig}; + use super::{Config, Node, ContractConfig, Transactions, Authorities}; + #[cfg(feature = "deploy")] + use super::TransactionConfig; #[test] fn load_full_setup_from_str() { @@ -313,10 +320,12 @@ home_deploy = { gas = 20 } keystore: "/keys/".into(), }; - expected.txs.home_deploy = TransactionConfig { - gas: 20, - gas_price: 0, - }; + #[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); diff --git a/cli/Cargo.toml b/cli/Cargo.toml index b4d477c..b6eb1d7 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -18,3 +18,7 @@ env_logger = "0.4" futures = "0.1.14" jsonrpc-core = "8.0" ctrlc = { version = "3.1", features = ["termination"] } + +[features] +default = [] +deploy = ["bridge/deploy"] diff --git a/cli/src/main.rs b/cli/src/main.rs index 39f9b33..f2a415c 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -151,7 +151,11 @@ fn execute(command: I, running: Arc) -> Result"] [dependencies] -bridge = { path = "../bridge" } +bridge = { path = "../bridge", features = ["deploy"] } futures = "0.1" jsonrpc-core = "8.0" web3 = "0.3" diff --git a/integration-tests/tests/basic_deposit_then_withdraw.rs b/integration-tests/tests/basic_deposit_then_withdraw.rs index 953c281..a439cb9 100644 --- a/integration-tests/tests/basic_deposit_then_withdraw.rs +++ b/integration-tests/tests/basic_deposit_then_withdraw.rs @@ -82,7 +82,7 @@ fn test_basic_deposit_then_withdraw() { assert!(Command::new("cargo") .env("RUST_BACKTRACE", "1") .current_dir("../cli") - .arg("build") + .args(&["build", "--features", "deploy"]) .status() .expect("failed to build bridge cli") .success()); diff --git a/integration-tests/tests/insufficient_funds.rs b/integration-tests/tests/insufficient_funds.rs index 836dbf2..f5246b3 100644 --- a/integration-tests/tests/insufficient_funds.rs +++ b/integration-tests/tests/insufficient_funds.rs @@ -83,7 +83,7 @@ fn test_insufficient_funds() { assert!(Command::new("cargo") .env("RUST_BACKTRACE", "1") .current_dir("../cli") - .arg("build") + .args(&["build", "--features", "deploy"]) .status() .expect("failed to build bridge cli") .success());