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

Epic: Support for RPC calls used by mining pools #5234

Closed
32 of 34 tasks
mpguerra opened this issue Sep 22, 2022 · 8 comments
Closed
32 of 34 tasks

Epic: Support for RPC calls used by mining pools #5234

mpguerra opened this issue Sep 22, 2022 · 8 comments
Labels
C-enhancement Category: This is an improvement Epic Zenhub Label. Denotes a theme of work under which related issues will be grouped

Comments

@mpguerra
Copy link
Contributor

mpguerra commented Sep 22, 2022

Motivation

We would like mining pools to be able to use zebrad to:

  1. generate a block template for mining, and
  2. broadcast a newly-mined block to the Zcash network.

Our use case assumption is that the miner will use Zebra to generate the block template, mine the block outside of Zebra, then use Zebra to broadcast the block to the Zcash network.

Miners should be able to use Zebra as a drop-in replacement for zcashd (i.e. the interface with Zebra should be compatible with zcashd’s).

Prerequisites

We did an initial scope and design in Zebra Mining RPC scope.pdf.

We also want to do this analysis and design work:

  • Get a list of all RPCs called by the s-nomp mining pool software to mine blocks (excluding payments), and where they are called from
  • Work out which parameters and fields we need for each RPC
  • Create a data flow diagram that shows how Zebra handles each RPC, so we know if we need state, mempool, or verifier changes
  • Decide which tests we need and which tests are out of scope

Scope

To begin with we would like to build an MVP (minimum viable product) that will allow mining pools to start using zebra. We believe that this will require us to support the following RPC calls:

Implement ZIP 317:

Fix the following bugs:

And test the RPCs (See #5937)

We also want to disable these RPCs in production zebrad builds, until they have been tested:

Once we have an MVP we would ideally engage with some mining pools to test it out and provide feedback.

Optional Tasks

These RPCs might be needed by some mining pools, but we're not sure yet:

These changes increase miner profits, but they are not required:

Speed up verification of new blocks, to decrease orphaned blocks and lost subsidies:

Speed up RPC responses:

Allow transactions that spend UTXOs created in the same block, to increase fees:

  • Send mempool UTXOs in verifier requests
  • Sort transactions that create UTXOs before transactions that spend them in getblocktemplate
  • This might not happen much in Zcash, we should check before we implement it

Out of Scope

  • Mining functionality
  • Mine blocks on testnet as part of automated testing and/or CI
    • Deploy (non-mining) testnet nodes each time we merge to main or tag a release
  • Node wallet RPCs used by mining pools or miners to manage funds (e.g. receiving block rewards, making mining pool payouts)
@mpguerra mpguerra added C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage Epic Zenhub Label. Denotes a theme of work under which related issues will be grouped labels Sep 22, 2022
@mpguerra mpguerra added this to Zebra Sep 22, 2022
@mpguerra mpguerra moved this to 🆕 New in Zebra Sep 22, 2022
@teor2345
Copy link
Contributor

@mpguerra you can see the full list of required RPCs and Zebra bug fixes here:
https://hackmd.io/gjxd0c4oTp-ZB81PtHdU2w

This is a draft list, it won't be final until we've found some open-source mining pool coordination software to test against.

@teor2345 teor2345 changed the title Epic: Support for RPC calls used by miners Epic: Support for RPC calls used by mining pools Sep 26, 2022
@mpguerra
Copy link
Contributor Author

So is the planning done for this already or do we need to do some more?

I'm keen to try to do this asynchronously using github discussions. I've started one for this purpose here: #5267

If it is not needed because all of the planning is done we can just close it...

@teor2345
Copy link
Contributor

So is the planning done for this already or do we need to do some more?

Planning is incomplete, I've added some missing tasks to this ticket and the discussion.

@teor2345
Copy link
Contributor

teor2345 commented Oct 5, 2022

@mpguerra I added a list of optional tasks that will increase miner profits (but I didn't create tickets yet).

@teor2345
Copy link
Contributor

teor2345 commented Oct 24, 2022

I checked s-nomp to see which RPCs it actually uses:

Current scope:

  • getblocktemplate - s-nomp and node-stratum-pool
  • submitblock - node-stratum-pool
  • getblockcount - node-stratum-pool
  • getblocksubsidy - might be optional in node-stratum-pool, falls back to getblocktemplate
  • getblockhash - no

Other mining RPCs:

Might be optional:

Not required:

  • prioritisetransaction - not used by s-nomp or node-stratum-pool
  • getlocalsolps, getnetworkhashps, getnetworksolps - not used by s-nomp or node-stratum-pool

We should also check what RPCs other mining pools use.

@mpguerra
Copy link
Contributor Author

For an MVP, I would stick to the RPCs that are actually in the list of mining RPCs in the rpc docs, so:

Other mining RPCs:

* [getinfo](https://zcash.github.io/rpc/getinfo.html) - uses fields that Zebra hasn't implemented yet
  
  * https://github.com/s-nomp/node-stratum-pool/blob/d86ae73f8ff968d9355bb61aac05e0ebef36ccb5/lib/pool.js#L441

This does not seem like it's strictly mining related

* [getblock(verbosity=1)](https://zcash.github.io/rpc/getblock.html) - uses fields that Zebra hasn't implemented yet
  
  * https://github.com/s-nomp/node-stratum-pool/blob/d86ae73f8ff968d9355bb61aac05e0ebef36ccb5/lib/pool.js#L710

This does not seem like it's strictly mining related

* [validateaddress](https://zcash.github.io/rpc/validateaddress.html) - `s-nomp` and `node-stratum-pool`
  
  * https://github.com/s-nomp/s-nomp/blob/8567a4c649ab6f5ea37ac3c3901f2d9c2597199a/libs/poolWorker.js#L140
  * https://github.com/s-nomp/node-stratum-pool/blob/d86ae73f8ff968d9355bb61aac05e0ebef36ccb5/lib/pool.js#L388

This does not seem like it's strictly mining related

* [getdifficulty](https://zcash.github.io/rpc/getdifficulty.html) and [getmininginfo](https://zcash.github.io/rpc/getmininginfo.html) - `node-stratum-pool`
  
  * https://github.com/s-nomp/node-stratum-pool/blob/d86ae73f8ff968d9355bb61aac05e0ebef36ccb5/lib/pool.js#L388

getdifficulty does not seem like it's strictly mining related, getmininginfo does seem to be mining related, we should probably add this one to the list

Might be optional:

* [getpeerinfo](https://zcash.github.io/rpc/getpeerinfo.html) - only used for sync progress
  
  * https://github.com/s-nomp/node-stratum-pool/blob/d86ae73f8ff968d9355bb61aac05e0ebef36ccb5/lib/pool.js#L173

* [getnetworkinfo](https://zcash.github.io/rpc/getnetworkinfo.html) - only used for payment processing, or when getinfo is not available:
  
  * https://github.com/s-nomp/s-nomp/blob/1fe93a6fabfd35a542e3c0fa606c3728bedaad9e/libs/paymentProcessor.js#L380
  * https://github.com/s-nomp/node-stratum-pool/blob/d86ae73f8ff968d9355bb61aac05e0ebef36ccb5/lib/pool.js#L397

Again, none of these seem to be strictly mining related

Not required:

* prioritisetransaction - no

* getlocalsolps, getnetworkhashps, getnetworksolps - no

We should also check what RPCs other mining pools use.

These are in the list in the docs so how do we know that they are not required? Are they not used by s-nomp? If so I'm happy to exclude from the MVP.

@teor2345
Copy link
Contributor

For an MVP, I would stick to the RPCs that are actually in the list of mining RPCs in the rpc docs

How can we be sure that the RPC docs are accurate for Zcash mining?

As far as I can see, they contain a lot of legacy RPCs that aren't used for Zcash, or aren't required when using getblocktemplate. Some of the documentation and fields of individual RPCs are also out of date.

If we don't implement the RPCs it expects, the mining pool software will fail to run. Can we set the scope based on the mining pool software code or discussions with engineers, rather than outdated documentation?

These are in the list in the docs so how do we know that they are not required? Are they not used by s-nomp? If so I'm happy to exclude from the MVP.

prioritisetransaction, getlocalsolps, getnetworkhashps, and getnetworksolps are not used by s-nomp or its mining pool dependency node-stratum-pool. This is another reason I think the documentation is outdated.

@mpguerra
Copy link
Contributor Author

All done here 👏 🎉

@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Zebra Feb 23, 2023
@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: This is an improvement Epic Zenhub Label. Denotes a theme of work under which related issues will be grouped
Projects
Archived in project
Development

No branches or pull requests

2 participants