Include RPC configurability for max tx gas limit.

Also Move the gas limit into the transaction queue from the miner.
This commit is contained in:
Gav Wood 2016-06-27 20:19:01 +02:00
parent a102015ecf
commit 10aa32b0f5
8 changed files with 82 additions and 16 deletions

View File

@ -53,6 +53,8 @@ pub struct MinerOptions {
/// Specify maximum amount of gas to bother considering for block insertion. /// Specify maximum amount of gas to bother considering for block insertion.
/// If `None`, then no limit. /// If `None`, then no limit.
pub max_tx_gas: Option<U256>, pub max_tx_gas: Option<U256>,
/// Maximum size of the transaction queue.
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.
pub pending_set: PendingSet, pub pending_set: PendingSet,
} }
@ -64,6 +66,7 @@ impl Default for MinerOptions {
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: None,
tx_queue_size: 1024,
pending_set: PendingSet::AlwaysQueue, pending_set: PendingSet::AlwaysQueue,
} }
} }
@ -107,7 +110,7 @@ impl Miner {
/// Creates new instance of miner /// Creates new instance of miner
pub fn new(options: MinerOptions, spec: Spec, accounts: Option<Arc<AccountProvider>>) -> Arc<Miner> { pub fn new(options: MinerOptions, spec: Spec, accounts: Option<Arc<AccountProvider>>) -> Arc<Miner> {
Arc::new(Miner { Arc::new(Miner {
transaction_queue: Mutex::new(TransactionQueue::new()), transaction_queue: Mutex::new(TransactionQueue::with_limits(options.tx_queue_size, options.max_tx_gas)),
sealing_enabled: AtomicBool::new(options.force_sealing), sealing_enabled: AtomicBool::new(options.force_sealing),
options: options, options: options,
sealing_block_last_request: Mutex::new(0), sealing_block_last_request: Mutex::new(0),
@ -396,6 +399,10 @@ 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>) {
self.transaction_queue.lock().unwrap().set_tx_gas_limit(limit)
}
/// Get the author that we will seal blocks as. /// Get the author that we will seal blocks as.
fn author(&self) -> Address { fn author(&self) -> Address {
*self.author.read().unwrap() *self.author.read().unwrap()

View File

@ -101,6 +101,9 @@ pub trait MinerService : Send + Sync {
/// Set maximal number of transactions kept in the queue (both current and future). /// Set maximal number of transactions kept in the queue (both current and future).
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.
fn set_tx_gas_limit(&self, limit: Option<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) ->
Vec<Result<TransactionImportResult, Error>> Vec<Result<TransactionImportResult, Error>>

View File

@ -334,6 +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.
tx_gas_limit: Option<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
@ -355,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_limit(1024) Self::with_limits(1024, None)
} }
/// Create new instance of this Queue with specified limits /// Create new instance of this Queue with specified limits
pub fn with_limit(limit: usize) -> Self { pub fn with_limits(limit: usize, tx_gas_limit: Option<U256>) -> Self {
let current = TransactionSet { let current = TransactionSet {
by_priority: BTreeSet::new(), by_priority: BTreeSet::new(),
by_address: Table::new(), by_address: Table::new(),
@ -374,6 +376,7 @@ impl TransactionQueue {
TransactionQueue { TransactionQueue {
minimal_gas_price: U256::zero(), minimal_gas_price: U256::zero(),
tx_gas_limit: tx_gas_limit,
gas_limit: !U256::zero(), gas_limit: !U256::zero(),
current: current, current: current,
future: future, future: future,
@ -418,6 +421,12 @@ impl TransactionQueue {
}; };
} }
/// Set the new limit for the amount of gas any individual transaction may have.
/// Any transaction already imported to the queue is not affected.
pub fn set_tx_gas_limit(&mut self, limit: Option<U256>) {
self.tx_gas_limit = limit;
}
/// Returns current status for this queue /// Returns current status for this queue
pub fn status(&self) -> TransactionQueueStatus { pub fn status(&self) -> TransactionQueueStatus {
TransactionQueueStatus { TransactionQueueStatus {
@ -435,7 +444,9 @@ impl TransactionQueue {
if tx.gas_price < self.minimal_gas_price { if tx.gas_price < self.minimal_gas_price {
trace!(target: "miner", trace!(target: "miner",
"Dropping transaction below minimal gas price threshold: {:?} (gp: {} < {})", "Dropping transaction below minimal gas price threshold: {:?} (gp: {} < {})",
tx.hash(), tx.gas_price, self.minimal_gas_price tx.hash(),
tx.gas_price,
self.minimal_gas_price
); );
return Err(Error::Transaction(TransactionError::InsufficientGasPrice { return Err(Error::Transaction(TransactionError::InsufficientGasPrice {
@ -446,10 +457,12 @@ impl TransactionQueue {
try!(tx.check_low_s()); try!(tx.check_low_s());
if tx.gas > self.gas_limit { if tx.gas > self.gas_limit || self.tx_gas_limit.map(|l| tx.gas > l).unwrap_or(false) {
trace!(target: "miner", trace!(target: "miner",
"Dropping transaction above gas limit: {:?} ({} > {})", "Dropping transaction above gas limit: {:?} ({} > {})",
tx.hash(), tx.gas, self.gas_limit tx.hash(),
tx.gas,
self.gas_limit
); );
return Err(Error::Transaction(TransactionError::GasLimitExceeded { return Err(Error::Transaction(TransactionError::GasLimitExceeded {
@ -463,8 +476,13 @@ impl TransactionQueue {
let cost = vtx.transaction.value + vtx.transaction.gas_price * vtx.transaction.gas; let cost = vtx.transaction.value + vtx.transaction.gas_price * vtx.transaction.gas;
if client_account.balance < cost { if client_account.balance < cost {
trace!(target: "miner", "Dropping transaction without sufficient balance: {:?} ({} < {})", trace!(target: "miner",
vtx.hash(), client_account.balance, cost); "Dropping transaction without sufficient balance: {:?} ({} < {})",
vtx.hash(),
client_account.balance,
cost
);
return Err(Error::Transaction(TransactionError::InsufficientBalance { return Err(Error::Transaction(TransactionError::InsufficientBalance {
cost: cost, cost: cost,
balance: client_account.balance balance: client_account.balance
@ -1304,7 +1322,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); let mut txq = TransactionQueue::with_limit(1, None);
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;
@ -1326,7 +1344,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); let mut txq = TransactionQueue::with_limit(2, None);
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();
@ -1347,7 +1365,7 @@ mod test {
#[test] #[test]
fn should_limit_future_transactions() { fn should_limit_future_transactions() {
let mut txq = TransactionQueue::with_limit(1); let mut txq = TransactionQueue::with_limit(1, None);
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));
@ -1607,7 +1625,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); let mut txq = TransactionQueue::with_limit(1, None);
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

@ -89,6 +89,7 @@ impl Configuration {
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(|d| Self::decode_u256(d, "--max-tx-gas")),
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,
"strict" => PendingSet::AlwaysSealing, "strict" => PendingSet::AlwaysSealing,

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::{Bytes}; use v1::types::{OptionalValue, 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
@ -86,6 +86,13 @@ impl<M> EthcoreSet for EthcoreSetClient<M> where M: MinerService + 'static {
}) })
} }
fn set_tx_gas_limit(&self, params: Params) -> Result<Value, Error> {
from_params::<(OptionalValue<U256>,)>(params).and_then(|(limit,)| {
take_weak!(self.miner).set_tx_gas_limit(limit.into());
to_value(&true)
})
}
fn add_reserved_peer(&self, params: Params) -> Result<Value, Error> { fn add_reserved_peer(&self, params: Params) -> Result<Value, Error> {
from_params::<(String,)>(params).and_then(|(peer,)| { from_params::<(String,)>(params).and_then(|(peer,)| {
match take_weak!(self.net).add_reserved_peer(&peer) { match take_weak!(self.net).add_reserved_peer(&peer) {

View File

@ -43,6 +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>>,
} }
impl Default for TestMinerService { impl Default for TestMinerService {
@ -58,6 +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),
} }
} }
} }
@ -99,6 +101,10 @@ impl MinerService for TestMinerService {
*self.limit.write().unwrap() = limit; *self.limit.write().unwrap() = limit;
} }
fn set_tx_gas_limit(&self, limit: Option<U256>) {
*self.tx_gas_limit.write().unwrap() = limit;
}
fn transactions_limit(&self) -> usize { fn transactions_limit(&self) -> usize {
*self.limit.read().unwrap() *self.limit.read().unwrap()
} }

View File

@ -40,6 +40,9 @@ pub trait EthcoreSet: Sized + Send + Sync + 'static {
/// Sets the limits for transaction queue. /// Sets the limits for transaction queue.
fn set_transactions_limit(&self, _: Params) -> Result<Value, Error>; fn set_transactions_limit(&self, _: Params) -> Result<Value, Error>;
/// Sets the maximum amount of gas a single transaction may consume.
fn set_tx_gas_limit(&self, _: Params) -> Result<Value, Error>;
/// Add a reserved peer. /// Add a reserved peer.
fn add_reserved_peer(&self, _: Params) -> Result<Value, Error>; fn add_reserved_peer(&self, _: Params) -> Result<Value, Error>;
@ -60,6 +63,7 @@ pub trait EthcoreSet: Sized + Send + Sync + 'static {
delegate.add_method("ethcore_setGasCeilTarget", EthcoreSet::set_gas_ceil_target); delegate.add_method("ethcore_setGasCeilTarget", EthcoreSet::set_gas_ceil_target);
delegate.add_method("ethcore_setExtraData", EthcoreSet::set_extra_data); delegate.add_method("ethcore_setExtraData", EthcoreSet::set_extra_data);
delegate.add_method("ethcore_setAuthor", EthcoreSet::set_author); delegate.add_method("ethcore_setAuthor", EthcoreSet::set_author);
delegate.add_method("ethcore_setMaxTransactionGas", EthcoreSet::set_tx_gas_limit);
delegate.add_method("ethcore_setTransactionsLimit", EthcoreSet::set_transactions_limit); delegate.add_method("ethcore_setTransactionsLimit", EthcoreSet::set_transactions_limit);
delegate.add_method("ethcore_addReservedPeer", EthcoreSet::add_reserved_peer); delegate.add_method("ethcore_addReservedPeer", EthcoreSet::add_reserved_peer);
delegate.add_method("ethcore_removeReservedPeer", EthcoreSet::remove_reserved_peer); delegate.add_method("ethcore_removeReservedPeer", EthcoreSet::remove_reserved_peer);

View File

@ -14,7 +14,7 @@
// You should have received a copy of the GNU General Public License // You should have received a copy of the GNU General Public License
// along with Parity. If not, see <http://www.gnu.org/licenses/>. // along with Parity. If not, see <http://www.gnu.org/licenses/>.
use serde::{Serialize, Serializer}; use serde::{Serialize, Serializer, Deserialize, Deserializer};
use serde_json::Value; use serde_json::Value;
/// Optional value /// Optional value
@ -26,13 +26,22 @@ pub enum OptionalValue<T> where T: Serialize {
Null Null
} }
impl<T> Default for OptionalValue<T> where T: Serialize { impl<T> Default for OptionalValue<T> where T: Serialize + Deserialize {
fn default() -> Self { fn default() -> Self {
OptionalValue::Null OptionalValue::Null
} }
} }
impl<T> Serialize for OptionalValue<T> where T: Serialize { impl<T> Into<Option<T>> for OptionalValue<T> where T: Serialize + Deserialize {
fn into(self) -> Option<T> {
match self {
OptionalValue::Null => None,
OptionalValue::Value(t) => Some(t),
}
}
}
impl<T> Serialize for OptionalValue<T> where T: Serialize + Deserialize {
fn serialize<S>(&self, serializer: &mut S) -> Result<(), S::Error> fn serialize<S>(&self, serializer: &mut S) -> Result<(), S::Error>
where S: Serializer { where S: Serializer {
match *self { match *self {
@ -42,6 +51,17 @@ impl<T> Serialize for OptionalValue<T> where T: Serialize {
} }
} }
impl<T> Deserialize for OptionalValue<T> where T: Serialize + Deserialize {
fn deserialize<D>(deserializer: &mut D) -> Result<OptionalValue<T>, D::Error>
where D: Deserializer {
let deser_result: Result<T, D::Error> = Deserialize::deserialize(deserializer);
match deser_result {
Ok(t) => Ok(OptionalValue::Value(t)),
Err(_) => Ok(OptionalValue::Null),
}
}
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use serde_json; use serde_json;