Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Prioritizing re-imported transactions #2372

Merged
merged 2 commits into from
Sep 28, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 5 additions & 2 deletions ethcore/src/client/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -853,9 +853,12 @@ impl BlockChainClient for Client {

fn transaction_receipt(&self, id: TransactionID) -> Option<LocalizedReceipt> {
let chain = self.chain.read();
self.transaction_address(id).and_then(|address| chain.block_number(&address.block_hash).and_then(|block_number| {
self.transaction_address(id)
.and_then(|address| chain.block_number(&address.block_hash).and_then(|block_number| {
let t = chain.block_body(&address.block_hash)
.and_then(|block| BodyView::new(&block).localized_transaction_at(&address.block_hash, block_number, address.index));
.and_then(|block| {
BodyView::new(&block).localized_transaction_at(&address.block_hash, block_number, address.index)
});

match (t, chain.transaction_receipt(&address)) {
(Some(tx), Some(receipt)) => {
Expand Down
2 changes: 1 addition & 1 deletion ethcore/src/miner/miner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -915,7 +915,7 @@ impl MinerService for Miner {
out_of_chain.for_each(|txs| {
let mut transaction_queue = self.transaction_queue.lock();
let _ = self.add_transactions_to_queue(
chain, txs, TransactionOrigin::External, &mut transaction_queue
chain, txs, TransactionOrigin::RetractedBlock, &mut transaction_queue
);
});
}
Expand Down
43 changes: 39 additions & 4 deletions ethcore/src/miner/transaction_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ pub enum TransactionOrigin {
Local,
/// External transaction received from network
External,
/// Transactions from retracted blocks
RetractedBlock,
}

impl PartialOrd for TransactionOrigin {
Expand All @@ -112,10 +114,11 @@ impl Ord for TransactionOrigin {
return Ordering::Equal;
}

if *self == TransactionOrigin::Local {
Ordering::Less
} else {
Ordering::Greater
match (*self, *other) {
(TransactionOrigin::RetractedBlock, _) => Ordering::Less,
(_, TransactionOrigin::RetractedBlock) => Ordering::Greater,
(TransactionOrigin::Local, _) => Ordering::Less,
_ => Ordering::Greater,
}
}
}
Expand Down Expand Up @@ -1014,6 +1017,17 @@ mod test {
new_tx_pair_default(0.into(), 1.into())
}

#[test]
fn test_ordering() {
assert_eq!(TransactionOrigin::Local.cmp(&TransactionOrigin::External), Ordering::Less);
assert_eq!(TransactionOrigin::RetractedBlock.cmp(&TransactionOrigin::Local), Ordering::Less);
assert_eq!(TransactionOrigin::RetractedBlock.cmp(&TransactionOrigin::External), Ordering::Less);

assert_eq!(TransactionOrigin::External.cmp(&TransactionOrigin::Local), Ordering::Greater);
assert_eq!(TransactionOrigin::Local.cmp(&TransactionOrigin::RetractedBlock), Ordering::Greater);
assert_eq!(TransactionOrigin::External.cmp(&TransactionOrigin::RetractedBlock), Ordering::Greater);
}

#[test]
fn should_return_correct_nonces_when_dropped_because_of_limit() {
// given
Expand Down Expand Up @@ -1375,6 +1389,27 @@ mod test {
assert_eq!(top.len(), 2);
}

#[test]
fn should_prioritize_reimported_transactions_within_same_nonce_height() {
// given
let mut txq = TransactionQueue::new();
let tx = new_tx_default();
// the second one has same nonce but higher `gas_price`
let (_, tx2) = new_similar_tx_pair();

// when
// first insert local one with higher gas price
txq.add(tx2.clone(), &default_account_details, TransactionOrigin::Local).unwrap();
// then the one with lower gas price, but from retracted block
txq.add(tx.clone(), &default_account_details, TransactionOrigin::RetractedBlock).unwrap();

// then
let top = txq.top_transactions();
assert_eq!(top[0], tx); // retracted should be first
assert_eq!(top[1], tx2);
assert_eq!(top.len(), 2);
}

#[test]
fn should_not_prioritize_local_transactions_with_different_nonce_height() {
// given
Expand Down
4 changes: 3 additions & 1 deletion util/bigint/src/hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,11 @@ pub fn clean_0x(s: &str) -> &str {

macro_rules! impl_hash {
($from: ident, $size: expr) => {
#[derive(Eq)]
#[repr(C)]
/// Unformatted binary data of fixed length.
pub struct $from (pub [u8; $size]);


impl From<[u8; $size]> for $from {
fn from(bytes: [u8; $size]) -> Self {
$from(bytes)
Expand Down Expand Up @@ -210,6 +210,8 @@ macro_rules! impl_hash {
}
}

impl Eq for $from {}

impl PartialEq for $from {
fn eq(&self, other: &Self) -> bool {
for i in 0..$size {
Expand Down