Planning: Support for RPC calls used by mining pools

[mpguerra]

Let's try to plan out the getblocktemplate work asynchronously here.

So far it seems like we need to (in no particular order...):

• implement getblocktemplate RPC call
• implement submitblock RPC call
• run open-source mining pool coordination software to test against

Anything else?

[teor2345]

Here are some of the other things we might want to do:

• Add a scope to the epic
• Add a getblocktemplate-rpcs Rust feature to zebrad and zebra-rpc, so that production builds don't have untested RPCs in them
• Find all the block generation RPCs used by mining pool software (node wallet RPCs are out of scope):
  • I could only find this open-source pool software: https://github.com/s-nomp/s-nomp
• Find all the parameters and fields that are actually used by the mining pool software, we can skip unused parameters/fields to save development time
  • We did an initial analysis for getblocktemplate in https://hackmd.io/gjxd0c4oTp-ZB81PtHdU2w
  • The other RPCs are simple enough that we can just implement all the parameters and fields
• Run mining pool software in CI so we can do integration tests

Here are some tasks we could add to each ticket:

• Testing: RPC snapshot, s-nomp integration, and manual zcashd comparison

And here are some bugs we'll need to fix:

• Send committed spent OutPoints & nullifiers to the mempool for rejection #2631
• The block part of Limit generated block and transaction size #483
  • we might want to close this ticket and open a ticket with a smaller scope
  • Edit: this actually works already, it's part of Zebra's block parser

I've added a draft list of tasks to the epic.

[teor2345]

We might also want to do some design diagrams, so we know where each RPC's data is coming from.
That will help us find the changes we need to make in other Zebra crates. (For example, extra state data.)

Here is what we did for the lightwalletd RPCs:

Data flow:
https://docs.google.com/drawings/d/1F3qCpIhzqhCCfWot5YEvsIt9gixvlwZd2TQNqxcOyBk/edit

Testing:
https://docs.google.com/drawings/d/1GDqvm_yC_pVmJV88fMHI6TfZmb2X-vKNWNQhb66i_Gk/edit

We won't need to track task dependencies, because they are automatically generated now:
https://zcash.github.io/developers/zcash-zf-dag

But we'll want to work out how to integration test each RPC, here's a rough guess:

• getblocktemplate: run the mining pool software, pretend to be a miner (if needed)
• submitblock: like sendrawtransaction, but with blocks
  • the current test is very slow and heavy, because it copies the entire state, so we might want to do Get transactions from the non-finalized state in the send transactions test #5015 first
• getblocksubsidy - check against the coinbase transactions of some recent blocks (after subtracting miner fees)
• getblockhash - like getblockchaininfo's tip hash field, or check against the hashes of some recent blocks
• getblockcount - like getblockchaininfo's block count field

[teor2345]

We might also want to find out the order that s-nomp calls RPCs.

If we implement them in that order, then we'll get good test coverage every time we add an RPC. (And we won't get any surprise failures, which can happen if we implement an RPC that unblocks an RPC we've already implemented.)

[oxarbitrage]

In addition to the mining pool software i think we might need to define a mining software that will be used to connect to the pool and do the real mining in a local device. It seems any equihash miner will do it so we might want to chose the most popular inside open source options. One option seems to be https://github.com/tpruvot/ccminer/

[teor2345]

Yep, let's keep a list of known supported mining software, and the features we support for it?

[oxarbitrage]

From zcashd rpc documentation (https://zcash-rpc.github.io/) we can know that at least the following rpc methods will be needed:

[teor2345]

Some of these RPC methods are out of scope, because they are only used for miner payouts.

We're just focusing on getting blocks mined in mining pools for now.

[oxarbitrage]

My opinion is to not add s-nomp or other mining (pool) software to the CI at least not initially, testing manually is fine.

This is different than lightwalletd where we knew zecwallet was really using lightwalletd and so other wallet implementations. In addition, lightwalletd is developed by ECC and/or adityapk00 which are trusted zcash developers. We don't know if this software is actually used by zcash miners. It seems we picked s-nomp just because it is open source, if we add it to the CI we are committing ourselves (forever) to maintain our versions updated, fix if they make changes, fix if it stops working, etc.

Also, i also think our CI already do too much and it will keep growing (we want integration, prop tests, snapshot tests, etc for every new feature).

I am afraid that we could spend more time making this integration work and maintaining it in the future than actually implementing and testing the mining RPC calls.

Don't get me wrong, i think it can be ok to add s-nomp to CI if it is easy enough but i don't think we should plan based on s-nomp alone right now . Figure out what rpc methods s-nomp call and try to support them as part of the planning is fine but i think we definitely at least also support the mining zcashd methods (#5267 (comment))

[teor2345]

My opinion is to not add s-nomp or other mining (pool) software to the CI at least not initially, testing manually is fine.

I think we should limit the scope of our integration tests. For example, I don't think the sendrawtransaction lightwalletd test was worth the effort we put into it. And I wonder if keeping lwd cached state was worth it.

So I don't think we should be mining blocks or doing anything complicated or stateful in CI for getblocktemplate

But I think starting up s-nomp and getting it to make some calls is worthwhile. We can stop adding new checks with s-nomp if it gets difficult or unreliable.

For the other RPCs we're going to be testing manually, we'll need to work out:

• How can we make sure we manually test every PR that makes changes to RPCs?
• How do we know that we got the RPC parameters, required fields, and required error codes correct?
  (zcash-rpc-diff doesn't cover parameters or error codes)
• How do we get bug reports from mining pools quickly, so they don't give up on using Zebra?

This is particularly important for performance issues like #2689 and #5307, which are tricky to diagnose, so we might never get good user bug reports for them.

It seems we picked s-nomp just because it is open source, if we add it to the CI we are committing ourselves (forever) to maintain our versions updated, fix if they make changes, fix if it stops working, etc.

Yeah, it's not great, there isn't a lot of open-source equihash mining pool software, and pools keep the software they use secret.

We can turn off parts of our CI if they stop working or they aren't needed any more.

Also, i also think our CI already do too much and it will keep growing (we want integration, prop tests, snapshot tests, etc for every new feature).

I agree, we need to match our testing effort to the value we get from those tests.

Current CI workflows are too big, and that makes turning them off difficult.
We want to make new CI workflows much smaller, so we can turn them off more easily.

And if the problem is CI speed, we can:

• move tests for code that isn't being actively worked on to the main branch, like Only run the lightwalletd full sync on the main branch #5316
• make specific Rust test changes that make CI faster

(This feels like we might want a separate discussion for CI priorities for the team.)

I am afraid that we could spend more time making this integration work and maintaining it in the future than actually implementing and testing the mining RPC calls.

It's pretty normal for integration tests to take a significant amount of engineering time.

We did integration tests for lightwalletd because it helped us keep the scope of the required parameters, fields, and error codes really small.

If we don't do integration testing, we risk:

• doing a lot of unnecessary RPC implementation work,
• accidentally releasing software that doesn't work, which means we would lose users to zcashd, and
• spending a lot of time seeking out and diagnosing user bug reports.

[mpguerra]

We do want to do some integration testing. For me the question is whether we want to do it by running s-nomp in our CI e.g this is how this is tested in zcash https://github.com/zcash/zcash/blob/master/qa/rpc-tests/getblocktemplate.py

We will also need to ask mining pool operators to test it for us whilst making it clear that we're asking them to test an MVP (minimum viable product) and not a finished product. We want to get some version out there and into people's hands quickly for real world testing.

[mpguerra]

this work is already implemented