Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem: hard-coded transaction gas #106

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Problem: hard-coded transaction gas
This makes it very to make a mistake when configuring
the bridge and have failing transactions as a result.

Solution: use `eth.estimateGas` call for every transaction

Configuration values for `gas` will be only used if they
are set and non-zero.

If `estimateGas` fails, the transaction will be thrown out
as it will be assumed that it is failing and therefore we
shouldn't waste resources on it.
yrashk committed Jun 13, 2018
commit 25426a3590da88722c3ddfe6df14ae1112dc4d88
11 changes: 3 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
@@ -100,11 +100,6 @@ password = "foreign_password.txt"

[authorities]
required_signatures = 2

[transactions]
deposit_relay = { gas = 3000000 }
withdraw_relay = { gas = 3000000 }
withdraw_confirm = { gas = 3000000 }
```

#### Options
@@ -133,9 +128,9 @@ withdraw_confirm = { gas = 3000000 }

#### transaction options

- `transaction.deposit_relay.gas` - specify how much gas should be consumed by deposit relay
- `transaction.withdraw_confirm.gas` - specify how much gas should be consumed by withdraw confirm
- `transaction.withdraw_relay.gas` - specify how much gas should be consumed by withdraw relay
- `transaction.deposit_relay.gas` - specify how much gas should be consumed by deposit relay (optional, default: computed with `eth.estimateGas`)
- `transaction.withdraw_confirm.gas` - specify how much gas should be consumed by withdraw confirm (optional, default: computed with `eth.estimateGas`)
- `transaction.withdraw_relay.gas` - specify how much gas should be consumed by withdraw relay (optional, default: computed with `eth.estimateGas`)

### Database file format

18 changes: 18 additions & 0 deletions bridge/src/api.rs
Original file line number Diff line number Diff line change
@@ -142,6 +142,24 @@ pub fn call<T: Transport>(transport: T, address: Address, payload: Bytes) -> Api
}
}

/// Imperative wrapper for web3 function.
pub fn estimate_gas<T: Transport>(transport: T, sender: Option<Address>, address: Address, payload: Bytes) -> ApiCall<U256, T::Out> {
let future = api::Eth::new(transport).estimate_gas(CallRequest {
from: sender,
to: address,
gas: None,
gas_price: None,
value: None,
data: Some(payload),
}, None);

ApiCall {
future,
message: "eth_estimateGas",
}
}


/// Returns a eth_sign-compatible hash of data to sign.
/// The data is prepended with special message to prevent
/// chosen-plaintext attacks.
37 changes: 36 additions & 1 deletion bridge/src/bridge/nonce.rs
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ use futures::{Future, Async, Poll, future::{MapErr}};
use tokio_timer::Timeout;
use web3::{self, Transport};
use web3::types::{U256, H256, Bytes};
use ethcore_transaction::Transaction;
use ethcore_transaction::{Transaction, Action};
use api::{self, ApiCall};
use error::{Error, ErrorKind};
use config::Node;
@@ -23,6 +23,12 @@ enum NonceCheckState<T: Transport, S: TransactionSender> {
},
/// Nonce available
Nonce(U256),
/// Request gas estimation
GasEstimateRequest {
future: Timeout<ApiCall<U256, T::Out>>,
},
/// Send transaction
SendTransaction,
/// Transaction is in progress
TransactionRequest {
future: Timeout<S::Future>,
@@ -90,6 +96,35 @@ impl<T: Transport, S: TransactionSender> Future for NonceCheck<T, S> {
},
NonceCheckState::Nonce(mut nonce) => {
self.transaction.nonce = nonce;
match self.transaction.action {
Action::Call(address) if self.transaction.gas.is_zero() => {
NonceCheckState::GasEstimateRequest {
future: self.app.timer.timeout(api::estimate_gas(&self.transport, Some(self.node.account), address, Bytes(self.transaction.data.clone())), self.node.request_timeout)
}
},
_ => NonceCheckState::SendTransaction,
}
},
NonceCheckState::GasEstimateRequest { ref mut future } => {
match future.poll() {
Ok(Async::Ready(gas)) => {
self.transaction.gas = gas;
NonceCheckState::SendTransaction
},
Ok(Async::NotReady) => return Ok(Async::NotReady),
Err(e) => {
match e {
Error(ErrorKind::Web3(web3::error::Error(web3::error::ErrorKind::Rpc(err), _)), _) => {
let hash = self.transaction.hash(Some(self.chain_id));
info!("{} fails to pass eth.estimateGas on {}, skipping (error: {:?})", hash, self.node.rpc_host, err);
return Ok(Async::Ready(self.sender.ignore(hash)))
},
e => return Err(From::from(e)),
}
},
}
},
NonceCheckState::SendTransaction => {
match prepare_raw_transaction(self.transaction.clone(), &self.app, &self.node, self.chain_id) {
Ok(tx) => NonceCheckState::TransactionRequest {
future: self.app.timer.timeout(self.sender.send(tx), self.node.request_timeout)
5 changes: 0 additions & 5 deletions examples/config.toml
Original file line number Diff line number Diff line change
@@ -19,8 +19,3 @@ default_gas_price = 5_000_000_000 # 5 GWEI

[authorities]
required_signatures = 1

[transactions]
home_deploy = { gas = 1000000 }
foreign_deploy = { gas = 3000000 }
deposit_relay = { gas = 100000 }
6 changes: 3 additions & 3 deletions integration-tests/bridge_config.toml
Original file line number Diff line number Diff line change
@@ -33,6 +33,6 @@ required_signatures = 1
[transactions]
home_deploy = { gas = 3000000 }
foreign_deploy = { gas = 3000000 }
deposit_relay = { gas = 3000000 }
withdraw_relay = { gas = 3000000 }
withdraw_confirm = { gas = 3000000 }
#deposit_relay = { gas = 3000000 }
#withdraw_relay = { gas = 3000000 }
#withdraw_confirm = { gas = 3000000 }