Errors & warnings for inappropriate RPCs #6029
Conversation
Function checks if sealing is currently underway, not to be confused with checking whether the engine performs internal sealing.
|
It looks like @sjeohp hasn'signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement at the following URL: https://cla.parity.io Once you've signed, plesae reply to this thread with Many thanks, Parity Technologies CLA Bot |
sjeohp-zz
changed the title
ethcore/src/miner/miner.rs
Outdated
| @@ -1083,6 +1083,11 @@ impl MinerService for Miner { | |||
| self.transaction_queue.read().last_nonce(address) | |||
| } | |||
|
|
|||
| fn engine_seals_internally(&self) -> bool | |||
| { | |||
There was a problem hiding this comment.
Style: keep the opening brace in the same line.
ethcore/src/miner/mod.rs
Outdated
| @@ -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]); | |||
|
|
|||
| /// Engine seals internally - no external input required (ie. not an Ethash chain) | |||
| fn engine_seals_internally(&self) -> bool; | |||
There was a problem hiding this comment.
I think I would call it more explicitly as: fn can_produce_work_package(&self) -> bool.
rpc/src/v1/helpers/errors.rs
Outdated
| pub fn no_work_required() -> Error { | ||
| Error { | ||
| code: ErrorCode::ServerError(codes::NO_WORK_REQUIRED), | ||
| message: "External work is only required for Ethash engines.".into(), |
There was a problem hiding this comment.
for Proof of Work engines?
There was a problem hiding this comment.
Hm? No work required for internal sealing engines, right?
There was a problem hiding this comment.
Yeah, but the comment says that it's required only for Ethash engines. In principle we could support other engines that work the same way as Ethash (Request work package -> Submit seal).
So my point was that the comment can be made more general.
|
[clabot:check] |
|
It looks like @sjeohp signed our Contributor License Agreement. 👍 Many thanks, Parity Technologies CLA Bot |
rphmeier
added
A0-pleasereview 🤓
Setting engine signer should fail if chain does not seal internally or client lacks account provider.
sjeohp-zz
changed the title
sjeohp-zz
changed the title
ethcore/src/account_provider/mod.rs
Outdated
| SStore(SSError) | ||
| SStore(SSError), | ||
| /// Inappropriate chain | ||
| InappropriateChain |
ethcore/src/account_provider/mod.rs
Outdated
| @@ -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") | |||
rpc/src/v1/helpers/errors.rs
Outdated
| Error { | ||
| code: ErrorCode::ServerError(codes::NO_WORK_REQUIRED), | ||
| message: "External work is only required for Proof of Work engines.".into(), | ||
| data: None |
rpc/src/v1/impls/eth.rs
Outdated
| @@ -553,6 +553,12 @@ 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> { | |||
|
|
|||
There was a problem hiding this comment.
superfluous (or at least non-conventional) empty line
gavofyork
added
A5-grumble 🔥
|
minor style grumbles. looks good otherwise. |
keorn
added
A8-looksgood 🦄
#4605