--reseal-on-uncle #5940
--reseal-on-uncle #5940
Conversation
arkpar
added
A0-pleasereview 🤓
| @@ -1652,6 +1652,32 @@ impl MiningBlockChainClient for Client { | |||
| open_block | |||
| } | |||
|
|
|||
| fn reopen_block(&self, block: ClosedBlock) -> OpenBlock { | |||
There was a problem hiding this comment.
this logic is specifically for the miner, so couldn't it be kept completely within there? client is bloated enough already
rather than fetching all valid uncles again, the miner could just check if any of the new headers in imported is a valid uncle.
There was a problem hiding this comment.
This is MiningBlockChainClient so it does indeed contain some mining-specific logic. See prepare_open_block above for example.
As for fetching all valid uncles - a chain ancestry iteration is required to check if an uncle is valid. It is actually faster to iterate once and check if uncle is already in the block.
I've added a small change to avoid reading headers unnecessarily though.
| .find_uncle_hashes(&h, engine.maximum_uncle_age()) | ||
| .unwrap_or_else(Vec::new); | ||
|
|
||
| for h in uncles { |
There was a problem hiding this comment.
I like the uncle-adding code in prepare_open_block more. Seems that the only difference is a check that uncle is already in the block, can't we use filter to get rid of such uncles in the list returned by find_uncle_hashes and re-use the same code in both methods?
There was a problem hiding this comment.
prepare_open_block uses find_uncle_headers and not find_uncle_hashes. Borrow checker won't allow using a single iterator sequence here because blocks would need to be used in two closures.
| @@ -1189,7 +1191,7 @@ impl MinerService for Miner { | |||
| transaction_queue.remove_old(&fetch_account, time); | |||
| } | |||
|
|
|||
| if enacted.len() > 0 { | |||
| if enacted.len() > 0 || (imported.len() > 0 && self.options.reseal_on_uncle) { | |||
There was a problem hiding this comment.
Wouldn't it cause spurious re-seals if the imported block cannot be a valid uncle?
There was a problem hiding this comment.
Such cases are extremely rare
ethcore/src/miner/miner.rs
Outdated
| @@ -1150,7 +1152,7 @@ impl MinerService for Miner { | |||
| }) | |||
| } | |||
|
|
|||
| fn chain_new_blocks(&self, chain: &MiningBlockChainClient, _imported: &[H256], _invalid: &[H256], enacted: &[H256], retracted: &[H256]) { | |||
| fn chain_new_blocks(&self, chain: &MiningBlockChainClient, imported: &[H256], _invalid: &[H256], enacted: &[H256], retracted: &[H256]) { | |||
| trace!(target: "miner", "chain_new_blocks"); | |||
|
|
|||
| // 1. We ignore blocks that were `imported` (because it means that they are not in canon-chain, and transactions | |||
There was a problem hiding this comment.
The comment is no longer valid.
gavofyork
added
A8-looksgood 🦄
* --reseal-on-uncle * Optimized uncle check * Additional uncle check * Updated comment
* --reseal-on-uncle (#5940) * --reseal-on-uncle * Optimized uncle check * Additional uncle check * Updated comment * v1.6.9 * CLI: Export error message and less verbose peer counter. (#5870) * Removed numbed of active connections from informant * Print error message when fatdb is required * Remove peers from UI
Closes #4785