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.
This commit is contained in:
Yurii Rashkovskii 2018-05-23 00:56:13 -07:00
parent 8a56c5cafb
commit 379973f315
No known key found for this signature in database
GPG Key ID: 1D60D7CFD80845FF
7 changed files with 36 additions and 146 deletions

View File

@ -142,23 +142,6 @@ pub fn call<T: Transport>(transport: T, address: Address, payload: Bytes) -> Api
}
}
pub fn call_at<T: Transport>(transport: T, address: Address, payload: Bytes, block: Option<BlockNumber>) -> ApiCall<Bytes, T::Out> {
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.

View File

@ -127,7 +127,6 @@ impl<T: Transport + Clone> Future for Deploy<T> {
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())
},

View File

@ -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<T: Transport + Clone>(app: Arc<App<T>>, init: &Database, home_chain_id: u64, foreign_chain_id: u64, foreign_validator_contract: Address) -> Bridge<T, FileBackend> {
pub fn create_bridge<T: Transport + Clone>(app: Arc<App<T>>, init: &Database, home_chain_id: u64, foreign_chain_id: u64) -> Bridge<T, FileBackend> {
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<T: Transport + Clone, F: BridgeBackend>(app: Arc<App<T>>, init: &Database, backend: F, home_chain_id: u64, foreign_chain_id: u64,
foreign_validator_contract: Address) -> Bridge<T, F> {
pub fn create_bridge_backed_by<T: Transport + Clone, F: BridgeBackend>(app: Arc<App<T>>, init: &Database, backend: F, home_chain_id: u64, foreign_chain_id: u64) -> Bridge<T, F> {
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<T: Transport + Clone, F: BridgeBackend>(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);

View File

@ -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<I: IntoIterator<Item = Address>>(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<RelayAssignment>, 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<Option<RelayAssignment>> {
// 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<u32> {
// 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<T: Transport> {
CheckRequiredSignatures(Timeout<ApiCall<Bytes, T::Out>>),
Wait,
FetchMessagesSignatures {
future: Join<
@ -103,31 +78,20 @@ pub enum WithdrawRelayState<T: Transport> {
Yield(Option<u64>),
}
pub fn create_withdraw_relay<T: Transport + Clone>(app: Arc<App<T>>, init: &Database, home_balance: Arc<RwLock<Option<U256>>>, home_chain_id: u64,
foreign_validator_contract: Address) -> WithdrawRelay<T> {
pub fn create_withdraw_relay<T: Transport + Clone>(app: Arc<App<T>>, init: &Database, home_balance: Arc<RwLock<Option<U256>>>, home_chain_id: u64) -> WithdrawRelay<T> {
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<T: Transport> {
home_contract: Address,
home_balance: Arc<RwLock<Option<U256>>>,
home_chain_id: u64,
required_signatures: u32,
}
impl<T: Transport> Stream for WithdrawRelay<T> {
type Item = (u64, u32);
type Item = u64;
type Error = Error;
fn poll(&mut self) -> Poll<Option<Self::Item>, Self::Error> {
@ -164,45 +127,17 @@ impl<T: Transport> Stream for WithdrawRelay<T> {
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::<error::Result<Vec<_>>>()?;
log))
.collect::<error::Result<Vec<_>>>()?;
let (signatures, messages): (Vec<_>, Vec<_>) = assignments.into_iter()
.filter_map(|a| a)
@ -315,7 +250,7 @@ impl<T: Transport> Stream for WithdrawRelay<T> {
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<Bytes> = 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);
}
}

View File

@ -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<u32>,
/// 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();

View File

@ -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<S, I>(command: I, running: Arc<AtomicBool>) -> Result<String, UserFac
},
};
let foreign_validator_contract: web3::types::Address = event_loop.run(app.timer.timeout(api::call_at(app.connections.foreign.clone(), database.foreign_contract_address,
app.foreign_bridge.functions().validator_contract().input().into(),
Some(database.checked_withdraw_relay.into())), app.config.foreign.request_timeout)).unwrap().0.as_slice().into();
info!(target: "bridge", "Foreign validator contract: {}", foreign_validator_contract);
info!(target: "bridge", "Starting listening to events");
let bridge = create_bridge(app.clone(), &database, home_chain_id, foreign_chain_id, foreign_validator_contract)
let bridge = create_bridge(app.clone(), &database, home_chain_id, foreign_chain_id)
.and_then(|_| future::ok(true)).collect();
let mut result = event_loop.run(bridge);
loop {

View File

@ -253,10 +253,6 @@ contract HomeBridge is BridgeDeploymentAddressStorage,
/// Must be lesser than number of authorities.
uint256 public requiredSignatures;
event RequiredSignaturesChanged (uint256 requiredSignatures);
address public validatorContract;
/// The gas cost of calling `HomeBridge.withdraw`.
///
/// Is subtracted from `value` on withdraw.
@ -288,8 +284,6 @@ contract HomeBridge is BridgeDeploymentAddressStorage,
requiredSignatures = requiredSignaturesParam;
authorities = authoritiesParam;
estimatedGasCostOfWithdraw = estimatedGasCostOfWithdrawParam;
validatorContract = this;
RequiredSignaturesChanged(requiredSignatures);
}
/// Should be used to deposit money.
@ -363,12 +357,9 @@ contract ForeignBridge is BridgeDeploymentAddressStorage,
///
/// Must be less than number of authorities.
uint256 public requiredSignatures;
event RequiredSignaturesChanged (uint256 requiredSignatures);
uint256 public estimatedGasCostOfWithdraw;
address public validatorContract;
// Original parity-bridge assumes that anyone could forward final
// withdraw confirmation to the HomeBridge contract. That's why
// they need to make sure that no one is trying to steal funds by
@ -409,7 +400,7 @@ contract ForeignBridge is BridgeDeploymentAddressStorage,
event Withdraw(address recipient, uint256 value, uint256 homeGasPrice);
/// Collected signatures which should be relayed to home chain.
event CollectedSignatures(address authorityResponsibleForRelay, bytes32 messageHash);
event CollectedSignatures(address authorityResponsibleForRelay, bytes32 messageHash, uint256 NumberOfCollectedSignatures);
/// Event created when new token address is set up.
event TokenAddress(address token);
@ -424,16 +415,12 @@ contract ForeignBridge is BridgeDeploymentAddressStorage,
require(_requiredSignatures != 0);
require(_requiredSignatures <= _authorities.length);
requiredSignatures = _requiredSignatures;
validatorContract = this;
for (uint i = 0; i < _authorities.length; i++) {
authorities[_authorities[i]] = true;
}
estimatedGasCostOfWithdraw = _estimatedGasCostOfWithdraw;
RequiredSignaturesChanged(requiredSignatures);
estimatedGasCostOfWithdraw = _estimatedGasCostOfWithdraw;
}
/// require that sender is an authority
@ -564,7 +551,7 @@ contract ForeignBridge is BridgeDeploymentAddressStorage,
// TODO: this may cause troubles if requiredSignatures len is changed
if (signed == requiredSignatures) {
CollectedSignatures(msg.sender, hash);
CollectedSignatures(msg.sender, hash, signed);
}
}