From 69d7c4f519e8f67025534815f85d035985f8e7a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Tue, 9 Jan 2018 12:43:36 +0100 Subject: [PATCH] Expose default gas price percentile configuration in CLI (#7497) * Expose gas price percentile. * Fix light eth_call. * fix gas_price in light client --- parity/cli/mod.rs | 8 ++++++++ parity/configuration.rs | 2 ++ parity/rpc_apis.rs | 10 +++++++++- parity/run.rs | 3 +++ rpc/src/v1/helpers/dispatch.rs | 25 ++++++++++++++++++------- rpc/src/v1/helpers/light_fetch.rs | 7 +++++-- rpc/src/v1/impls/eth.rs | 5 ++++- rpc/src/v1/impls/eth_pubsub.rs | 4 +++- rpc/src/v1/impls/light/eth.rs | 19 ++++++++++++------- rpc/src/v1/impls/light/parity.rs | 6 +++++- rpc/src/v1/tests/eth.rs | 2 +- rpc/src/v1/tests/mocked/eth.rs | 3 ++- rpc/src/v1/tests/mocked/personal.rs | 4 ++-- rpc/src/v1/tests/mocked/signer.rs | 2 +- rpc/src/v1/tests/mocked/signing.rs | 2 +- util/stats/src/lib.rs | 26 +++++++++++++++++++++++++- 16 files changed, 101 insertions(+), 27 deletions(-) diff --git a/parity/cli/mod.rs b/parity/cli/mod.rs index a38d36d242d..855f28444a7 100644 --- a/parity/cli/mod.rs +++ b/parity/cli/mod.rs @@ -346,6 +346,7 @@ usage! { ARG arg_password: (Vec) = Vec::new(), or |c: &Config| otry!(c.account).password.clone(), "--password=[FILE]...", "Provide a file containing a password for unlocking an account. Leading and trailing whitespace is trimmed.", + ["UI options"] FLAG flag_force_ui: (bool) = false, or |c: &Config| otry!(c.ui).force.clone(), "--force-ui", @@ -696,6 +697,10 @@ usage! { "--min-gas-price=[STRING]", "Minimum amount of Wei per GAS to be paid for a transaction to be accepted for mining. Overrides --usd-per-tx.", + ARG arg_gas_price_percentile: (usize) = 50usize, or |c: &Config| otry!(c.mining).gas_price_percentile, + "--gas-price-percentile=[PCT]", + "Set PCT percentile gas price value from last 100 blocks as default gas price when sending transactions.", + ARG arg_author: (Option) = None, or |c: &Config| otry!(c.mining).author.clone(), "--author=[ADDRESS]", "Specify the block author (aka \"coinbase\") address for sending block rewards from sealed blocks. NOTE: MINING WILL NOT WORK WITHOUT THIS OPTION.", // Sealing/Mining Option @@ -1142,6 +1147,7 @@ struct Mining { tx_time_limit: Option, relay_set: Option, min_gas_price: Option, + gas_price_percentile: Option, usd_per_tx: Option, usd_per_eth: Option, price_update_period: Option, @@ -1546,6 +1552,7 @@ mod tests { arg_tx_time_limit: Some(100u64), arg_relay_set: "cheap".into(), arg_min_gas_price: Some(0u64), + arg_gas_price_percentile: 50usize, arg_usd_per_tx: "0.0025".into(), arg_usd_per_eth: "auto".into(), arg_price_update_period: "hourly".into(), @@ -1794,6 +1801,7 @@ mod tests { work_queue_size: None, relay_set: None, min_gas_price: None, + gas_price_percentile: None, usd_per_tx: None, usd_per_eth: None, price_update_period: Some("hourly".into()), diff --git a/parity/configuration.rs b/parity/configuration.rs index 1ab1ad7c04d..641f5cffa80 100644 --- a/parity/configuration.rs +++ b/parity/configuration.rs @@ -339,6 +339,7 @@ impl Configuration { daemon: daemon, logger_config: logger_config.clone(), miner_options: self.miner_options()?, + gas_price_percentile: self.args.arg_gas_price_percentile, ntp_servers: self.ntp_servers(), ws_conf: ws_conf, http_conf: http_conf, @@ -1357,6 +1358,7 @@ mod tests { daemon: None, logger_config: Default::default(), miner_options: Default::default(), + gas_price_percentile: 50, ntp_servers: vec![ "0.parity.pool.ntp.org:123".into(), "1.parity.pool.ntp.org:123".into(), diff --git a/parity/rpc_apis.rs b/parity/rpc_apis.rs index f2d97f892f0..e39b42352de 100644 --- a/parity/rpc_apis.rs +++ b/parity/rpc_apis.rs @@ -226,6 +226,7 @@ pub struct FullDependencies { pub fetch: FetchClient, pub remote: parity_reactor::Remote, pub whisper_rpc: Option<::whisper::RpcFactory>, + pub gas_price_percentile: usize, } impl FullDependencies { @@ -241,7 +242,7 @@ impl FullDependencies { ($namespace:ident, $handler:expr, $deps:expr, $nonces:expr) => { { let deps = &$deps; - let dispatcher = FullDispatcher::new(deps.client.clone(), deps.miner.clone(), $nonces); + let dispatcher = FullDispatcher::new(deps.client.clone(), deps.miner.clone(), $nonces, deps.gas_price_percentile); if deps.signer_service.is_enabled() { $handler.extend_with($namespace::to_delegate(SigningQueueClient::new(&deps.signer_service, dispatcher, deps.remote.clone(), &deps.secret_store))) } else { @@ -256,6 +257,7 @@ impl FullDependencies { self.client.clone(), self.miner.clone(), nonces.clone(), + self.gas_price_percentile, ); for api in apis { match *api { @@ -277,6 +279,7 @@ impl FullDependencies { pending_nonce_from_queue: self.geth_compatibility, allow_pending_receipt_query: !self.geth_compatibility, send_block_number_in_get_work: !self.geth_compatibility, + gas_price_percentile: self.gas_price_percentile, } ); handler.extend_with(client.to_delegate()); @@ -422,6 +425,7 @@ pub struct LightDependencies { pub geth_compatibility: bool, pub remote: parity_reactor::Remote, pub whisper_rpc: Option<::whisper::RpcFactory>, + pub gas_price_percentile: usize, } impl LightDependencies { @@ -440,6 +444,7 @@ impl LightDependencies { self.cache.clone(), self.transaction_queue.clone(), Arc::new(Mutex::new(dispatch::Reservations::with_pool(self.fetch.pool()))), + self.gas_price_percentile, ); macro_rules! add_signing_methods { @@ -477,6 +482,7 @@ impl LightDependencies { self.transaction_queue.clone(), self.secret_store.clone(), self.cache.clone(), + self.gas_price_percentile, ); handler.extend_with(Eth::to_delegate(client.clone())); @@ -492,6 +498,7 @@ impl LightDependencies { self.sync.clone(), self.cache.clone(), self.remote.clone(), + self.gas_price_percentile, ); self.client.add_listener( Arc::downgrade(&client.handler()) as Weak<::light::client::LightChainNotify> @@ -521,6 +528,7 @@ impl LightDependencies { signer, self.dapps_address.clone(), self.ws_address.clone(), + self.gas_price_percentile, ).to_delegate()); if !for_generic_pubsub { diff --git a/parity/run.rs b/parity/run.rs index 369c63b5c7f..2d620c1e269 100644 --- a/parity/run.rs +++ b/parity/run.rs @@ -87,6 +87,7 @@ pub struct RunCmd { pub daemon: Option, pub logger_config: LogConfig, pub miner_options: MinerOptions, + pub gas_price_percentile: usize, pub ntp_servers: Vec, pub ws_conf: rpc::WsConfiguration, pub http_conf: rpc::HttpConfiguration, @@ -358,6 +359,7 @@ fn execute_light(cmd: RunCmd, can_restart: bool, logger: Arc) -> geth_compatibility: cmd.geth_compatibility, remote: event_loop.remote(), whisper_rpc: whisper_factory, + gas_price_percentile: cmd.gas_price_percentile, }); let dependencies = rpc::Dependencies { @@ -761,6 +763,7 @@ pub fn execute(cmd: RunCmd, can_restart: bool, logger: Arc) -> R fetch: fetch.clone(), remote: event_loop.remote(), whisper_rpc: whisper_factory, + gas_price_percentile: cmd.gas_price_percentile, }); let dependencies = rpc::Dependencies { diff --git a/rpc/src/v1/helpers/dispatch.rs b/rpc/src/v1/helpers/dispatch.rs index ec8e10a9a15..bd5137c2e3b 100644 --- a/rpc/src/v1/helpers/dispatch.rs +++ b/rpc/src/v1/helpers/dispatch.rs @@ -88,6 +88,7 @@ pub struct FullDispatcher { client: Arc, miner: Arc, nonces: Arc>, + gas_price_percentile: usize, } impl FullDispatcher { @@ -96,11 +97,13 @@ impl FullDispatcher { client: Arc, miner: Arc, nonces: Arc>, + gas_price_percentile: usize, ) -> Self { FullDispatcher { client, miner, nonces, + gas_price_percentile, } } } @@ -111,6 +114,7 @@ impl Clone for FullDispatcher { client: self.client.clone(), miner: self.miner.clone(), nonces: self.nonces.clone(), + gas_price_percentile: self.gas_price_percentile, } } } @@ -148,7 +152,9 @@ impl Dispatcher for FullDispatcher = prices.into(); cache.lock().set_gas_price_corpus(corpus.clone()); corpus @@ -258,6 +264,8 @@ pub struct LightDispatcher { pub transaction_queue: Arc>, /// Nonce reservations pub nonces: Arc>, + /// Gas Price percentile value used as default gas price. + pub gas_price_percentile: usize, } impl LightDispatcher { @@ -271,6 +279,7 @@ impl LightDispatcher { cache: Arc>, transaction_queue: Arc>, nonces: Arc>, + gas_price_percentile: usize, ) -> Self { LightDispatcher { sync, @@ -279,6 +288,7 @@ impl LightDispatcher { cache, transaction_queue, nonces, + gas_price_percentile, } } @@ -345,6 +355,7 @@ impl Dispatcher for LightDispatcher { }; // fast path for known gas price. + let gas_price_percentile = self.gas_price_percentile; let gas_price = match request_gas_price { Some(gas_price) => Either::A(future::ok(with_gas_price(gas_price))), None => Either::B(fetch_gas_price_corpus( @@ -352,8 +363,8 @@ impl Dispatcher for LightDispatcher { self.client.clone(), self.on_demand.clone(), self.cache.clone() - ).and_then(|corp| match corp.median() { - Some(median) => Ok(*median), + ).and_then(move |corp| match corp.percentile(gas_price_percentile) { + Some(percentile) => Ok(*percentile), None => Ok(DEFAULT_GAS_PRICE), // fall back to default on error. }).map(with_gas_price)) }; @@ -738,11 +749,11 @@ fn decrypt(accounts: &AccountProvider, address: Address, msg: Bytes, password: S } /// Extract the default gas price from a client and miner. -pub fn default_gas_price(client: &C, miner: &M) -> U256 where +pub fn default_gas_price(client: &C, miner: &M, percentile: usize) -> U256 where C: MiningBlockChainClient, M: MinerService, { - client.gas_price_corpus(100).median().cloned().unwrap_or_else(|| miner.sensible_gas_price()) + client.gas_price_corpus(100).percentile(percentile).cloned().unwrap_or_else(|| miner.sensible_gas_price()) } /// Convert RPC confirmation payload to signer confirmation payload. diff --git a/rpc/src/v1/helpers/light_fetch.rs b/rpc/src/v1/helpers/light_fetch.rs index 64f075b016d..d7436d8cdac 100644 --- a/rpc/src/v1/helpers/light_fetch.rs +++ b/rpc/src/v1/helpers/light_fetch.rs @@ -60,6 +60,8 @@ pub struct LightFetch { pub sync: Arc, /// The light data cache. pub cache: Arc>, + /// Gas Price percentile + pub gas_price_percentile: usize, } /// Extract a transaction at given index. @@ -203,6 +205,7 @@ impl LightFetch { None => Either::B(self.account(from, id).map(|acc| acc.map(|a| a.nonce))), }; + let gas_price_percentile = self.gas_price_percentile; let gas_price_fut = match req.gas_price { Some(price) => Either::A(future::ok(price)), None => Either::B(dispatch::fetch_gas_price_corpus( @@ -210,8 +213,8 @@ impl LightFetch { self.client.clone(), self.on_demand.clone(), self.cache.clone(), - ).map(|corp| match corp.median() { - Some(median) => *median, + ).map(move |corp| match corp.percentile(gas_price_percentile) { + Some(percentile) => *percentile, None => DEFAULT_GAS_PRICE.into(), })) }; diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index f9b684a3112..dc450afccb3 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -66,6 +66,8 @@ pub struct EthClientOptions { pub allow_pending_receipt_query: bool, /// Send additional block number when asking for work pub send_block_number_in_get_work: bool, + /// Gas Price Percentile used as default gas price. + pub gas_price_percentile: usize, } impl EthClientOptions { @@ -84,6 +86,7 @@ impl Default for EthClientOptions { pending_nonce_from_queue: false, allow_pending_receipt_query: true, send_block_number_in_get_work: true, + gas_price_percentile: 50, } } } @@ -338,7 +341,7 @@ impl Eth for EthClient where } fn gas_price(&self) -> Result { - Ok(RpcU256::from(default_gas_price(&*self.client, &*self.miner))) + Ok(RpcU256::from(default_gas_price(&*self.client, &*self.miner, self.options.gas_price_percentile))) } fn accounts(&self, meta: Metadata) -> Result> { diff --git a/rpc/src/v1/impls/eth_pubsub.rs b/rpc/src/v1/impls/eth_pubsub.rs index 795ed760c0f..e9919335c19 100644 --- a/rpc/src/v1/impls/eth_pubsub.rs +++ b/rpc/src/v1/impls/eth_pubsub.rs @@ -92,12 +92,14 @@ impl EthPubSubClient { sync: Arc, cache: Arc>, remote: Remote, + gas_price_percentile: usize, ) -> Self { let fetch = LightFetch { client, on_demand, sync, - cache + cache, + gas_price_percentile, }; EthPubSubClient::new(Arc::new(fetch), remote) } diff --git a/rpc/src/v1/impls/light/eth.rs b/rpc/src/v1/impls/light/eth.rs index 768fd4705fd..1929b1cb75c 100644 --- a/rpc/src/v1/impls/light/eth.rs +++ b/rpc/src/v1/impls/light/eth.rs @@ -62,6 +62,7 @@ pub struct EthClient { accounts: Arc, cache: Arc>, polls: Mutex>, + gas_price_percentile: usize, } impl Clone for EthClient { @@ -75,6 +76,7 @@ impl Clone for EthClient { accounts: self.accounts.clone(), cache: self.cache.clone(), polls: Mutex::new(PollManager::new()), + gas_price_percentile: self.gas_price_percentile, } } } @@ -89,15 +91,17 @@ impl EthClient { transaction_queue: Arc>, accounts: Arc, cache: Arc>, + gas_price_percentile: usize, ) -> Self { EthClient { - sync: sync, - client: client, - on_demand: on_demand, - transaction_queue: transaction_queue, - accounts: accounts, - cache: cache, + sync, + client, + on_demand, + transaction_queue, + accounts, + cache, polls: Mutex::new(PollManager::new()), + gas_price_percentile, } } @@ -108,6 +112,7 @@ impl EthClient { on_demand: self.on_demand.clone(), sync: self.sync.clone(), cache: self.cache.clone(), + gas_price_percentile: self.gas_price_percentile, } } @@ -239,7 +244,7 @@ impl Eth for EthClient { fn gas_price(&self) -> Result { Ok(self.cache.lock().gas_price_corpus() - .and_then(|c| c.median().cloned()) + .and_then(|c| c.percentile(self.gas_price_percentile).cloned()) .map(RpcU256::from) .unwrap_or_else(Default::default)) } diff --git a/rpc/src/v1/impls/light/parity.rs b/rpc/src/v1/impls/light/parity.rs index 1be734df444..925919e5de0 100644 --- a/rpc/src/v1/impls/light/parity.rs +++ b/rpc/src/v1/impls/light/parity.rs @@ -60,6 +60,7 @@ pub struct ParityClient { dapps_address: Option, ws_address: Option, eip86_transition: u64, + gas_price_percentile: usize, } impl ParityClient { @@ -74,6 +75,7 @@ impl ParityClient { signer: Option>, dapps_address: Option, ws_address: Option, + gas_price_percentile: usize, ) -> Self { ParityClient { light_dispatch, @@ -85,7 +87,8 @@ impl ParityClient { dapps_address, ws_address, eip86_transition: client.eip86_transition(), - client: client, + client, + gas_price_percentile, } } @@ -96,6 +99,7 @@ impl ParityClient { on_demand: self.light_dispatch.on_demand.clone(), sync: self.light_dispatch.sync.clone(), cache: self.light_dispatch.cache.clone(), + gas_price_percentile: self.gas_price_percentile, } } } diff --git a/rpc/src/v1/tests/eth.rs b/rpc/src/v1/tests/eth.rs index 27b98f7f55f..bd39ccf06ae 100644 --- a/rpc/src/v1/tests/eth.rs +++ b/rpc/src/v1/tests/eth.rs @@ -153,7 +153,7 @@ impl EthTester { let reservations = Arc::new(Mutex::new(nonce::Reservations::new())); - let dispatcher = FullDispatcher::new(client.clone(), miner_service.clone(), reservations); + let dispatcher = FullDispatcher::new(client.clone(), miner_service.clone(), reservations, 50); let eth_sign = SigningUnsafeClient::new( &opt_account_provider, dispatcher, diff --git a/rpc/src/v1/tests/mocked/eth.rs b/rpc/src/v1/tests/mocked/eth.rs index 209d55127fb..504cef43a01 100644 --- a/rpc/src/v1/tests/mocked/eth.rs +++ b/rpc/src/v1/tests/mocked/eth.rs @@ -93,11 +93,12 @@ impl EthTester { let snapshot = snapshot_service(); let hashrates = Arc::new(Mutex::new(HashMap::new())); let external_miner = Arc::new(ExternalMiner::new(hashrates.clone())); + let gas_price_percentile = options.gas_price_percentile; let eth = EthClient::new(&client, &snapshot, &sync, &opt_ap, &miner, &external_miner, options).to_delegate(); let filter = EthFilterClient::new(client.clone(), miner.clone()).to_delegate(); let reservations = Arc::new(Mutex::new(nonce::Reservations::new())); - let dispatcher = FullDispatcher::new(client.clone(), miner.clone(), reservations); + let dispatcher = FullDispatcher::new(client.clone(), miner.clone(), reservations, gas_price_percentile); let sign = SigningUnsafeClient::new(&opt_ap, dispatcher).to_delegate(); let mut io: IoHandler = IoHandler::default(); io.extend_with(eth); diff --git a/rpc/src/v1/tests/mocked/personal.rs b/rpc/src/v1/tests/mocked/personal.rs index ce74ae686f6..fd0825f2849 100644 --- a/rpc/src/v1/tests/mocked/personal.rs +++ b/rpc/src/v1/tests/mocked/personal.rs @@ -56,7 +56,7 @@ fn setup() -> PersonalTester { let miner = miner_service(); let reservations = Arc::new(Mutex::new(nonce::Reservations::new())); - let dispatcher = FullDispatcher::new(client, miner.clone(), reservations); + let dispatcher = FullDispatcher::new(client, miner.clone(), reservations, 50); let personal = PersonalClient::new(opt_accounts, dispatcher, false); let mut io = IoHandler::default(); @@ -112,7 +112,7 @@ fn invalid_password_test(method: &str) "value": "0x9184e72a" }, "password321"], "id": 1 - }"#; + }"#; let response = r#"{"jsonrpc":"2.0","error":{"code":-32021,"message":"Account password is invalid or account does not exist.","data":"SStore(InvalidPassword)"},"id":1}"#; diff --git a/rpc/src/v1/tests/mocked/signer.rs b/rpc/src/v1/tests/mocked/signer.rs index 2fb95a1d640..b21a06e2d60 100644 --- a/rpc/src/v1/tests/mocked/signer.rs +++ b/rpc/src/v1/tests/mocked/signer.rs @@ -65,7 +65,7 @@ fn signer_tester() -> SignerTester { let reservations = Arc::new(Mutex::new(nonce::Reservations::new())); let event_loop = EventLoop::spawn(); - let dispatcher = FullDispatcher::new(client, miner.clone(), reservations); + let dispatcher = FullDispatcher::new(client, miner.clone(), reservations, 50); let mut io = IoHandler::default(); io.extend_with(SignerClient::new(&opt_accounts, dispatcher, &signer, event_loop.remote()).to_delegate()); diff --git a/rpc/src/v1/tests/mocked/signing.rs b/rpc/src/v1/tests/mocked/signing.rs index 8a6a790f8ed..81dd1f90f45 100644 --- a/rpc/src/v1/tests/mocked/signing.rs +++ b/rpc/src/v1/tests/mocked/signing.rs @@ -61,7 +61,7 @@ impl Default for SigningTester { let reservations = Arc::new(Mutex::new(nonce::Reservations::new())); let mut io = IoHandler::default(); - let dispatcher = FullDispatcher::new(client.clone(), miner.clone(), reservations); + let dispatcher = FullDispatcher::new(client.clone(), miner.clone(), reservations, 50); let remote = Remote::new_thread_per_future(); diff --git a/util/stats/src/lib.rs b/util/stats/src/lib.rs index 01c988232fd..74fda927265 100644 --- a/util/stats/src/lib.rs +++ b/util/stats/src/lib.rs @@ -46,6 +46,18 @@ impl Deref for Corpus { } impl Corpus { + /// Get given percentile (approximated). + pub fn percentile(&self, val: usize) -> Option<&T> { + let len = self.0.len(); + let x = val * len / 100; + let x = ::std::cmp::min(x, len); + if x == 0 { + return None; + } + + self.0.get(x - 1) + } + /// Get the median element, if it exists. pub fn median(&self) -> Option<&T> { self.0.get(self.0.len() / 2) @@ -121,7 +133,19 @@ impl Histogram #[cfg(test)] mod tests { - use super::Histogram; + use super::*; + + #[test] + fn check_corpus() { + let corpus = Corpus::from(vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10]); + assert_eq!(corpus.percentile(0), None); + assert_eq!(corpus.percentile(1), None); + assert_eq!(corpus.percentile(101), Some(&10)); + assert_eq!(corpus.percentile(100), Some(&10)); + assert_eq!(corpus.percentile(50), Some(&5)); + assert_eq!(corpus.percentile(60), Some(&6)); + assert_eq!(corpus.median(), Some(&6)); + } #[test] fn check_histogram() {