Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Parity can accept older work packages #811

Merged
merged 20 commits into from Mar 28, 2016
Merged

Parity can accept older work packages #811

merged 20 commits into from Mar 28, 2016

Conversation

gavofyork
Copy link
Contributor

Utilise UsingQueue in order to push ClosedBlocks into a queue for (if used) later retrieval from work.

@gavofyork gavofyork added the A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. label Mar 24, 2016
@gavofyork gavofyork added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Mar 26, 2016
// TODO [todr] Should be moved to miner crate eventually.
fn prepare_sealing(&self, author: Address, gas_floor_target: U256, extra_data: Bytes, transactions: Vec<SignedTransaction>)
-> Option<(ClosedBlock, HashSet<H256>)> {
-> (Option<ClosedBlock>, HashSet<H256>) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the caller, we always want to know which transactions were invalid, even if there's no new block to mine.

@@ -339,6 +339,18 @@ impl fmt::Debug for State {
}
}

impl Clone for State {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this used? I implemented state snapshot specifically to avoid state cloning, which is quite expensive.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in block.rs, ClosedBlock and also in miner.rs. it's necessary so that we can reopen a ClosedBlock later, add a new transaction and possibly seal.

cloning should only be used in those two circumstances, both are only when mining. it's certainly necessary for allowing additional transactions in a block since a possibly arbitrary block-finalization state-change must be reverted. it may not strictly be necessary for the usage in miner.rs, but i don't particularly want to rewrite that code for some optimisation of as-yet unknown benefit.

@arkpar arkpar added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Mar 27, 2016
…cannot).

`ClosedBlock`s still keep the pre-finalised state (i.e. state after the last transaction).
`LockedBlock`s do not. New mining algo needs to reopen these `ClosedBlock`s, however enactment
system does not (and `ClosedBlock`s are slower & more hungry), hence the distinction.
@gavofyork gavofyork added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Mar 27, 2016
@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Mar 28, 2016
@gavofyork gavofyork added the A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. label Mar 28, 2016
@gavofyork gavofyork removed the A8-looksgood 🦄 Pull request is reviewed well. label Mar 28, 2016
@gavofyork gavofyork added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Mar 28, 2016
@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Mar 28, 2016
@gavofyork gavofyork merged commit 3ebfd01 into master Mar 28, 2016
@gavofyork gavofyork deleted the bettermining branch March 28, 2016 08:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants