From 379973f3159756b55fbc96511b2c9f720646bde0 Mon Sep 17 00:00:00 2001 From: Yurii Rashkovskii Date: Wed, 23 May 2018 00:56:13 -0700 Subject: [PATCH] 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