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

Mining #547

Merged
merged 22 commits into from Mar 2, 2016
Merged

Mining #547

merged 22 commits into from Mar 2, 2016

Conversation

gavofyork
Copy link
Contributor

  • Implemented three functions in JSONRPC for mining, updated a couple more.
  • Added version_data function to misc to get the standard RLP version data.
  • Added mining to Client class; whenever the best block changes, or work is queried and no ClosedBlock exists, a new ClosedBlock is authored as a child of the best block. For 1.0 this somewhat monolithic design is ok; with Civility we will refactor this into a separate crate & process.
  • ClosedBlock no longer retains an Engine reference; reopen() thus doesn't work in its old form - nothing was using it so removing is fine. seal() used the Engine reference. This is now supplied as a parameter. This allows us to hold onto a ClosedBlock within the Client as the "currently sealing" block without having to worry about the Engine's lifetime.
  • Added try_seal function to ClosedBlock to check whether seal is any good (seal does something similar but doesn't check the seal).
  • Added verify_block_seal to Engine.
  • Exposed a few methods from ethash module to aid in work provision/collection.
  • Lowercase v of Unverified and Preverified to stay consistent with rest of codebase.
  • Added U256 <-> U512 conversions with pass-by-reference.
  • Update JSON tests and re-enabled transition.

Still todo:

  • Search for and add all uncles to the mined block.
  • Add transactions into the block (blocking on TransactionQueue merge).
  • Avoid reprocessing the block for import.

@gavofyork gavofyork added the A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. label Feb 29, 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 1, 2016
@@ -403,8 +417,79 @@ impl Client {
BlockId::Latest => Some(self.chain.read().unwrap().best_block_number())
}
}

/// Set the author that we will seal blocks as.
Copy link
Contributor

Choose a reason for hiding this comment

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

"Get the author" probably

}

/// Get the standard version data for this software.
pub fn version_data() -> Bytes {
Copy link
Contributor

Choose a reason for hiding this comment

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

no test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

given the output is based on environmental factors outside the control of the test environment, any test would simply be a copy'n'paste of the code in the function.

Copy link
Contributor

Choose a reason for hiding this comment

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

still I believe we need one so that further possible modifications of this code are introduced with confidence

}

/// Grab the `ClosedBlock` that we want to be sealed. Comes as a mutex that you have to lock.
pub fn sealing_block(&self) -> &Mutex<Option<ClosedBlock>> {
Copy link
Contributor

Choose a reason for hiding this comment

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

no test

@NikVolf NikVolf added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. and removed A0-pleasereview 🤓 Pull request needs code review. labels Mar 1, 2016
@NikVolf
Copy link
Contributor

NikVolf commented Mar 1, 2016

though it possibly works in this state, no tests actually prove this

&Ethash::to_ethash(header.mix_hash()))));
&Ethash::to_ethash(header.bare_hash()),
header.nonce().low_u64(),
&Ethash::to_ethash(header.mix_hash()) )));
Copy link
Contributor

Choose a reason for hiding this comment

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

redundant whitespace

@gavofyork
Copy link
Contributor Author

will add tests for the Client struct.

eth.rs will have to be a separate PR - #561 created to track it.

@gavofyork gavofyork added A0-pleasereview 🤓 Pull request needs code review. and removed A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. labels Mar 1, 2016
@NikVolf NikVolf added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Mar 2, 2016
gavofyork pushed a commit that referenced this pull request Mar 2, 2016
@gavofyork gavofyork merged commit 162300a into master Mar 2, 2016
@gavofyork gavofyork deleted the mining branch March 2, 2016 12:01
@gavofyork gavofyork mentioned this pull request Mar 5, 2016
4 tasks
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