signet mining utility

ajtowns commented at 10:38 pm on September 10, 2020: member

Adds contrib/signet/miner for mining signet blocks.

Adds bitcoin-util cli utility, with the idea being it can provide bitcoin related functionality that does not rely on the ability to access a running node. Only subcommand currently is “grind” which takes a hex-encoded header and grinds its nonce until its nBits is satisfied.

Updates getblocktemplate to include signet_challenge field, and makes getblocktemplate require the signet rule when invoked on the signet change. Removes connectivity and IBD checks from getblocktemplate when applied to a test chain (regtest, testnet, signet).

DrahtBot added the label Build system on Sep 10, 2020

DrahtBot added the label GUI on Sep 10, 2020

DrahtBot added the label Mining on Sep 10, 2020

DrahtBot added the label RPC/REST/ZMQ on Sep 10, 2020

DrahtBot added the label Scripts and tools on Sep 10, 2020

DrahtBot added the label Utils/log/libs on Sep 10, 2020

DrahtBot added the label Validation on Sep 10, 2020

DrahtBot commented at 1:39 pm on September 19, 2020: member

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

Conflicts

No conflicts as of last run.

ajtowns force-pushed on Sep 21, 2020

ajtowns removed the label GUI on Sep 21, 2020

ajtowns removed the label Validation on Sep 21, 2020

ajtowns marked this as ready for review on Sep 21, 2020

ajtowns commented at 10:48 pm on September 21, 2020: member

EDIT: the following is no longer accurate, see contrib/signet/README.md instead

Some examples:

Generate blocks indefinitely, at 10 minute intervals, paying block reward to the given address, using bitcoin-util to do multi-threaded proof-of-work: $ ./contrib/signet/generate.py --cli='./bitcoin-cli' generate 10 --block-time=600 --address="tb1..." --grind-cmd='./bitcoin-util grind'
Generate 10 blocks, at 10 minute intervals, paying block reward to the given address, with timestamps starting at the genesis block, using single-threaded python code to do proof-of-work: $ ./contrib/signet/generate.py --cli='./bitcoin-cli' generate 10 --block-time=600 --address="tb1..." --backdate 0
Generate blocks indefinitely at 30 minute intervals, but only if there hasn’t been a block in the meantime, paying the reward for block height X to the X’th address from the given descriptor: $ ./contrib/signet/generate.py --cli='./bitcoin-cli' generate -1 --block-time=1800 --descriptor="wpkh(...)#..." --secondary

Using a “–block-time=600” will get blocks exactly on 10 minute boundaries, with very little variation. You can alternatively use --mining-time=20 to target spending an average of 20 seconds of cpu time per block, which should get a mainnet-like distribution of block times once difficulty has stabilised, assuming only one miner.

You can also generate blocks more manually by running:

./bitcoin-cli -signet getblocktemplate '{"rules": ["signet","segwit"]}' | ../contrib/signet/generate.py --cli='./bitcoin-cli' genpsbt --address=tb1... – to generate a psbt, encoding the signet txs and the raw block
./bitcoin-cli -signet -stdin walletprocesspsbt to sign the psbt from stdin, or some other method of signing a psbt
../contrib/signet/generate.py solvepsbt --grind-cmd='./bitcoin-util grind' | ./bitcoin-cli -signet submitblock to convert the signed psbt into a signet commitment and submit the block

Using the --cli option to specify ./bitcoin-cli -conf=/path/to/custom-signet.conf and having signet=1, signetchallenge and a custom datadir all specified in the conf file is probably sensible.

MarcoFalke removed the label Build system on Sep 22, 2020

MarcoFalke removed the label Mining on Sep 22, 2020

MarcoFalke removed the label RPC/REST/ZMQ on Sep 22, 2020

in contrib/signet/generate.py:4 in e077aa7055 outdated

0@@ -0,0 +1,422 @@
1+#!/usr/bin/env python3
2+# Copyright (c) 2010 ArtForz -- public domain half-a-node
3+# Copyright (c) 2012 Jeff Garzik
4+# Copyright (c) 2010-2020 The Bitcoin Core developers

MarcoFalke commented at 1:34 pm on September 22, 2020:

0# Copyright (c) 2020 The Bitcoin Core developers

:eyes:

ajtowns commented at 4:33 pm on September 22, 2020:

Hmm, think those are leftover from when the code from test_framework/messages.py etc was copied in rather than imported

in src/bitcoin-util.cpp:158 in e077aa7055 outdated

158+            --argc;
159+            ++argv;
160+        }
161+
162+        char* command = argv[1];
163+        if (strcmp(command, "grind") == 0) {

MarcoFalke commented at 1:40 pm on September 22, 2020:

Any reason to not use our ArgsManager here, so that -help documentation will be created?

ajtowns commented at 4:34 pm on September 22, 2020:

I don’t think ArgsManager supports subcommands that have different suboptions?

MarcoFalke commented at 4:49 pm on September 22, 2020:

It would support -grind=block_header_hex. Is something else needed here that I am missing?

ajtowns commented at 6:16 pm on October 1, 2020:

I was thinking it would make sense for bitcoin-util would have subcommands like git, each with their own different options (like --amend for git commit vs --continue for git rebase). bitcoin-util grind could have a -jobs=8 argument specifying how many jobs to run in parallel, eg, which presumably wouldn’t be appropriate elsewhere.

MarcoFalke commented at 8:29 am on October 23, 2020:

The wallet tool is doing exactly that already with argsman. E.g. it has the info subcommand, or a dump subcommand with conditional options like -dumpfile (#19137)

sipa commented at 8:34 pm on December 23, 2020:

This seems worth addressing still: https://github.com/bitcoin/bitcoin/pull/19937/commits/e6473d274fe01d58c9eed75bc56415fcce6d63db#r492742101

in src/bitcoin-util.cpp:72 in e077aa7055 outdated

67+        return EXIT_FAILURE;
68+    }
69+
70+    if (argc < 2 || HelpRequested(gArgs)) {
71+        // First part of help message is specific to this utility
72+        std::string strUsage = PACKAGE_NAME " bitcoin-tx utility version " + FormatFullVersion() + "\n\n" +

MarcoFalke commented at 1:41 pm on September 22, 2020:

0        std::string strUsage = PACKAGE_NAME " bitcoin-util utility version " + FormatFullVersion() + "\n\n" +

MarcoFalke commented at 1:43 pm on September 22, 2020: member

Is there a risk in exposing the miner on RPC as well? Bitcoind is running anyway somewhere, and some miners might want to run everything in one process for convenience? No strong opinion.

MarcoFalke added the label RPC/REST/ZMQ on Sep 22, 2020

MarcoFalke added the label Mining on Sep 22, 2020

ajtowns commented at 5:00 pm on September 22, 2020: member

I don’t think there’s a risk in doing it over RPC; it’s that it puts the wallet code in the middle of two mining codes bits (ie, generate a template, sign, grind proof of work). Merging mining and wallet code seemed a bit ugly, but keeping them distinct means you need a script to combine them for you, so it was easier to put everything in the script. Also seemed easier to add an RPC later if that makes sense, than to remove it if it doesn’t make sense.

in configure.ac:580 in a55abfe425 outdated

560@@ -560,6 +561,12 @@ AC_ARG_ENABLE([util-wallet],
561   [build_bitcoin_wallet=$enableval],
562   [build_bitcoin_wallet=$build_bitcoin_utils])
563 
564+AC_ARG_ENABLE([util-util],
565+  [AS_HELP_STRING([--enable-util-util],

kallewoof commented at 7:12 am on September 30, 2020:

Why is this needed, since there’s already –with-util above? (Also, why util-util?)

ajtowns commented at 6:01 pm on October 1, 2020:

--with-utils gives the bitcoin-cli, bitcoin-tx, bitcoin-util, bitcoin-wallet utils; --with-util-X just gives bitcoin-X. Not really needed, (and util-util is certainly weird) but don’t see any reason to be inconsistent.

in src/bitcoin-util.cpp:103 in a55abfe425 outdated

 98+    finish = finish - (finish % step) + offset;
 99+
100+    while (!found && header.nNonce < finish) {
101+        const uint32_t next = (finish - header.nNonce < 5000*step) ? finish : header.nNonce + 5000*step;
102+        do {
103+            if (!(UintToArith256(header.GetHash()) > target)) {

kallewoof commented at 7:22 am on September 30, 2020:

operator<= exists, so using that seems more straightforward.

in src/bitcoin-util.cpp:54 in a55abfe425 outdated

54+    //
55+    SetupBitcoinUtilArgs(gArgs);
56+    std::string error;
57+    if (!gArgs.ParseParameters(argc, argv, error)) {
58+        tfm::format(std::cerr, "Error parsing command line arguments: %s\n", error);
59+        return EXIT_FAILURE;

kallewoof commented at 7:34 am on September 30, 2020:

Wouldn’t it be totally fine if these simply did exit(…) instead of the EXIT_/CONTINUE_ stuff?

ajtowns commented at 6:05 pm on October 1, 2020:

Sure? It’s just doing the same thing bitcoin-tx does.

kallewoof commented at 7:35 am on September 30, 2020: member

Tested ACK e077aa7055e71f08a7d2cfbb29702832899fee04

in src/bitcoin-util.cpp:62 in e077aa7055 outdated

57+    if (!gArgs.ParseParameters(argc, argv, error)) {
58+        tfm::format(std::cerr, "Error parsing command line arguments: %s\n", error);
59+        return EXIT_FAILURE;
60+    }
61+
62+    // Check for -chain, -testnet or -regtest parameter (Params() calls are only valid after this clause)

MarcoFalke commented at 12:32 pm on September 30, 2020:

Could cherry-pick the syle-changes from commit faf0a26711eed9264113463e56b988cf9fe549fd?

jsarenik commented at 9:50 am on October 1, 2020: none

I still have no idea how to run a custom signet on this PR’s commit f4c6ec1deaf8cf4dbbbc2633886112ebdddb31dc (which has conflicts when I try to rebase it to master locally).

The documentation at bitcoin.it is out of date. Please help me so that I can test.

kallewoof commented at 10:16 am on October 1, 2020: member

@jsarenik I’ve updated the running an issuer section of the docs. The rest are fine I think. Lemme know if not.

jsarenik commented at 11:30 am on October 1, 2020: none

@kallewoof have a look at #15454 and add the createwallet lines to the doc please.

jsarenik commented at 11:55 am on October 1, 2020: none

@kallewoof or start all bitcoind with -wallet command line argument.

jsarenik commented at 12:05 pm on October 1, 2020: none

Or actually rather -wallet=""

jsarenik commented at 12:32 pm on October 1, 2020: none

@kallewoof Maybe adding a new bitcoin-cli createwallet line after starting a bitcoind would be the best according to #20034 (because -wallet="" may change behavior in next releases as far as I understood).

ajtowns force-pushed on Oct 1, 2020

ajtowns commented at 6:47 pm on October 1, 2020: member

Rebased, with the a bunch of the suggestions applied

laanwj added this to the "Blockers" column in a project

kallewoof commented at 7:01 am on October 2, 2020: member

@jsarenik Gave you edit privileges on the Bitcoin Wiki.

jsarenik commented at 7:11 am on October 2, 2020: none

@kallewoof Thank you! Now I need to figure out how to run a custom signet…

jsarenik commented at 10:27 am on October 2, 2020: none

Tested ACK f2ee4a95b3d9eac7b717f2ed4316dcb5df2d2795

Editing the Wiki now.

decryp2kanon commented at 4:57 pm on October 4, 2020: contributor

Concept ACK

in src/rpc/mining.cpp:825 in dd62762ff2 outdated

823@@ -818,6 +824,10 @@ static RPCHelpMan getblocktemplate()
824     UniValue aRules(UniValue::VARR);
825     aRules.push_back("csv");
826     if (!fPreSegWit) aRules.push_back("!segwit");
827+    if (consensusParams.signet_blocks) {
828+        aRules.push_back("!signet");

Sjors commented at 9:02 am on October 17, 2020:

It would be useful to add a comment for non mining experts, such as yours truly, to explain what this does (especially the ! part, which I guess doesn’t mean “not” here). I found the discussion around !segwit rather confusing: https://github.com/bitcoin/bitcoin/pull/17946

ajtowns commented at 2:37 am on October 22, 2020:

Added a comment

in src/rpc/mining.cpp:671 in 0857bf9e6c outdated

666@@ -667,11 +667,13 @@ static RPCHelpMan getblocktemplate()
667     if(!node.connman)
668         throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
669 
670-    if (node.connman->GetNodeCount(CConnman::CONNECTIONS_ALL) == 0)
671-        throw JSONRPCError(RPC_CLIENT_NOT_CONNECTED, PACKAGE_NAME " is not connected!");
672+    if (!Params().IsTestChain()) {
673+        if (node.connman->GetNodeCount(CConnman::CONNECTIONS_ALL) == 0)

Sjors commented at 9:08 am on October 17, 2020:

Could add missing brackets since you’re touching this area.

ajtowns commented at 2:36 am on October 22, 2020:

Done

in contrib/signet/generate.py:286 in f2ee4a95b3 outdated

278+    if args.target_mining_time is not None:
279+        if args.target_mining_time <= 0 or args.target_mining_time > 600:
280+            logging.error("Target mining time must be between 1 and 600")
281+            return 1
282+
283+    mi = json.loads(args.bcli("getmininginfo"))

Sjors commented at 9:24 am on October 17, 2020:

This is not Python 3.5.6 friendly:

0JSON object must be str, not 'bytes'

ajtowns commented at 2:36 am on October 22, 2020:

args.bcli / bitcoin_cli always returns a string now

in contrib/signet/generate.py:348 in f2ee4a95b3 outdated

339+        mining_start = time.time()
340+        mined_blocks += 1
341+        psbt = generate_psbt(tmpl, reward_spk, blocktime=start)
342+        psbt_signed = json.loads(args.bcli("-stdin", "walletprocesspsbt", input=psbt.encode('utf8')))
343+        if not psbt_signed.get("complete",False):
344+            sys.stderr.write("PSBT signing failed")

Sjors commented at 9:30 am on October 17, 2020:

Dumping the PSBT result is useful here for debugging.

ajtowns commented at 2:36 am on October 22, 2020:

Should dump psbt now if debug logging is enabled. That will contain the whole block as well, so it will be long.

Sjors commented at 9:42 am on October 17, 2020: member

Concept ACK. Using the wiki instructions, I was able to launch my own signet and mine blocks on it!

It’s nice to see bitcoin-util #14671 come to live.

The commits that touch the node (3d1c2ce3c2684d4435fe17118d2fbdd10d04a2e0, dd62762ff2d4eac87ce12dff0e8f7ac3dd535f39, 0857bf9e6cb23792de94cfd164289d3eb08c608f) look good to me, but I didn’t review the bitcoin-util and generate.py script.

I get a compiler warning:

 0test/validation_tests.cpp:68:88: warning: braces around scalar initializer [-Wbraced-scalar-init]
 1    BOOST_CHECK(signet_params->GetConsensus().signet_challenge == std::vector<uint8_t>{{OP_TRUE}});
 2                                                                                       ^~~~~~~~~
 3/usr/local/include/boost/test/tools/old/interface.hpp:83:6: note: expanded from macro 'BOOST_CHECK'
 4    (P), BOOST_TEST_STRINGIZE( P ), CHECK, CHECK_PRED, _ )
 5     ^
 6/usr/local/include/boost/test/tools/old/interface.hpp:68:61: note: expanded from macro 'BOOST_TEST_TOOL_IMPL'
 7        BOOST_JOIN( BOOST_TEST_TOOL_PASS_PRED, frwd_type )( P, ARGS ),          \
 8                                                            ^
 9/usr/local/include/boost/test/tools/old/interface.hpp:51:47: note: expanded from macro 'BOOST_TEST_TOOL_PASS_PRED2'
10#define BOOST_TEST_TOOL_PASS_PRED2( P, ARGS ) P

@jsarenik in the wiki it’s better to redact the entire public key, because idiots like myself might end up copy-pasting their public key into REDACTED and end up with an invalid -signetchallenge :-)

jsarenik commented at 9:25 am on October 18, 2020: none

@jsarenik in the wiki it’s better to redact the entire public key, because idiots like myself might end up copy-pasting their public key into REDACTED and end up with an invalid -signetchallenge :-)

Thank you @Sjors for feed-back. Wiki edited. Have a look.

0xB10C commented at 8:36 am on October 19, 2020: member

I successfully launched my own signet and mined a few hundred blocks on it, too. Haven’t had a chance to take a closer look at the code.

decryp2kanon commented at 4:39 pm on October 21, 2020: contributor

just a question about MAGIC_BYTES

https://github.com/bitcoin/bitcoin/blob/b46f37ba5ec4fbd2e4c82343fc4f353d7f34837a/src/chainparams.cpp#L342

https://github.com/bitcoin/bitcoin/blob/b46f37ba5ec4fbd2e4c82343fc4f353d7f34837a/test/functional/test_framework/p2p.py#L116

both are same?

jsarenik commented at 5:18 pm on October 21, 2020: none

just a question about MAGIC_BYTES

https://github.com/bitcoin/bitcoin/blob/b46f37ba5ec4fbd2e4c82343fc4f353d7f34837a/src/chainparams.cpp#L342

https://github.com/bitcoin/bitcoin/blob/b46f37ba5ec4fbd2e4c82343fc4f353d7f34837a/test/functional/test_framework/p2p.py#L116

both are same? @decryp2kanon Yes. In the other endianness it is 0x40CF030A and for Signet IV this is the network marker.

in contrib/signet/generate.py:18 in f2ee4a95b3 outdated

13+import subprocess
14+
15+from binascii import unhexlify
16+from io import BytesIO
17+
18+sys.path.insert(0, "../test/functional/")

decryp2kanon commented at 5:47 pm on October 21, 2020:

0sys.path.insert(0, "../../test/functional/")

in my case, it was wrong path.

0cd ./contrib/signet/
1./generate.py --help

jsarenik commented at 5:56 pm on October 21, 2020:

Yes, the paths are a bit confusing. It is meant to be run from main source root. See for example https://en.bitcoin.it/wiki/Signet:Custom:Script

jsarenik commented at 6:00 pm on October 21, 2020:

And the general documentation for Custom Signet is at https://en.bitcoin.it/wiki/Signet#Custom_Signet

ajtowns commented at 2:34 am on October 22, 2020:

Should be fixed

decryp2kanon commented at 5:51 pm on October 21, 2020: contributor

I am trying to test. could you give an example of this?

0usage: generate.py generate [-h] [--block-time BLOCK_TIME]
1                            [--target-mining-time TARGET_MINING_TIME]
2                            [--backdate BACKDATE] [--secondary]
3                            [--signcmd SIGNCMD] [--address ADDRESS]
4                            [--descriptor DESCRIPTOR] [--grind-cmd GRIND_CMD]

jsarenik commented at 5:57 pm on October 21, 2020: none

I am trying to test. could you give an example of this? @decryp2kanon See #19937 (comment) (above)

ajtowns force-pushed on Oct 22, 2020

ajtowns commented at 2:37 am on October 22, 2020: member

Rebased to master to pull in #20060 which fixes the compiler warning and #20157 which activates taproot, and made some changes based on feedback above.

kallewoof commented at 7:01 am on October 22, 2020: member

Miner now running this version on my end.

ajtowns force-pushed on Oct 22, 2020

Sjors commented at 8:35 am on October 26, 2020: member

The commits that touch the node still good to me as of 09671f2, thanks for the changes. Mining also still works. The bitcoin-util command help text is missing a list of commands. It’s also confusing why signetseednode is listed as a potential argument.

MarcoFalke commented at 8:49 am on October 26, 2020: member

command help text is missing a list of commands

This can probably be fixed by switching to ArgsMan: #19937 (review)

ajtowns force-pushed on Nov 11, 2020

ajtowns commented at 7:28 am on November 11, 2020: member

Bunch of changes:

adds contrib/signet/README.md
renames contrib/signet/generate.py to contrib/signet/miner
changes many of the parameters (--block-time and --mining-time replaced by --nbits and --poisson; --secondary replaced by --multiminer, --backup-delay and --standby-delay)
makes block times predictable
adds a calibrate subcommand to see how long your cpu takes to mine a block to pick out a value for --nbits
improves the output so it’s easier to tell what’s going on
does not add signet challenge to getmininginfo anymore

DrahtBot added the label Needs rebase on Nov 19, 2020

ajtowns force-pushed on Nov 19, 2020

DrahtBot removed the label Needs rebase on Nov 19, 2020

laanwj commented at 3:11 pm on November 23, 2020: member

I’m a bit in doubt about adding yet another binary just for this. In principle we already have bitcoin-tx with for idea being to provide bitcoin related functionality that does not rely on the ability to access a running node. It’s named a bit inconvenient to add this functionality I guess but …

ajtowns commented at 9:48 pm on November 23, 2020: member

@laanwj bitcoin-util could be used for node-free versions of a bunch of rpc calls (see #14671) and/or to avoid introducing a separate binary for managing asmap stuff (see #18573). I think it would make more sense to aim to eventually deprecate bitcoin-tx in favour of psbt operations, and instead keep around bitcoin-cli for interacting with a node over rpc, bitcoin-wallet for doing wallet operations that don’t require a node, and bitcoin-util for anything that doesn’t require a node or a wallet (or a gui).

muxator commented at 11:45 am on December 1, 2020: none

Hi,

a very minor nit: 34b8e1742cb756016bae94a2775a0f7d617050b1 introduces contrib/signet/generate.py. The subsequent commit a2ada2db9c16d50c920ddb56e1b82d30b68987fe updates the documentation, while also renaming contrib/signet/generate.py → contrib/signet/miner.

Shouldn’t the first commit be modified to directly introduce miner instead of generate.py?

Depending on the project policies, it may also make sense to use a single commit to introduce the new script and update the relevant documentation.

ajtowns force-pushed on Dec 2, 2020

ajtowns commented at 0:19 am on December 2, 2020: member

@muxator good catch; fixed to introduce the script as contrib/signet/miner directly. Don’t see any reason to merge the docs into the same commit – they’re already in the same PR so won’t be merged separately.

in src/Makefile.am:683 in 066ee99cea outdated

679+
680+bitcoin_util_LDADD = \
681+  $(LIBUNIVALUE) \
682+  $(LIBBITCOIN_COMMON) \
683+  $(LIBBITCOIN_UTIL) \
684+  $(LIBBITCOIN_CONSENSUS) \

ajtowns commented at 2:29 am on December 2, 2020:

“grind” needs libconsensus for both primitives/block.h for the block header definition and arith_uint256.h for converting nbits to a target and comparing the target to a sha256. Since libconsensus isn’t linked into bitcoin-cli, that probably rules out bitcoin-cli --grind=HEADER as an alternative.

Making bitcoin-tx -grind HEADER work seems plausible; should look something like https://github.com/ajtowns/bitcoin/commits/202009-signet-generate-util2tx

Long term I think bitcoin-util is a good idea, but it might be better to introduce it in its own PR, in which case putting the functionality in bitcoin-tx in the meantime seems plausible? Thoughts?

kallewoof commented at 7:15 am on December 2, 2020:

bitcoin-tx --grind feels weird to me. Grinding is unrelated to transactions. Unless we decide bitcoin-util is not desired, I think introducing it here makes more sense, personally.

in contrib/signet/README.md:59 in 066ee99cea outdated

54+
55+Instead of calculating a specific nbits value, --min-nbits can be specified instead, in which case the mininmum signet difficulty will be targeted.
56+
57+By default, the signet miner mines blocks at fixed intervals with minimal variation. If you want blocks to appear more randomly, as they do in mainnet, specify the --poisson option.
58+
59+Using the --multiminer parameter allows mining to be distributed amongst multiple miners. For example, if you have 3 miners and want to share blocks between them, specify --multiminer=1/3 on one, --multiminer=2/3 on another, and --multiminer=3/3 on the last one. If you want one to do 10% of blocks and two others to do 45% each, --multiminer=1-10/100 on the first, and --multiminer=11-55 and --multiminer=56-100 on the others. Note that which miner mines which block is determined by the previous block hash, so occassional runs of one miner doing many blocks in a row is to be expected.

muxator commented at 9:12 pm on December 2, 2020:

“occassional runs” -> “occasional runs”

ajtowns commented at 2:40 am on December 3, 2020:

Done

in contrib/signet/miner:21 in 066ee99cea outdated

16+import subprocess
17+
18+from binascii import unhexlify
19+from io import BytesIO
20+
21+PATH_BASE_CONTRIB_SIGNET = os.path.abspath(os.path.dirname(__file__))

muxator commented at 9:56 pm on December 2, 2020:

Hi,

I usually run my binaries putting a symlink to them in a directory somewhere in my PATH (let’s say /home/.local/bin, but it can be anything, really).

When doing this with miner, however, it won’t work:

0$ ln -s <base>/contrib/signet/miner /tmp/miner
1$ /tmp/miner --help
2Traceback (most recent call last):
3  File "/tmp/miner", line 25, in <module>
4    from test_framework.blocktools import WITNESS_COMMITMENT_HEADER, script_BIP34_coinbase_height # noqa: E402
5ModuleNotFoundError: No module named 'test_framework'

The reason is that, when computing the absolute path to the directory containing miner, links are not followed. This would work if it included an invocation to realpath(), for example:

 0diff --git a/contrib/signet/miner b/contrib/signet/miner
 1--- a/contrib/signet/miner
 2+++ b/contrib/signet/miner
 3@@ -18,7 +18,7 @@ import subprocess
 4 from binascii import unhexlify
 5 from io import BytesIO
 6 
 7-PATH_BASE_CONTRIB_SIGNET = os.path.abspath(os.path.dirname(__file__))
 8+PATH_BASE_CONTRIB_SIGNET = os.path.abspath(os.path.dirname(os.path.realpath(__file__)))
 9 PATH_BASE_TEST_FUNCTIONAL = os.path.join(PATH_BASE_CONTRIB_SIGNET, "..", "..", "test", "functional")
10 sys.path.insert(0, PATH_BASE_TEST_FUNCTIONAL)

On my copy, I also called abspath() when computing the final value for PATH_BASE_TEST_FUNCTIONAL, but that’s just a question of taste, because I prefer <base>/test/functional instead of <base>/contrib/signet/../../test/functional in my include path (it’s more explicit to debug if something goes wrong).

The final version would be:

 0diff --git a/contrib/signet/miner b/contrib/signet/miner
 1--- a/contrib/signet/miner
 2+++ b/contrib/signet/miner
 3@@ -18,8 +18,8 @@ import subprocess
 4 from binascii import unhexlify
 5 from io import BytesIO
 6 
 7-PATH_BASE_CONTRIB_SIGNET = os.path.abspath(os.path.dirname(__file__))
 8-PATH_BASE_TEST_FUNCTIONAL = os.path.join(PATH_BASE_CONTRIB_SIGNET, "..", "..", "test", "functional")
 9+PATH_BASE_CONTRIB_SIGNET = os.path.abspath(os.path.dirname(os.path.realpath(__file__)))
10+PATH_BASE_TEST_FUNCTIONAL = os.path.abspath(os.path.join(PATH_BASE_CONTRIB_SIGNET, "..", "..", "test", "functional"))
11 sys.path.insert(0, PATH_BASE_TEST_FUNCTIONAL)
12 
13 from test_framework.blocktools import WITNESS_COMMITMENT_HEADER, script_BIP34_coinbase_height # noqa: E402

From my experience with other Python projects, there should not be any undesired side effects, but I am not the expert here. Do you think supporting this use case makes sense?

Thanks!

ajtowns commented at 2:40 am on December 3, 2020:

Done

ajtowns force-pushed on Dec 3, 2020

ajtowns commented at 2:42 am on December 3, 2020: member

Added @muxator’s suggestions and updated bitcoin-util-res.rc to match the new version number scheme (and rebased to master to catch up with the new version scheme)

kallewoof commented at 7:32 am on December 3, 2020: member

Tested ACK 6258e7eb3c9c4cb519cb519d640b73a059b1cc11

in contrib/signet/miner:541 in 6258e7eb3c outdated

536+    #CHECKS=[]
537+    for i in range(TRIALS):
538+        header.nTime = i
539+        header.nNonce = 0
540+        headhex = header.serialize().hex()
541+        cmd = args.grind_cmd.split(" ") + [headhex]

muxator commented at 2:15 pm on December 7, 2020:

If miner calibrate is called without any additional arguments, there is a stack trace:

0$ ./miner calibrate 
1Traceback (most recent call last):
2  File "./miner", line 637, in <module>
3    main()
4  File "./miner", line 631, in main
5    return args.fn(args)
6  File "./miner", line 541, in do_calibrate
7    cmd = args.grind_cmd.split(" ") + [headhex]
8AttributeError: 'NoneType' object has no attribute 'split'

One possible way to fix it could be making --grind-cmd required for the calibrate command. For --solvepsbt and --generate, apparently, this is not needed, since finish_block() checks for None already.

 0diff --git a/contrib/signet/miner b/contrib/signet/miner
 1--- a/contrib/signet/miner
 2+++ b/contrib/signet/miner
 3@@ -604,8 +604,8 @@ def main():
 4         sp.add_argument("--address", default=None, type=str, help="Address for block reward payment")
 5         sp.add_argument("--descriptor", default=None, type=str, help="Descriptor for block reward payment")
 6 
 7-    for sp in [solvepsbt, generate, calibrate]:
 8-        sp.add_argument("--grind-cmd", default=None, type=str, help="Command to grind a block header for proof-of-work")
 9+    for (sp, is_required) in [(solvepsbt, False), (generate, False), (calibrate, True)]:
10+        sp.add_argument("--grind-cmd", default=None, type=str, required=is_required, help="Command to grind a block header for proof-of-work")
11 
12     args = parser.parse_args(sys.argv[1:])

Behaviour after the proposed change:

0$ ./miner calibrate 
1usage: miner calibrate [-h] [--nbits NBITS] [--seconds SECONDS] --grind-cmd GRIND_CMD
2miner calibrate: error: the following arguments are required: --grind-cmd

in contrib/signet/miner:523 in 6258e7eb3c outdated

518+def do_calibrate(args):
519+    if args.nbits is not None and args.seconds is not None:
520+        sys.stderr.write("Can only specify one of --nbits or --seconds\n")
521+        return 1
522+    if args.nbits is not None and len(args.nbits) != 8:
523+        sys.stderr.write("Must specify 8 hex digits for --nbits")

muxator commented at 2:17 pm on December 7, 2020:

A trailing '\n' at the end of this line would make is consistent with the previous sys.stderr.write():

 0diff --git a/contrib/signet/miner b/contrib/signet/miner
 1--- a/contrib/signet/miner
 2+++ b/contrib/signet/miner
 3@@ -520,7 +520,7 @@ def do_calibrate(args):
 4         sys.stderr.write("Can only specify one of --nbits or --seconds\n")
 5         return 1
 6     if args.nbits is not None and len(args.nbits) != 8:
 7-        sys.stderr.write("Must specify 8 hex digits for --nbits")
 8+        sys.stderr.write("Must specify 8 hex digits for --nbits\n")
 9         return 1
10 
11     TRIALS = 600 # gets variance down pretty low

DrahtBot added the label Needs rebase on Dec 10, 2020

rpc: update getblocktemplate with signet rule, include signet_challenge 81c54dec20

rpc: allow getblocktemplate for test chains when unconnected or in IBD 95d5d5e625

ajtowns force-pushed on Dec 14, 2020

DrahtBot removed the label Needs rebase on Dec 14, 2020

in contrib/signet/README.md:28 in 69c6e47a3f outdated

23+
24+To mine the first block in your custom chain, you can run:
25+
26+  cd src/
27+  CLI="./bitcoin-cli -conf=mysignet.conf"
28+  MINER="..contrib/signet/miner"

JeremyRubin commented at 0:53 am on December 18, 2020:

typo here, should be ../contrib!

in contrib/signet/README.md:26 in 69c6e47a3f outdated

21+miner
22+=====
23+
24+To mine the first block in your custom chain, you can run:
25+
26+  cd src/

JeremyRubin commented at 0:53 am on December 18, 2020:

maybe just make this a one-liner with some ; or && ’s?

in contrib/signet/README.md:57 in 69c6e47a3f outdated

52+
53+Instead of using a single address, a ranged descriptor may be provided instead (via the --descriptor parameter), with the reward for the block at height H being sent to the H'th address generated from the descriptor.
54+
55+Instead of calculating a specific nbits value, --min-nbits can be specified instead, in which case the mininmum signet difficulty will be targeted.
56+
57+By default, the signet miner mines blocks at fixed intervals with minimal variation. If you want blocks to appear more randomly, as they do in mainnet, specify the --poisson option.

JeremyRubin commented at 0:58 am on December 18, 2020:

How can i set this fixed interval? Slightly confused the difference between nbits and the interval itself. It sounds like interval is every 5 minutes try to mine a 25 seconds difficult block? What if I just want “blocks every 30 seconds”?

kallewoof commented at 8:10 am on December 18, 2020:

$GENERATE –cli="$CLI" “$@” generate –grind-cmd="$GRIND" –address="$ADDR" –nbits=$NBITS –multiminer=$MULTIMINER –ongoing –poisson

is what it looks like on my end. I think you want –ongoing –poisson.

JeremyRubin commented at 6:59 pm on December 18, 2020:

I don’t think –poisson is the right thing, as –poisson just adds noise?

I guess the parameter is standby delay, which defaults to 0?

Where I think I’m confused is how setting nbits works because we use the next_block_delta to define how long to wait before mining (via sleeping), but then the grind command conceivably takes est_time(nbits) to mine? so it feels like we’re double delaying based on nbits?

ajtowns commented at 9:19 pm on December 18, 2020:

You can’t choose the interval; the difficulty will just get adjusted so the interval levels out at around 10 minutes. What you can target is a difficulty, so that once you reach that difficulty you keep mining a block every 10 minutes so that you stay at that difficulty. If you’re not already at that difficulty, you instead choose a shorter/longer interval, so the difficulty is increased/decreased at the next retarget.

If you want to mine a bunch of blocks to get the chain started, then backdating via --set-block-time works. You could also use --nbits=170f1372 (current mainnet nbits) which would make it try to raise the difficulty by mining quickly (though not faster than an average of 2m30).

There’s no double delay, because everything operates on the block time field – ie, last block had timestamp T, next block has timestamp T+INTERVAL. The standby-delay and backup-delay parameters mean you won’t start mining the block until T+INTERVAL+DELAY, and you won’t finish mining the block until T+INTERVAL+DELAY+GRINDTIME, but those delays don’t affect the following block, which will have a timestamp of T+INTERVAL+INTERVAL2 (though again you might not start mining it for a further DELAY, and it won’t be finished until GRINDTIME has also passed)

JeremyRubin commented at 6:36 pm on December 19, 2020:

Ahhh this makes sense I didn’t realize normal difficulty adjustment rules are still used.

This text could go great in the Readme btw :)

Maybe incl a shell script template to backdate 100 blocks on start to seed a usable wallet?

ajtowns commented at 6:59 pm on December 19, 2020:

I think just START=$(date +%s -d"1000 minutes ago") and --set-block-time=$START would work for that – even with poisson timing you should end up mining 100+ blocks before you catch up with real time. Maybe something like that should just be the default if you’re mining the first block of a signet chain?

JeremyRubin approved

JeremyRubin commented at 0:59 am on December 18, 2020: contributor

Tested ACK; could do more direct review of the code itself.

Few docs nits.

Utility works like a charm to mine blocks!

in contrib/signet/README.md:55 in 69c6e47a3f outdated

50+
51+Instead of specifying --ongoing, you can specify --max-blocks=N to mine N blocks and stop.
52+
53+Instead of using a single address, a ranged descriptor may be provided instead (via the --descriptor parameter), with the reward for the block at height H being sent to the H'th address generated from the descriptor.
54+
55+Instead of calculating a specific nbits value, --min-nbits can be specified instead, in which case the mininmum signet difficulty will be targeted.

gruve-p commented at 9:17 pm on December 18, 2020:

mininmum > minimum

in contrib/signet/README.md:38 in 69c6e47a3f outdated

33+This will mine a block with the current timestamp. If you want to backdate the chain, you can give a different timestamp to --set-block-time.
34+
35+You will then need to pick a difficulty target. Since signet chains are primarily protected by a signature rather than proof of work, there is no need to spend as much energy as possible mining, however you may wish to choose to spend more time than the absolute minimum. The calibrate subcommand can be used to pick a target, eg:
36+
37+  $MINER calibrate --grind-cmd="$GRIND"
38+  nbits=1e00f403 for 25s average mining time

gruve-p commented at 9:46 pm on December 18, 2020:

nbits=1e00f403 > –nbits=1e00f403

ajtowns commented at 6:51 pm on December 19, 2020:

That line is an example of output of the command

gruve-p commented at 11:00 pm on December 20, 2020:

The formatting looks strange when reading the README.md

ajtowns commented at 2:20 am on December 21, 2020:

Ah, apparently 4 spaces is the standard, not wordpress’s 2 spaces. Fixed.

in contrib/signet/README.md:77 in 69c6e47a3f outdated

72+  $CLI -signet getblocktemplate '{"rules": ["signet","segwit"]}' |
73+    $MINER --cli="$CLI" genpsbt --address="$ADDR" |
74+    $CLI -signet -stdin walletprocesspsbt |
75+    jq -r .psbt |
76+    $MINER --cli="$CLI" solvepsbt --grind-cmd="$GRIND" |
77+    $CLI -signet -stdin submitblock

gruve-p commented at 11:00 pm on December 20, 2020:

Maybe fix the formatting here also

in contrib/signet/README.md:31 in 69c6e47a3f outdated

26+  cd src/
27+  CLI="./bitcoin-cli -conf=mysignet.conf"
28+  MINER="..contrib/signet/miner"
29+  GRIND="./bitcoin-util grind"
30+  ADDR=$($CLI -signet getnewaddress)
31+  $MINER --cli="$CLI" generate --grind-cmd="$GRIND" --address="$ADDR" --set-block-time=-1

gruve-p commented at 11:01 pm on December 20, 2020:

Formatting here also

ajtowns force-pushed on Dec 21, 2020

in contrib/signet/README.md:28 in f19148a9dd outdated

23+
24+To mine the first block in your custom chain, you can run:
25+
26+    cd src/
27+    CLI="./bitcoin-cli -conf=mysignet.conf"
28+    MINER="..contrib/signet/miner"

gruve-p commented at 3:29 pm on December 21, 2020:

MINER="..contrib/signet/miner" > MINER="../contrib/signet/miner"

laanwj commented at 4:02 pm on December 21, 2020: member

code review ACK f19148a9ddab01af80067982f2639dbee2cdbf52

nit: your new utility doesn’t implement --version and no manual page is generated for it in contrib/devtools/gen-manpages.sh. No opinion on whether that’s necessary here.

sipa commented at 8:36 pm on December 23, 2020: member

Concept ACK. I think it makes sense to have a bitcoin-util tool with subcommands for wallet- and node-free operations (and deprecate bitcoin-tx in favor of PSBT support in bitcoin-util).

gruve-p commented at 3:58 pm on December 24, 2020: contributor

tACK https://github.com/bitcoin/bitcoin/pull/19937/commits/f19148a9ddab01af80067982f2639dbee2cdbf52

Few docs nits.

practicalswift commented at 11:03 pm on December 27, 2020: contributor

Concept ACK

bitcoin-util makes sense!

in src/rpc/mining.cpp:723 in f19148a9dd outdated

717@@ -714,6 +718,13 @@ static RPCHelpMan getblocktemplate()
718         // TODO: Maybe recheck connections/IBD and (if something wrong) send an expires-immediately template to stop miners?
719     }
720 
721+    const Consensus::Params& consensusParams = Params().GetConsensus();
722+
723+    // GBT must be called with 'signet' set in the rules for signet chains

laanwj commented at 1:42 pm on January 11, 2021:

What happens when signet rules are set when calling getblocktemplate on a non-signet network, I suppose this is (or should be) an error?

ajtowns commented at 8:21 am on January 12, 2021:

The way I read BIP9 is that by including “signet” in the rules when doing a GBT request, that just indicates you can handle it if signet rules are active – if they’re not active, then there’s no reason why you can’t handle that too. So adding signet to rules on mainnet should be fine as far as bitcoind is concerned – it will just return a regular template without any signet stuff, just as if you’d indicated support for .. extension block mining or something.

laanwj commented at 11:31 am on January 12, 2021:

Thanks, clear.

Add bitcoin-util command line utility 13762bcc96

contrib/signet: Add script for generating a signet chain ff7dbdc08a

contrib/signet: Document miner script in README.md 595a34dbea

in src/bitcoin-util.cpp:189 in f19148a9dd outdated

184+         * ArgsManager does these ops but for some reason they're not
185+         * available.
186+         * "argc < 0" should be enough to be unreachable, but not
187+         * so unreachable that it's optimised away entirely
188+         */
189+        try { UniValue uv; uv["txid"]; uv.get_str(); uv.write(4); } catch (...) { }

laanwj commented at 1:45 pm on January 11, 2021:

I think this is awful. Can someone of the build system people please weigh in here?

fanquake commented at 2:11 pm on January 11, 2021:

This works for me:

 0diff --git a/src/Makefile.am b/src/Makefile.am
 1index b974bed66..f37db7bde 100644
 2--- a/src/Makefile.am
 3+++ b/src/Makefile.am
 4@@ -677,9 +677,9 @@ bitcoin_util_SOURCES += bitcoin-util-res.rc
 5 endif
 6 
 7 bitcoin_util_LDADD = \
 8-  $(LIBUNIVALUE) \
 9   $(LIBBITCOIN_COMMON) \
10   $(LIBBITCOIN_UTIL) \
11+  $(LIBUNIVALUE) \
12   $(LIBBITCOIN_CONSENSUS) \
13   $(LIBBITCOIN_CRYPTO) \
14   $(LIBSECP256K1)
15diff --git a/src/bitcoin-util.cpp b/src/bitcoin-util.cpp
16index ff6bedec0..f7e670f4e 100644
17--- a/src/bitcoin-util.cpp
18+++ b/src/bitcoin-util.cpp
19@@ -179,16 +179,6 @@ static int CommandLineUtil(int argc, char* argv[])
20 
21 int main(int argc, char* argv[])
22 {
23-    if (argc < 0) {
24-        /* this nonsense is needed or you get linker errors because
25-         * ArgsManager does these ops but for some reason they're not
26-         * available.
27-         * "argc < 0" should be enough to be unreachable, but not
28-         * so unreachable that it's optimised away entirely
29-         */
30-        try { UniValue uv; uv["txid"]; uv.get_str(); uv.write(4); } catch (...) { }
31-    }
32-
33     SetupEnvironment();
34 
35     try {

ajtowns commented at 8:36 am on January 12, 2021:

Thanks @fanquake !

ajtowns force-pushed on Jan 12, 2021

ajtowns commented at 8:38 am on January 12, 2021: member

Rebased to fix the bad library ordering.

Suggest reviewing #20715 which is probably the future for bitcoin-util’s argument parsing. I think we could update bitcoin-util to work that way either before or after merging this PR.

laanwj commented at 11:53 am on January 12, 2021: member

code review ACK 595a34dbea01954cb0372b0210d2fd64357a1762

laanwj merged this on Jan 12, 2021

laanwj closed this on Jan 12, 2021

laanwj removed this from the "Blockers" column in a project

sidhujag referenced this in commit 0e8638b64f on Jan 12, 2021

fanquake deleted a comment on Jan 13, 2021

weedcoder commented at 2:21 pm on January 15, 2021: none

kallewoof commented at 4:24 am on January 16, 2021: member

@weedcoder I’d say it is missing a lot of the logic required to do what liquid does, but I do believe this + a permanent elements chain with a 2-way-peg can do a great job as a test-chain for liquid. :)

laanwj referenced this in commit c763cacb88 on Jan 18, 2021

fanquake deleted a comment on Jan 19, 2021

sidhujag referenced this in commit f2485c00b8 on Jan 20, 2021

in src/bitcoin-util.cpp:33 in 595a34dbea

28+#include <functional>
29+#include <memory>
30+#include <stdio.h>
31+#include <thread>
32+
33+#include <boost/algorithm/string.hpp>

hebasto commented at 7:49 pm on February 1, 2021:

Not sure if this include is required.

MarcoFalke referenced this in commit 4e946ebcf1 on Feb 4, 2021

laanwj referenced this in commit 0f47e01d7d on Jun 18, 2021

DrahtBot locked this on Aug 16, 2022

signet mining utility #19937

Conflicts