Errors & warnings for inappropriate RPCs (#6029)

* Clarify function name

Function checks if sealing is currently underway, not to be confused
with checking whether the engine performs internal sealing.

* Error when work called on internal sealing engine

* Error submitting work for internal sealing engine

* Fix inverted bool and style grumbles

* Add can_produce_work_package to TestMinerService

* Error when setting engine signer on PoW chain

* Unit tests for engine signing

Setting engine signer should fail if chain does not seal internally
or client lacks account provider.

* Tweak TestMinerService

* Fix minor style grumbles
This commit is contained in:
Joseph Mark 2017-07-12 13:52:18 +07:00 committed by Gav Wood
parent 02f2c611d4
commit 0fca4f95d6
8 changed files with 75 additions and 13 deletions

View File

@ -64,7 +64,9 @@ pub enum SignError {
/// Low-level hardware device error.
Hardware(HardwareError),
/// Low-level error from store
SStore(SSError)
SStore(SSError),
/// Inappropriate chain
InappropriateChain,
}
impl fmt::Display for SignError {
@ -74,6 +76,7 @@ impl fmt::Display for SignError {
SignError::NotFound => write!(f, "Account does not exist"),
SignError::Hardware(ref e) => write!(f, "{}", e),
SignError::SStore(ref e) => write!(f, "{}", e),
SignError::InappropriateChain => write!(f, "Inappropriate chain"),
}
}
}

View File

@ -804,9 +804,15 @@ impl MinerService for Miner {
// | Make sure to release the locks before calling that method. |
// --------------------------------------------------------------------------
self.engine.set_signer(ap.clone(), address, password);
}
}
Ok(())
} else {
warn!(target: "miner", "No account provider");
Err(AccountError::NotFound)
}
} else {
warn!(target: "miner", "Cannot set engine signer on a PoW chain.");
Err(AccountError::InappropriateChain)
}
}
fn set_extra_data(&self, extra_data: Bytes) {
@ -1085,6 +1091,10 @@ impl MinerService for Miner {
self.transaction_queue.read().last_nonce(address)
}
fn can_produce_work_package(&self) -> bool {
self.engine.seals_internally().is_none()
}
/// Update sealing if required.
/// Prepare the block and work if the Engine does not seal internally.
fn update_sealing(&self, chain: &MiningBlockChainClient) {
@ -1113,7 +1123,7 @@ impl MinerService for Miner {
}
}
fn is_sealing(&self) -> bool {
fn is_currently_sealing(&self) -> bool {
self.sealing_work.lock().queue.is_in_use()
}
@ -1272,7 +1282,7 @@ mod tests {
use header::BlockNumber;
use types::transaction::{SignedTransaction, Transaction, PendingTransaction, Action};
use spec::Spec;
use tests::helpers::{generate_dummy_client};
use tests::helpers::{generate_dummy_client, generate_dummy_client_with_spec_and_accounts};
#[test]
fn should_prepare_block_to_seal() {
@ -1440,4 +1450,22 @@ mod tests {
assert!(miner.pending_block().is_none());
assert_eq!(client.chain_info().best_block_number, 4 as BlockNumber);
}
#[test]
fn should_fail_setting_engine_signer_on_pow() {
let spec = Spec::new_pow_test_spec;
let tap = Arc::new(AccountProvider::transient_provider());
let addr = tap.insert_account("1".sha3().into(), "").unwrap();
let client = generate_dummy_client_with_spec_and_accounts(spec, Some(tap.clone()));
assert!(match client.miner().set_engine_signer(addr, "".into()) { Err(AccountError::InappropriateChain) => true, _ => false })
}
#[test]
fn should_fail_setting_engine_signer_without_account_provider() {
let spec = Spec::new_instant;
let tap = Arc::new(AccountProvider::transient_provider());
let addr = tap.insert_account("1".sha3().into(), "").unwrap();
let client = generate_dummy_client_with_spec_and_accounts(spec, None);
assert!(match client.miner().set_engine_signer(addr, "".into()) { Err(AccountError::NotFound) => true, _ => false });
}
}

View File

@ -136,6 +136,9 @@ pub trait MinerService : Send + Sync {
/// Called when blocks are imported to chain, updates transactions queue.
fn chain_new_blocks(&self, chain: &MiningBlockChainClient, imported: &[H256], invalid: &[H256], enacted: &[H256], retracted: &[H256]);
/// PoW chain - can produce work package
fn can_produce_work_package(&self) -> bool;
/// New chain head event. Restart mining operation.
fn update_sealing(&self, chain: &MiningBlockChainClient);
@ -176,7 +179,7 @@ pub trait MinerService : Send + Sync {
fn last_nonce(&self, address: &Address) -> Option<U256>;
/// Is it currently sealing?
fn is_sealing(&self) -> bool;
fn is_currently_sealing(&self) -> bool;
/// Suggested gas price.
fn sensible_gas_price(&self) -> U256;

View File

@ -451,6 +451,9 @@ impl Spec {
/// Create a new Spec with BasicAuthority which uses multiple validator sets changing with height.
/// Account with secrets "0".sha3() is the validator for block 1 and with "1".sha3() onwards.
pub fn new_validator_multi() -> Self { load_bundled!("validator_multi") }
/// Create a new spec for a PoW chain
pub fn new_pow_test_spec() -> Self { load_bundled!("ethereum/olympic") }
}
#[cfg(test)]

View File

@ -21,6 +21,7 @@ export const ERROR_CODES = {
NO_WORK: -32001,
NO_AUTHOR: -32002,
NO_NEW_WORK: -32003,
NO_WORK_REQUIRED: -32004,
NOT_ENOUGH_DATA: -32006,
UNKNOWN_ERROR: -32009,
TRANSACTION_ERROR: -32010,
@ -38,7 +39,7 @@ export const ERROR_CODES = {
COMPILATION_ERROR: -32050,
ENCRYPTION_ERROR: -32055,
FETCH_ERROR: -32060,
INVALID_PARAMS: -32602
INVALID_PARAMS: -32602,
};
export default class TransportError extends ExtendableError {

View File

@ -28,6 +28,7 @@ mod codes {
pub const NO_WORK: i64 = -32001;
pub const NO_AUTHOR: i64 = -32002;
pub const NO_NEW_WORK: i64 = -32003;
pub const NO_WORK_REQUIRED: i64 = -32004;
pub const UNKNOWN_ERROR: i64 = -32009;
pub const TRANSACTION_ERROR: i64 = -32010;
pub const EXECUTION_ERROR: i64 = -32015;
@ -133,7 +134,7 @@ pub fn state_pruned() -> Error {
Error {
code: ErrorCode::ServerError(codes::UNSUPPORTED_REQUEST),
message: "This request is not supported because your node is running with state pruning. Run with --pruning=archive.".into(),
data: None
data: None,
}
}
@ -145,7 +146,7 @@ pub fn exceptional() -> Error {
Error {
code: ErrorCode::ServerError(codes::EXCEPTION_ERROR),
message: "The execution failed due to an exception.".into(),
data: None
data: None,
}
}
@ -153,7 +154,7 @@ pub fn no_work() -> Error {
Error {
code: ErrorCode::ServerError(codes::NO_WORK),
message: "Still syncing.".into(),
data: None
data: None,
}
}
@ -161,7 +162,7 @@ pub fn no_new_work() -> Error {
Error {
code: ErrorCode::ServerError(codes::NO_NEW_WORK),
message: "Work has not changed.".into(),
data: None
data: None,
}
}
@ -169,7 +170,15 @@ pub fn no_author() -> Error {
Error {
code: ErrorCode::ServerError(codes::NO_AUTHOR),
message: "Author not configured. Run Parity with --author to configure.".into(),
data: None
data: None,
}
}
pub fn no_work_required() -> Error {
Error {
code: ErrorCode::ServerError(codes::NO_WORK_REQUIRED),
message: "External work is only required for Proof of Work engines.".into(),
data: None,
}
}

View File

@ -332,7 +332,7 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM> Eth for EthClient<C, SN, S, M, EM> where
}
fn is_mining(&self) -> Result<bool, Error> {
Ok(self.miner.is_sealing())
Ok(self.miner.is_currently_sealing())
}
fn hashrate(&self) -> Result<RpcU256, Error> {
@ -553,6 +553,11 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM> Eth for EthClient<C, SN, S, M, EM> where
}
fn work(&self, no_new_work_timeout: Trailing<u64>) -> Result<Work, Error> {
if !self.miner.can_produce_work_package() {
warn!(target: "miner", "Cannot give work package - engine seals internally.");
return Err(errors::no_work_required())
}
let no_new_work_timeout = no_new_work_timeout.unwrap_or_default();
// check if we're still syncing and return empty strings in that case
@ -602,6 +607,11 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM> Eth for EthClient<C, SN, S, M, EM> where
}
fn submit_work(&self, nonce: RpcH64, pow_hash: RpcH256, mix_hash: RpcH256) -> Result<bool, Error> {
if !self.miner.can_produce_work_package() {
warn!(target: "miner", "Cannot submit work - engine seals internally.");
return Err(errors::no_work_required())
}
let nonce: H64 = nonce.into();
let pow_hash: H256 = pow_hash.into();
let mix_hash: H256 = mix_hash.into();

View File

@ -207,6 +207,11 @@ impl MinerService for TestMinerService {
unimplemented!();
}
/// PoW chain - can produce work package
fn can_produce_work_package(&self) -> bool {
true
}
/// New chain head event. Restart mining operation.
fn update_sealing(&self, _chain: &MiningBlockChainClient) {
unimplemented!();
@ -265,7 +270,7 @@ impl MinerService for TestMinerService {
self.last_nonces.read().get(address).cloned()
}
fn is_sealing(&self) -> bool {
fn is_currently_sealing(&self) -> bool {
false
}