Merge pull request #114 from paritytech/aj-tx-error-chain

tx-pool: replace error-chain with vanilla Error impl
This commit is contained in:
Andrew Jones 2019-03-01 22:06:35 +00:00 committed by GitHub
commit a31b51ca31
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 56 additions and 50 deletions

View File

@ -7,7 +7,6 @@ authors = ["Parity Technologies <admin@parity.io>"]
repository = "https://github.com/paritytech/parity-common"
[dependencies]
error-chain = "0.12"
log = "0.4"
smallvec = "0.6"
trace-time = { path = "../trace-time", version = "0.1" }

View File

@ -14,34 +14,41 @@
// You should have received a copy of the GNU General Public License
// along with Parity. If not, see <http://www.gnu.org/licenses/>.
/// Error chain doesn't let us have generic types.
/// So the hashes are converted to debug strings for easy display.
type Hash = String;
use std::{error, fmt, result};
error_chain! {
errors {
/// Transaction is already imported
AlreadyImported(hash: Hash) {
description("transaction is already in the pool"),
display("[{}] already imported", hash)
}
/// Transaction is too cheap to enter the queue
TooCheapToEnter(hash: Hash, min_score: String) {
description("the pool is full and transaction is too cheap to replace any transaction"),
display("[{}] too cheap to enter the pool. Min score: {}", hash, min_score)
}
/// Transaction is too cheap to replace existing transaction that occupies the same slot.
TooCheapToReplace(old_hash: Hash, hash: Hash) {
description("transaction is too cheap to replace existing transaction in the pool"),
display("[{}] too cheap to replace: {}", hash, old_hash)
/// Transaction Pool Error
#[derive(Debug)]
pub enum Error<Hash: fmt::Debug + fmt::LowerHex> {
/// Transaction is already imported
AlreadyImported(Hash),
/// Transaction is too cheap to enter the queue
TooCheapToEnter(Hash, String),
/// Transaction is too cheap to replace existing transaction that occupies the same slot.
TooCheapToReplace(Hash, Hash),
}
/// Transaction Pool Result
pub type Result<T, H> = result::Result<T, Error<H>>;
impl<H: fmt::Debug + fmt::LowerHex> fmt::Display for Error<H> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
Error::AlreadyImported(h) =>
write!(f, "[{:?}] already imported", h),
Error::TooCheapToEnter(hash, min_score) =>
write!(f, "[{:x}] too cheap to enter the pool. Min score: {}", hash, min_score),
Error::TooCheapToReplace(old_hash, hash) =>
write!(f, "[{:x}] too cheap to replace: {:x}", hash, old_hash),
}
}
}
impl<H: fmt::Debug + fmt::LowerHex> error::Error for Error<H> {}
#[cfg(test)]
impl PartialEq for ErrorKind {
impl<H: fmt::Debug + fmt::LowerHex> PartialEq for Error<H> where H: PartialEq {
fn eq(&self, other: &Self) -> bool {
use self::ErrorKind::*;
use self::Error::*;
match (self, other) {
(&AlreadyImported(ref h1), &AlreadyImported(ref h2)) => h1 == h2,

View File

@ -71,8 +71,6 @@
extern crate smallvec;
extern crate trace_time;
#[macro_use]
extern crate error_chain;
#[macro_use]
extern crate log;
@ -93,7 +91,7 @@ mod verifier;
pub mod scoring;
pub use self::error::{Error, ErrorKind};
pub use self::error::Error;
pub use self::listener::{Listener, NoopListener};
pub use self::options::Options;
pub use self::pool::{Pool, PendingIterator, UnorderedIterator, Transaction};

View File

@ -14,8 +14,8 @@
// You should have received a copy of the GNU General Public License
// along with Parity. If not, see <http://www.gnu.org/licenses/>.
use std::sync::Arc;
use error::ErrorKind;
use std::{fmt::{Debug, LowerHex}, sync::Arc};
use error::Error;
/// Transaction pool listener.
///
@ -29,7 +29,7 @@ pub trait Listener<T> {
/// The transaction was rejected from the pool.
/// It means that it was too cheap to replace any transaction already in the pool.
fn rejected(&mut self, _tx: &Arc<T>, _reason: &ErrorKind) {}
fn rejected<H: Debug + LowerHex>(&mut self, _tx: &Arc<T>, _reason: &Error<H>) {}
/// The transaction was pushed out from the pool because of the limit.
fn dropped(&mut self, _tx: &Arc<T>, _by: Option<&T>) {}
@ -58,7 +58,7 @@ impl<T, A, B> Listener<T> for (A, B) where
self.1.added(tx, old);
}
fn rejected(&mut self, tx: &Arc<T>, reason: &ErrorKind) {
fn rejected<H: Debug + LowerHex>(&mut self, tx: &Arc<T>, reason: &Error<H>) {
self.0.rejected(tx, reason);
self.1.rejected(tx, reason);
}

View File

@ -133,10 +133,12 @@ impl<T, S, L> Pool<T, S, L> where
/// If any limit is reached the transaction with the lowest `Score` is evicted to make room.
///
/// The `Listener` will be informed on any drops or rejections.
pub fn import(&mut self, transaction: T) -> error::Result<Arc<T>> {
pub fn import(&mut self, transaction: T) -> error::Result<Arc<T>, T::Hash> {
let mem_usage = transaction.mem_usage();
ensure!(!self.by_hash.contains_key(transaction.hash()), error::ErrorKind::AlreadyImported(format!("{:?}", transaction.hash())));
if self.by_hash.contains_key(transaction.hash()) {
return Err(error::Error::AlreadyImported(transaction.hash().clone()))
}
self.insertion_id += 1;
let transaction = Transaction {
@ -150,7 +152,7 @@ impl<T, S, L> Pool<T, S, L> where
let remove_worst = |s: &mut Self, transaction| {
match s.remove_worst(transaction) {
Err(err) => {
s.listener.rejected(transaction, err.kind());
s.listener.rejected(transaction, &err);
Err(err)
},
Ok(None) => Ok(false),
@ -202,14 +204,14 @@ impl<T, S, L> Pool<T, S, L> where
Ok(new.transaction)
},
AddResult::TooCheap { new, old } => {
let error = error::ErrorKind::TooCheapToReplace(format!("{:x}", old.hash()), format!("{:x}", new.hash()));
let error = error::Error::TooCheapToReplace(old.hash().clone(), new.hash().clone());
self.listener.rejected(&new, &error);
bail!(error)
return Err(error)
},
AddResult::TooCheapToEnter(new, score) => {
let error = error::ErrorKind::TooCheapToEnter(format!("{:x}", new.hash()), format!("{:#x}", score));
let error = error::Error::TooCheapToEnter(new.hash().clone(), format!("{:#x}", score));
self.listener.rejected(&new, &error);
bail!(error)
return Err(error)
}
}
}
@ -281,12 +283,12 @@ impl<T, S, L> Pool<T, S, L> where
///
/// Returns `None` in case we couldn't decide if the transaction should replace the worst transaction or not.
/// In such case we will accept the transaction even though it is going to exceed the limit.
fn remove_worst(&mut self, transaction: &Transaction<T>) -> error::Result<Option<Transaction<T>>> {
fn remove_worst(&mut self, transaction: &Transaction<T>) -> error::Result<Option<Transaction<T>>, T::Hash> {
let to_remove = match self.worst_transactions.iter().next_back() {
// No elements to remove? and the pool is still full?
None => {
warn!("The pool is full but there are no transactions to remove.");
return Err(error::ErrorKind::TooCheapToEnter(format!("{:?}", transaction.hash()), "unknown".into()).into());
return Err(error::Error::TooCheapToEnter(transaction.hash().clone(), "unknown".into()))
},
Some(old) => match self.scoring.should_replace(&old.transaction, transaction) {
// We can't decide which of them should be removed, so accept both.
@ -295,7 +297,7 @@ impl<T, S, L> Pool<T, S, L> where
scoring::Choice::ReplaceOld => Some(old.clone()),
// otherwise fail
scoring::Choice::RejectNew => {
return Err(error::ErrorKind::TooCheapToEnter(format!("{:?}", transaction.hash()), format!("{:#x}", old.score)).into())
return Err(error::Error::TooCheapToEnter(transaction.hash().clone(), format!("{:#x}", old.score)))
},
},
};

View File

@ -133,9 +133,9 @@ fn should_reject_if_above_count() {
// Reject second
let tx1 = b.tx().nonce(0).new();
let tx2 = b.tx().nonce(1).new();
let hash = format!("{:?}", tx2.hash());
let hash = tx2.hash.clone();
txq.import(tx1).unwrap();
assert_eq!(txq.import(tx2).unwrap_err().kind(), &error::ErrorKind::TooCheapToEnter(hash, "0x0".into()));
assert_eq!(txq.import(tx2).unwrap_err(), error::Error::TooCheapToEnter(hash, "0x0".into()));
assert_eq!(txq.light_status().transaction_count, 1);
txq.clear();
@ -159,9 +159,9 @@ fn should_reject_if_above_mem_usage() {
// Reject second
let tx1 = b.tx().nonce(1).mem_usage(1).new();
let tx2 = b.tx().nonce(2).mem_usage(2).new();
let hash = format!("{:?}", tx2.hash());
let hash = tx2.hash.clone();
txq.import(tx1).unwrap();
assert_eq!(txq.import(tx2).unwrap_err().kind(), &error::ErrorKind::TooCheapToEnter(hash, "0x0".into()));
assert_eq!(txq.import(tx2).unwrap_err(), error::Error::TooCheapToEnter(hash, "0x0".into()));
assert_eq!(txq.light_status().transaction_count, 1);
txq.clear();
@ -185,9 +185,9 @@ fn should_reject_if_above_sender_count() {
// Reject second
let tx1 = b.tx().nonce(1).new();
let tx2 = b.tx().nonce(2).new();
let hash = format!("{:x}", tx2.hash());
let hash = tx2.hash.clone();
txq.import(tx1).unwrap();
assert_eq!(txq.import(tx2).unwrap_err().kind(), &error::ErrorKind::TooCheapToEnter(hash, "0x0".into()));
assert_eq!(txq.import(tx2).unwrap_err(), error::Error::TooCheapToEnter(hash, "0x0".into()));
assert_eq!(txq.light_status().transaction_count, 1);
txq.clear();
@ -195,10 +195,10 @@ fn should_reject_if_above_sender_count() {
// Replace first
let tx1 = b.tx().nonce(1).new();
let tx2 = b.tx().nonce(2).gas_price(2).new();
let hash = format!("{:x}", tx2.hash());
let hash = tx2.hash.clone();
txq.import(tx1).unwrap();
// This results in error because we also compare nonces
assert_eq!(txq.import(tx2).unwrap_err().kind(), &error::ErrorKind::TooCheapToEnter(hash, "0x0".into()));
assert_eq!(txq.import(tx2).unwrap_err(), error::Error::TooCheapToEnter(hash, "0x0".into()));
assert_eq!(txq.light_status().transaction_count, 1);
}
@ -587,8 +587,7 @@ fn should_not_import_even_if_limit_is_reached_and_should_replace_returns_false()
let err = txq.import(b.tx().nonce(1).gas_price(5).new()).unwrap_err();
// then
assert_eq!(err.kind(),
&error::ErrorKind::TooCheapToEnter("0x00000000000000000000000000000000000000000000000000000000000001f5".into(), "0x5".into()));
assert_eq!(err, error::Error::TooCheapToEnter("0x00000000000000000000000000000000000000000000000000000000000001f5".into(), "0x5".into()));
assert_eq!(txq.light_status(), LightStatus {
transaction_count: 1,
senders: 1,
@ -626,6 +625,7 @@ fn should_import_even_if_sender_limit_is_reached() {
mod listener {
use std::cell::RefCell;
use std::rc::Rc;
use std::fmt;
use super::*;
@ -637,7 +637,7 @@ mod listener {
self.0.borrow_mut().push(if old.is_some() { "replaced" } else { "added" });
}
fn rejected(&mut self, _tx: &SharedTransaction, _reason: &error::ErrorKind) {
fn rejected<H: fmt::Debug + fmt::LowerHex>(&mut self, _tx: &SharedTransaction, _reason: &error::Error<H>) {
self.0.borrow_mut().push("rejected".into());
}