Add waitFeesChanged() to Mining interface

Sjors commented at 6:15 pm on September 30, 2024: member

The Stratum v2 protocol allows pushing out templates as fees in the mempool increase. This interface lets us know when it’s time to do so.

I split the implementation into two parts because I’m not sure if we should include the second commit now, or wait until cluster mempool #30289 is merged.

The first commit contains a mock implementation very similiar to how longpolling in getblocktemplate works. It calls getTransactionsUpdated once per second. This returns true anytime a transaction is added to the mempool, even if it has no impact on fees in the best block.

The second commit creates a new block template so it can actually measure the fees. It’s slightly faster getNewBlock() because it skips verification.

On my 2019 Intel MacBook Pro it takes about 20ms to generate a block template. The waitFeesChanged() waits 1 second between each CreateNewBlock call so as to not burden the node too much. But on (very) low end hardware this may still be problematic.

The second commit does not change the interface, so it could be left out if people prefer that. I’m not sure what performance increase to expect from cluster mempool.

The interface intentionally does not return the resulting template, so that the implementation can be optimized further. Downside is that the caller needs to do another getNewBlock().

The changes here can use to make longpolling getblocktemplate more efficient, by only returning the RPC call when template fees actually increased. However this PR does not touch that RPC code.

Introduce waitFeesChanged() mining interface a87589abfd

Have waitFeesChanged() frequently make a template 9a723c784a

DrahtBot commented at 6:15 pm on September 30, 2024: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	danielabrozzoni

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Sjors renamed this:
~~Introduce waitFeesChanged() mining interface~~
Add waitFeesChanged() to Mining interface
on Sep 30, 2024

in src/ipc/capnp/mining.capnp:20 in a87589abfd outdated

16@@ -17,10 +17,11 @@ interface Mining $Proxy.wrap("interfaces::Mining") {
17     isInitialBlockDownload @1 (context :Proxy.Context) -> (result: Bool);
18     getTip @2 (context :Proxy.Context) -> (result: Common.BlockRef, hasResult: Bool);
19     waitTipChanged @3 (context :Proxy.Context, currentTip: Data, timeout: Float64) -> (result: Common.BlockRef);
20-    createNewBlock @4 (scriptPubKey: Data, options: BlockCreateOptions) -> (result: BlockTemplate);
21-    processNewBlock @5 (context :Proxy.Context, block: Data) -> (newBlock: Bool, result: Bool);
22-    getTransactionsUpdated @6 (context :Proxy.Context) -> (result: UInt32);
23-    testBlockValidity @7 (context :Proxy.Context, block: Data, checkMerkleRoot: Bool) -> (state: BlockValidationState, result: Bool);
24+    waitFeesChanged @4 (context :Proxy.Context, currentTip: Data, feeThreshold: Int64, options: BlockCreateOptions, timeout: Float64) -> (result: Bool);

Sjors commented at 6:31 pm on September 30, 2024:

a87589abfde4f2f1dfba0f3f1f5745f7acfef365: @ryanofsky or should I make this @9 and leave the other numbers untouched?

ryanofsky commented at 7:39 pm on September 30, 2024:

a87589a: @ryanofsky or should I make this @9 and leave the other numbers untouched?

You can but it shouldn’t matter, as long as nothing outside the build is depending on the interface. So far I’ve just renumbered everything like your current PR does. I even have a hacky renumbering script capnp-renum, that I pipe iinto from vim to renumber structs and interfaces:

 0#!/usr/bin/env python
 1
 2import sys
 3import re
 4
 5next = 0 if len(sys.argv) < 2 else int(sys.argv[1])
 6
 7def f(m):
 8  global next
 9  next += 1
10  return "@" + str(next - 1)
11
12sys.stdout.write(re.sub(r"@\d+", f, sys.stdin.read()))

Sjors commented at 7:22 am on October 1, 2024:

Ok, well the numbering here doesn’t conflict with #30955 and there’s no other proposed interface change pending. So I’ll keep the renumbering here.

danielabrozzoni commented at 1:37 pm on October 1, 2024: contributor

Code review ACK 9a723c784ad5170b1e01e91f018e5794a51dd038 - code looks good to me, I don’t have an opinion on merging both commits vs waiting for cluster mempool for the second one.

Add waitFeesChanged() to Mining interface #31003

Code Coverage

Reviews