From 8a56c5cafb6a845f0c4c8e225b62736a7e836f31 Mon Sep 17 00:00:00 2001 From: Yurii Rashkovskii Date: Sat, 19 May 2018 08:25:39 -0700 Subject: [PATCH 1/2] Problem: required_signatures is static Validators' information is completely configured through validators contracts and does not depend on `authorities.required_signatures` parameter of bridge's configuration. The number of validators also could be changed during run-time and therefore `authorities.required_signatures` parameter will not reflect the actual number of signatures required for transaction validation. Solution: retrieve required_signatures from RequiredSignaturesChanged event and requiredSignatures() method Closes #74 --- README.md | 2 - bridge/src/api.rs | 17 ++++ bridge/src/bridge/deploy.rs | 1 + bridge/src/bridge/deposit_relay.rs | 2 +- bridge/src/bridge/mod.rs | 21 +++-- bridge/src/bridge/withdraw_confirm.rs | 2 +- bridge/src/bridge/withdraw_relay.rs | 129 ++++++++++++++++++++------ bridge/src/config.rs | 8 +- bridge/src/database.rs | 4 + bridge/src/error.rs | 3 + bridge/src/util.rs | 4 +- cli/src/main.rs | 10 +- contracts/bridge.sol | 13 +++ examples/config.toml | 1 - 14 files changed, 166 insertions(+), 51 deletions(-) diff --git a/README.md b/README.md index 9002352..9a28422 100644 --- a/README.md +++ b/README.md @@ -96,7 +96,6 @@ accounts = [ "0x006e27b6a72e1f34c626762f3c4761547aff1421", "0x006e27b6a72e1f34c626762f3c4761547aff1421" ] -required_signatures = 2 [transactions] deposit_relay = { gas = 3000000, gas_price = 1000000000 } @@ -121,7 +120,6 @@ withdraw_confirm = { gas = 3000000, gas_price = 1000000000 } #### authorities options - `authorities.account` - all authorities (**required**) -- `authorities.required_signatures` - number of authorities signatures required to consider action final (**required**) #### transaction options diff --git a/bridge/src/api.rs b/bridge/src/api.rs index cf31055..0a79459 100644 --- a/bridge/src/api.rs +++ b/bridge/src/api.rs @@ -142,6 +142,23 @@ pub fn call(transport: T, address: Address, payload: Bytes) -> Api } } +pub fn call_at(transport: T, address: Address, payload: Bytes, block: Option) -> ApiCall { + let future = api::Eth::new(transport).call(CallRequest { + from: None, + to: address, + gas: None, + gas_price: None, + value: None, + data: Some(payload), + }, block); + + ApiCall { + future, + message: "eth_call", + } +} + + /// Returns a eth_sign-compatible hash of data to sign. /// The data is prepended with special message to prevent /// chosen-plaintext attacks. diff --git a/bridge/src/bridge/deploy.rs b/bridge/src/bridge/deploy.rs index c7a91e0..18ef53e 100644 --- a/bridge/src/bridge/deploy.rs +++ b/bridge/src/bridge/deploy.rs @@ -127,6 +127,7 @@ impl Future for Deploy { checked_deposit_relay: main_receipt.block_number.low_u64(), checked_withdraw_relay: test_receipt.block_number.low_u64(), checked_withdraw_confirm: test_receipt.block_number.low_u64(), + withdraw_relay_required_signatures: Some(self.app.config.authorities.required_signatures), }; return Ok(Deployed::New(database).into()) }, diff --git a/bridge/src/bridge/deposit_relay.rs b/bridge/src/bridge/deposit_relay.rs index 22f5d7d..e8f7dfc 100644 --- a/bridge/src/bridge/deposit_relay.rs +++ b/bridge/src/bridge/deposit_relay.rs @@ -15,7 +15,7 @@ use itertools::Itertools; fn deposits_filter(home: &home::HomeBridge, address: Address) -> FilterBuilder { let filter = home.events().deposit().create_filter(); - web3_filter(filter, address) + web3_filter(filter, ::std::iter::once(address)) } fn deposit_relay_payload(home: &home::HomeBridge, foreign: &foreign::ForeignBridge, log: Log) -> Result { diff --git a/bridge/src/bridge/mod.rs b/bridge/src/bridge/mod.rs index 88fec38..0f05271 100644 --- a/bridge/src/bridge/mod.rs +++ b/bridge/src/bridge/mod.rs @@ -11,7 +11,7 @@ use std::sync::{Arc, RwLock}; use std::path::PathBuf; use futures::{Stream, Poll, Async}; use web3::Transport; -use web3::types::U256; +use web3::types::{U256, Address}; use app::App; use database::Database; use error::{Error, ErrorKind, Result}; @@ -27,7 +27,7 @@ pub use self::withdraw_confirm::{WithdrawConfirm, create_withdraw_confirm}; #[derive(Clone, Copy)] pub enum BridgeChecked { DepositRelay(u64), - WithdrawRelay(u64), + WithdrawRelay((u64, u32)), WithdrawConfirm(u64), } @@ -47,8 +47,9 @@ impl BridgeBackend for FileBackend { BridgeChecked::DepositRelay(n) => { self.database.checked_deposit_relay = n; }, - BridgeChecked::WithdrawRelay(n) => { + BridgeChecked::WithdrawRelay((n, sigs)) => { self.database.checked_withdraw_relay = n; + self.database.withdraw_relay_required_signatures = Some(sigs); }, BridgeChecked::WithdrawConfirm(n) => { self.database.checked_withdraw_confirm = n; @@ -71,17 +72,18 @@ enum BridgeStatus { } /// Creates new bridge. -pub fn create_bridge(app: Arc>, init: &Database, home_chain_id: u64, foreign_chain_id: u64) -> Bridge { +pub fn create_bridge(app: Arc>, init: &Database, home_chain_id: u64, foreign_chain_id: u64, foreign_validator_contract: Address) -> Bridge { let backend = FileBackend { path: app.database_path.clone(), database: init.clone(), }; - create_bridge_backed_by(app, init, backend, home_chain_id, foreign_chain_id) + create_bridge_backed_by(app, init, backend, home_chain_id, foreign_chain_id, foreign_validator_contract) } /// Creates new bridge writing to custom backend. -pub fn create_bridge_backed_by(app: Arc>, init: &Database, backend: F, home_chain_id: u64, foreign_chain_id: u64) -> Bridge { +pub fn create_bridge_backed_by(app: Arc>, init: &Database, backend: F, home_chain_id: u64, foreign_chain_id: u64, + foreign_validator_contract: Address) -> Bridge { let home_balance = Arc::new(RwLock::new(None)); let foreign_balance = Arc::new(RwLock::new(None)); Bridge { @@ -90,7 +92,7 @@ pub fn create_bridge_backed_by(app: Arc< foreign_balance: foreign_balance.clone(), home_balance: home_balance.clone(), deposit_relay: create_deposit_relay(app.clone(), init, foreign_balance.clone(), foreign_chain_id), - withdraw_relay: create_withdraw_relay(app.clone(), init, home_balance.clone(), home_chain_id), + withdraw_relay: create_withdraw_relay(app.clone(), init, home_balance.clone(), home_chain_id, foreign_validator_contract), withdraw_confirm: create_withdraw_confirm(app.clone(), init, foreign_balance.clone(), foreign_chain_id), state: BridgeStatus::Wait, backend, @@ -168,6 +170,7 @@ impl Stream for Bridge { 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); @@ -175,6 +178,7 @@ impl Stream for Bridge { 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); @@ -226,10 +230,11 @@ mod tests { 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(); + backend.save(vec![BridgeChecked::DepositRelay(2), BridgeChecked::WithdrawConfirm(3), BridgeChecked::WithdrawRelay((2, 1))]).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); + assert_eq!(1, backend.database.withdraw_relay_required_signatures.unwrap()); let loaded = Database::load(path).unwrap(); assert_eq!(backend.database, loaded); diff --git a/bridge/src/bridge/withdraw_confirm.rs b/bridge/src/bridge/withdraw_confirm.rs index 804beb3..130a0d7 100644 --- a/bridge/src/bridge/withdraw_confirm.rs +++ b/bridge/src/bridge/withdraw_confirm.rs @@ -16,7 +16,7 @@ use super::nonce::{NonceCheck, SendRawTransaction}; fn withdraws_filter(foreign: &foreign::ForeignBridge, address: Address) -> FilterBuilder { let filter = foreign.events().withdraw().create_filter(); - web3_filter(filter, address) + web3_filter(filter, ::std::iter::once(address)) } fn withdraw_submit_signature_payload(foreign: &foreign::ForeignBridge, withdraw_message: Vec, signature: H520) -> Bytes { diff --git a/bridge/src/bridge/withdraw_relay.rs b/bridge/src/bridge/withdraw_relay.rs index 778ce72..e28de40 100644 --- a/bridge/src/bridge/withdraw_relay.rs +++ b/bridge/src/bridge/withdraw_relay.rs @@ -1,10 +1,10 @@ use std::sync::{Arc, RwLock}; -use futures::{self, Future, Stream, stream::{Collect, iter_ok, IterOk, Buffered}, Poll}; +use futures::{self, Async, Future, Stream, stream::{Collect, iter_ok, IterOk, Buffered}, Poll}; use futures::future::{JoinAll, join_all, Join}; use tokio_timer::Timeout; use web3::Transport; use web3::types::{U256, Address, FilterBuilder, Log, Bytes}; -use ethabi::{RawLog, self}; +use ethabi::{RawLog, Topic, self}; use app::App; use api::{self, LogStream, ApiCall}; use contracts::foreign; @@ -18,9 +18,18 @@ use super::nonce::{NonceCheck, SendRawTransaction}; use itertools::Itertools; /// returns a filter for `ForeignBridge.CollectedSignatures` events -fn collected_signatures_filter(foreign: &foreign::ForeignBridge, address: Address) -> FilterBuilder { - let filter = foreign.events().collected_signatures().create_filter(); - web3_filter(filter, address) +fn collected_signatures_filter>(foreign: &foreign::ForeignBridge, addresses: I) -> FilterBuilder { + let mut filter = foreign.events().collected_signatures().create_filter(); + let sig_filter = foreign.events().required_signatures_changed().create_filter(); + // Combine with the `RequiredSignaturesChanged` event + match filter.topic0 { + Topic::This(t) => filter.topic0 = Topic::OneOf(vec![t]), + Topic::OneOf(ref mut vec) => { + vec.append(&mut sig_filter.topic0.into()); + }, + _ => (), + } + web3_filter(filter, addresses) } /// payloads for calls to `ForeignBridge.signature` and `ForeignBridge.message` @@ -33,7 +42,12 @@ struct RelayAssignment { message_payload: Bytes, } -fn signatures_payload(foreign: &foreign::ForeignBridge, required_signatures: u32, my_address: Address, log: Log) -> error::Result> { +fn signatures_payload(foreign: &foreign::ForeignBridge, required_signatures: u32, my_address: Address, log: Log) -> error::Result<(Option, u32)> { + // check if this is a RequiredSignaturesChanged event + match get_required_signatures(foreign, log.clone()) { + Some(signatures) => return Ok((None, signatures)), + None => (), + } // convert web3::Log to ethabi::RawLog since ethabi events can // only be parsed from the latter let raw_log = RawLog { @@ -45,7 +59,7 @@ fn signatures_payload(foreign: &foreign::ForeignBridge, required_signatures: u32 info!("bridge not responsible for relaying transaction to home. tx hash: {}", log.transaction_hash.unwrap()); // this authority is not responsible for relaying this transaction. // someone else will relay this transaction to home. - return Ok(None); + return Ok((None, required_signatures)); } let signature_payloads = (0..required_signatures).into_iter() .map(|index| foreign.functions().signature().input(collected_signatures.message_hash, index)) @@ -53,14 +67,27 @@ fn signatures_payload(foreign: &foreign::ForeignBridge, required_signatures: u32 .collect(); let message_payload = foreign.functions().message().input(collected_signatures.message_hash).into(); - Ok(Some(RelayAssignment { + Ok((Some(RelayAssignment { signature_payloads, message_payload, - })) + }), required_signatures)) +} + +fn get_required_signatures(foreign: &foreign::ForeignBridge, log: Log) -> Option { + // convert web3::Log to ethabi::RawLog since ethabi events can + // only be parsed from the latter + let raw_log = RawLog { + topics: log.topics.into_iter().map(|t| t.0.into()).collect(), + data: log.data.0, + }; + foreign.events().required_signatures_changed().parse_log(raw_log) + .ok().map(|v| v.required_signatures.low_u32()) } + /// state of the withdraw relay state machine pub enum WithdrawRelayState { + CheckRequiredSignatures(Timeout>), Wait, FetchMessagesSignatures { future: Join< @@ -76,20 +103,31 @@ pub enum WithdrawRelayState { Yield(Option), } -pub fn create_withdraw_relay(app: Arc>, init: &Database, home_balance: Arc>>, home_chain_id: u64) -> WithdrawRelay { +pub fn create_withdraw_relay(app: Arc>, init: &Database, home_balance: Arc>>, home_chain_id: u64, + foreign_validator_contract: Address) -> WithdrawRelay { let logs_init = api::LogStreamInit { after: init.checked_withdraw_relay, request_timeout: app.config.foreign.request_timeout, poll_interval: app.config.foreign.poll_interval, confirmations: app.config.foreign.required_confirmations, - filter: collected_signatures_filter(&app.foreign_bridge, init.foreign_contract_address), + filter: collected_signatures_filter(&app.foreign_bridge, vec![init.foreign_contract_address, foreign_validator_contract]), + }; + + let state = if init.withdraw_relay_required_signatures.is_none() { + let call = app.timer.timeout(api::call_at(app.connections.foreign.clone(), foreign_validator_contract, + app.foreign_bridge.functions().required_signatures().input().into(), + Some(init.checked_withdraw_relay.into())), app.config.foreign.request_timeout); + WithdrawRelayState::CheckRequiredSignatures(call) + } else { + WithdrawRelayState::Wait }; WithdrawRelay { logs: api::log_stream(app.connections.foreign.clone(), app.timer.clone(), logs_init), home_contract: init.home_contract_address, foreign_contract: init.foreign_contract_address, - state: WithdrawRelayState::Wait, + required_signatures: init.withdraw_relay_required_signatures.clone().unwrap_or(app.config.authorities.accounts.len() as u32), + state, app, home_balance, home_chain_id, @@ -104,10 +142,11 @@ pub struct WithdrawRelay { home_contract: Address, home_balance: Arc>>, home_chain_id: u64, + required_signatures: u32, } impl Stream for WithdrawRelay { - type Item = u64; + type Item = (u64, u32); type Error = Error; fn poll(&mut self) -> Poll, Self::Error> { @@ -116,24 +155,54 @@ impl Stream for WithdrawRelay { let contract = self.home_contract.clone(); let home = &self.app.config.home; let t = &self.app.connections.home; + let foreign = &self.app.connections.foreign; let chain_id = self.home_chain_id; + let foreign_bridge = &self.app.foreign_bridge; + let foreign_account = self.app.config.foreign.account; + let timer = &self.app.timer; + let foreign_contract = self.foreign_contract; + let foreign_request_timeout = self.app.config.foreign.request_timeout; loop { + let required_signatures = self.required_signatures; let next_state = match self.state { + WithdrawRelayState::CheckRequiredSignatures(ref mut logs) => { + let mut required_signatures = try_ready!(logs.poll().map_err(|e| ErrorKind::ContextualizedError(Box::new(e), "checking foreign for requiredSignatures value"))); + self.required_signatures = U256::from(required_signatures.0.as_slice()).low_u32(); + info!("Required signatures: {}", self.required_signatures); + WithdrawRelayState::Wait + }, WithdrawRelayState::Wait => { let item = try_stream!(self.logs.poll().map_err(|e| ErrorKind::ContextualizedError(Box::new(e), "polling foreign for collected signatures"))); info!("got {} new signed withdraws to relay", item.logs.len()); let assignments = item.logs .into_iter() - .map(|log| { + .fold((self.required_signatures, vec![]), |mut acc, log| { info!("collected signature is ready for relay: tx hash: {}", log.transaction_hash.unwrap()); - signatures_payload( - &self.app.foreign_bridge, - self.app.config.authorities.required_signatures, - self.app.config.foreign.account, - log) - }) - .collect::>>()?; + let res = signatures_payload( + foreign_bridge, + acc.0, + foreign_account, + log); + match res { + Ok((value, required_signatures)) => { + acc.1.push(Ok(value)); + (required_signatures, acc.1) + }, + Err(err) => { + acc.1.push(Err(err)); + (acc.0, acc.1) + }, + } + }); + + + if assignments.0 != self.required_signatures { + self.required_signatures = assignments.0; + info!("Required signatures: {} (block #{})", self.required_signatures, item.to); + } + + let assignments = assignments.1.into_iter().collect::>>()?; let (signatures, messages): (Vec<_>, Vec<_>) = assignments.into_iter() .filter_map(|a| a) @@ -142,9 +211,9 @@ impl Stream for WithdrawRelay { let message_calls = messages.into_iter() .map(|payload| { - self.app.timer.timeout( - api::call(&self.app.connections.foreign, self.foreign_contract.clone(), payload), - self.app.config.foreign.request_timeout) + timer.timeout( + api::call(foreign, foreign_contract.clone(), payload), + foreign_request_timeout) }) .collect::>(); @@ -152,9 +221,9 @@ impl Stream for WithdrawRelay { .map(|payloads| { payloads.into_iter() .map(|payload| { - self.app.timer.timeout( - api::call(&self.app.connections.foreign, self.foreign_contract.clone(), payload), - self.app.config.foreign.request_timeout) + timer.timeout( + api::call(foreign, foreign_contract.clone(), payload), + foreign_request_timeout) }) .collect::>() }) @@ -246,7 +315,7 @@ impl Stream for WithdrawRelay { info!("waiting for signed withdraws to relay"); WithdrawRelayState::Wait }, - some => return Ok(some.into()), + Some(block) => return Ok(Async::Ready(Some((block, required_signatures)))), } }; self.state = next_state; @@ -282,7 +351,7 @@ mod tests { removed: None, }; - let assignment = signatures_payload(&foreign, 2, my_address, log).unwrap().unwrap(); + let assignment = signatures_payload(&foreign, 2, my_address, log).unwrap().0.unwrap(); let expected_message: Bytes = "490a32c600000000000000000000000000000000000000000000000000000000000000f0".from_hex().unwrap().into(); let expected_signatures: Vec = vec![ "1812d99600000000000000000000000000000000000000000000000000000000000000f00000000000000000000000000000000000000000000000000000000000000000".from_hex().unwrap().into(), @@ -314,6 +383,6 @@ mod tests { }; let assignment = signatures_payload(&foreign, 2, my_address, log).unwrap(); - assert_eq!(None, assignment); + assert_eq!(None, assignment.0); } } diff --git a/bridge/src/config.rs b/bridge/src/config.rs index 4683339..4e47bb1 100644 --- a/bridge/src/config.rs +++ b/bridge/src/config.rs @@ -47,6 +47,7 @@ impl Config { foreign: Node::from_load_struct(config.foreign)?, authorities: Authorities { accounts: config.authorities.accounts, + #[cfg(feature = "deploy")] required_signatures: config.authorities.required_signatures, }, txs: config.transactions.map(Transactions::from_load_struct).unwrap_or_default(), @@ -181,6 +182,7 @@ pub struct ContractConfig { #[derive(Debug, PartialEq, Clone)] pub struct Authorities { pub accounts: Vec
, + #[cfg(feature = "deploy")] pub required_signatures: u32, } @@ -241,9 +243,9 @@ mod load { } #[derive(Deserialize)] - #[serde(deny_unknown_fields)] pub struct Authorities { pub accounts: Vec
, + #[cfg(feature = "deploy")] pub required_signatures: u32, } } @@ -292,7 +294,6 @@ accounts = [ "0x0000000000000000000000000000000000000002", "0x0000000000000000000000000000000000000003" ] -required_signatures = 2 [transactions] home_deploy = { gas = 20 } @@ -335,7 +336,6 @@ home_deploy = { gas = 20 } "0000000000000000000000000000000000000002".into(), "0000000000000000000000000000000000000003".into(), ], - required_signatures: 2, }, #[cfg(feature = "deploy")] estimated_gas_cost_of_withdraw: 100_000, @@ -381,7 +381,6 @@ accounts = [ "0x0000000000000000000000000000000000000002", "0x0000000000000000000000000000000000000003" ] -required_signatures = 2 "#; let expected = Config { txs: Transactions::default(), @@ -419,7 +418,6 @@ required_signatures = 2 "0000000000000000000000000000000000000002".into(), "0000000000000000000000000000000000000003".into(), ], - required_signatures: 2, }, #[cfg(feature = "deploy")] estimated_gas_cost_of_withdraw: 200_000_000, diff --git a/bridge/src/database.rs b/bridge/src/database.rs index 6671e4f..579939e 100644 --- a/bridge/src/database.rs +++ b/bridge/src/database.rs @@ -20,6 +20,8 @@ pub struct Database { pub checked_deposit_relay: u64, /// Number of last block which has been checked for withdraw relays. pub checked_withdraw_relay: u64, + /// Number of required signatures on the foreign side + pub withdraw_relay_required_signatures: Option, /// Number of last block which has been checked for withdraw confirms. pub checked_withdraw_confirm: u64, } @@ -70,6 +72,7 @@ home_deploy = 100 foreign_deploy = 101 checked_deposit_relay = 120 checked_withdraw_relay = 121 +withdraw_relay_required_signatures = 2 checked_withdraw_confirm = 121 "#; @@ -81,6 +84,7 @@ checked_withdraw_confirm = 121 checked_deposit_relay: 120, checked_withdraw_relay: 121, checked_withdraw_confirm: 121, + withdraw_relay_required_signatures: Some(2), }; let database = toml.parse().unwrap(); diff --git a/bridge/src/error.rs b/bridge/src/error.rs index c82862e..d52a424 100644 --- a/bridge/src/error.rs +++ b/bridge/src/error.rs @@ -22,6 +22,9 @@ error_chain! { errors { ShutdownRequested InsufficientFunds + NoRequiredSignaturesChanged { + description("No RequiredSignaturesChanged has been observed") + } // api timeout Timeout(request: &'static str) { description("Request timeout"), diff --git a/bridge/src/util.rs b/bridge/src/util.rs index dcdb2a4..dc989f8 100644 --- a/bridge/src/util.rs +++ b/bridge/src/util.rs @@ -11,12 +11,12 @@ fn web3_topic(topic: ethabi::Topic) -> Option> { } } -pub fn web3_filter(filter: ethabi::TopicFilter, address: Address) -> FilterBuilder { +pub fn web3_filter>(filter: ethabi::TopicFilter, addresses: I) -> FilterBuilder { let t0 = web3_topic(filter.topic0); let t1 = web3_topic(filter.topic1); let t2 = web3_topic(filter.topic2); let t3 = web3_topic(filter.topic3); FilterBuilder::default() - .address(vec![address]) + .address(addresses.into_iter().collect()) .topics(t0, t1, t2, t3) } diff --git a/cli/src/main.rs b/cli/src/main.rs index a084d41..ee15c09 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -18,6 +18,7 @@ use docopt::Docopt; use futures::{Stream, future}; use tokio_core::reactor::Core; +use bridge::api; use bridge::app::App; use bridge::bridge::{create_bridge, create_deploy, create_chain_id_retrieval, Deployed}; use bridge::config::Config; @@ -175,8 +176,15 @@ fn execute(command: I, running: Arc) -> Result Date: Wed, 23 May 2018 00:56:13 -0700 Subject: [PATCH 2/2] Problem: tracking RequiredSignaturesChanged is complicated This requires retreiving validators contract, calling `requiredSignatures()` and carefully tracking `RequiredSignaturesChanged` events. Solution: use recently introduced additional parameter in CollectedSignatures to obtain the number of required signatures. --- bridge/src/api.rs | 17 ----- bridge/src/bridge/deploy.rs | 1 - bridge/src/bridge/mod.rs | 19 ++--- bridge/src/bridge/withdraw_relay.rs | 113 ++++++---------------------- bridge/src/database.rs | 4 - cli/src/main.rs | 9 +-- contracts/bridge.sol | 19 +---- 7 files changed, 36 insertions(+), 146 deletions(-) diff --git a/bridge/src/api.rs b/bridge/src/api.rs index 0a79459..cf31055 100644 --- a/bridge/src/api.rs +++ b/bridge/src/api.rs @@ -142,23 +142,6 @@ pub fn call(transport: T, address: Address, payload: Bytes) -> Api } } -pub fn call_at(transport: T, address: Address, payload: Bytes, block: Option) -> ApiCall { - let future = api::Eth::new(transport).call(CallRequest { - from: None, - to: address, - gas: None, - gas_price: None, - value: None, - data: Some(payload), - }, block); - - ApiCall { - future, - message: "eth_call", - } -} - - /// Returns a eth_sign-compatible hash of data to sign. /// The data is prepended with special message to prevent /// chosen-plaintext attacks. diff --git a/bridge/src/bridge/deploy.rs b/bridge/src/bridge/deploy.rs index 18ef53e..c7a91e0 100644 --- a/bridge/src/bridge/deploy.rs +++ b/bridge/src/bridge/deploy.rs @@ -127,7 +127,6 @@ impl Future for Deploy { checked_deposit_relay: main_receipt.block_number.low_u64(), checked_withdraw_relay: test_receipt.block_number.low_u64(), checked_withdraw_confirm: test_receipt.block_number.low_u64(), - withdraw_relay_required_signatures: Some(self.app.config.authorities.required_signatures), }; return Ok(Deployed::New(database).into()) }, diff --git a/bridge/src/bridge/mod.rs b/bridge/src/bridge/mod.rs index 0f05271..d36bcdb 100644 --- a/bridge/src/bridge/mod.rs +++ b/bridge/src/bridge/mod.rs @@ -11,7 +11,7 @@ use std::sync::{Arc, RwLock}; use std::path::PathBuf; use futures::{Stream, Poll, Async}; use web3::Transport; -use web3::types::{U256, Address}; +use web3::types::U256; use app::App; use database::Database; use error::{Error, ErrorKind, Result}; @@ -27,7 +27,7 @@ pub use self::withdraw_confirm::{WithdrawConfirm, create_withdraw_confirm}; #[derive(Clone, Copy)] pub enum BridgeChecked { DepositRelay(u64), - WithdrawRelay((u64, u32)), + WithdrawRelay(u64), WithdrawConfirm(u64), } @@ -47,9 +47,8 @@ impl BridgeBackend for FileBackend { BridgeChecked::DepositRelay(n) => { self.database.checked_deposit_relay = n; }, - BridgeChecked::WithdrawRelay((n, sigs)) => { + BridgeChecked::WithdrawRelay(n) => { self.database.checked_withdraw_relay = n; - self.database.withdraw_relay_required_signatures = Some(sigs); }, BridgeChecked::WithdrawConfirm(n) => { self.database.checked_withdraw_confirm = n; @@ -72,18 +71,17 @@ enum BridgeStatus { } /// Creates new bridge. -pub fn create_bridge(app: Arc>, init: &Database, home_chain_id: u64, foreign_chain_id: u64, foreign_validator_contract: Address) -> Bridge { +pub fn create_bridge(app: Arc>, init: &Database, home_chain_id: u64, foreign_chain_id: u64) -> Bridge { let backend = FileBackend { path: app.database_path.clone(), database: init.clone(), }; - create_bridge_backed_by(app, init, backend, home_chain_id, foreign_chain_id, foreign_validator_contract) + create_bridge_backed_by(app, init, backend, 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, home_chain_id: u64, foreign_chain_id: u64, - foreign_validator_contract: Address) -> Bridge { +pub fn create_bridge_backed_by(app: Arc>, init: &Database, backend: F, home_chain_id: u64, foreign_chain_id: u64) -> Bridge { let home_balance = Arc::new(RwLock::new(None)); let foreign_balance = Arc::new(RwLock::new(None)); Bridge { @@ -92,7 +90,7 @@ pub fn create_bridge_backed_by(app: Arc< foreign_balance: foreign_balance.clone(), home_balance: home_balance.clone(), deposit_relay: create_deposit_relay(app.clone(), init, foreign_balance.clone(), foreign_chain_id), - withdraw_relay: create_withdraw_relay(app.clone(), init, home_balance.clone(), home_chain_id, foreign_validator_contract), + withdraw_relay: create_withdraw_relay(app.clone(), init, home_balance.clone(), home_chain_id), withdraw_confirm: create_withdraw_confirm(app.clone(), init, foreign_balance.clone(), foreign_chain_id), state: BridgeStatus::Wait, backend, @@ -230,11 +228,10 @@ mod tests { 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, 1))]).unwrap(); + 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); - assert_eq!(1, backend.database.withdraw_relay_required_signatures.unwrap()); let loaded = Database::load(path).unwrap(); assert_eq!(backend.database, loaded); diff --git a/bridge/src/bridge/withdraw_relay.rs b/bridge/src/bridge/withdraw_relay.rs index e28de40..66779e2 100644 --- a/bridge/src/bridge/withdraw_relay.rs +++ b/bridge/src/bridge/withdraw_relay.rs @@ -4,7 +4,7 @@ use futures::future::{JoinAll, join_all, Join}; use tokio_timer::Timeout; use web3::Transport; use web3::types::{U256, Address, FilterBuilder, Log, Bytes}; -use ethabi::{RawLog, Topic, self}; +use ethabi::{RawLog, self}; use app::App; use api::{self, LogStream, ApiCall}; use contracts::foreign; @@ -19,16 +19,7 @@ use itertools::Itertools; /// returns a filter for `ForeignBridge.CollectedSignatures` events fn collected_signatures_filter>(foreign: &foreign::ForeignBridge, addresses: I) -> FilterBuilder { - let mut filter = foreign.events().collected_signatures().create_filter(); - let sig_filter = foreign.events().required_signatures_changed().create_filter(); - // Combine with the `RequiredSignaturesChanged` event - match filter.topic0 { - Topic::This(t) => filter.topic0 = Topic::OneOf(vec![t]), - Topic::OneOf(ref mut vec) => { - vec.append(&mut sig_filter.topic0.into()); - }, - _ => (), - } + let filter = foreign.events().collected_signatures().create_filter(); web3_filter(filter, addresses) } @@ -42,12 +33,7 @@ struct RelayAssignment { message_payload: Bytes, } -fn signatures_payload(foreign: &foreign::ForeignBridge, required_signatures: u32, my_address: Address, log: Log) -> error::Result<(Option, u32)> { - // check if this is a RequiredSignaturesChanged event - match get_required_signatures(foreign, log.clone()) { - Some(signatures) => return Ok((None, signatures)), - None => (), - } +fn signatures_payload(foreign: &foreign::ForeignBridge, my_address: Address, log: Log) -> error::Result> { // convert web3::Log to ethabi::RawLog since ethabi events can // only be parsed from the latter let raw_log = RawLog { @@ -59,35 +45,24 @@ fn signatures_payload(foreign: &foreign::ForeignBridge, required_signatures: u32 info!("bridge not responsible for relaying transaction to home. tx hash: {}", log.transaction_hash.unwrap()); // this authority is not responsible for relaying this transaction. // someone else will relay this transaction to home. - return Ok((None, required_signatures)); + return Ok(None); } - let signature_payloads = (0..required_signatures).into_iter() + + let required_signatures: U256 = (&foreign.functions().message().input(collected_signatures.number_of_collected_signatures)[4..]).into(); + let signature_payloads = (0..required_signatures.low_u32()).into_iter() .map(|index| foreign.functions().signature().input(collected_signatures.message_hash, index)) .map(Into::into) .collect(); let message_payload = foreign.functions().message().input(collected_signatures.message_hash).into(); - Ok((Some(RelayAssignment { + Ok(Some(RelayAssignment { signature_payloads, message_payload, - }), required_signatures)) -} - -fn get_required_signatures(foreign: &foreign::ForeignBridge, log: Log) -> Option { - // convert web3::Log to ethabi::RawLog since ethabi events can - // only be parsed from the latter - let raw_log = RawLog { - topics: log.topics.into_iter().map(|t| t.0.into()).collect(), - data: log.data.0, - }; - foreign.events().required_signatures_changed().parse_log(raw_log) - .ok().map(|v| v.required_signatures.low_u32()) + })) } - /// state of the withdraw relay state machine pub enum WithdrawRelayState { - CheckRequiredSignatures(Timeout>), Wait, FetchMessagesSignatures { future: Join< @@ -103,31 +78,20 @@ pub enum WithdrawRelayState { Yield(Option), } -pub fn create_withdraw_relay(app: Arc>, init: &Database, home_balance: Arc>>, home_chain_id: u64, - foreign_validator_contract: Address) -> WithdrawRelay { +pub fn create_withdraw_relay(app: Arc>, init: &Database, home_balance: Arc>>, home_chain_id: u64) -> WithdrawRelay { let logs_init = api::LogStreamInit { after: init.checked_withdraw_relay, request_timeout: app.config.foreign.request_timeout, poll_interval: app.config.foreign.poll_interval, confirmations: app.config.foreign.required_confirmations, - filter: collected_signatures_filter(&app.foreign_bridge, vec![init.foreign_contract_address, foreign_validator_contract]), - }; - - let state = if init.withdraw_relay_required_signatures.is_none() { - let call = app.timer.timeout(api::call_at(app.connections.foreign.clone(), foreign_validator_contract, - app.foreign_bridge.functions().required_signatures().input().into(), - Some(init.checked_withdraw_relay.into())), app.config.foreign.request_timeout); - WithdrawRelayState::CheckRequiredSignatures(call) - } else { - WithdrawRelayState::Wait + filter: collected_signatures_filter(&app.foreign_bridge, vec![init.foreign_contract_address]), }; WithdrawRelay { logs: api::log_stream(app.connections.foreign.clone(), app.timer.clone(), logs_init), home_contract: init.home_contract_address, foreign_contract: init.foreign_contract_address, - required_signatures: init.withdraw_relay_required_signatures.clone().unwrap_or(app.config.authorities.accounts.len() as u32), - state, + state: WithdrawRelayState::Wait, app, home_balance, home_chain_id, @@ -142,11 +106,10 @@ pub struct WithdrawRelay { home_contract: Address, home_balance: Arc>>, home_chain_id: u64, - required_signatures: u32, } impl Stream for WithdrawRelay { - type Item = (u64, u32); + type Item = u64; type Error = Error; fn poll(&mut self) -> Poll, Self::Error> { @@ -164,45 +127,17 @@ impl Stream for WithdrawRelay { let foreign_request_timeout = self.app.config.foreign.request_timeout; loop { - let required_signatures = self.required_signatures; let next_state = match self.state { - WithdrawRelayState::CheckRequiredSignatures(ref mut logs) => { - let mut required_signatures = try_ready!(logs.poll().map_err(|e| ErrorKind::ContextualizedError(Box::new(e), "checking foreign for requiredSignatures value"))); - self.required_signatures = U256::from(required_signatures.0.as_slice()).low_u32(); - info!("Required signatures: {}", self.required_signatures); - WithdrawRelayState::Wait - }, WithdrawRelayState::Wait => { let item = try_stream!(self.logs.poll().map_err(|e| ErrorKind::ContextualizedError(Box::new(e), "polling foreign for collected signatures"))); info!("got {} new signed withdraws to relay", item.logs.len()); let assignments = item.logs .into_iter() - .fold((self.required_signatures, vec![]), |mut acc, log| { - info!("collected signature is ready for relay: tx hash: {}", log.transaction_hash.unwrap()); - let res = signatures_payload( + .map(|log| signatures_payload( foreign_bridge, - acc.0, foreign_account, - log); - match res { - Ok((value, required_signatures)) => { - acc.1.push(Ok(value)); - (required_signatures, acc.1) - }, - Err(err) => { - acc.1.push(Err(err)); - (acc.0, acc.1) - }, - } - }); - - - if assignments.0 != self.required_signatures { - self.required_signatures = assignments.0; - info!("Required signatures: {} (block #{})", self.required_signatures, item.to); - } - - let assignments = assignments.1.into_iter().collect::>>()?; + log)) + .collect::>>()?; let (signatures, messages): (Vec<_>, Vec<_>) = assignments.into_iter() .filter_map(|a| a) @@ -315,7 +250,7 @@ impl Stream for WithdrawRelay { info!("waiting for signed withdraws to relay"); WithdrawRelayState::Wait }, - Some(block) => return Ok(Async::Ready(Some((block, required_signatures)))), + Some(block) => return Ok(Async::Ready(Some(block))), } }; self.state = next_state; @@ -335,11 +270,11 @@ mod tests { let foreign = foreign::ForeignBridge::default(); let my_address = "aff3454fce5edbc8cca8697c15331677e6ebcccc".into(); - let data = "000000000000000000000000aff3454fce5edbc8cca8697c15331677e6ebcccc00000000000000000000000000000000000000000000000000000000000000f0".from_hex().unwrap(); + let data = "000000000000000000000000aff3454fce5edbc8cca8697c15331677e6ebcccc00000000000000000000000000000000000000000000000000000000000000f00000000000000000000000000000000000000000000000000000000000000002".from_hex().unwrap(); let log = Log { data: data.into(), - topics: vec!["eb043d149eedb81369bec43d4c3a3a53087debc88d2525f13bfaa3eecda28b5c".into()], + topics: vec!["415557404d88a0c0b8e3b16967cafffc511213fd9c465c16832ee17ed57d7237".into()], transaction_hash: Some("884edad9ce6fa2440d8a54cc123490eb96d2768479d49ff9c7366125a9424364".into()), address: Address::zero(), block_hash: None, @@ -351,7 +286,7 @@ mod tests { removed: None, }; - let assignment = signatures_payload(&foreign, 2, my_address, log).unwrap().0.unwrap(); + let assignment = signatures_payload(&foreign, my_address, log).unwrap().unwrap(); let expected_message: Bytes = "490a32c600000000000000000000000000000000000000000000000000000000000000f0".from_hex().unwrap().into(); let expected_signatures: Vec = vec![ "1812d99600000000000000000000000000000000000000000000000000000000000000f00000000000000000000000000000000000000000000000000000000000000000".from_hex().unwrap().into(), @@ -366,11 +301,11 @@ mod tests { let foreign = foreign::ForeignBridge::default(); let my_address = "aff3454fce5edbc8cca8697c15331677e6ebcccd".into(); - let data = "000000000000000000000000aff3454fce5edbc8cca8697c15331677e6ebcccc00000000000000000000000000000000000000000000000000000000000000f0".from_hex().unwrap(); + let data = "000000000000000000000000aff3454fce5edbc8cca8697c15331677e6ebcccc00000000000000000000000000000000000000000000000000000000000000f00000000000000000000000000000000000000000000000000000000000000002".from_hex().unwrap(); let log = Log { data: data.into(), - topics: vec!["eb043d149eedb81369bec43d4c3a3a53087debc88d2525f13bfaa3eecda28b5c".into()], + topics: vec!["415557404d88a0c0b8e3b16967cafffc511213fd9c465c16832ee17ed57d7237".into()], transaction_hash: Some("884edad9ce6fa2440d8a54cc123490eb96d2768479d49ff9c7366125a9424364".into()), address: Address::zero(), block_hash: None, @@ -382,7 +317,7 @@ mod tests { removed: None, }; - let assignment = signatures_payload(&foreign, 2, my_address, log).unwrap(); - assert_eq!(None, assignment.0); + let assignment = signatures_payload(&foreign, my_address, log).unwrap(); + assert_eq!(None, assignment); } } diff --git a/bridge/src/database.rs b/bridge/src/database.rs index 579939e..6671e4f 100644 --- a/bridge/src/database.rs +++ b/bridge/src/database.rs @@ -20,8 +20,6 @@ pub struct Database { pub checked_deposit_relay: u64, /// Number of last block which has been checked for withdraw relays. pub checked_withdraw_relay: u64, - /// Number of required signatures on the foreign side - pub withdraw_relay_required_signatures: Option, /// Number of last block which has been checked for withdraw confirms. pub checked_withdraw_confirm: u64, } @@ -72,7 +70,6 @@ home_deploy = 100 foreign_deploy = 101 checked_deposit_relay = 120 checked_withdraw_relay = 121 -withdraw_relay_required_signatures = 2 checked_withdraw_confirm = 121 "#; @@ -84,7 +81,6 @@ checked_withdraw_confirm = 121 checked_deposit_relay: 120, checked_withdraw_relay: 121, checked_withdraw_confirm: 121, - withdraw_relay_required_signatures: Some(2), }; let database = toml.parse().unwrap(); diff --git a/cli/src/main.rs b/cli/src/main.rs index ee15c09..b1be7ba 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -18,7 +18,6 @@ use docopt::Docopt; use futures::{Stream, future}; use tokio_core::reactor::Core; -use bridge::api; use bridge::app::App; use bridge::bridge::{create_bridge, create_deploy, create_chain_id_retrieval, Deployed}; use bridge::config::Config; @@ -176,14 +175,8 @@ fn execute(command: I, running: Arc) -> Result