change(rpc): Provide and parse a long poll ID, but don't use it yet

[teor2345]

Motivation

We want to implement long polling support in Zebra.

Depends-On: #5772

Specifications

getblocktemplate request:

capabilities: miners which support long polling SHOULD provide a list including the String "longpoll"
longpollid: "longpollid" of job to monitor for expiration; required and valid only for long poll requests

getblocktemplate response:

longpollid: identifier for long poll request; MUST be omitted if the server does not support long polling

https://en.bitcoin.it/wiki/BIP_0022#Optional:_Long_Polling

Designs

Add a LongPollInput type which contains all the data long polling can depend on.
Lossily convert it into a LongPollId type, which almost always changes when the LongPollInput changes.
Convert the LongPollId type to and from a string in the getblocktemplate RPC.

Solution

RPC request:

• Declare longpoll support in the getblocktemplate RPC capabilities
• Provide a longpollid field in the RPC response, which represents the contents of the tip and mempool used to create the response.
• Accept a longpollid parameter to the RPC request, but don't implement long polling yet.

RPC server:

• Increase the default number of RPC threads when the getblocktemplate-rpcs feature is on, because every long polling client could use one thread.

This is part of #5720.

Review

I'd like an initial review of the design and the data we're using, before I add long polling waits and tests in the next PR.

Reviewer Checklist

• Will the PR name make sense to users?
  • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
• Are the PR labels correct?
• Does the code do what the ticket and PR says?
• How do you know it works? Does it have tests?

Follow Up Work

• Actually wait when long polling
• Add the submit_old field
• Check if jsonrpc_core blocks when a RPC waits
• Add tests

[codecov]

Codecov Report

Merging #5796 (7f0a612) into main (77b85cf) will decrease coverage by 0.06%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5796      +/-   ##
==========================================
- Coverage   78.82%   78.75%   -0.07%     
==========================================
  Files         308      308              
  Lines       38742    38747       +5     
==========================================
- Hits        30539    30517      -22     
- Misses       8203     8230      +27     

[arya2]

What's the advantage of this design over tip hash + mempool version?

[arya2]

Alternative proposal: we could generate the template with a uuid in a different task when there's a tip change or mempool update then return the uuid as the longpollid.

[teor2345]

What's the advantage of this design over tip hash + mempool version?

This is a partial implementation of a tip hash + mempool design, without the part where we check if the long poll id is the same, and wait to return the result.

zcashd's long poll id implementation effectively places two restrictions on us:

• must use hex digits
• must be less than or equal to 69 hex digits

We might want to have exactly 69 digits, or we might want fewer, so our IDs can't be confused. I've gone for fewer here, we can always pad later if we find out that's needed during testing.

A block hash is 64 hex digits, but we also want to include the max time, so we can send a new template if there are no transaction changes in the mempool, and the template expires. We also need to fit some kind of mempool transaction content marker into the 69 digits.

There can be multiple transactions in the mempool, so we need some way of condensing their hashes into a smaller number of bytes. And once we have that, we might as well use it on the block hash.

The tip height and mempool transaction count are optional, but they are useful for avoiding duplicate checksums, and for debugging. We have space to write these fields and the time as decimal, which makes them more readable.

[teor2345]

Alternative proposal: we could generate the template with a uuid in a different task when there's a tip change or mempool update then return the uuid as the longpollid.

I think I see how this could work, but how would we avoid generating templates when there were no requests?

This also adds concurrency, which increases the complexity of this change. So if we can do it without concurrency, I'd like to try that first.

[teor2345]

I need to handle parsing the long poll id correctly, it should be valid here:

---- methods::tests::vectors::rpc_getblocktemplate stdout ----

The application panicked (crashed).
Message:  assertion failed: `(left == right)`
  left: `ServerError(-10)`,
 right: `InvalidParams`
Location: zebra-rpc/src/methods/tests/vectors.rs:1001

https://github.com/ZcashFoundation/zebra/actions/runs/3625573409/jobs/6114169640#step:3:3563

[arya2]

but how would we avoid generating templates when there were no requests?

Tower service driven by polling!

This also adds concurrency, which increases the complexity of this change. So if we can do it without concurrency, I'd like to try that first.

I see your point. We could also do a uuid with a cached template and check the max_time there.

[teor2345]

This also adds concurrency, which increases the complexity of this change. So if we can do it without concurrency, I'd like to try that first.

I see your point. We could also do a uuid with a cached template and check the max_time there.

I'd like to try for the simplest working implementation first, and then make it more efficient.

[teor2345]

Just a rebase and some minor tidy-ups.

[teor2345]

And finally a fix to the config file and its tests.

[arya2]

Looks good, I like that you used a checksum of the tip hash instead of the full hash.

[str4d]

zcashd's long poll id implementation effectively places two restrictions on us:

• must use hex digits
• must be less than or equal to 69 hex digits

Does it? The only restriction I see is that it must be encoded as a JSON string.

A long poll ID is effectively an identifier for the returned template, so the next call can know what you are currently using, and therefore what template state to measure "is updated" against. It therefore only makes sense in the context of the node instance that generated it, because only that instance can remember what the template was given the current API. Long poll IDs aren't even portable across different instances of the same implementation (zcashd's nTransactionsUpdatedLastLP is derived from local state transitions, not global).

So unless there's something else in (Bitcoin Core's partial implementation of) BIP 22 that constrains it (which appears to not be the case; in fact "clients must not assume the long poll ID has any specific meaning" is stated) then you can do whatever you want here.

[arya2]

The only restriction I see is that it must be encoded as a JSON string.

That's my understanding as well, I don't think it could be used as anything besides an id.

This should work either way.

[teor2345]

So unless there's something else in (Bitcoin Core's partial implementation of) BIP 22 that constrains it (which appears to not be the case; in fact "clients must not assume the long poll ID has any specific meaning" is stated) then you can do whatever you want here.

While this is technically true according to the spec, there is nothing stopping client implementations:

• using a 69 byte array for the long poll id
• checking the length of the long poll id
• checking the format of the long poll id

So if we want to be maximally compatible with existing clients, we should avoid going over 69 bytes, and avoid using non-hex characters.

[teor2345]

There's also no guarantee that any incompatibilities will reliably cause errors. In a memory-unsafe language, it's possible to write a few bytes past a 69 byte array without detecting it, depending on the allocator and memory pressure.