Skip to content

Commit

Permalink
Separate status for canceled local transactions. (openethereum#5319)
Browse files Browse the repository at this point in the history
  • Loading branch information
tomusdrw authored and gavofyork committed Mar 29, 2017
1 parent 5fa0881 commit d4684d6
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 14 deletions.
10 changes: 9 additions & 1 deletion ethcore/src/miner/local_transactions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
//! Local Transactions List.
use linked_hash_map::LinkedHashMap;
use transaction::SignedTransaction;
use transaction::{SignedTransaction, PendingTransaction};
use error::TransactionError;
use util::{U256, H256};

Expand All @@ -40,6 +40,8 @@ pub enum Status {
Rejected(SignedTransaction, TransactionError),
/// Transaction is invalid.
Invalid(SignedTransaction),
/// Transaction was canceled.
Canceled(PendingTransaction),
}

impl Status {
Expand Down Expand Up @@ -99,6 +101,12 @@ impl LocalTransactionsList {
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) {
warn!(target: "own_tx", "Transaction dropped (hash {:?})", tx.hash());
self.transactions.insert(tx.hash(), Status::Dropped(tx));
Expand Down
6 changes: 3 additions & 3 deletions ethcore/src/miner/miner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use transaction::{Action, UnverifiedTransaction, PendingTransaction, SignedTrans
use receipt::{Receipt, RichReceipt};
use spec::Spec;
use engines::{Engine, Seal};
use miner::{MinerService, MinerStatus, TransactionQueue, TransactionQueueDetailsProvider, PrioritizationStrategy,
use miner::{MinerService, MinerStatus, TransactionQueue, RemovalReason, TransactionQueueDetailsProvider, PrioritizationStrategy,
AccountDetails, TransactionOrigin};
use miner::banning_queue::{BanningTransactionQueue, Threshold};
use miner::work_notify::{WorkPoster, NotifyWork};
Expand Down Expand Up @@ -430,7 +430,7 @@ impl Miner {
{
let mut queue = self.transaction_queue.write();
for hash in invalid_transactions {
queue.remove_invalid(&hash, &fetch_nonce);
queue.remove(&hash, &fetch_nonce, RemovalReason::Invalid);
}
for hash in transactions_to_penalize {
queue.penalize(&hash);
Expand Down Expand Up @@ -1021,7 +1021,7 @@ impl MinerService for Miner {
let tx = queue.find(hash);
if tx.is_some() {
let fetch_nonce = |a: &Address| chain.latest_nonce(a);
queue.remove_invalid(hash, &fetch_nonce);
queue.remove(hash, &fetch_nonce, RemovalReason::Canceled);
}
tx
}
Expand Down
2 changes: 1 addition & 1 deletion ethcore/src/miner/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ mod stratum;
pub use self::external::{ExternalMiner, ExternalMinerService};

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};
pub use self::local_transactions::{Status as LocalTransactionStatus};
pub use client::TransactionImportResult;
Expand Down
32 changes: 24 additions & 8 deletions ethcore/src/miner/transaction_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
//!
//! use util::{Uint, U256, Address};
//! use ethkey::{Random, Generator};
//! use ethcore::miner::{TransactionQueue, TransactionQueueDetailsProvider, AccountDetails, TransactionOrigin};
//! use ethcore::miner::{TransactionQueue, RemovalReason, TransactionQueueDetailsProvider, AccountDetails, TransactionOrigin};
//! use ethcore::transaction::*;
//! use rustc_serialize::hex::FromHex;
//!
Expand Down Expand Up @@ -80,7 +80,7 @@
//!
//! // And when transaction is removed (but nonce haven't changed)
//! // 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().future, 1);
//! assert_eq!(txq.top_transactions().len(), 0);
Expand Down Expand Up @@ -510,6 +510,15 @@ pub enum PrioritizationStrategy {
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.
pub type QueuingInstant = BlockNumber;
const DEFAULT_QUEUING_PERIOD: BlockNumber = 128;
Expand Down Expand Up @@ -897,7 +906,7 @@ impl TransactionQueue {
.expect("We fetch details for all senders from both current and future")
.nonce;
for hash in invalid {
self.remove_invalid(&hash, &fetch_nonce);
self.remove(&hash, &fetch_nonce, RemovalReason::Invalid);
}
}

Expand Down Expand Up @@ -945,7 +954,7 @@ impl TransactionQueue {
/// so transactions left in queue are processed according to client nonce.
///
/// 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 {

assert_eq!(self.future.by_priority.len() + self.current.by_priority.len(), self.by_hash.len());
Expand All @@ -964,7 +973,14 @@ impl TransactionQueue {

// Mark in locals
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
Expand Down Expand Up @@ -2277,7 +2293,7 @@ pub mod test {
assert_eq!(txq.status().pending, 3);

// when
txq.remove_invalid(&tx.hash(), &|_| default_nonce());
txq.remove(&tx.hash(), &|_| default_nonce(), RemovalReason::Invalid);

// then
let stats = txq.status();
Expand Down Expand Up @@ -2420,7 +2436,7 @@ pub mod test {
assert_eq!(txq.status().pending, 2);

// 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().future, 1);
txq.add(tx1.clone(), TransactionOrigin::External, 0, None, &default_tx_provider()).unwrap();
Expand Down Expand Up @@ -2518,7 +2534,7 @@ pub mod test {
assert_eq!(txq.status().future, 2);

// when
txq.remove_invalid(&tx1.hash(), &|_| default_nonce() + 1.into());
txq.remove(&tx1.hash(), &|_| default_nonce() + 1.into(), RemovalReason::Invalid);

// then
let stats = txq.status();
Expand Down
9 changes: 8 additions & 1 deletion rpc/src/v1/types/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ pub enum LocalTransactionStatus {
Rejected(Transaction, String),
/// Transaction is invalid.
Invalid(Transaction),
/// Transaction was canceled.
Canceled(Transaction),
}

impl Serialize for LocalTransactionStatus {
Expand All @@ -101,7 +103,7 @@ impl Serialize for LocalTransactionStatus {

let elems = match *self {
Pending | Future => 1,
Mined(..) | Dropped(..) | Invalid(..) => 2,
Mined(..) | Dropped(..) | Invalid(..) | Canceled(..) => 2,
Rejected(..) => 3,
Replaced(..) => 4,
};
Expand All @@ -121,6 +123,10 @@ impl Serialize for LocalTransactionStatus {
struc.serialize_field(status, "dropped")?;
struc.serialize_field(transaction, tx)?;
},
Canceled(ref tx) => {
struc.serialize_field(status, "canceled")?;
struc.serialize_field(transaction, tx)?;
},
Invalid(ref tx) => {
struc.serialize_field(status, "invalid")?;
struc.serialize_field(transaction, tx)?;
Expand Down Expand Up @@ -249,6 +255,7 @@ impl From<miner::LocalTransactionStatus> for LocalTransactionStatus {
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()),
Invalid(tx) => LocalTransactionStatus::Invalid(tx.into()),
Canceled(tx) => LocalTransactionStatus::Canceled(tx.into()),
}
}
}
Expand Down

0 comments on commit d4684d6

Please sign in to comment.