Ignoring transactions slightly above gas_limit

This commit is contained in:
Tomasz Drwięga 2016-03-17 15:20:33 +01:00 committed by arkpar
parent b3dd72ab0a
commit a0cdf5c420
3 changed files with 87 additions and 4 deletions

View File

@ -79,6 +79,13 @@ pub enum TransactionError {
/// Transaction cost
cost: U256,
},
/// Transactions gas is higher then current gas limit
GasLimitExceeded {
/// Current gas limit
limit: U256,
/// Declared transaction gas
got: U256,
},
/// Transaction's gas limit (aka gas) is invalid.
InvalidGasLimit(OutOfBounds<U256>),
}

View File

@ -21,7 +21,7 @@ use std::sync::atomic::AtomicBool;
use std::collections::HashSet;
use util::{H256, U256, Address, Bytes, Uint};
use ethcore::views::{BlockView};
use ethcore::views::{BlockView, HeaderView};
use ethcore::client::{BlockChainClient, BlockId};
use ethcore::block::{ClosedBlock, IsBlock};
use ethcore::error::{Error};
@ -94,6 +94,12 @@ impl Miner {
pub fn set_minimal_gas_price(&self, min_gas_price: U256) {
self.transaction_queue.lock().unwrap().set_minimal_gas_price(min_gas_price);
}
fn update_gas_limit(&self, chain: &BlockChainClient) {
let gas_limit = HeaderView::new(&chain.best_block_header()).gas_limit();
let mut queue = self.transaction_queue.lock().unwrap();
queue.set_gas_limit(gas_limit);
}
}
impl MinerService for Miner {
@ -185,6 +191,11 @@ impl MinerService for Miner {
let block = BlockView::new(&block);
block.transactions()
}
// First update gas limit in transaction queue
self.update_gas_limit(chain);
// Then import all transactions...
{
let out_of_chain = retracted
.par_iter()
@ -201,7 +212,8 @@ impl MinerService for Miner {
});
});
}
// First import all transactions and after that remove old ones
// ...and after that remove old ones
{
let in_chain = {
let mut in_chain = HashSet::new();
@ -227,6 +239,7 @@ impl MinerService for Miner {
});
}
// Update mined block
if self.sealing_enabled.load(atomic::Ordering::Relaxed) {
self.prepare_sealing(chain);
}

View File

@ -252,10 +252,16 @@ pub struct AccountDetails {
pub balance: U256,
}
/// Transactions with `gas > (gas_limit + gas_limit * Factor(in percents))` are not imported to the queue.
const GAS_LIMIT_HYSTERESIS: usize = 10; // %
/// TransactionQueue implementation
pub struct TransactionQueue {
/// Gas Price threshold for transactions that can be imported to this queue (defaults to 0)
minimal_gas_price: U256,
/// Current gas limit (block gas limit * factor). Transactions above the limit will not be accepted (default to !0)
gas_limit: U256,
/// Priority queue for transactions that can go to block
current: TransactionSet,
/// Priority queue for transactions that has been received but are not yet valid to go to block
@ -293,6 +299,7 @@ impl TransactionQueue {
TransactionQueue {
minimal_gas_price: U256::zero(),
gas_limit: !U256::zero(),
current: current,
future: future,
by_hash: HashMap::new(),
@ -301,11 +308,24 @@ impl TransactionQueue {
}
/// Sets new gas price threshold for incoming transactions.
/// Any transactions already imported to the queue are not affected.
/// Any transaction already imported to the queue is not affected.
pub fn set_minimal_gas_price(&mut self, min_gas_price: U256) {
self.minimal_gas_price = min_gas_price;
}
/// Sets new gas limit. Transactions with gas slightly (`GAS_LIMIT_HYSTERESIS`) above the limit won't be imported.
/// Any transaction already imported to the queue is not affected.
pub fn set_gas_limit(&mut self, gas_limit: U256) {
let extra = gas_limit / U256::from(GAS_LIMIT_HYSTERESIS);
self.gas_limit = match gas_limit.overflowing_add(extra) {
(_, true) => !U256::zero(),
(val, false) => val,
};
}
// Will be used when rpc merged
#[allow(dead_code)]
/// Returns current status for this queue
pub fn status(&self) -> TransactionQueueStatus {
TransactionQueueStatus {
@ -335,12 +355,24 @@ impl TransactionQueue {
tx.hash(), tx.gas_price, self.minimal_gas_price
);
return Err(Error::Transaction(TransactionError::InsufficientGasPrice{
return Err(Error::Transaction(TransactionError::InsufficientGasPrice {
minimal: self.minimal_gas_price,
got: tx.gas_price,
}));
}
if tx.gas > self.gas_limit {
trace!(target: "miner",
"Dropping transaction above gas limit: {:?} ({} > {})",
tx.hash(), tx.gas, self.gas_limit
);
return Err(Error::Transaction(TransactionError::GasLimitExceeded {
limit: self.gas_limit,
got: tx.gas,
}));
}
let vtx = try!(VerifiedTransaction::new(tx));
let account = fetch_account(&vtx.sender());
@ -677,6 +709,37 @@ mod test {
assert_eq!(stats.pending, 1);
}
#[test]
fn gas_limit_should_never_overflow() {
// given
let mut txq = TransactionQueue::new();
txq.set_gas_limit(U256::zero());
assert_eq!(txq.gas_limit, U256::zero());
// when
txq.set_gas_limit(!U256::zero());
// then
assert_eq!(txq.gas_limit, !U256::zero());
}
#[test]
fn should_not_import_transaction_above_gas_limit() {
// given
let mut txq = TransactionQueue::new();
let tx = new_tx();
txq.set_gas_limit(tx.gas / U256::from(2));
// when
txq.add(tx, &default_nonce).unwrap_err();
// then
let stats = txq.status();
assert_eq!(stats.pending, 0);
assert_eq!(stats.future, 0);
}
#[test]
fn should_drop_transactions_from_senders_without_balance() {
// given