U256 instead of Option<U256>. Fix up tests.

This commit is contained in:
Gav Wood 2016-06-28 10:21:29 +02:00
parent af935df553
commit 31de739122
7 changed files with 26 additions and 42 deletions

View File

@ -50,9 +50,8 @@ pub struct MinerOptions {
pub reseal_on_external_tx: bool, pub reseal_on_external_tx: bool,
/// Reseal on receipt of new local transactions. /// Reseal on receipt of new local transactions.
pub reseal_on_own_tx: bool, pub reseal_on_own_tx: bool,
/// Specify maximum amount of gas to bother considering for block insertion. /// Maximum amount of gas to bother considering for block insertion.
/// If `None`, then no limit. pub max_tx_gas: U256,
pub max_tx_gas: Option<U256>,
/// Maximum size of the transaction queue. /// Maximum size of the transaction queue.
pub tx_queue_size: usize, pub tx_queue_size: usize,
/// Whether we should fallback to providing all the queue's transactions or just pending. /// Whether we should fallback to providing all the queue's transactions or just pending.
@ -65,7 +64,7 @@ impl Default for MinerOptions {
force_sealing: false, force_sealing: false,
reseal_on_external_tx: true, reseal_on_external_tx: true,
reseal_on_own_tx: true, reseal_on_own_tx: true,
max_tx_gas: None, max_tx_gas: !U256::zero(),
tx_queue_size: 1024, tx_queue_size: 1024,
pending_set: PendingSet::AlwaysQueue, pending_set: PendingSet::AlwaysQueue,
} }
@ -134,7 +133,7 @@ impl Miner {
trace!(target: "miner", "prepare_sealing: entering"); trace!(target: "miner", "prepare_sealing: entering");
let (transactions, mut open_block) = { let (transactions, mut open_block) = {
let transactions = {self.transaction_queue.lock().unwrap().top_transactions_maybe_limit(&self.options.max_tx_gas)}; let transactions = {self.transaction_queue.lock().unwrap().top_transactions()};
let mut sealing_work = self.sealing_work.lock().unwrap(); let mut sealing_work = self.sealing_work.lock().unwrap();
let best_hash = chain.best_block_header().sha3(); let best_hash = chain.best_block_header().sha3();
/* /*
@ -399,7 +398,7 @@ impl MinerService for Miner {
self.transaction_queue.lock().unwrap().set_limit(limit) self.transaction_queue.lock().unwrap().set_limit(limit)
} }
fn set_tx_gas_limit(&self, limit: Option<U256>) { fn set_tx_gas_limit(&self, limit: U256) {
self.transaction_queue.lock().unwrap().set_tx_gas_limit(limit) self.transaction_queue.lock().unwrap().set_tx_gas_limit(limit)
} }

View File

@ -102,7 +102,7 @@ pub trait MinerService : Send + Sync {
fn set_transactions_limit(&self, limit: usize); fn set_transactions_limit(&self, limit: usize);
/// Set maximum amount of gas allowed for any single transaction to mine. /// Set maximum amount of gas allowed for any single transaction to mine.
fn set_tx_gas_limit(&self, limit: Option<U256>); fn set_tx_gas_limit(&self, limit: U256);
/// Imports transactions to transaction queue. /// Imports transactions to transaction queue.
fn import_transactions<T>(&self, chain: &MiningBlockChainClient, transactions: Vec<SignedTransaction>, fetch_account: T) -> fn import_transactions<T>(&self, chain: &MiningBlockChainClient, transactions: Vec<SignedTransaction>, fetch_account: T) ->

View File

@ -334,8 +334,8 @@ const GAS_LIMIT_HYSTERESIS: usize = 10; // %
pub struct TransactionQueue { pub struct TransactionQueue {
/// Gas Price threshold for transactions that can be imported to this queue (defaults to 0) /// Gas Price threshold for transactions that can be imported to this queue (defaults to 0)
minimal_gas_price: U256, minimal_gas_price: U256,
/// If `Some`, then the maximum amount of gas any individual transaction may use. /// The maximum amount of gas any individual transaction may use.
tx_gas_limit: Option<U256>, tx_gas_limit: U256,
/// Current gas limit (block gas limit * factor). Transactions above the limit will not be accepted (default to !0) /// Current gas limit (block gas limit * factor). Transactions above the limit will not be accepted (default to !0)
gas_limit: U256, gas_limit: U256,
/// Priority queue for transactions that can go to block /// Priority queue for transactions that can go to block
@ -357,11 +357,11 @@ impl Default for TransactionQueue {
impl TransactionQueue { impl TransactionQueue {
/// Creates new instance of this Queue /// Creates new instance of this Queue
pub fn new() -> Self { pub fn new() -> Self {
Self::with_limits(1024, None) Self::with_limits(1024, !U256::zero())
} }
/// Create new instance of this Queue with specified limits /// Create new instance of this Queue with specified limits
pub fn with_limits(limit: usize, tx_gas_limit: Option<U256>) -> Self { pub fn with_limits(limit: usize, tx_gas_limit: U256) -> Self {
let current = TransactionSet { let current = TransactionSet {
by_priority: BTreeSet::new(), by_priority: BTreeSet::new(),
by_address: Table::new(), by_address: Table::new(),
@ -423,7 +423,7 @@ impl TransactionQueue {
/// Set the new limit for the amount of gas any individual transaction may have. /// Set the new limit for the amount of gas any individual transaction may have.
/// Any transaction already imported to the queue is not affected. /// Any transaction already imported to the queue is not affected.
pub fn set_tx_gas_limit(&mut self, limit: Option<U256>) { pub fn set_tx_gas_limit(&mut self, limit: U256) {
self.tx_gas_limit = limit; self.tx_gas_limit = limit;
} }
@ -457,9 +457,9 @@ impl TransactionQueue {
try!(tx.check_low_s()); try!(tx.check_low_s());
if tx.gas > self.gas_limit || self.tx_gas_limit.map(|l| tx.gas > l).unwrap_or(false) { if tx.gas > self.gas_limit || tx.gas > self.tx_gas_limit {
trace!(target: "miner", trace!(target: "miner",
"Dropping transaction above gas limit: {:?} ({} > min({}, {:?}))", "Dropping transaction above gas limit: {:?} ({} > min({}, {}))",
tx.hash(), tx.hash(),
tx.gas, tx.gas,
self.gas_limit, self.gas_limit,
@ -602,22 +602,6 @@ impl TransactionQueue {
.collect() .collect()
} }
/// Returns top transactions from the queue ordered by priority with a maximum gas limit.
pub fn top_transactions_with_limit(&self, max_tx_gas: &U256) -> Vec<SignedTransaction> {
self.current.by_priority
.iter()
.map(|t| self.by_hash.get(&t.hash).expect("All transactions in `current` and `future` are always included in `by_hash`"))
.map(|t| &t.transaction).filter(|t| t.gas <= *max_tx_gas).cloned()
.collect()
}
/// Returns top transactions from the queue ordered by priority with a maximum gas limit.
pub fn top_transactions_maybe_limit(&self, max_tx_gas: &Option<U256>) -> Vec<SignedTransaction> {
match *max_tx_gas {
None => self.top_transactions(),
Some(ref m) => self.top_transactions_with_limit(m),
}
}
/// Returns hashes of all transactions from current, ordered by priority. /// Returns hashes of all transactions from current, ordered by priority.
pub fn pending_hashes(&self) -> Vec<H256> { pub fn pending_hashes(&self) -> Vec<H256> {
self.current.by_priority self.current.by_priority
@ -1323,7 +1307,7 @@ mod test {
#[test] #[test]
fn should_drop_old_transactions_when_hitting_the_limit() { fn should_drop_old_transactions_when_hitting_the_limit() {
// given // given
let mut txq = TransactionQueue::with_limit(1, None); let mut txq = TransactionQueue::with_limits(1, !U256::zero());
let (tx, tx2) = new_txs(U256::one()); let (tx, tx2) = new_txs(U256::one());
let sender = tx.sender().unwrap(); let sender = tx.sender().unwrap();
let nonce = tx.nonce; let nonce = tx.nonce;
@ -1345,7 +1329,7 @@ mod test {
#[test] #[test]
fn should_return_correct_nonces_when_dropped_because_of_limit() { fn should_return_correct_nonces_when_dropped_because_of_limit() {
// given // given
let mut txq = TransactionQueue::with_limit(2, None); let mut txq = TransactionQueue::with_limits(2, !U256::zero());
let tx = new_tx(); let tx = new_tx();
let (tx1, tx2) = new_txs(U256::one()); let (tx1, tx2) = new_txs(U256::one());
let sender = tx1.sender().unwrap(); let sender = tx1.sender().unwrap();
@ -1366,7 +1350,7 @@ mod test {
#[test] #[test]
fn should_limit_future_transactions() { fn should_limit_future_transactions() {
let mut txq = TransactionQueue::with_limit(1, None); let mut txq = TransactionQueue::with_limits(1, !U256::zero());
txq.current.set_limit(10); txq.current.set_limit(10);
let (tx1, tx2) = new_txs_with_gas_price_diff(U256::from(4), U256::from(1)); let (tx1, tx2) = new_txs_with_gas_price_diff(U256::from(4), U256::from(1));
let (tx3, tx4) = new_txs_with_gas_price_diff(U256::from(4), U256::from(2)); let (tx3, tx4) = new_txs_with_gas_price_diff(U256::from(4), U256::from(2));
@ -1626,7 +1610,7 @@ mod test {
#[test] #[test]
fn should_keep_right_order_in_future() { fn should_keep_right_order_in_future() {
// given // given
let mut txq = TransactionQueue::with_limit(1, None); let mut txq = TransactionQueue::with_limits(1, !U256::zero());
let (tx1, tx2) = new_txs(U256::from(1)); let (tx1, tx2) = new_txs(U256::from(1));
let prev_nonce = |a: &Address| AccountDetails { nonce: default_nonce(a).nonce - U256::one(), balance: let prev_nonce = |a: &Address| AccountDetails { nonce: default_nonce(a).nonce - U256::one(), balance:
default_nonce(a).balance }; default_nonce(a).balance };

View File

@ -88,7 +88,7 @@ impl Configuration {
force_sealing: self.args.flag_force_sealing, force_sealing: self.args.flag_force_sealing,
reseal_on_external_tx: ext, reseal_on_external_tx: ext,
reseal_on_own_tx: own, reseal_on_own_tx: own,
max_tx_gas: self.args.flag_max_tx_gas.as_ref().map(|d| Self::decode_u256(d, "--max-tx-gas")), max_tx_gas: self.args.flag_max_tx_gas.as_ref().map_or(!U256::zero(), |d| Self::decode_u256(d, "--max-tx-gas")),
tx_queue_size: self.args.flag_tx_queue_size, tx_queue_size: self.args.flag_tx_queue_size,
pending_set: match self.args.flag_relay_set.as_str() { pending_set: match self.args.flag_relay_set.as_str() {
"cheap" => PendingSet::AlwaysQueue, "cheap" => PendingSet::AlwaysQueue,

View File

@ -22,7 +22,7 @@ use jsonrpc_core::*;
use ethcore::miner::MinerService; use ethcore::miner::MinerService;
use ethcore::service::SyncMessage; use ethcore::service::SyncMessage;
use v1::traits::EthcoreSet; use v1::traits::EthcoreSet;
use v1::types::{OptionalValue, Bytes}; use v1::types::Bytes;
/// Ethcore-specific rpc interface for operations altering the settings. /// Ethcore-specific rpc interface for operations altering the settings.
pub struct EthcoreSetClient<M> where pub struct EthcoreSetClient<M> where
@ -87,7 +87,7 @@ impl<M> EthcoreSet for EthcoreSetClient<M> where M: MinerService + 'static {
} }
fn set_tx_gas_limit(&self, params: Params) -> Result<Value, Error> { fn set_tx_gas_limit(&self, params: Params) -> Result<Value, Error> {
from_params::<(OptionalValue<U256>,)>(params).and_then(|(limit,)| { from_params::<(U256,)>(params).and_then(|(limit,)| {
take_weak!(self.miner).set_tx_gas_limit(limit.into()); take_weak!(self.miner).set_tx_gas_limit(limit.into());
to_value(&true) to_value(&true)
}) })

View File

@ -29,7 +29,7 @@ use ethcore::account_provider::AccountProvider;
use devtools::RandomTempPath; use devtools::RandomTempPath;
use util::Hashable; use util::Hashable;
use util::io::IoChannel; use util::io::IoChannel;
use util::{U256, H256}; use util::{U256, H256, Uint};
use jsonrpc_core::IoHandler; use jsonrpc_core::IoHandler;
use ethjson::blockchain::BlockChain; use ethjson::blockchain::BlockChain;
@ -54,7 +54,8 @@ fn miner_service(spec: Spec, accounts: Arc<AccountProvider>) -> Arc<Miner> {
force_sealing: true, force_sealing: true,
reseal_on_external_tx: true, reseal_on_external_tx: true,
reseal_on_own_tx: true, reseal_on_own_tx: true,
max_tx_gas: None, tx_queue_size: 1024,
max_tx_gas: !U256::zero(),
pending_set: PendingSet::SealingOrElseQueue, pending_set: PendingSet::SealingOrElseQueue,
}, },
spec, spec,

View File

@ -43,7 +43,7 @@ pub struct TestMinerService {
author: RwLock<Address>, author: RwLock<Address>,
extra_data: RwLock<Bytes>, extra_data: RwLock<Bytes>,
limit: RwLock<usize>, limit: RwLock<usize>,
tx_gas_limit: RwLock<Option<U256>>, tx_gas_limit: RwLock<U256>,
} }
impl Default for TestMinerService { impl Default for TestMinerService {
@ -59,7 +59,7 @@ impl Default for TestMinerService {
author: RwLock::new(Address::zero()), author: RwLock::new(Address::zero()),
extra_data: RwLock::new(vec![1, 2, 3, 4]), extra_data: RwLock::new(vec![1, 2, 3, 4]),
limit: RwLock::new(1024), limit: RwLock::new(1024),
tx_gas_limit: RwLock::new(None), tx_gas_limit: RwLock::new(!U256::zero()),
} }
} }
} }
@ -101,7 +101,7 @@ impl MinerService for TestMinerService {
*self.limit.write().unwrap() = limit; *self.limit.write().unwrap() = limit;
} }
fn set_tx_gas_limit(&self, limit: Option<U256>) { fn set_tx_gas_limit(&self, limit: U256) {
*self.tx_gas_limit.write().unwrap() = limit; *self.tx_gas_limit.write().unwrap() = limit;
} }