contrib/signet/miner updates

ajtowns commented at 4:12 pm on September 5, 2023: contributor

Refactors the code a bunch, and adds --poolnum / --poolid options so that signers can tag their coinbases in a way that explorers can recognise (see also https://github.com/bitcoin-data/mining-pools/pull/82 and https://github.com/mempool/mempool/issues/2903).

The refactoring in particular helps enable the “try using inquisition’s getblocktemplate, and if that doesn’t work fall back to core’s getblocktemplate” logic, as described/implemented in https://github.com/bitcoin-inquisition/bitcoin/pull/7

DrahtBot commented at 4:12 pm on September 5, 2023: 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	achow101, danielabrozzoni
Concept ACK	0xB10C

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#30130 (contrib/signet/miner: increase miner search space by edilmedeiros)
#29032 (signet: fixing mining for OP_TRUE challenge by Sjors)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

0xB10C commented at 6:11 am on September 7, 2023: contributor

Concept ACK

DrahtBot added the label CI failed on Jan 12, 2024

signet/miner: drop get_reward_address function 3aed0a4284

signet/miner: drop do_createpsbt function 16951f549e

signet/miner: drop create_coinbase function aac040b439

signet/miner: rename do_decode_psbt to decode_psbt 35f4631196

signet/miner: move next_block_* functions into new Generator class 5540e6ca49

signet/miner: add Generate.next_block_time function 85c5c0bea9

signet/miner: add Generate.gbt function 7b31332370

signet/miner: add Generate.mine function 409ab7d35b

signet/miner: add support for a poolnum/poolid tag in mined blocks 338a266a9a

ajtowns force-pushed on Feb 13, 2024

DrahtBot removed the label CI failed on Feb 13, 2024

maflcko requested review from kallewoof on Feb 22, 2024

kallewoof commented at 1:58 am on March 14, 2024: member

Sorry for delay. Code looks reasonable.

in contrib/signet/miner:336 in 338a266a9a outdated

354+        if not psbt_signed.get("complete",False):
355+            logging.debug("Generated PSBT: %s" % (psbt,))
356+            sys.stderr.write("PSBT signing failed\n")
357+            return None
358+        block, signet_solution = decode_psbt(psbt_signed["psbt"])
359+        return finish_block(block, signet_solution, grind_cmd)

edilmedeiros commented at 6:23 pm on May 23, 2024:

I have an open PR (#30130) with an associated issue (#30102) that touches this code. I’m still reviewing this PR, but in the meanwhile do you think it would be a good idea to rebase mine on top of this (or maybe open a pull request on this, whatever makes our lives easier)? As it is, I believe it can’t be cherry-picked.

In summary, finish_block will call bitcoin-util to grind PoW, which can fail if it exhausts the nonce search space. This will be (extremely) rarely seem in signets that keep difficulty low, but it will be quite common if someone wants to keep a high difficulty chain. It’s a recoverable failure and the fix is to detect that with a try/except block, change the timestamp and remine. At a first glance, this would be all contained in the new mine function.

ajtowns added this to the milestone 28.0 on Aug 7, 2024

in contrib/signet/miner:525 in 338a266a9a outdated

520@@ -501,6 +521,8 @@ def main():
521     cmds = parser.add_subparsers(help="sub-commands")
522     genpsbt = cmds.add_parser("genpsbt", help="Generate a block PSBT for signing")
523     genpsbt.set_defaults(fn=do_genpsbt)
524+    genpsbt.add_argument("--poolnum", default=None, type=int, help="Identify blocks that you mine")
525+    genpsbt.add_argument("--poolid", default=None, type=str, help="Identify blocks that you mine (eg: /signet:1/)")

achow101 commented at 10:57 pm on August 8, 2024:

In 338a266a9a08e47bc6dd02175c8fa649f701515d “signet/miner: add support for a poolnum/poolid tag in mined blocks”

Using a mutually exclusive group here and below will let the argument parser enforce that these are mutually exclusive rather than having to enforce it in get_poolid.

0    poolid_group = genpsbt.add_mutually_exclusive_group()
1    poolid_group.add_argument("--poolnum", default=None, type=int, help="Identify blocks that you mine")
2    poolid_group.add_argument("--poolid", default=None, type=str, help="Identify blocks that you mine (eg: /signet:1/)")

ajtowns commented at 4:26 pm on August 9, 2024:

Added a commit to use mutually exclusive groups here and in a few other places.

in contrib/signet/miner:264 in 85c5c0bea9 outdated

259+            logging.debug("Setting start time to %d", self.set_block_time)
260+            self.mine_time = self.set_block_time
261+            self.action_time = now
262+            self.is_mine = True
263+        elif bestheader["height"] == 0:
264+            time_delta = self.INTERVAL * 100 # plenty of time to mine 100 blocks

achow101 commented at 10:59 pm on August 8, 2024:

In 85c5c0bea9d45e93a9fb20988457480798d68637 “signet/miner: add Generate.next_block_time function”

Why change the time delta to use self.INTERVAL instead of using next_block_delta as previously?

ajtowns commented at 4:26 am on August 9, 2024:

next_block_delta can be somewhat small if you have a high difficulty target (ie 2.5m instead of 10m) and if --poisson is specified will be smaller again (the deterministic random calculation used against the genesis block reduces the target by ~73%). The --poisson result alone is already annoying – it will generally only take 25 blocks before it’s caught up to real time; but even without that if you switch from mining with a high nbits target to a minimal nbits target, then even without --poisson you’ll also fail to mine 100 blocks before catching up to real time.

signet/miner: Use argparse exclusive groups

Let argparse take care of making arguments make sense in more cases.

Co-Authored-By: Ava Chow <github@achow101.com>

fb6d51eb25

achow101 commented at 8:34 pm on August 9, 2024: member

ACK fb6d51eb25a2bb69a3ecdfc4796a88d4d1aacc65

DrahtBot requested review from 0xB10C on Aug 9, 2024

achow101 removed this from the milestone 28.0 on Aug 15, 2024

achow101 added this to the milestone 29.0 on Aug 15, 2024

danielabrozzoni approved

danielabrozzoni commented at 10:02 am on August 16, 2024: contributor

Code review ACK fb6d51eb25a2bb69a3ecdfc4796a88d4d1aacc65

achow101 merged this on Sep 4, 2024

achow101 closed this on Sep 4, 2024

Sjors commented at 6:41 am on September 5, 2024: member

If you liked this PR, you may also like the freshly rebased #29032!

contrib/signet/miner updates #28417

Code Coverage

Reviews

Conflicts