Revive parity_setMinGasPrice RPC call (#10294)

* Add function to update minimum gas price

* Update TestMinerService to handle min_gas_price changes

* Place minimum gas price test behind feature flag

* Update check for fixed gas pricer to be more explicit

* Use errors::unsupported instead of errors::request_rejected

* Add test that fails to set minimum gas price

* Fix test that should fail when setting new gas price

* Put dev dependencies behind feature flag

* Fix deadlock in set_minimal_gas_price()

* Update RPC tests with mocked error response

* Remove unnecessary cfg flag

* Remove duplicate crate imports

Cargo.lock

@@ -707,6 +707,7 @@ dependencies = [
  "ethjson 0.1.0",
  "ethkey 0.3.0",
  "evm 0.1.0",
+ "fetch 0.1.0",
  "hashdb 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "heapsize 0.4.2 (git+https://github.com/cheme/heapsize.git?branch=ec-macfix)",
  "itertools 0.5.10 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -728,6 +729,7 @@ dependencies = [
  "parity-bytes 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "parity-crypto 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "parity-machine 0.1.0",
+ "parity-runtime 0.1.0",
  "parity-snappy 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "parking_lot 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "patricia-trie 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)",

ethcore/Cargo.toml

@@ -76,7 +76,9 @@ blooms-db = { path = "../util/blooms-db" }
 criterion = "0.2"
 env_logger = "0.5"
 ethcore-accounts = { path = "../accounts" }
+fetch = { path = "../util/fetch" }
 kvdb-rocksdb = "0.1.3"
+parity-runtime = { path = "../util/runtime" }
 rlp_compress = { path = "../util/rlp-compress" }
 tempdir = "0.3"
 trie-standardmap = "0.1"

ethcore/src/lib.rs

@@ -143,6 +143,12 @@ extern crate serde_derive;
 #[cfg_attr(test, macro_use)]
 extern crate evm;
 
+#[cfg(all(test, feature = "price-info"))]
+extern crate fetch;
+
+#[cfg(all(test, feature = "price-info"))]
+extern crate parity_runtime;
+
 pub mod block;
 pub mod builtin;
 pub mod client;

ethcore/src/miner/miner.rs

@@ -873,6 +873,32 @@ impl miner::MinerService for Miner {
 		self.params.read().gas_range_target.0 / 5
 	}
 
+	fn set_minimal_gas_price(&self, new_price: U256) -> Result<bool, &str> {
+		match *self.gas_pricer.lock() {
+			// Binding the gas pricer to `gp` here to prevent
+			// a deadlock when calling recalibrate()
+			ref mut gp @ GasPricer::Fixed(_) => {
+				trace!(target: "miner", "minimal_gas_price: recalibrating fixed...");
+				*gp = GasPricer::new_fixed(new_price);
+
+				let txq = self.transaction_queue.clone();
+				let mut options = self.options.pool_verification_options.clone();
+				gp.recalibrate(move |gas_price| {
+					debug!(target: "miner", "minimal_gas_price: Got gas price! {}", gas_price);
+					options.minimal_gas_price = gas_price;
+					txq.set_verifier_options(options);
+				});
+
+				Ok(true)
+			},
+			#[cfg(feature = "price-info")]
+			GasPricer::Calibrated(_) => {
+				let error_msg = "Can't update fixed gas price while automatic gas calibration is enabled.";
+				return Err(error_msg);
+			},
+		}
+	}
+
 	fn import_external_transactions<C: miner::BlockChainClient>(
 		&self,
 		chain: &C,
@@ -1654,4 +1680,60 @@ mod tests {
 
 		assert!(miner.is_currently_sealing());
 	}
+
+	#[test]
+	fn should_set_new_minimum_gas_price() {
+		// Creates a new GasPricer::Fixed behind the scenes
+		let miner = Miner::new_for_tests(&Spec::new_test(), None);
+
+		let expected_minimum_gas_price: U256 = 0x1337.into();
+		miner.set_minimal_gas_price(expected_minimum_gas_price).unwrap();
+
+		let txq_options = miner.transaction_queue.status().options;
+		let current_minimum_gas_price = txq_options.minimal_gas_price;
+
+		assert!(current_minimum_gas_price == expected_minimum_gas_price);
+	}
+
+	#[cfg(feature = "price-info")]
+	fn dynamic_gas_pricer() -> GasPricer {
+		use std::time::Duration;
+		use parity_runtime::Executor;
+		use fetch::Client as FetchClient;
+		use ethcore_miner::gas_price_calibrator::{GasPriceCalibrator, GasPriceCalibratorOptions};
+
+		// Don't really care about any of these settings since
+		// the gas pricer is never actually going to be used
+		let fetch = FetchClient::new(1).unwrap();
+		let p = Executor::new_sync();
+
+		GasPricer::new_calibrated(
+			GasPriceCalibrator::new(
+				GasPriceCalibratorOptions {
+					usd_per_tx: 0.0,
+					recalibration_period: Duration::from_secs(0),
+				},
+				fetch,
+				p,
+			)
+		)
+	}
+
+	#[test]
+	#[cfg(feature = "price-info")]
+	fn should_fail_to_set_new_minimum_gas_price() {
+		// We get a fixed gas pricer by default, need to change that
+		let miner = Miner::new_for_tests(&Spec::new_test(), None);
+		let calibrated_gas_pricer = dynamic_gas_pricer();
+		*miner.gas_pricer.lock() = calibrated_gas_pricer;
+
+		let expected_minimum_gas_price: U256 = 0x1337.into();
+		let result = miner.set_minimal_gas_price(expected_minimum_gas_price);
+		assert!(result.is_err());
+
+		let received_error_msg = result.unwrap_err();
+		let expected_error_msg = "Can't update fixed gas price while automatic gas calibration is enabled.";
+
+		assert!(received_error_msg == expected_error_msg);
+	}
 }

ethcore/src/miner/mod.rs

@@ -205,4 +205,8 @@ pub trait MinerService : Send + Sync {
 
 	/// Suggested gas limit.
 	fn sensible_gas_limit(&self) -> U256;
+
+	/// Set a new minimum gas limit.
+	/// Will not work if dynamic gas calibration is set.
+	fn set_minimal_gas_price(&self, gas_price: U256) -> Result<bool, &str>;
 }

miner/src/gas_pricer.rs

@@ -45,7 +45,7 @@ impl GasPricer {
 	/// Recalibrate current gas price.
 	pub fn recalibrate<F: FnOnce(U256) + Sync + Send + 'static>(&mut self, set_price: F) {
 		match *self {
-			GasPricer::Fixed(ref max) => set_price(max.clone()),
+			GasPricer::Fixed(ref curr) => set_price(curr.clone()),
 			#[cfg(feature = "price-info")]
 			GasPricer::Calibrated(ref mut cal) => cal.recalibrate(set_price),
 		}

rpc/src/v1/impls/parity_set.rs

@@ -118,9 +118,11 @@ impl<C, M, U, F> ParitySet for ParitySetClient<C, M, U, F> where
 	F: Fetch + 'static,
 {
 
-	fn set_min_gas_price(&self, _gas_price: U256) -> Result<bool> {
+	fn set_min_gas_price(&self, gas_price: U256) -> Result<bool> {
-		warn!("setMinGasPrice is deprecated. Ignoring request.");
+		match self.miner.set_minimal_gas_price(gas_price.into()) {
-		Ok(false)
+			Ok(success) => Ok(success),
+			Err(e) => Err(errors::unsupported(e, None)),
+		}
 	}
 
 	fn set_transactions_limit(&self, _limit: usize) -> Result<bool> {

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

@@ -49,6 +49,8 @@ pub struct TestMinerService {
 	pub pending_receipts: Mutex<Vec<RichReceipt>>,
 	/// Next nonces.
 	pub next_nonces: RwLock<HashMap<Address, U256>>,
+	/// Minimum gas price
+	pub min_gas_price: RwLock<Option<U256>>,
 	/// Signer (if any)
 	pub signer: RwLock<Option<Box<EngineSigner>>>,
 
@@ -63,6 +65,7 @@ impl Default for TestMinerService {
 			local_transactions: Default::default(),
 			pending_receipts: Default::default(),
 			next_nonces: Default::default(),
+			min_gas_price: RwLock::new(Some(0.into())),
 			authoring_params: RwLock::new(AuthoringParams {
 				author: Address::zero(),
 				gas_range_target: (12345.into(), 54321.into()),
@@ -279,4 +282,18 @@ impl MinerService for TestMinerService {
 	fn sensible_gas_limit(&self) -> U256 {
 		0x5208.into()
 	}
+
+	fn set_minimal_gas_price(&self, gas_price: U256) -> Result<bool, &str> {
+		let mut new_price = self.min_gas_price.write();
+		match *new_price {
+			Some(ref mut v) => {
+				*v = gas_price;
+				Ok(true)
+			},
+			None => {
+				let error_msg = "Can't update fixed gas price while automatic gas calibration is enabled.";
+				Err(error_msg)
+			},
+		}
+	}
 }

rpc/src/v1/tests/mocked/parity_set.rs

@@ -112,7 +112,25 @@ fn rpc_parity_set_min_gas_price() {
 	io.extend_with(parity_set_client(&client, &miner, &updater, &network).to_delegate());
 
 	let request = r#"{"jsonrpc": "2.0", "method": "parity_setMinGasPrice", "params":["0xcd1722f3947def4cf144679da39c4c32bdc35681"], "id": 1}"#;
-	let response = r#"{"jsonrpc":"2.0","result":false,"id":1}"#;
+	let response = r#"{"jsonrpc":"2.0","result":true,"id":1}"#;
+
+	assert_eq!(io.handle_request_sync(request), Some(response.to_owned()));
+}
+
+#[test]
+fn rpc_parity_set_min_gas_price_with_automated_calibration_enabled() {
+	let miner = miner_service();
+	*miner.min_gas_price.write() = None;
+
+	let client = client_service();
+	let network = network_service();
+	let updater = updater_service();
+
+	let mut io = IoHandler::new();
+	io.extend_with(parity_set_client(&client, &miner, &updater, &network).to_delegate());
+
+	let request = r#"{"jsonrpc": "2.0", "method": "parity_setMinGasPrice", "params":["0xdeadbeef"], "id": 1}"#;
+	let response = r#"{"jsonrpc":"2.0","error":{"code":-32000,"message":"Can't update fixed gas price while automatic gas calibration is enabled."},"id":1}"#;
 
 	assert_eq!(io.handle_request_sync(request), Some(response.to_owned()));
 }