Add a new RPC `parity_submitWorkDetail` similar `eth_submitWork` but return block hash #9404

YihaoPeng · 2018-08-23T08:19:30Z

Purpose

There is a problem with Ethereum's current RPC eth_submitWork: the miner cannot know the real block hash after a submission, so it is impossible to query the block from a blockchain browser or perform any subsequent automated processing.

So I added a new RPC parity_submitWorkDetail to Parity. It is similar to eth_submitWork but will return the block hash on success, and return an explicit error message on failure.

Params and Response

  // RPC parity_submitWorkDetail

  // Params (same as `eth_submitWork`):
       [
           "<nonce>",
           "<pow_hash>",
           "<mix_hash>"
       ]
   
  // Result on success:
       "block_hash"
  
  // Error on failure:
       {code: -32005, message: "Cannot submit work.", data: "<reason for submission failure>"}

Example:

call RPC with curl:

curl --data-binary '{"jsonrpc": "2.0", "method": "parity_submitWorkDetail", "params": ["0xccfb958c9f08dc52","0x86f29a6fb5d3fa0b8e32e9c1c1d53f2496a91d414d5707f7c88da39c109ceb66","0x95fc749e53339cd9187a58d28d686ef7b1e9c3261c1c47046ea2873e8bf36684"], "id": 5}' -H 'content-type: application/json' http://localhost:8545/

response with error message:

{"jsonrpc":"2.0","error":{"code":-32005,"message":"Cannot submit work.","data":"PoW hash is invalid or out of date."},"id":5}

response with block hash:

{"jsonrpc":"2.0","result":"0x07a992176ab51ee50539c1ba287bef937fe49c9a96dafa03954fb6fefa594691","id":5}

Personal Description

I am a complete Rust newcomer and this is the first time I read and change Rust's code. I have completed the function and tested it and it works fine. But I don't know if it is enough as a pull request.

If I need to do something else, please let me know.

Testing and Usage

I tested it on the Ropsten chain and the submission worked fine. (This is a very simple patch and I think it may not require further testing.)

Open source POW pool BTCPool uses this patch to record its submited block to database.

BTC.COM pool has mined 17 blocks in Ethereum Mainnet with its Parity node with this patch.

It has more details (block hash, error message, and more in future) in its response and not only the `true` or `false`.

parity-cla-bot · 2018-08-23T08:19:33Z

It looks like @YihaoPeng hasn't signed our Contributor License Agreement, yet.

The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence.
Wikipedia

You can read and sign our full Contributor License Agreement at the following URL: https://cla.parity.io

Once you've signed, please reply to this thread with [clabot:check] to prove it.

Many thanks,

Parity Technologies CLA Bot

YihaoPeng · 2018-08-23T08:23:11Z

[clabot:check]

parity-cla-bot · 2018-08-23T08:23:14Z

It looks like @YihaoPeng signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

sorpaas · 2018-08-23T09:47:00Z

The issue is the same as #9126 -- it's recommended to have an EIP first before modifying the eth_ jsonrpc namespace, in order to keep it tidy and to keep cross-client compatible.

However, if you want, you can change this PR to use parity_ namespace and we may be able to accept it!

sorpaas · 2018-08-23T09:52:33Z

rpc/src/v1/impls/eth.rs

@@ -801,6 +801,36 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM, T: StateInfo + 'static> Eth for EthClient<
 		}
 	}

+	fn submit_work_detail(&self, nonce: RpcH64, pow_hash: RpcH256, mix_hash: RpcH256) -> Result<SubmitDetailResult> {


And just a small grumble -- we can refactor L806 to L813 to a helper function to avoid duplicate codes with submit_work.

thanks for your advice. I have modified it.

YihaoPeng · 2018-08-23T10:03:58Z

Does I need to move codes from eth.rs to parity.rs, or just rename RPC is ok?

sorpaas · 2018-08-23T10:35:34Z

@YihaoPeng For parity_ namespace it would need to be moved to parity.rs.

YihaoPeng · 2018-08-23T10:41:23Z

OK, I will try to move the code.

The problem is that the implementation of this feature is in eth.rs and is now referenced by the implementation of eth_submitWork.

YihaoPeng · 2018-08-23T10:41:50Z

Opps, a wrong click... I'm sorry.

YihaoPeng · 2018-08-23T11:54:36Z

@sorpaas The code is moved to parity.rs and the RPC is renamed to parity_submitWorkDetail.

I reseted my master branch. The old code is here: master_submitwork

However, I cannot avoid to duplicate codes because my low Rust skills. 😅
I don't know how to call a function which in another trait and whether this good.

sorpaas

I realized that this new RPC is nearly identical to eth_submitWork, where the only change is the return type (from bool to H256). However, it indeed looks like a valid usecase and cannot be fit it into eth_submitWork without breaking existing users' code.

Not sure if there're better ways to handle this.

rpc/src/v1/impls/parity.rs

…failed

rpc/src/v1/impls/parity.rs

5chdn · 2018-09-30T13:05:41Z

Can you have another look at this @sorpaas @dvdplm :)

sorpaas

A final grumble and I think this should be ready for merge!

YihaoPeng · 2018-10-01T05:17:52Z

rpc/src/v1/impls/parity.rs

@@ -449,4 +450,26 @@ impl<C, M, U, S> Parity for ParityClient<C, M, U> where
 				.map(|res| res.into_iter().map(|res| res.output.into()).collect())
 				.map_err(errors::call)
 	}
+
+	fn submit_work_detail(&self, nonce: H64, pow_hash: H256, mix_hash: H256) -> Result<H256> {


I am trying to make such a change, but I am having trouble.
The question is what type of miner and client should be.
I noticed that in paraity.rs and eth.rs they are all defined as generics and two definitions are different.
I tried to use generics but I encountered an error like this:

pub fn submit_work_detail<C: BlockChainClient, M: MinerService>(client: C, miner: M, nonce: H64, pow_hash: H256, mix_hash: H256) -> Result<H256, Error> {...}

The trait bound `std::sync::Arc<C>: ethcore::miner::BlockChainClient` is not satisfied rpc/src/v1/impls/eth.rs:785:9 the trait `ethcore::miner::BlockChainClient` is not implemented for `std::sync::Arc<C>`

OK, I have got it. This works:

pub fn submit_work_detail<C: BlockChainClient, M: MinerService>(client: &Arc<C>, miner: &Arc<M>, nonce: H64, pow_hash: H256, mix_hash: H256) -> Result<H256, Error> {...}

5chdn · 2018-10-01T08:56:23Z

Please rebase on master

SwimmingTiger · 2018-10-02T15:18:52Z

I tried rebase but every commit needs to resolve the conflict. A merge or squash is obviously easier and can be done automatically. I think squashing is good because my refactor process is not important.

---- (@SwimmingTiger == @YihaoPeng)

sorpaas · 2018-10-02T15:46:43Z

@SwimmingTiger I think you'll just need to do a merge (git pull origin master). We squash merge PRs anyway so merge commits won't matter. :)

SwimmingTiger · 2018-10-02T16:42:47Z

OK, I have pushed the merging.

…mon-deps * origin/master: Add a new RPC `parity_submitWorkDetail` similar `eth_submitWork` but return block hash (#9404) Resumable EVM and heap-allocated callstack (#9360) update parity-wordlist library (#9682)

add a new RPC eth_submitWorkDetailsimilar eth_submitWork.

34ea01b

It has more details (block hash, error message, and more in future) in its response and not only the `true` or `false`.

sorpaas reviewed Aug 23, 2018

View reviewed changes

YihaoPeng closed this Aug 23, 2018

YihaoPeng reopened this Aug 23, 2018

move RPC submitWorkDetail from namespace eth_ to parity_

4456cd0

YihaoPeng force-pushed the master branch from 9a49396 to 4456cd0 Compare August 23, 2018 11:47

5chdn added this to the 2.1 milestone Aug 23, 2018

5chdn added A0-pleasereview 🤓 Pull request needs code review. M6-rpcapi 📣 RPC API. labels Aug 23, 2018

sorpaas reviewed Aug 24, 2018

View reviewed changes

rpc/src/v1/impls/parity.rs Outdated Show resolved Hide resolved

remove SubmitDetailResult type; submitWorkDetail return a error when …

a6d8eaa

…failed

YihaoPeng changed the title ~~Add a new RPC eth_submitWorkDetail similar eth_submitWork but response block hash~~ Add a new RPC parity_submitWorkDetail similar eth_submitWork but return block hash Aug 24, 2018

change error message of RPC error cannot_submit_work.

dc07275

shamardy mentioned this pull request Aug 24, 2018

Add option to include transaction receipts in eth_getBlockByHash #9075

Closed

dvdplm reviewed Aug 27, 2018

View reviewed changes

rpc/src/v1/impls/parity.rs Outdated Show resolved Hide resolved

remove double imported H256.

2fe6766

YihaoPeng force-pushed the master branch from 15bc64a to 2fe6766 Compare September 6, 2018 04:11

5chdn modified the milestones: 2.1, 2.2 Sep 11, 2018

DinoStray approved these changes Sep 14, 2018

View reviewed changes

sorpaas reviewed Sep 30, 2018

View reviewed changes

put submit_work_detail into a helper to avoid the duplicate codes.

55daec4

sorpaas approved these changes Oct 1, 2018

View reviewed changes

sorpaas added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Oct 1, 2018

NikVolf approved these changes Oct 2, 2018

View reviewed changes

Merge branch 'parity-master'

8f8978d

sorpaas merged commit 7ba5652 into openethereum:master Oct 2, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a new RPC `parity_submitWorkDetail` similar `eth_submitWork` but return block hash #9404

Add a new RPC `parity_submitWorkDetail` similar `eth_submitWork` but return block hash #9404

YihaoPeng commented Aug 23, 2018 •

edited

parity-cla-bot commented Aug 23, 2018

YihaoPeng commented Aug 23, 2018

parity-cla-bot commented Aug 23, 2018

sorpaas commented Aug 23, 2018

sorpaas Aug 23, 2018

YihaoPeng Aug 23, 2018

YihaoPeng commented Aug 23, 2018

sorpaas commented Aug 23, 2018

YihaoPeng commented Aug 23, 2018 •

edited

YihaoPeng commented Aug 23, 2018 •

edited

YihaoPeng commented Aug 23, 2018 •

edited

sorpaas left a comment

5chdn commented Sep 30, 2018

sorpaas left a comment

This comment was marked as resolved.

YihaoPeng Oct 1, 2018 •

edited

YihaoPeng Oct 1, 2018

5chdn commented Oct 1, 2018

SwimmingTiger commented Oct 2, 2018 •

edited

sorpaas commented Oct 2, 2018

SwimmingTiger commented Oct 2, 2018

Add a new RPC parity_submitWorkDetail similar eth_submitWork but return block hash #9404

Add a new RPC parity_submitWorkDetail similar eth_submitWork but return block hash #9404

Conversation

YihaoPeng commented Aug 23, 2018 • edited

Purpose

Params and Response

Example:

Personal Description

Testing and Usage

parity-cla-bot commented Aug 23, 2018

YihaoPeng commented Aug 23, 2018

parity-cla-bot commented Aug 23, 2018

sorpaas commented Aug 23, 2018

sorpaas Aug 23, 2018

Choose a reason for hiding this comment

YihaoPeng Aug 23, 2018

Choose a reason for hiding this comment

YihaoPeng commented Aug 23, 2018

sorpaas commented Aug 23, 2018

YihaoPeng commented Aug 23, 2018 • edited

YihaoPeng commented Aug 23, 2018 • edited

YihaoPeng commented Aug 23, 2018 • edited

sorpaas left a comment

Choose a reason for hiding this comment

5chdn commented Sep 30, 2018

sorpaas left a comment

Choose a reason for hiding this comment

This comment was marked as resolved.

YihaoPeng Oct 1, 2018 • edited

Choose a reason for hiding this comment

YihaoPeng Oct 1, 2018

Choose a reason for hiding this comment

5chdn commented Oct 1, 2018

SwimmingTiger commented Oct 2, 2018 • edited

sorpaas commented Oct 2, 2018

SwimmingTiger commented Oct 2, 2018

Add a new RPC `parity_submitWorkDetail` similar `eth_submitWork` but return block hash #9404

Add a new RPC `parity_submitWorkDetail` similar `eth_submitWork` but return block hash #9404

YihaoPeng commented Aug 23, 2018 •

edited

YihaoPeng commented Aug 23, 2018 •

edited

YihaoPeng commented Aug 23, 2018 •

edited

YihaoPeng commented Aug 23, 2018 •

edited

YihaoPeng Oct 1, 2018 •

edited

SwimmingTiger commented Oct 2, 2018 •

edited