Even more detailed errors for transaction queue
This commit is contained in:
parent
5aa54e0c1a
commit
881678b613
@ -62,13 +62,18 @@ pub enum ExecutionError {
|
||||
Internal
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
#[derive(Debug, PartialEq)]
|
||||
/// Errors concerning transaction processing.
|
||||
pub enum TransactionError {
|
||||
/// Transaction is already imported to the queue
|
||||
AlreadyImported,
|
||||
/// Transaction is not valid anymore (state already has higher nonce)
|
||||
Old,
|
||||
/// Transaction has too low fee
|
||||
/// (there is already a transaction with the same sender-nonce but higher gas price)
|
||||
TooCheap,
|
||||
/// Transaction was not imported to the queue because limit has been reached.
|
||||
LimitReached,
|
||||
/// Transaction's gas price is below threshold.
|
||||
InsufficientGasPrice {
|
||||
/// Minimal expected gas price
|
||||
@ -153,7 +158,7 @@ pub enum BlockError {
|
||||
UnknownUncleParent(H256),
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
#[derive(Debug, PartialEq)]
|
||||
/// Import to the block queue result
|
||||
pub enum ImportError {
|
||||
/// Already in the block chain.
|
||||
|
@ -61,7 +61,7 @@ extern crate rayon;
|
||||
mod miner;
|
||||
mod transaction_queue;
|
||||
|
||||
pub use transaction_queue::{TransactionQueue, AccountDetails};
|
||||
pub use transaction_queue::{TransactionQueue, AccountDetails, TransactionImportResult};
|
||||
pub use miner::{Miner};
|
||||
|
||||
use util::{H256, U256, Address, Bytes};
|
||||
@ -145,15 +145,6 @@ pub trait MinerService : Send + Sync {
|
||||
fn sensible_gas_limit(&self) -> U256 { x!(21000) }
|
||||
}
|
||||
|
||||
/// Represents the result of importing transaction.
|
||||
#[derive(Debug)]
|
||||
pub enum TransactionImportResult {
|
||||
/// Transaction was imported to current queue.
|
||||
Current,
|
||||
/// Transaction was imported to future queue.
|
||||
Future
|
||||
}
|
||||
|
||||
/// Mining status
|
||||
#[derive(Debug)]
|
||||
pub struct MinerStatus {
|
||||
|
@ -86,14 +86,30 @@
|
||||
|
||||
use std::default::Default;
|
||||
use std::cmp::{Ordering};
|
||||
use std::collections::{HashMap, BTreeSet};
|
||||
use std::collections::{HashMap, HashSet, BTreeSet};
|
||||
use util::numbers::{Uint, U256};
|
||||
use util::hash::{Address, H256};
|
||||
use util::table::*;
|
||||
use ethcore::transaction::*;
|
||||
use ethcore::error::{Error, TransactionError};
|
||||
use super::TransactionImportResult;
|
||||
|
||||
macro_rules! check_too_cheap {
|
||||
($is_in:expr) => {
|
||||
if !($is_in) {
|
||||
return Err(TransactionError::TooCheap);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
macro_rules! check_limit {
|
||||
($hash:expr, $dropped:expr) => {
|
||||
if let Some(dropped) = $dropped {
|
||||
if dropped.contains(&$hash) {
|
||||
return Err(TransactionError::LimitReached);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
/// Light structure used to identify transaction and it's order
|
||||
@ -206,10 +222,11 @@ impl TransactionSet {
|
||||
/// Remove low priority transactions if there is more then specified by given `limit`.
|
||||
///
|
||||
/// It drops transactions from this set but also removes associated `VerifiedTransaction`.
|
||||
fn enforce_limit(&mut self, by_hash: &mut HashMap<H256, VerifiedTransaction>) {
|
||||
/// Returns hashes of transactions removed because of limit.
|
||||
fn enforce_limit(&mut self, by_hash: &mut HashMap<H256, VerifiedTransaction>) -> Option<HashSet<H256>> {
|
||||
let len = self.by_priority.len();
|
||||
if len <= self.limit {
|
||||
return;
|
||||
return None;
|
||||
}
|
||||
|
||||
let to_drop : Vec<(Address, U256)> = {
|
||||
@ -222,10 +239,17 @@ impl TransactionSet {
|
||||
.collect()
|
||||
};
|
||||
|
||||
for (sender, nonce) in to_drop {
|
||||
let order = self.drop(&sender, &nonce).expect("Transaction has just been found in `by_priority`; so it is in `by_address` also.");
|
||||
by_hash.remove(&order.hash).expect("Hash found in `by_priorty` matches the one dropped; so it is included in `by_hash`");
|
||||
}
|
||||
Some(to_drop
|
||||
.into_iter()
|
||||
.map(|(sender, nonce)| {
|
||||
let order = self.drop(&sender, &nonce)
|
||||
.expect("Transaction has just been found in `by_priority`; so it is in `by_address` also.");
|
||||
|
||||
by_hash.remove(&order.hash)
|
||||
.expect("Hash found in `by_priorty` matches the one dropped; so it is included in `by_hash`");
|
||||
order.hash
|
||||
})
|
||||
.collect::<HashSet<H256>>())
|
||||
}
|
||||
|
||||
/// Drop transaction from this set (remove from `by_priority` and `by_address`)
|
||||
@ -253,6 +277,15 @@ pub struct TransactionQueueStatus {
|
||||
pub future: usize,
|
||||
}
|
||||
|
||||
#[derive(Debug, PartialEq)]
|
||||
/// Represents the result of importing transaction.
|
||||
pub enum TransactionImportResult {
|
||||
/// Transaction was imported to current queue.
|
||||
Current,
|
||||
/// Transaction was imported to future queue.
|
||||
Future
|
||||
}
|
||||
|
||||
/// Details of account
|
||||
pub struct AccountDetails {
|
||||
/// Most recent account nonce
|
||||
@ -580,6 +613,7 @@ impl TransactionQueue {
|
||||
|
||||
let address = tx.sender();
|
||||
let nonce = tx.nonce();
|
||||
let hash = tx.hash();
|
||||
|
||||
let next_nonce = self.last_nonces
|
||||
.get(&address)
|
||||
@ -589,8 +623,8 @@ impl TransactionQueue {
|
||||
// Check height
|
||||
if nonce > next_nonce {
|
||||
// We have a gap - put to future
|
||||
Self::replace_transaction(tx, next_nonce, &mut self.future, &mut self.by_hash);
|
||||
self.future.enforce_limit(&mut self.by_hash);
|
||||
check_too_cheap!(Self::replace_transaction(tx, next_nonce, &mut self.future, &mut self.by_hash));
|
||||
check_limit!(hash, self.future.enforce_limit(&mut self.by_hash));
|
||||
return Ok(TransactionImportResult::Future);
|
||||
} else if nonce < state_nonce {
|
||||
// Droping transaction
|
||||
@ -598,7 +632,7 @@ impl TransactionQueue {
|
||||
return Err(TransactionError::Old);
|
||||
}
|
||||
|
||||
Self::replace_transaction(tx, state_nonce, &mut self.current, &mut self.by_hash);
|
||||
check_too_cheap!(Self::replace_transaction(tx, state_nonce, &mut self.current, &mut self.by_hash));
|
||||
// Keep track of highest nonce stored in current
|
||||
self.last_nonces.insert(address, nonce);
|
||||
// Update nonces of transactions in future
|
||||
@ -610,10 +644,10 @@ impl TransactionQueue {
|
||||
if let Some(order) = self.future.drop(&address, &nonce) {
|
||||
// Let's insert that transaction to current (if it has higher gas_price)
|
||||
let future_tx = self.by_hash.remove(&order.hash).unwrap();
|
||||
Self::replace_transaction(future_tx, state_nonce, &mut self.current, &mut self.by_hash);
|
||||
check_too_cheap!(Self::replace_transaction(future_tx, state_nonce, &mut self.current, &mut self.by_hash));
|
||||
}
|
||||
// Also enforce the limit
|
||||
self.current.enforce_limit(&mut self.by_hash);
|
||||
check_limit!(hash, self.current.enforce_limit(&mut self.by_hash));
|
||||
|
||||
trace!(target: "miner", "status: {:?}", self.status());
|
||||
Ok(TransactionImportResult::Current)
|
||||
@ -623,13 +657,17 @@ impl TransactionQueue {
|
||||
///
|
||||
/// If there is already transaction with same `(sender, nonce)` it will be replaced iff `gas_price` is higher.
|
||||
/// One of the transactions is dropped from set and also removed from queue entirely (from `by_hash`).
|
||||
fn replace_transaction(tx: VerifiedTransaction, base_nonce: U256, set: &mut TransactionSet, by_hash: &mut HashMap<H256, VerifiedTransaction>) {
|
||||
///
|
||||
/// Returns `true` if transaction actually got to the queue (`false` if there was already a transaction with higher
|
||||
/// gas_price)
|
||||
fn replace_transaction(tx: VerifiedTransaction, base_nonce: U256, set: &mut TransactionSet, by_hash: &mut HashMap<H256, VerifiedTransaction>) -> bool {
|
||||
let order = TransactionOrder::for_transaction(&tx, base_nonce);
|
||||
let hash = tx.hash();
|
||||
let address = tx.sender();
|
||||
let nonce = tx.nonce();
|
||||
|
||||
by_hash.insert(hash, tx);
|
||||
|
||||
if let Some(old) = set.insert(address, nonce, order.clone()) {
|
||||
// There was already transaction in queue. Let's check which one should stay
|
||||
let old_fee = old.gas_price;
|
||||
@ -639,14 +677,17 @@ impl TransactionQueue {
|
||||
set.insert(address, nonce, old);
|
||||
// and remove new one
|
||||
by_hash.remove(&hash);
|
||||
false
|
||||
} else {
|
||||
// Make sure we remove old transaction entirely
|
||||
by_hash.remove(&old.hash);
|
||||
true
|
||||
}
|
||||
} else {
|
||||
true
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
#[cfg(test)]
|
||||
mod test {
|
||||
@ -654,9 +695,17 @@ mod test {
|
||||
use util::table::*;
|
||||
use util::*;
|
||||
use ethcore::transaction::*;
|
||||
use ethcore::error::{Error, TransactionError};
|
||||
use super::*;
|
||||
use super::{TransactionSet, TransactionOrder, VerifiedTransaction};
|
||||
|
||||
fn unwrap_tx_err(err: Result<TransactionImportResult, Error>) -> TransactionError {
|
||||
match err.unwrap_err() {
|
||||
Error::Transaction(e) => e,
|
||||
_ => panic!("Expected transaction error!"),
|
||||
}
|
||||
}
|
||||
|
||||
fn new_unsigned_tx(nonce: U256) -> Transaction {
|
||||
Transaction {
|
||||
action: Action::Create,
|
||||
@ -698,11 +747,16 @@ mod test {
|
||||
}
|
||||
|
||||
fn new_txs(second_nonce: U256) -> (SignedTransaction, SignedTransaction) {
|
||||
new_txs_with_gas_price_diff(second_nonce, U256::zero())
|
||||
}
|
||||
|
||||
fn new_txs_with_gas_price_diff(second_nonce: U256, gas_price: U256) -> (SignedTransaction, SignedTransaction) {
|
||||
let keypair = KeyPair::create().unwrap();
|
||||
let secret = &keypair.secret();
|
||||
let nonce = U256::from(123);
|
||||
let tx = new_unsigned_tx(nonce);
|
||||
let tx2 = new_unsigned_tx(nonce + second_nonce);
|
||||
let mut tx2 = new_unsigned_tx(nonce + second_nonce);
|
||||
tx2.gas_price = tx2.gas_price + gas_price;
|
||||
|
||||
(tx.sign(secret), tx2.sign(secret))
|
||||
}
|
||||
@ -795,14 +849,14 @@ mod test {
|
||||
|
||||
// First insert one transaction to future
|
||||
let res = txq.add(tx, &prev_nonce);
|
||||
assert!(res.is_ok());
|
||||
assert_eq!(res.unwrap(), TransactionImportResult::Future);
|
||||
assert_eq!(txq.status().future, 1);
|
||||
|
||||
// now import second transaction to current
|
||||
let res = txq.add(tx2.clone(), &default_nonce);
|
||||
|
||||
// and then there should be only one transaction in current (the one with higher gas_price)
|
||||
assert!(res.is_ok());
|
||||
assert_eq!(unwrap_tx_err(res), TransactionError::TooCheap);
|
||||
assert_eq!(txq.status().pending, 1);
|
||||
assert_eq!(txq.status().future, 0);
|
||||
assert_eq!(txq.current.by_priority.len(), 1);
|
||||
@ -821,7 +875,7 @@ mod test {
|
||||
let res = txq.add(tx, &default_nonce);
|
||||
|
||||
// then
|
||||
assert!(res.is_ok());
|
||||
assert_eq!(res.unwrap(), TransactionImportResult::Current);
|
||||
let stats = txq.status();
|
||||
assert_eq!(stats.pending, 1);
|
||||
}
|
||||
@ -845,12 +899,18 @@ mod test {
|
||||
// given
|
||||
let mut txq = TransactionQueue::new();
|
||||
let tx = new_tx();
|
||||
txq.set_gas_limit(tx.gas / U256::from(2));
|
||||
let gas = tx.gas;
|
||||
let limit = gas / U256::from(2);
|
||||
txq.set_gas_limit(limit);
|
||||
|
||||
// when
|
||||
txq.add(tx, &default_nonce).unwrap_err();
|
||||
let res = txq.add(tx, &default_nonce);
|
||||
|
||||
// then
|
||||
assert_eq!(unwrap_tx_err(res), TransactionError::GasLimitExceeded {
|
||||
limit: U256::from(55_000), // Should be 110% of set_gas_limit
|
||||
got: gas,
|
||||
});
|
||||
let stats = txq.status();
|
||||
assert_eq!(stats.pending, 0);
|
||||
assert_eq!(stats.future, 0);
|
||||
@ -868,9 +928,13 @@ mod test {
|
||||
};
|
||||
|
||||
// when
|
||||
txq.add(tx, &account).unwrap_err();
|
||||
let res = txq.add(tx, &account);
|
||||
|
||||
// then
|
||||
assert_eq!(unwrap_tx_err(res), TransactionError::InsufficientBalance {
|
||||
balance: U256::from(1),
|
||||
cost: U256::from(100_100),
|
||||
});
|
||||
let stats = txq.status();
|
||||
assert_eq!(stats.pending, 0);
|
||||
assert_eq!(stats.future, 0);
|
||||
@ -884,9 +948,13 @@ mod test {
|
||||
txq.set_minimal_gas_price(tx.gas_price + U256::one());
|
||||
|
||||
// when
|
||||
txq.add(tx, &default_nonce).unwrap_err();
|
||||
let res = txq.add(tx, &default_nonce);
|
||||
|
||||
// then
|
||||
assert_eq!(unwrap_tx_err(res), TransactionError::InsufficientGasPrice {
|
||||
minimal: U256::from(2),
|
||||
got: U256::from(1),
|
||||
});
|
||||
let stats = txq.status();
|
||||
assert_eq!(stats.pending, 0);
|
||||
assert_eq!(stats.future, 0);
|
||||
@ -961,10 +1029,12 @@ mod test {
|
||||
let (tx, tx2) = new_txs(U256::from(2));
|
||||
|
||||
// when
|
||||
txq.add(tx.clone(), &default_nonce).unwrap();
|
||||
txq.add(tx2.clone(), &default_nonce).unwrap();
|
||||
let res1 = txq.add(tx.clone(), &default_nonce).unwrap();
|
||||
let res2 = txq.add(tx2.clone(), &default_nonce).unwrap();
|
||||
|
||||
// then
|
||||
assert_eq!(res1, TransactionImportResult::Current);
|
||||
assert_eq!(res2, TransactionImportResult::Future);
|
||||
let stats = txq.status();
|
||||
assert_eq!(stats.pending, 1);
|
||||
assert_eq!(stats.future, 1);
|
||||
@ -1091,10 +1161,11 @@ mod test {
|
||||
assert_eq!(txq.status().pending, 1);
|
||||
|
||||
// when
|
||||
txq.add(tx2.clone(), &default_nonce).unwrap();
|
||||
let res = txq.add(tx2.clone(), &default_nonce);
|
||||
|
||||
// then
|
||||
let t = txq.top_transactions();
|
||||
assert_eq!(unwrap_tx_err(res), TransactionError::LimitReached);
|
||||
assert_eq!(txq.status().pending, 1);
|
||||
assert_eq!(t.len(), 1);
|
||||
assert_eq!(t[0], tx);
|
||||
@ -1103,8 +1174,8 @@ mod test {
|
||||
#[test]
|
||||
fn should_limit_future_transactions() {
|
||||
let mut txq = TransactionQueue::with_limits(10, 1);
|
||||
let (tx1, tx2) = new_txs(U256::from(4));
|
||||
let (tx3, tx4) = new_txs(U256::from(4));
|
||||
let (tx1, tx2) = new_txs_with_gas_price_diff(U256::from(4), U256::from(1));
|
||||
let (tx3, tx4) = new_txs_with_gas_price_diff(U256::from(4), U256::from(2));
|
||||
txq.add(tx1.clone(), &default_nonce).unwrap();
|
||||
txq.add(tx3.clone(), &default_nonce).unwrap();
|
||||
assert_eq!(txq.status().pending, 2);
|
||||
@ -1126,9 +1197,10 @@ mod test {
|
||||
let fetch_last_nonce = |_a: &Address| AccountDetails{ nonce: last_nonce, balance: !U256::zero() };
|
||||
|
||||
// when
|
||||
txq.add(tx, &fetch_last_nonce).unwrap_err();
|
||||
let res = txq.add(tx, &fetch_last_nonce);
|
||||
|
||||
// then
|
||||
assert_eq!(unwrap_tx_err(res), TransactionError::Old);
|
||||
let stats = txq.status();
|
||||
assert_eq!(stats.pending, 0);
|
||||
assert_eq!(stats.future, 0);
|
||||
@ -1146,9 +1218,10 @@ mod test {
|
||||
assert_eq!(txq.status().pending, 0);
|
||||
|
||||
// when
|
||||
txq.add(tx2.clone(), &nonce).unwrap_err();
|
||||
let res = txq.add(tx2.clone(), &nonce);
|
||||
|
||||
// then
|
||||
assert_eq!(unwrap_tx_err(res), TransactionError::AlreadyImported);
|
||||
let stats = txq.status();
|
||||
assert_eq!(stats.future, 1);
|
||||
assert_eq!(stats.pending, 0);
|
||||
|
Loading…
Reference in New Issue
Block a user