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

Expose default gas price percentile configuration in CLI #7497

Merged
merged 4 commits into from
Jan 9, 2018
Merged
Show file tree
Hide file tree
Changes from 2 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
8 changes: 8 additions & 0 deletions parity/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,7 @@ usage! {
ARG arg_password: (Vec<String>) = 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",
Expand Down Expand Up @@ -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<String>) = 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
Expand Down Expand Up @@ -1142,6 +1147,7 @@ struct Mining {
tx_time_limit: Option<u64>,
relay_set: Option<String>,
min_gas_price: Option<u64>,
gas_price_percentile: Option<usize>,
usd_per_tx: Option<String>,
usd_per_eth: Option<String>,
price_update_period: Option<String>,
Expand Down Expand Up @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe override the default for the test in tests/config.full.toml.

arg_usd_per_tx: "0.0025".into(),
arg_usd_per_eth: "auto".into(),
arg_price_update_period: "hourly".into(),
Expand Down Expand Up @@ -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()),
Expand Down
2 changes: 2 additions & 0 deletions parity/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down
7 changes: 6 additions & 1 deletion parity/rpc_apis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -256,6 +257,7 @@ impl FullDependencies {
self.client.clone(),
self.miner.clone(),
nonces.clone(),
self.gas_price_percentile,
);
for api in apis {
match *api {
Expand All @@ -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());
Expand Down Expand Up @@ -422,6 +425,7 @@ pub struct LightDependencies<T> {
pub geth_compatibility: bool,
pub remote: parity_reactor::Remote,
pub whisper_rpc: Option<::whisper::RpcFactory>,
pub gas_price_percentile: usize,
}

impl<C: LightChainClient + 'static> LightDependencies<C> {
Expand All @@ -440,6 +444,7 @@ impl<C: LightChainClient + 'static> LightDependencies<C> {
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 {
Expand Down
3 changes: 3 additions & 0 deletions parity/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ pub struct RunCmd {
pub daemon: Option<String>,
pub logger_config: LogConfig,
pub miner_options: MinerOptions,
pub gas_price_percentile: usize,
pub ntp_servers: Vec<String>,
pub ws_conf: rpc::WsConfiguration,
pub http_conf: rpc::HttpConfiguration,
Expand Down Expand Up @@ -358,6 +359,7 @@ fn execute_light(cmd: RunCmd, can_restart: bool, logger: Arc<RotatingLogger>) ->
geth_compatibility: cmd.geth_compatibility,
remote: event_loop.remote(),
whisper_rpc: whisper_factory,
gas_price_percentile: cmd.gas_price_percentile,
});

let dependencies = rpc::Dependencies {
Expand Down Expand Up @@ -761,6 +763,7 @@ pub fn execute(cmd: RunCmd, can_restart: bool, logger: Arc<RotatingLogger>) -> R
fetch: fetch.clone(),
remote: event_loop.remote(),
whisper_rpc: whisper_factory,
gas_price_percentile: cmd.gas_price_percentile,
});

let dependencies = rpc::Dependencies {
Expand Down
21 changes: 16 additions & 5 deletions rpc/src/v1/helpers/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ pub struct FullDispatcher<C, M> {
client: Arc<C>,
miner: Arc<M>,
nonces: Arc<Mutex<nonce::Reservations>>,
gas_price_percentile: usize,
}

impl<C, M> FullDispatcher<C, M> {
Expand All @@ -96,11 +97,13 @@ impl<C, M> FullDispatcher<C, M> {
client: Arc<C>,
miner: Arc<M>,
nonces: Arc<Mutex<nonce::Reservations>>,
gas_price_percentile: usize,
) -> Self {
FullDispatcher {
client,
miner,
nonces,
gas_price_percentile,
}
}
}
Expand All @@ -111,6 +114,7 @@ impl<C, M> Clone for FullDispatcher<C, M> {
client: self.client.clone(),
miner: self.miner.clone(),
nonces: self.nonces.clone(),
gas_price_percentile: self.gas_price_percentile,
}
}
}
Expand Down Expand Up @@ -148,7 +152,9 @@ impl<C: MiningBlockChainClient, M: MinerService> Dispatcher for FullDispatcher<C
used_default_from: request.from.is_none(),
to: request.to,
nonce,
gas_price: request.gas_price.unwrap_or_else(|| default_gas_price(&*self.client, &*self.miner)),
gas_price: request.gas_price.unwrap_or_else(|| {
default_gas_price(&*self.client, &*self.miner, self.gas_price_percentile)
}),
gas: request.gas.unwrap_or_else(|| self.miner.sensible_gas_limit()),
value: request.value.unwrap_or_else(|| 0.into()),
data: request.data.unwrap_or_else(Vec::new),
Expand Down Expand Up @@ -258,6 +264,8 @@ pub struct LightDispatcher {
pub transaction_queue: Arc<RwLock<LightTransactionQueue>>,
/// Nonce reservations
pub nonces: Arc<Mutex<nonce::Reservations>>,
/// Gas Price percentile value used as default gas price.
pub gas_price_percentile: usize,
}

impl LightDispatcher {
Expand All @@ -271,6 +279,7 @@ impl LightDispatcher {
cache: Arc<Mutex<LightDataCache>>,
transaction_queue: Arc<RwLock<LightTransactionQueue>>,
nonces: Arc<Mutex<nonce::Reservations>>,
gas_price_percentile: usize,
) -> Self {
LightDispatcher {
sync,
Expand All @@ -279,6 +288,7 @@ impl LightDispatcher {
cache,
transaction_queue,
nonces,
gas_price_percentile,
}
}

Expand Down Expand Up @@ -345,15 +355,16 @@ 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(
self.sync.clone(),
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))
};
Expand Down Expand Up @@ -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<C, M>(client: &C, miner: &M) -> U256 where
pub fn default_gas_price<C, M>(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.
Expand Down
5 changes: 4 additions & 1 deletion rpc/src/v1/impls/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
}
}
}
Expand Down Expand Up @@ -338,7 +341,7 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM> Eth for EthClient<C, SN, S, M, EM> where
}

fn gas_price(&self) -> Result<RpcU256> {
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<Vec<RpcH160>> {
Expand Down
2 changes: 1 addition & 1 deletion rpc/src/v1/tests/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion rpc/src/v1/tests/mocked/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Metadata> = IoHandler::default();
io.extend_with(eth);
Expand Down
4 changes: 2 additions & 2 deletions rpc/src/v1/tests/mocked/personal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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}"#;

Expand Down
2 changes: 1 addition & 1 deletion rpc/src/v1/tests/mocked/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down
2 changes: 1 addition & 1 deletion rpc/src/v1/tests/mocked/signing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
26 changes: 25 additions & 1 deletion util/stats/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,18 @@ impl<T> Deref for Corpus<T> {
}

impl<T: Ord> Corpus<T> {
/// 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)
Expand Down Expand Up @@ -121,7 +133,19 @@ impl<T: Ord + Copy + ::std::fmt::Display> Histogram<T>

#[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));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for a bigger data set this should be the same as median.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have even number of elements here.

assert_eq!(corpus.percentile(60), Some(&6));
assert_eq!(corpus.median(), Some(&6));
}

#[test]
fn check_histogram() {
Expand Down