Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BESU-1919] Add proper support for eth_hashrate #1063

Merged
merged 15 commits into from
Jun 15, 2020

Conversation

matkt
Copy link
Contributor

@matkt matkt commented Jun 9, 2020

PR description

  • Implemented submitHashrate endpoint which is by default limited to 1000 sealers. Adding a 1001 will remove the oldest from the list (0x0 hashrate is refused)
  • Updated eth_hashrateendpoint so that it returns the cumulative hashrate of all sealers if the list is not empty. Otherwise it returns the local hashrate
  • Added hashrate submission with Stratum1EthProxyProtocol and Stratum1Protocol
  • Added CLI Option to modify the number of sealers limit

Tested

  • Tested the hashrate submission with stratum
  • Tested the hashrate submission with json rpc api
{"id":1,"jsonrpc":"2.0","method":"eth_submitHashrate","params":["0x00","0x01"]}
{"id":1,"jsonrpc":"2.0","method":"eth_submitHashrate","params":["0x01","0x01"]}
  • Tested with CPU and GPU mining
  • Tested submission TTL

@matkt matkt added the doc-change-required Indicates an issue or PR that requires doc to be updated label Jun 9, 2020
@matkt matkt linked an issue Jun 9, 2020 that may be closed by this pull request
matkt added 2 commits June 9, 2020 17:27
Signed-off-by: Karim TAAM <[email protected]>
Signed-off-by: Karim TAAM <[email protected]>
@matkt matkt marked this pull request as ready for review June 9, 2020 16:08
Copy link
Contributor

@shemnon shemnon left a comment

Choose a reason for hiding this comment

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

I think we need to move the hashrate logic out of the AbstractMiningCoordinator into EthashMiningCoordinator and not expose the SealerInfo map and instead expose the aggregate hashrate.

@@ -215,4 +232,28 @@ public void setExtraData(final Bytes extraData) {

protected abstract boolean newChainHeadInvalidatesMiningOperation(
final BlockHeader newChainHeadHeader);

@Override
public Map<String, SealerInfo> getSealerInfos() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be better to move all the logic around submitting hashrate and storing the sealerInfo down into the EthashMiningCoordinator. IBFT and Clique will never use these.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@MadelineMurray
Copy link
Contributor

Raised hyperledger/besu-docs#414 to doc this so this can be closed.

@matkt matkt deleted the feature/besu-1919-eth-hashrate branch June 29, 2020 08:30
@bgravenorst bgravenorst removed the doc-change-required Indicates an issue or PR that requires doc to be updated label Mar 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add proper support for eth_hashrate
4 participants