Modify gas price statistics (#2947)

* gas price distribution + median + tests

* put histogram in util

* use the util histogram

* remove the default gas price implementation

* histogram rpc

* fix empty corpus

* Add JS ethcore_gasPriceHistogram

* Fix typo (s/types/type/) & subsequent failing test

* Fix return type & formatting

* bucketBounds

* Add jsapi e2e test verification

ethcore/src/client/traits.rs

@@ -16,6 +16,7 @@
 
 use std::collections::BTreeMap;
 use util::{U256, Address, H256, H2048, Bytes, Itertools};
+use util::stats::Histogram;
 use blockchain::TreeRoute;
 use verification::queue::QueueInfo as BlockQueueInfo;
 use block::{OpenBlock, SealedBlock};
@@ -190,8 +191,8 @@ pub trait BlockChainClient : Sync + Send {
 	/// list all transactions
 	fn pending_transactions(&self) -> Vec<SignedTransaction>;
 
-	/// Get the gas price distribution.
+	/// Sorted list of transaction gas prices from at least last sample_size blocks.
-	fn gas_price_statistics(&self, sample_size: usize, distribution_size: usize) -> Result<Vec<U256>, ()> {
+	fn gas_price_corpus(&self, sample_size: usize) -> Vec<U256> {
 		let mut h = self.chain_info().best_block_hash;
 		let mut corpus = Vec::new();
 		while corpus.is_empty() {
@@ -200,25 +201,29 @@ pub trait BlockChainClient : Sync + Send {
 				let block = BlockView::new(&block_bytes);
 				let header = block.header_view();
 				if header.number() == 0 {
-					if corpus.is_empty() {
+					return corpus;
-						corpus.push(20_000_000_000u64.into());	// we have literally no information - it' as good a number as any.
-					}
-					break;
 				}
 				block.transaction_views().iter().foreach(|t| corpus.push(t.gas_price()));
 				h = header.parent_hash().clone();
 			}
 		}
 		corpus.sort();
-		let n = corpus.len();
+		corpus
-		if n > 0 {
-			Ok((0..(distribution_size + 1))
-				.map(|i| corpus[i * (n - 1) / distribution_size])
-				.collect::<Vec<_>>()
-			)
-		} else {
-			Err(())
 	}
+
+	/// Calculate median gas price from recent blocks if they have any transactions.
+	fn gas_price_median(&self, sample_size: usize) -> Option<U256> {
+		let corpus = self.gas_price_corpus(sample_size);
+		corpus.get(corpus.len()/2).cloned()
+	}
+
+	/// Get the gas price distribution based on recent blocks if they have any transactions.
+	fn gas_price_histogram(&self, sample_size: usize, bucket_number: usize) -> Option<Histogram> {
+		let raw_corpus = self.gas_price_corpus(sample_size);
+		let raw_len = raw_corpus.len();
+		// Throw out outliers.
+		let (corpus, _) = raw_corpus.split_at(raw_len-raw_len/40);
+		Histogram::new(corpus, bucket_number)
 	}
 }
 

ethcore/src/miner/mod.rs

@@ -158,7 +158,7 @@ pub trait MinerService : Send + Sync {
 	fn is_sealing(&self) -> bool;
 
 	/// Suggested gas price.
-	fn sensible_gas_price(&self) -> U256 { 20000000000u64.into() }
+	fn sensible_gas_price(&self) -> U256;
 
 	/// Suggested gas limit.
 	fn sensible_gas_limit(&self) -> U256 { 21000.into() }

ethcore/src/tests/client.rs

@@ -26,6 +26,7 @@ use miner::Miner;
 use rlp::{Rlp, View};
 use spec::Spec;
 use views::BlockView;
+use util::stats::Histogram;
 
 #[test]
 fn imports_from_empty() {
@@ -198,19 +199,37 @@ fn can_collect_garbage() {
 	assert!(client.blockchain_cache_info().blocks < 100 * 1024);
 }
 
+
 #[test]
-#[cfg_attr(feature="dev", allow(useless_vec))]
+fn can_generate_gas_price_median() {
-fn can_generate_gas_price_statistics() {
+	let client_result = generate_dummy_client_with_data(3, 1, &vec_into![1, 2, 3]);
-	let client_result = generate_dummy_client_with_data(16, 1, &vec_into![0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]);
 	let client = client_result.reference();
-	let s = client.gas_price_statistics(8, 8).unwrap();
+	assert_eq!(Some(U256::from(2)), client.gas_price_median(3));
-	assert_eq!(s, vec_into![8, 8, 9, 10, 11, 12, 13, 14, 15]);
+
-	let s = client.gas_price_statistics(16, 8).unwrap();
+	let client_result = generate_dummy_client_with_data(4, 1, &vec_into![1, 4, 3, 2]);
-	assert_eq!(s, vec_into![0, 1, 3, 5, 7, 9, 11, 13, 15]);
+	let client = client_result.reference();
-	let s = client.gas_price_statistics(32, 8).unwrap();
+	assert_eq!(Some(U256::from(3)), client.gas_price_median(4));
-	assert_eq!(s, vec_into![0, 1, 3, 5, 7, 9, 11, 13, 15]);
 }
 
+#[test]
+fn can_generate_gas_price_histogram() {
+	let client_result = generate_dummy_client_with_data(20, 1, &vec_into![6354,8593,6065,4842,7845,7002,689,4958,4250,6098,5804,4320,643,8895,2296,8589,7145,2000,2512,1408]);
+	let client = client_result.reference();
+
+	let hist = client.gas_price_histogram(20, 5).unwrap();
+	let correct_hist = Histogram { bucket_bounds: vec_into![643,2293,3943,5593,7243,8893], counts: vec![4,2,4,6,3] };
+	assert_eq!(hist, correct_hist);
+}
+
+#[test]
+fn empty_gas_price_histogram() {
+	let client_result = generate_dummy_client_with_data(20, 0, &vec_into![]);
+	let client = client_result.reference();
+
+	assert!(client.gas_price_histogram(20, 5).is_none());
+}
+
+
 #[test]
 fn can_handle_long_fork() {
 	let client_result = generate_dummy_client(1200);

js/src/api/format/output.js

@@ -70,6 +70,20 @@ export function outDate (date) {
   return new Date(outNumber(date).toNumber() * 1000);
 }
 
+export function outHistogram (histogram) {
+  if (histogram) {
+    Object.keys(histogram).forEach((key) => {
+      switch (key) {
+        case 'bucketBounds':
+        case 'counts':
+          histogram[key] = histogram[key].map(outNumber);
+      }
+    });
+  }
+
+  return histogram;
+}
+
 export function outLog (log) {
   Object.keys(log).forEach((key) => {
     switch (key) {

js/src/api/format/output.spec.js

@@ -16,7 +16,7 @@
 
 import BigNumber from 'bignumber.js';
 
-import { outBlock, outAccountInfo, outAddress, outDate, outNumber, outPeers, outReceipt, outTransaction, outTrace } from './output';
+import { outBlock, outAccountInfo, outAddress, outDate, outHistogram, outNumber, outPeers, outReceipt, outTransaction, outTrace } from './output';
 import { isAddress, isBigNumber, isInstanceOf } from '../../../test/types';
 
 describe('api/format/output', () => {
@@ -120,6 +120,18 @@ describe('api/format/output', () => {
     });
   });
 
+  describe('outHistogram', () => {
+    ['bucketBounds', 'counts'].forEach((type) => {
+      it(`formats ${type} as number arrays`, () => {
+        expect(
+          outHistogram({ [type]: [0x123, 0x456, 0x789] })
+        ).to.deep.equal({
+          [type]: [new BigNumber(0x123), new BigNumber(0x456), new BigNumber(0x789)]
+        });
+      });
+    });
+  });
+
   describe('outNumber', () => {
     it('returns a BigNumber equalling the value', () => {
       const bn = outNumber('0x123456');

js/src/api/rpc/ethcore/ethcore.e2e.js

@@ -27,6 +27,16 @@ describe('ethapi.ethcore', () => {
     });
   });
 
+  describe('gasPriceHistogram', () => {
+    it('returns and translates the target', () => {
+      return ethapi.ethcore.gasPriceHistogram().then((result) => {
+        expect(Object.keys(result)).to.deep.equal(['bucketBounds', 'counts']);
+        expect(result.bucketBounds.length > 0).to.be.true;
+        expect(result.counts.length > 0).to.be.true;
+      });
+    });
+  });
+
   describe('netChain', () => {
     it('returns and the chain', () => {
       return ethapi.ethcore.netChain().then((value) => {

js/src/api/rpc/ethcore/ethcore.js

@@ -15,7 +15,7 @@
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
 import { inAddress, inData, inNumber16 } from '../../format/input';
-import { outAddress, outNumber, outPeers } from '../../format/output';
+import { outAddress, outHistogram, outNumber, outPeers } from '../../format/output';
 
 export default class Ethcore {
   constructor (transport) {
@@ -69,6 +69,12 @@ export default class Ethcore {
       .then(outNumber);
   }
 
+  gasPriceHistogram () {
+    return this._transport
+      .execute('ethcore_gasPriceHistogram')
+      .then(outHistogram);
+  }
+
   generateSecretPhrase () {
     return this._transport
       .execute('ethcore_generateSecretPhrase');

js/src/jsonrpc/interfaces/ethcore.js

@@ -104,6 +104,25 @@ export default {
     }
   },
 
+  gasPriceHistogram: {
+    desc: 'Returns a snapshot of the historic gas prices',
+    params: [],
+    returns: {
+      type: Object,
+      desc: 'Historic values',
+      details: {
+        bucketBounds: {
+          type: Array,
+          desc: 'Array of U256 bound values'
+        },
+        count: {
+          type: Array,
+          desc: 'Array of U64 counts'
+        }
+      }
+    }
+  },
+
   generateSecretPhrase: {
     desc: 'Creates a secret phrase that can be associated with an account',
     params: [],

rpc/src/v1/helpers/dispatch.rs

@@ -92,8 +92,5 @@ fn prepare_transaction<C, M>(client: &C, miner: &M, request: TransactionRequest)
 }
 
 pub fn default_gas_price<C, M>(client: &C, miner: &M) -> U256 where C: MiningBlockChainClient, M: MinerService {
-	client
+	client.gas_price_median(100).unwrap_or_else(|| miner.sensible_gas_price())
-		.gas_price_statistics(100, 8)
-		.map(|x| x[4])
-		.unwrap_or_else(|_| miner.sensible_gas_price())
 }

rpc/src/v1/helpers/errors.rs

@@ -32,6 +32,7 @@ mod codes {
 	pub const NO_WORK: i64 = -32001;
 	pub const NO_AUTHOR: i64 = -32002;
 	pub const NO_NEW_WORK: i64 = -32003;
+	pub const NOT_ENOUGH_DATA: i64 = -32006;
 	pub const UNKNOWN_ERROR: i64 = -32009;
 	pub const TRANSACTION_ERROR: i64 = -32010;
 	pub const EXECUTION_ERROR: i64 = -32015;
@@ -152,6 +153,14 @@ pub fn no_author() -> Error {
 	}
 }
 
+pub fn not_enough_data() -> Error {
+	Error {
+		code: ErrorCode::ServerError(codes::NOT_ENOUGH_DATA),
+		message: "The node does not have enough data to compute the given statistic.".into(),
+		data: None
+	}
+}
+
 pub fn token(e: String) -> Error {
 	Error {
 		code: ErrorCode::ServerError(codes::UNKNOWN_ERROR),

rpc/src/v1/impls/ethcore.rs

@@ -33,7 +33,7 @@ use ethcore::ids::BlockID;
 
 use jsonrpc_core::Error;
 use v1::traits::Ethcore;
-use v1::types::{Bytes, U256, H160, H256, H512, Peers, Transaction, RpcSettings};
+use v1::types::{Bytes, U256, H160, H256, H512, Peers, Transaction, RpcSettings, Histogram};
 use v1::helpers::{errors, SigningQueue, SignerService, NetworkSettings};
 use v1::helpers::dispatch::DEFAULT_MAC;
 use v1::helpers::auto_args::Ready;
@@ -222,13 +222,9 @@ impl<C, M, S: ?Sized, F> Ethcore for EthcoreClient<C, M, S, F> where
 		Ok(Bytes::new(version_data()))
 	}
 
-	fn gas_price_statistics(&self) -> Result<Vec<U256>, Error> {
+	fn gas_price_histogram(&self) -> Result<Histogram, Error> {
 		try!(self.active());
-
+		take_weak!(self.client).gas_price_histogram(100, 10).ok_or_else(errors::not_enough_data).map(Into::into)
-		match take_weak!(self.client).gas_price_statistics(100, 8) {
-			Ok(stats) => Ok(stats.into_iter().map(Into::into).collect()),
-			_ => Err(Error::internal_error()),
-		}
 	}
 
 	fn unsigned_transactions_count(&self) -> Result<usize, Error> {

rpc/src/v1/tests/helpers/miner_service.rs

@@ -253,4 +253,7 @@ impl MinerService for TestMinerService {
 		self.latest_closed_block.lock().as_ref().map_or(None, |b| b.block().fields().state.code(address).map(|c| (*c).clone()))
 	}
 
+	fn sensible_gas_price(&self) -> U256 {
+		20000000000u64.into()
+	}
 }

rpc/src/v1/traits/ethcore.rs

@@ -18,7 +18,7 @@
 use jsonrpc_core::Error;
 
 use v1::helpers::auto_args::{Wrap, WrapAsync, Ready};
-use v1::types::{H160, H256, H512, U256, Bytes, Peers, Transaction, RpcSettings};
+use v1::types::{H160, H256, H512, U256, Bytes, Peers, Transaction, RpcSettings, Histogram};
 
 build_rpc_trait! {
 	/// Ethcore-specific rpc interface.
@@ -76,8 +76,8 @@ build_rpc_trait! {
 		fn default_extra_data(&self) -> Result<Bytes, Error>;
 
 		/// Returns distribution of gas price in latest blocks.
-		#[rpc(name = "ethcore_gasPriceStatistics")]
+		#[rpc(name = "ethcore_gasPriceHistogram")]
-		fn gas_price_statistics(&self) -> Result<Vec<U256>, Error>;
+		fn gas_price_histogram(&self) -> Result<Histogram, Error>;
 
 		/// Returns number of unsigned transactions waiting in the signer queue (if signer enabled)
 		/// Returns error when signer is disabled

rpc/src/v1/types/histogram.rs

@@ -0,0 +1,39 @@
+// Copyright 2015, 2016 Ethcore (UK) Ltd.
+// This file is part of Parity.
+
+// Parity is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Parity is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Parity. If not, see <http://www.gnu.org/licenses/>.
+
+//! Gas prices histogram.
+
+use v1::types::U256;
+use util::stats;
+
+/// Values of RPC settings.
+#[derive(Serialize, Deserialize)]
+pub struct Histogram {
+	/// Gas prices for bucket edges.
+	#[serde(rename="bucketBounds")]
+	pub bucket_bounds: Vec<U256>,
+	/// Transacion counts for each bucket.
+	pub counts: Vec<u64>,
+}
+
+impl From<stats::Histogram> for Histogram {
+	fn from(h: stats::Histogram) -> Self {
+		Histogram {
+			bucket_bounds: h.bucket_bounds.into_iter().map(Into::into).collect(),
+			counts: h.counts
+		}
+	}
+}

rpc/src/v1/types/mod.rs.in

@@ -32,6 +32,7 @@ mod trace;
 mod trace_filter;
 mod uint;
 mod work;
+mod histogram;
 
 pub use self::bytes::Bytes;
 pub use self::block::{Block, BlockTransactions};
@@ -51,3 +52,4 @@ pub use self::trace::{LocalizedTrace, TraceResults};
 pub use self::trace_filter::TraceFilter;
 pub use self::uint::U256;
 pub use self::work::Work;
+pub use self::histogram::Histogram;

util/src/lib.rs

@@ -144,6 +144,7 @@ pub mod semantic_version;
 pub mod log;
 pub mod path;
 pub mod snappy;
+pub mod stats;
 pub mod cache;
 mod timer;
 

util/src/stats.rs

@@ -0,0 +1,70 @@
+// Copyright 2015, 2016 Ethcore (UK) Ltd.
+// This file is part of Parity.
+
+// Parity is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Parity is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Parity.  If not, see <http://www.gnu.org/licenses/>.
+
+//! Statistical functions.
+
+use bigint::uint::*;
+
+/// Discretised histogram.
+#[derive(Debug, PartialEq)]
+pub struct Histogram {
+	/// Bounds of each bucket.
+	pub bucket_bounds: Vec<U256>,
+	/// Count within each bucket.
+	pub counts: Vec<u64>
+}
+
+impl Histogram {
+	/// Histogram if a sorted corpus is at least fills the buckets.
+	pub fn new(corpus: &[U256], bucket_number: usize) -> Option<Histogram> {
+		if corpus.len() < bucket_number { return None; }
+		let corpus_end = corpus.last().expect("there are at least bucket_number elements; qed").clone();
+		// If there are extremely few transactions, go from zero.
+		let corpus_start = corpus.first().expect("there are at least bucket_number elements; qed").clone();
+		let bucket_size = (corpus_end - corpus_start + 1.into()) / bucket_number.into();
+		let mut bucket_end = corpus_start + bucket_size;
+
+		let mut bucket_bounds = vec![corpus_start; bucket_number + 1];
+		let mut counts = vec![0; bucket_number];
+		let mut corpus_i = 0;
+		// Go through the corpus adding to buckets.
+		for bucket in 0..bucket_number {
+			while corpus[corpus_i] < bucket_end {
+				counts[bucket] += 1;
+				corpus_i += 1;
+			}
+			bucket_bounds[bucket + 1] = bucket_end;
+			bucket_end = bucket_end + bucket_size;
+		}
+		Some(Histogram { bucket_bounds: bucket_bounds, counts: counts })
+	}
+}
+
+
+#[cfg(test)]
+mod tests {
+	use bigint::uint::U256;
+	use super::Histogram;
+
+	#[test]
+	fn check_histogram() {
+		let hist = Histogram::new(&vec_into![643,689,1408,2000,2296,2512,4250,4320,4842,4958,5804,6065,6098,6354,7002,7145,7845,8589,8593,8895], 5).unwrap();
+		let correct_bounds: Vec<U256> = vec_into![643,2293,3943,5593,7243,8893];
+		assert_eq!(Histogram { bucket_bounds: correct_bounds, counts: vec![4,2,4,6,3] }, hist);
+
+		assert!(Histogram::new(&vec_into![1, 2], 5).is_none());
+	}
+}