Add reseal max period #4903
Add reseal max period #4903
Conversation
keorn
added
A0-pleasereview 🤓
gavofyork
added
A8-looksgood 🦄
| @@ -482,6 +489,7 @@ impl Miner { | |||
| // Save proposal for later seal submission and broadcast it. | |||
| Seal::Proposal(seal) => { | |||
| trace!(target: "miner", "Received a Proposal seal."); | |||
| *self.next_mandatory_reseal.write() = Instant::now() + self.options.reseal_max_period; | |||
There was a problem hiding this comment.
Just a question: do "partially-sealed" blocks awaiting PoW submission follow this (Seal::Proposal) code path?
There was a problem hiding this comment.
PoW blocks awaiting work are not sealed. So PoW only uses the Seal::Regular code path.
Both PoW blocks and proposal blocks go into the "work" queue to be later released with a final seal.
There was a problem hiding this comment.
Then when mining Ethash chains, the next_mandatory_reseal will only be updated when you're actually successful in producing a solution? It's quite rare that you would do that every 2 minutes, so wouldn't miners end up periodically discarding non-stale candidates because of the max reseal period?
There was a problem hiding this comment.
Indeed, it would start sealing on transactions after 2 min. Either can make this period very large or make it specific for internal sealing, which I think makes more sense.
There was a problem hiding this comment.
Why not just reset the period on prepare_sealing_work?
There was a problem hiding this comment.
Sealing may sometimes fail when the Engine does not generate a seal, so it would be good to keep trying.
keorn
added
the
B7-releasenotes 📜
Useful for chains where you do not want block bloat but want to see blocks from time to time to establish liveness.