Penalize transactions with gas above gas limit
Conflicts: ethcore/src/miner/transaction_queue.rs
This commit is contained in:
parent
62cbf9ce97
commit
2874f464aa
@ -292,6 +292,7 @@ impl Miner {
|
|||||||
};
|
};
|
||||||
|
|
||||||
let mut invalid_transactions = HashSet::new();
|
let mut invalid_transactions = HashSet::new();
|
||||||
|
let mut too_big_transactions = HashSet::new();
|
||||||
let block_number = open_block.block().fields().header.number();
|
let block_number = open_block.block().fields().header.number();
|
||||||
// TODO: push new uncles, too.
|
// TODO: push new uncles, too.
|
||||||
for tx in transactions {
|
for tx in transactions {
|
||||||
@ -299,6 +300,8 @@ impl Miner {
|
|||||||
match open_block.push_transaction(tx, None) {
|
match open_block.push_transaction(tx, None) {
|
||||||
Err(Error::Execution(ExecutionError::BlockGasLimitReached { gas_limit, gas_used, gas })) => {
|
Err(Error::Execution(ExecutionError::BlockGasLimitReached { gas_limit, gas_used, gas })) => {
|
||||||
debug!(target: "miner", "Skipping adding transaction to block because of gas limit: {:?} (limit: {:?}, used: {:?}, gas: {:?})", hash, gas_limit, gas_used, gas);
|
debug!(target: "miner", "Skipping adding transaction to block because of gas limit: {:?} (limit: {:?}, used: {:?}, gas: {:?})", hash, gas_limit, gas_used, gas);
|
||||||
|
too_big_transactions.insert(hash);
|
||||||
|
|
||||||
// Exit early if gas left is smaller then min_tx_gas
|
// Exit early if gas left is smaller then min_tx_gas
|
||||||
let min_tx_gas: U256 = 21000.into(); // TODO: figure this out properly.
|
let min_tx_gas: U256 = 21000.into(); // TODO: figure this out properly.
|
||||||
if gas_limit - gas_used < min_tx_gas {
|
if gas_limit - gas_used < min_tx_gas {
|
||||||
@ -334,6 +337,9 @@ impl Miner {
|
|||||||
for hash in invalid_transactions.into_iter() {
|
for hash in invalid_transactions.into_iter() {
|
||||||
queue.remove_invalid(&hash, &fetch_account);
|
queue.remove_invalid(&hash, &fetch_account);
|
||||||
}
|
}
|
||||||
|
for hash in too_big_transactions {
|
||||||
|
queue.penalize(&hash);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
(block, original_work_hash)
|
(block, original_work_hash)
|
||||||
}
|
}
|
||||||
|
@ -134,6 +134,8 @@ struct TransactionOrder {
|
|||||||
hash: H256,
|
hash: H256,
|
||||||
/// Origin of the transaction
|
/// Origin of the transaction
|
||||||
origin: TransactionOrigin,
|
origin: TransactionOrigin,
|
||||||
|
/// Penalties
|
||||||
|
penalties: usize,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
@ -144,6 +146,7 @@ impl TransactionOrder {
|
|||||||
gas_price: tx.transaction.gas_price,
|
gas_price: tx.transaction.gas_price,
|
||||||
hash: tx.hash(),
|
hash: tx.hash(),
|
||||||
origin: tx.origin,
|
origin: tx.origin,
|
||||||
|
penalties: 0,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -151,6 +154,15 @@ impl TransactionOrder {
|
|||||||
self.nonce_height = nonce - base_nonce;
|
self.nonce_height = nonce - base_nonce;
|
||||||
self
|
self
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn penalize(mut self) -> Self {
|
||||||
|
let current = self.penalties;
|
||||||
|
self.penalties = match self.penalties.overflowing_add(1) {
|
||||||
|
(_, true) => current,
|
||||||
|
(val, false) => val,
|
||||||
|
};
|
||||||
|
self
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Eq for TransactionOrder {}
|
impl Eq for TransactionOrder {}
|
||||||
@ -167,6 +179,11 @@ impl PartialOrd for TransactionOrder {
|
|||||||
|
|
||||||
impl Ord for TransactionOrder {
|
impl Ord for TransactionOrder {
|
||||||
fn cmp(&self, b: &TransactionOrder) -> Ordering {
|
fn cmp(&self, b: &TransactionOrder) -> Ordering {
|
||||||
|
// First check number of penalties
|
||||||
|
if self.penalties != b.penalties {
|
||||||
|
return self.penalties.cmp(&b.penalties);
|
||||||
|
}
|
||||||
|
|
||||||
// First check nonce_height
|
// First check nonce_height
|
||||||
if self.nonce_height != b.nonce_height {
|
if self.nonce_height != b.nonce_height {
|
||||||
return self.nonce_height.cmp(&b.nonce_height);
|
return self.nonce_height.cmp(&b.nonce_height);
|
||||||
@ -591,6 +608,37 @@ impl TransactionQueue {
|
|||||||
assert_eq!(self.future.by_priority.len() + self.current.by_priority.len(), self.by_hash.len());
|
assert_eq!(self.future.by_priority.len() + self.current.by_priority.len(), self.by_hash.len());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Penalize transactions from sender of transaction with given hash.
|
||||||
|
/// I.e. it should change the priority of the transaction in the queue.
|
||||||
|
pub fn penalize(&mut self, transaction_hash: &H256) {
|
||||||
|
let transaction = self.by_hash.get(transaction_hash);
|
||||||
|
if transaction.is_none() {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
let transaction = transaction.expect("Early-exit for None is above.");
|
||||||
|
let sender = transaction.sender();
|
||||||
|
|
||||||
|
// Penalize all transactions from this sender
|
||||||
|
let nonces_from_sender = match self.current.by_address.row(&sender) {
|
||||||
|
Some(row_map) => row_map.keys().cloned().collect::<Vec<U256>>(),
|
||||||
|
None => vec![],
|
||||||
|
};
|
||||||
|
for k in nonces_from_sender {
|
||||||
|
let order = self.current.drop(&sender, &k).unwrap();
|
||||||
|
self.current.insert(sender, k, order.penalize());
|
||||||
|
}
|
||||||
|
// Same thing for future
|
||||||
|
let nonces_from_sender = match self.future.by_address.row(&sender) {
|
||||||
|
Some(row_map) => row_map.keys().cloned().collect::<Vec<U256>>(),
|
||||||
|
None => vec![],
|
||||||
|
};
|
||||||
|
for k in nonces_from_sender {
|
||||||
|
let order = self.future.drop(&sender, &k).unwrap();
|
||||||
|
self.current.insert(sender, k, order.penalize());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Removes invalid transaction identified by hash from queue.
|
/// Removes invalid transaction identified by hash from queue.
|
||||||
/// Assumption is that this transaction nonce is not related to client nonce,
|
/// Assumption is that this transaction nonce is not related to client nonce,
|
||||||
/// so transactions left in queue are processed according to client nonce.
|
/// so transactions left in queue are processed according to client nonce.
|
||||||
@ -948,6 +996,17 @@ mod test {
|
|||||||
(tx1.sign(secret), tx2.sign(secret))
|
(tx1.sign(secret), tx2.sign(secret))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Returns two consecutive transactions, both with increased gas price
|
||||||
|
fn new_tx_pair_with_gas_price_increment(gas_price_increment: U256) -> (SignedTransaction, SignedTransaction) {
|
||||||
|
let gas = default_gas_price() + gas_price_increment;
|
||||||
|
let tx1 = new_unsigned_tx(default_nonce(), gas);
|
||||||
|
let tx2 = new_unsigned_tx(default_nonce() + 1.into(), gas);
|
||||||
|
|
||||||
|
let keypair = Random.generate().unwrap();
|
||||||
|
let secret = &keypair.secret();
|
||||||
|
(tx1.sign(secret), tx2.sign(secret))
|
||||||
|
}
|
||||||
|
|
||||||
fn new_tx_pair_default(nonce_increment: U256, gas_price_increment: U256) -> (SignedTransaction, SignedTransaction) {
|
fn new_tx_pair_default(nonce_increment: U256, gas_price_increment: U256) -> (SignedTransaction, SignedTransaction) {
|
||||||
new_tx_pair(default_nonce(), default_gas_price(), nonce_increment, gas_price_increment)
|
new_tx_pair(default_nonce(), default_gas_price(), nonce_increment, gas_price_increment)
|
||||||
}
|
}
|
||||||
@ -1335,6 +1394,39 @@ mod test {
|
|||||||
assert_eq!(top.len(), 2);
|
assert_eq!(top.len(), 2);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn should_penalize_transactions_from_sender() {
|
||||||
|
// given
|
||||||
|
let mut txq = TransactionQueue::new();
|
||||||
|
// txa, txb - slightly bigger gas price to have consistent ordering
|
||||||
|
let (txa, txb) = new_tx_pair_default(1.into(), 0.into());
|
||||||
|
let (tx1, tx2) = new_tx_pair_with_gas_price_increment(3.into());
|
||||||
|
|
||||||
|
// insert everything
|
||||||
|
txq.add(txa.clone(), &default_account_details, TransactionOrigin::External).unwrap();
|
||||||
|
txq.add(txb.clone(), &default_account_details, TransactionOrigin::External).unwrap();
|
||||||
|
txq.add(tx1.clone(), &default_account_details, TransactionOrigin::External).unwrap();
|
||||||
|
txq.add(tx2.clone(), &default_account_details, TransactionOrigin::External).unwrap();
|
||||||
|
|
||||||
|
let top = txq.top_transactions();
|
||||||
|
assert_eq!(top[0], tx1);
|
||||||
|
assert_eq!(top[1], txa);
|
||||||
|
assert_eq!(top[2], tx2);
|
||||||
|
assert_eq!(top[3], txb);
|
||||||
|
assert_eq!(top.len(), 4);
|
||||||
|
|
||||||
|
// when
|
||||||
|
txq.penalize(&tx1.hash());
|
||||||
|
|
||||||
|
// then
|
||||||
|
let top = txq.top_transactions();
|
||||||
|
assert_eq!(top[0], txa);
|
||||||
|
assert_eq!(top[1], txb);
|
||||||
|
assert_eq!(top[2], tx1);
|
||||||
|
assert_eq!(top[3], tx2);
|
||||||
|
assert_eq!(top.len(), 4);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn should_return_pending_hashes() {
|
fn should_return_pending_hashes() {
|
||||||
// given
|
// given
|
||||||
|
Loading…
Reference in New Issue
Block a user