grassrootseconomics/openethereum
5
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fixing future order and drops because of limit errors

Browse Source
This commit is contained in:
Tomasz Drwięga 2016-06-20 12:13:10 +02:00
parent 18c35a031b
commit c348508b40
1 changed files with 30 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
ethcore/src/miner/transaction_queue.rs
View File

@ -244,13 +244,12 @@ impl TransactionSet {
/// Remove low priority transactions if there is more then specified by given `limit`. /// Remove low priority transactions if there is more then specified by given `limit`.
/// ///
/// It drops transactions from this set but also removes associated `VerifiedTransaction`. /// It drops transactions from this set but also removes associated `VerifiedTransaction`.
/// Returns addresses and highes nonces of transactions removed because of limit. /// Returns addresses and lowest nonces of transactions removed because of limit.
fn enforce_limit(&mut self, by_hash: &mut HashMap<H256, VerifiedTransaction>) -> Option<HashMap<Address, U256>> { fn enforce_limit(&mut self, by_hash: &mut HashMap<H256, VerifiedTransaction>) -> Option<HashMap<Address, U256>> {
let len = self.by_priority.len(); let len = self.by_priority.len();
if len <= self.limit { if len <= self.limit {
return None; return None;
} }
let to_drop : Vec<(Address, U256)> = { let to_drop : Vec<(Address, U256)> = {
self.by_priority self.by_priority
.iter() .iter()
@ -269,8 +268,8 @@ impl TransactionSet {
by_hash.remove(&order.hash) by_hash.remove(&order.hash)
.expect("Hash found in `by_priorty` matches the one dropped; so it is included in `by_hash`"); .expect("Hash found in `by_priorty` matches the one dropped; so it is included in `by_hash`");
let max = removed.get(&sender).map_or(nonce, |val| cmp::max(*val, nonce)); let min = removed.get(&sender).map_or(nonce, |val| cmp::min(*val, nonce));
removed.insert(sender, max); removed.insert(sender, min);
removed removed
})) }))
} }
@ -666,8 +665,12 @@ impl TransactionQueue {
// Check height // Check height
if nonce > next_nonce { if nonce > next_nonce {
// We have a gap - put to future // We have a gap - put to future.
try!(check_too_cheap(Self::replace_transaction(tx, next_nonce, &mut self.future, &mut self.by_hash))); // Update nonces of transactions in future (remove old transactions)
self.update_future(&address, state_nonce);
// Insert transaction (or replace old one with lower gas price)
try!(check_too_cheap(Self::replace_transaction(tx, state_nonce, &mut self.future, &mut self.by_hash)));
// Return an error if this transaction is not imported because of limit.
try!(check_if_removed(&address, &nonce, self.future.enforce_limit(&mut self.by_hash))); try!(check_if_removed(&address, &nonce, self.future.enforce_limit(&mut self.by_hash)));
return Ok(TransactionImportResult::Future); return Ok(TransactionImportResult::Future);
} else if nonce < state_nonce { } else if nonce < state_nonce {
@ -703,9 +706,9 @@ impl TransactionQueue {
} }
/// Updates /// Updates
fn update_last_nonces(&mut self, removed_max_nonces: &Option<HashMap<Address, U256>>) { fn update_last_nonces(&mut self, removed_min_nonces: &Option<HashMap<Address, U256>>) {
if let Some(ref max_nonces) = *removed_max_nonces { if let Some(ref min_nonces) = *removed_min_nonces {
for (sender, nonce) in max_nonces.iter() { for (sender, nonce) in min_nonces.iter() {
if *nonce == U256::zero() { if *nonce == U256::zero() {
self.last_nonces.remove(sender); self.last_nonces.remove(sender);
} else { } else {
@ -762,7 +765,7 @@ fn check_too_cheap(is_in: bool) -> Result<(), TransactionError> {
fn check_if_removed(sender: &Address, nonce: &U256, dropped: Option<HashMap<Address, U256>>) -> Result<(), TransactionError> { fn check_if_removed(sender: &Address, nonce: &U256, dropped: Option<HashMap<Address, U256>>) -> Result<(), TransactionError> {
match dropped { match dropped {
Some(ref dropped) => match dropped.get(sender) { Some(ref dropped) => match dropped.get(sender) {
Some(max) if nonce <= max => { Some(min) if nonce >= min => {
Err(TransactionError::LimitReached) Err(TransactionError::LimitReached)
}, },
_ => Ok(()), _ => Ok(()),
@ -1555,4 +1558,21 @@ mod test {
assert_eq!(txq.has_local_pending_transactions(), true); assert_eq!(txq.has_local_pending_transactions(), true);
} }
#[test]
fn should_keep_right_order_in_future() {
// given
let mut txq = TransactionQueue::with_limit(1);
let (tx1, tx2) = new_txs(U256::from(1));
let prev_nonce = |a: &Address| AccountDetails { nonce: default_nonce(a).nonce - U256::one(), balance:
default_nonce(a).balance };
// when
assert_eq!(txq.add(tx2, &prev_nonce, TransactionOrigin::External).unwrap(), TransactionImportResult::Future);
assert_eq!(txq.add(tx1.clone(), &prev_nonce, TransactionOrigin::External).unwrap(), TransactionImportResult::Future);
// then
assert_eq!(txq.future.by_priority.len(), 1);
assert_eq!(txq.future.by_priority.iter().next().unwrap().hash, tx1.hash());
}
} }