Separate status for canceled local transactions. (#5319)

This commit is contained in:
Tomasz Drwięga 2017-03-29 14:43:55 +02:00 committed by Gav Wood
parent 5fa088114c
commit d4684d6302
5 changed files with 45 additions and 14 deletions

View File

@ -17,7 +17,7 @@
//! Local Transactions List. //! Local Transactions List.
use linked_hash_map::LinkedHashMap; use linked_hash_map::LinkedHashMap;
use transaction::SignedTransaction; use transaction::{SignedTransaction, PendingTransaction};
use error::TransactionError; use error::TransactionError;
use util::{U256, H256}; use util::{U256, H256};
@ -40,6 +40,8 @@ pub enum Status {
Rejected(SignedTransaction, TransactionError), Rejected(SignedTransaction, TransactionError),
/// Transaction is invalid. /// Transaction is invalid.
Invalid(SignedTransaction), Invalid(SignedTransaction),
/// Transaction was canceled.
Canceled(PendingTransaction),
} }
impl Status { impl Status {
@ -99,6 +101,12 @@ impl LocalTransactionsList {
self.clear_old(); self.clear_old();
} }
pub fn mark_canceled(&mut self, tx: PendingTransaction) {
warn!(target: "own_tx", "Transaction canceled (hash {:?})", tx.hash());
self.transactions.insert(tx.hash(), Status::Canceled(tx));
self.clear_old();
}
pub fn mark_dropped(&mut self, tx: SignedTransaction) { pub fn mark_dropped(&mut self, tx: SignedTransaction) {
warn!(target: "own_tx", "Transaction dropped (hash {:?})", tx.hash()); warn!(target: "own_tx", "Transaction dropped (hash {:?})", tx.hash());
self.transactions.insert(tx.hash(), Status::Dropped(tx)); self.transactions.insert(tx.hash(), Status::Dropped(tx));

View File

@ -29,7 +29,7 @@ use transaction::{Action, UnverifiedTransaction, PendingTransaction, SignedTrans
use receipt::{Receipt, RichReceipt}; use receipt::{Receipt, RichReceipt};
use spec::Spec; use spec::Spec;
use engines::{Engine, Seal}; use engines::{Engine, Seal};
use miner::{MinerService, MinerStatus, TransactionQueue, TransactionQueueDetailsProvider, PrioritizationStrategy, use miner::{MinerService, MinerStatus, TransactionQueue, RemovalReason, TransactionQueueDetailsProvider, PrioritizationStrategy,
AccountDetails, TransactionOrigin}; AccountDetails, TransactionOrigin};
use miner::banning_queue::{BanningTransactionQueue, Threshold}; use miner::banning_queue::{BanningTransactionQueue, Threshold};
use miner::work_notify::{WorkPoster, NotifyWork}; use miner::work_notify::{WorkPoster, NotifyWork};
@ -430,7 +430,7 @@ impl Miner {
{ {
let mut queue = self.transaction_queue.write(); let mut queue = self.transaction_queue.write();
for hash in invalid_transactions { for hash in invalid_transactions {
queue.remove_invalid(&hash, &fetch_nonce); queue.remove(&hash, &fetch_nonce, RemovalReason::Invalid);
} }
for hash in transactions_to_penalize { for hash in transactions_to_penalize {
queue.penalize(&hash); queue.penalize(&hash);
@ -1021,7 +1021,7 @@ impl MinerService for Miner {
let tx = queue.find(hash); let tx = queue.find(hash);
if tx.is_some() { if tx.is_some() {
let fetch_nonce = |a: &Address| chain.latest_nonce(a); let fetch_nonce = |a: &Address| chain.latest_nonce(a);
queue.remove_invalid(hash, &fetch_nonce); queue.remove(hash, &fetch_nonce, RemovalReason::Canceled);
} }
tx tx
} }

View File

@ -54,7 +54,7 @@ mod stratum;
pub use self::external::{ExternalMiner, ExternalMinerService}; pub use self::external::{ExternalMiner, ExternalMinerService};
pub use self::miner::{Miner, MinerOptions, Banning, PendingSet, GasPricer, GasPriceCalibratorOptions, GasLimit}; pub use self::miner::{Miner, MinerOptions, Banning, PendingSet, GasPricer, GasPriceCalibratorOptions, GasLimit};
pub use self::transaction_queue::{TransactionQueue, TransactionDetailsProvider as TransactionQueueDetailsProvider, pub use self::transaction_queue::{TransactionQueue, RemovalReason, TransactionDetailsProvider as TransactionQueueDetailsProvider,
PrioritizationStrategy, AccountDetails, TransactionOrigin}; PrioritizationStrategy, AccountDetails, TransactionOrigin};
pub use self::local_transactions::{Status as LocalTransactionStatus}; pub use self::local_transactions::{Status as LocalTransactionStatus};
pub use client::TransactionImportResult; pub use client::TransactionImportResult;

View File

@ -31,7 +31,7 @@
//! //!
//! use util::{Uint, U256, Address}; //! use util::{Uint, U256, Address};
//! use ethkey::{Random, Generator}; //! use ethkey::{Random, Generator};
//! use ethcore::miner::{TransactionQueue, TransactionQueueDetailsProvider, AccountDetails, TransactionOrigin}; //! use ethcore::miner::{TransactionQueue, RemovalReason, TransactionQueueDetailsProvider, AccountDetails, TransactionOrigin};
//! use ethcore::transaction::*; //! use ethcore::transaction::*;
//! use rustc_serialize::hex::FromHex; //! use rustc_serialize::hex::FromHex;
//! //!
@ -80,7 +80,7 @@
//! //!
//! // And when transaction is removed (but nonce haven't changed) //! // And when transaction is removed (but nonce haven't changed)
//! // it will move subsequent transactions to future //! // it will move subsequent transactions to future
//! txq.remove_invalid(&st1.hash(), &|_| 10.into()); //! txq.remove(&st1.hash(), &|_| 10.into(), RemovalReason::Invalid);
//! assert_eq!(txq.status().pending, 0); //! assert_eq!(txq.status().pending, 0);
//! assert_eq!(txq.status().future, 1); //! assert_eq!(txq.status().future, 1);
//! assert_eq!(txq.top_transactions().len(), 0); //! assert_eq!(txq.top_transactions().len(), 0);
@ -510,6 +510,15 @@ pub enum PrioritizationStrategy {
GasFactorAndGasPrice, GasFactorAndGasPrice,
} }
/// Reason to remove single transaction from the queue.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum RemovalReason {
/// Transaction is invalid
Invalid,
/// Transaction was canceled
Canceled,
}
/// Point in time when transaction was inserted. /// Point in time when transaction was inserted.
pub type QueuingInstant = BlockNumber; pub type QueuingInstant = BlockNumber;
const DEFAULT_QUEUING_PERIOD: BlockNumber = 128; const DEFAULT_QUEUING_PERIOD: BlockNumber = 128;
@ -897,7 +906,7 @@ impl TransactionQueue {
.expect("We fetch details for all senders from both current and future") .expect("We fetch details for all senders from both current and future")
.nonce; .nonce;
for hash in invalid { for hash in invalid {
self.remove_invalid(&hash, &fetch_nonce); self.remove(&hash, &fetch_nonce, RemovalReason::Invalid);
} }
} }
@ -945,7 +954,7 @@ impl TransactionQueue {
/// so transactions left in queue are processed according to client nonce. /// so transactions left in queue are processed according to client nonce.
/// ///
/// If gap is introduced marks subsequent transactions as future /// If gap is introduced marks subsequent transactions as future
pub fn remove_invalid<F>(&mut self, transaction_hash: &H256, fetch_nonce: &F) pub fn remove<F>(&mut self, transaction_hash: &H256, fetch_nonce: &F, reason: RemovalReason)
where F: Fn(&Address) -> U256 { where F: Fn(&Address) -> U256 {
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());
@ -964,7 +973,14 @@ impl TransactionQueue {
// Mark in locals // Mark in locals
if self.local_transactions.contains(transaction_hash) { if self.local_transactions.contains(transaction_hash) {
self.local_transactions.mark_invalid(transaction.transaction.into()); match reason {
RemovalReason::Invalid => self.local_transactions.mark_invalid(
transaction.transaction.into()
),
RemovalReason::Canceled => self.local_transactions.mark_canceled(
PendingTransaction::new(transaction.transaction, transaction.condition)
),
}
} }
// Remove from future // Remove from future
@ -2277,7 +2293,7 @@ pub mod test {
assert_eq!(txq.status().pending, 3); assert_eq!(txq.status().pending, 3);
// when // when
txq.remove_invalid(&tx.hash(), &|_| default_nonce()); txq.remove(&tx.hash(), &|_| default_nonce(), RemovalReason::Invalid);
// then // then
let stats = txq.status(); let stats = txq.status();
@ -2420,7 +2436,7 @@ pub mod test {
assert_eq!(txq.status().pending, 2); assert_eq!(txq.status().pending, 2);
// when // when
txq.remove_invalid(&tx1.hash(), &|_| default_nonce()); txq.remove(&tx1.hash(), &|_| default_nonce(), RemovalReason::Invalid);
assert_eq!(txq.status().pending, 0); assert_eq!(txq.status().pending, 0);
assert_eq!(txq.status().future, 1); assert_eq!(txq.status().future, 1);
txq.add(tx1.clone(), TransactionOrigin::External, 0, None, &default_tx_provider()).unwrap(); txq.add(tx1.clone(), TransactionOrigin::External, 0, None, &default_tx_provider()).unwrap();
@ -2518,7 +2534,7 @@ pub mod test {
assert_eq!(txq.status().future, 2); assert_eq!(txq.status().future, 2);
// when // when
txq.remove_invalid(&tx1.hash(), &|_| default_nonce() + 1.into()); txq.remove(&tx1.hash(), &|_| default_nonce() + 1.into(), RemovalReason::Invalid);
// then // then
let stats = txq.status(); let stats = txq.status();

View File

@ -91,6 +91,8 @@ pub enum LocalTransactionStatus {
Rejected(Transaction, String), Rejected(Transaction, String),
/// Transaction is invalid. /// Transaction is invalid.
Invalid(Transaction), Invalid(Transaction),
/// Transaction was canceled.
Canceled(Transaction),
} }
impl Serialize for LocalTransactionStatus { impl Serialize for LocalTransactionStatus {
@ -101,7 +103,7 @@ impl Serialize for LocalTransactionStatus {
let elems = match *self { let elems = match *self {
Pending | Future => 1, Pending | Future => 1,
Mined(..) | Dropped(..) | Invalid(..) => 2, Mined(..) | Dropped(..) | Invalid(..) | Canceled(..) => 2,
Rejected(..) => 3, Rejected(..) => 3,
Replaced(..) => 4, Replaced(..) => 4,
}; };
@ -121,6 +123,10 @@ impl Serialize for LocalTransactionStatus {
struc.serialize_field(status, "dropped")?; struc.serialize_field(status, "dropped")?;
struc.serialize_field(transaction, tx)?; struc.serialize_field(transaction, tx)?;
}, },
Canceled(ref tx) => {
struc.serialize_field(status, "canceled")?;
struc.serialize_field(transaction, tx)?;
},
Invalid(ref tx) => { Invalid(ref tx) => {
struc.serialize_field(status, "invalid")?; struc.serialize_field(status, "invalid")?;
struc.serialize_field(transaction, tx)?; struc.serialize_field(transaction, tx)?;
@ -249,6 +255,7 @@ impl From<miner::LocalTransactionStatus> for LocalTransactionStatus {
Rejected(tx, err) => LocalTransactionStatus::Rejected(tx.into(), errors::transaction_message(err)), Rejected(tx, err) => LocalTransactionStatus::Rejected(tx.into(), errors::transaction_message(err)),
Replaced(tx, gas_price, hash) => LocalTransactionStatus::Replaced(tx.into(), gas_price.into(), hash.into()), Replaced(tx, gas_price, hash) => LocalTransactionStatus::Replaced(tx.into(), gas_price.into(), hash.into()),
Invalid(tx) => LocalTransactionStatus::Invalid(tx.into()), Invalid(tx) => LocalTransactionStatus::Invalid(tx.into()),
Canceled(tx) => LocalTransactionStatus::Canceled(tx.into()),
} }
} }
} }