Allow configuring target block time for a signet

benthecarman commented at 2:06 am on April 11, 2023: contributor

This pull request introduces the ability to configure the block time of a custom Bitcoin signet network, allowing people to have a signet that is easier to test with. The change helps to improve the flexibility of signet, making it more useful for diverse testing purposes. For example, I am trying to setup a signet with a 30 second block time and this caused a bunch of difficulty adjustments to happen making the network inconsistent. Regtest isn’t a real viable alternative to me here because we would like defaults to use our custom signet if configured, without hindering of local regtest development.

DrahtBot commented at 2:06 am on April 11, 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	rsafier, RandyMcMillan, ekzyis
Concept ACK	stickies-v, ghost, kristapsk, litch, dangershony
Approach NACK	TheCharlatan

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

Conflicts

No conflicts as of last run.

ajtowns commented at 2:33 am on April 11, 2023: contributor

Configurable consensus parameters seems like a great way to make the network inconsistent in general (in particular, if you get the parameter wrong, you’ll end up marking valid blocks invalid, with various consequences).

What’s the actual problem you’re trying to solve here? Would it work to have the signet miner only generate blocks when the mempool has entries, and sit idle when it’s empty, for instance? If you’re trying to generate many blocks, then regtest is already better as it doesn’t require a baseline proof of work, and simply having multiple regtest configs probably makes more sense.

in src/chainparamsbase.cpp:28 in 1bdb26a20a outdated

24@@ -25,6 +25,7 @@ void SetupChainParamsBaseOptions(ArgsManager& argsman)
25     argsman.AddArg("-vbparams=deployment:start:end[:min_activation_height]", "Use given start/end times and min_activation_height for specified version bits deployment (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
26     argsman.AddArg("-signet", "Use the signet chain. Equivalent to -chain=signet. Note that the network is defined by the -signetchallenge parameter", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS);
27     argsman.AddArg("-signetchallenge", "Blocks must satisfy the given script to be considered valid (only for signet networks; defaults to the global default signet test network challenge)", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::CHAINPARAMS);
28+    argsman.AddArg("-signetblocktime", "Difficult adjustment will target a block time of the given amount in seconds (only for signet networks; defaults to 10 minutes)", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::CHAINPARAMS);

unknown commented at 6:05 am on April 11, 2023:

0    argsman.AddArg("-signetblocktime", "Difficulty adjustment will target a block time of the given amount in seconds (only for signet networks; defaults to 10 minutes)", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::CHAINPARAMS);

RandyMcMillan commented at 2:54 pm on April 11, 2023: contributor

Concept ACK

Having more control over custom signet parameters/conditions seems really useful IMO.

ghost commented at 4:57 pm on April 11, 2023: none

Concept ACK

To address the concern in #27446 (comment) maybe this works only for custom signet.

How?

Maybe if bitcoin.conf has signetchallenge=xxx in it?

benthecarman commented at 4:58 pm on April 11, 2023: contributor

Configurable consensus parameters seems like a great way to make the network inconsistent in general (in particular, if you get the parameter wrong, you’ll end up marking valid blocks invalid, with various consequences).

I don’t know if that is a really fair criticism for signet, -signetchallenge is a consensus param as well.

If you’re trying to generate many blocks, then regtest is already better as it doesn’t require a baseline proof of work, and simply having multiple regtest configs probably makes more sense.

This doesn’t really fit our usecase because we can’t have app defaults that differ between different regtests.

benthecarman commented at 5:01 pm on April 11, 2023: contributor

Maybe if bitcoin.conf has signetchallenge=xxx in it?

Yes, this currently will throw an error if you don’t have signet challenge set.

benthecarman force-pushed on Apr 11, 2023

unknown approved

unknown commented at 5:43 pm on April 11, 2023: none

utACK https://github.com/bitcoin/bitcoin/pull/27446/commits/15ca24bb49cd6a5027dcf57fc7b9bd1d33d367db

earonesty commented at 6:00 pm on April 11, 2023: none

concept ack. signet is for tinkering

rsafier commented at 9:06 pm on April 11, 2023: none

Concept ACK. Been wanting this for a while

in src/chainparams.cpp:38 in 15ca24bb49 outdated

33+            throw std::runtime_error(strprintf("%s: -signetblocktime cannot be multiple values.", __func__));
34+        }
35+        if (!args.IsArgSet("-signetchallenge")) {
36+            throw std::runtime_error(strprintf("%s: -signetblocktime cannot be set without -signetchallenge", __func__));
37+        }
38+        options.nPowTargetSpacing = std::stoi(signetblocktime[0]);

mzumsande commented at 9:12 pm on April 11, 2023:

This fails the linter, could use ParseInt64().

maflcko commented at 7:30 am on April 12, 2023:

This fails the linter, could use ParseInt64().

Is there a reason to not just use GetIntArg directly? If not, for new code, ToIntegral should be used, unless the legacy ParseInt behavior of accepting a leading + is needed?

in src/chainparams.cpp:30 in 15ca24bb49 outdated

26@@ -27,6 +27,16 @@ void ReadSigNetArgs(const ArgsManager& args, CChainParams::SigNetOptions& option
27         }
28         options.challenge.emplace(ParseHex(signet_challenge[0]));
29     }
30+    if (args.IsArgSet("-signetblocktime")) {

mzumsande commented at 9:14 pm on April 11, 2023:

Maybe require that signetblocktime > 0 ? Probably bad things would happen otherwise.

benthecarman commented at 9:34 pm on April 11, 2023:

Did signetblocktime >= 0 just to be sure

benthecarman force-pushed on Apr 11, 2023

benthecarman renamed this:
~~Allow configuirng target block time for a signet~~
Allow configuring target block time for a signet
on Apr 12, 2023

in src/chainparams.cpp:40 in 2a599a3cb6 outdated

35+        if (!args.IsArgSet("-signetchallenge")) {
36+            throw std::runtime_error(strprintf("%s: -signetblocktime cannot be set without -signetchallenge", __func__));
37+        }
38+        int64_t block_time;
39+        if (!ParseInt64(signetblocktime[0], &block_time) || block_time <= 0) {
40+            throw std::runtime_error(strprintf("%s: -signetblocktime must be greater than 0", __func__));

maflcko commented at 7:31 am on April 12, 2023:

Any reason to leak implementation details (__func__) to the user when it doesn’t add any value? -signetblocktime must be greater than 0 should be self-explanatory?

in src/chainparams.cpp:33 in 2a599a3cb6 outdated

32+        if (signetblocktime.size() != 1) {
33+            throw std::runtime_error(strprintf("%s: -signetblocktime cannot be multiple values.", __func__));
34+        }
35+        if (!args.IsArgSet("-signetchallenge")) {
36+            throw std::runtime_error(strprintf("%s: -signetblocktime cannot be set without -signetchallenge", __func__));
37+        }

stickies-v commented at 10:48 am on April 12, 2023:

Would InitParameterInteraction() (init.cpp) be a better place for this?

benthecarman commented at 5:36 pm on April 12, 2023:

Looks like it is only networking settings in there and all of them only print error logs, I think we’d want to explicitly throw in this case

stickies-v commented at 6:09 pm on April 12, 2023:

You’re right, we do want to throw. AppInitParameterInteraction() does seem to raise InitError based on bad parameter interaction. I’m not sure why we have both AppInitParameterInteraction() and InitParameterInteraction() and which needs to be used when, it just seems like since this effectively is parameter interaction we’d like to raise that somewhere in here. Hopefully someone else can chime in?

in src/chainparams.cpp:31 in 2a599a3cb6 outdated

26@@ -27,6 +27,20 @@ void ReadSigNetArgs(const ArgsManager& args, CChainParams::SigNetOptions& option
27         }
28         options.challenge.emplace(ParseHex(signet_challenge[0]));
29     }
30+    if (args.IsArgSet("-signetblocktime")) {
31+        const auto signetblocktime = args.GetArgs("-signetblocktime");

stickies-v commented at 10:55 am on April 12, 2023:

I think this can be simplified with GetIntArg which returns a std::optional:

 0diff --git a/src/chainparams.cpp b/src/chainparams.cpp
 1index f41d2aaf8..02d51039e 100644
 2--- a/src/chainparams.cpp
 3+++ b/src/chainparams.cpp
 4@@ -27,19 +27,14 @@ void ReadSigNetArgs(const ArgsManager& args, CChainParams::SigNetOptions& option
 5         }
 6         options.challenge.emplace(ParseHex(signet_challenge[0]));
 7     }
 8-    if (args.IsArgSet("-signetblocktime")) {
 9-        const auto signetblocktime = args.GetArgs("-signetblocktime");
10-        if (signetblocktime.size() != 1) {
11-            throw std::runtime_error(strprintf("%s: -signetblocktime cannot be multiple values.", __func__));
12-        }
13+    if (const auto signetblocktime{args.GetIntArg("-signetblocktime")}) {
14         if (!args.IsArgSet("-signetchallenge")) {
15             throw std::runtime_error(strprintf("%s: -signetblocktime cannot be set without -signetchallenge", __func__));
16         }
17-        int64_t block_time;
18-        if (!ParseInt64(signetblocktime[0], &block_time) || block_time <= 0) {
19+        if (*signetblocktime <= 0) {
20             throw std::runtime_error(strprintf("%s: -signetblocktime must be greater than 0", __func__));
21         }
22-        options.nPowTargetSpacing = block_time;
23+        options.nPowTargetSpacing = signetblocktime;
24     }
25 }
26 
27</details>

in src/kernel/chainparams.h:138 in 2a599a3cb6 outdated

134@@ -135,6 +135,7 @@ class CChainParams
135     struct SigNetOptions {
136         std::optional<std::vector<uint8_t>> challenge{};
137         std::optional<std::vector<std::string>> seeds{};
138+        std::optional<int64_t> nPowTargetSpacing{};

stickies-v commented at 10:57 am on April 12, 2023:

nit: we use snake case now (developer notes), I think it’s fine to have this one be snake case and Consensus::Params::nPowTargetSpacing remain camel case.

0        std::optional<int64_t> pow_target_spacing{};

in src/chainparamsbase.cpp:28 in 2a599a3cb6 outdated

24@@ -25,6 +25,7 @@ void SetupChainParamsBaseOptions(ArgsManager& argsman)
25     argsman.AddArg("-vbparams=deployment:start:end[:min_activation_height]", "Use given start/end times and min_activation_height for specified version bits deployment (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
26     argsman.AddArg("-signet", "Use the signet chain. Equivalent to -chain=signet. Note that the network is defined by the -signetchallenge parameter", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS);
27     argsman.AddArg("-signetchallenge", "Blocks must satisfy the given script to be considered valid (only for signet networks; defaults to the global default signet test network challenge)", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::CHAINPARAMS);
28+    argsman.AddArg("-signetblocktime", "Difficulty adjustment will target a block time of the given amount in seconds (only for signet networks; defaults to 10 minutes)", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::CHAINPARAMS);

stickies-v commented at 11:04 am on April 12, 2023:

nit: I’d define the default in a variable and use it here to avoid hardcoding the default here so it doesn’t go out of sync

stickies-v commented at 11:18 am on April 12, 2023:

I think the doc also needs to specify that -signetchallenge is required when using this option

in src/kernel/chainparams.cpp:1 in 2a599a3cb6

stickies-v commented at 11:11 am on April 12, 2023:

I think this can be simplified quite a bit by just initializing SigNetOptions::nPowTargetSpacing with a default value:

 0diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp
 1index 7f66a6b8e..32905af48 100644
 2--- a/src/kernel/chainparams.cpp
 3+++ b/src/kernel/chainparams.cpp
 4@@ -290,7 +290,6 @@ public:
 5     explicit SigNetParams(const SigNetOptions& options)
 6     {
 7         std::vector<uint8_t> bin;
 8-        int64_t nPowTargetSpacing = 10 * 60;
 9         vSeeds.clear();
10 
11         if (!options.challenge) {
12@@ -329,10 +328,6 @@ public:
13             vSeeds = *options.seeds;
14         }
15 
16-        if (options.nPowTargetSpacing) {
17-            nPowTargetSpacing = *options.nPowTargetSpacing;
18-        }
19-
20         strNetworkID = CBaseChainParams::SIGNET;
21         consensus.signet_blocks = true;
22         consensus.signet_challenge.assign(bin.begin(), bin.end());
23@@ -344,7 +339,7 @@ public:
24         consensus.CSVHeight = 1;
25         consensus.SegwitHeight = 1;
26         consensus.nPowTargetTimespan = 14 * 24 * 60 * 60; // two weeks
27-        consensus.nPowTargetSpacing = nPowTargetSpacing;
28+        consensus.nPowTargetSpacing = options.nPowTargetSpacing;
29         consensus.fPowAllowMinDifficultyBlocks = false;
30         consensus.fPowNoRetargeting = false;
31         consensus.nRuleChangeActivationThreshold = 1815; // 90% of 2016
32diff --git a/src/kernel/chainparams.h b/src/kernel/chainparams.h
33index 43d0b4590..20d95f039 100644
34--- a/src/kernel/chainparams.h
35+++ b/src/kernel/chainparams.h
36@@ -135,7 +135,7 @@ public:
37     struct SigNetOptions {
38         std::optional<std::vector<uint8_t>> challenge{};
39         std::optional<std::vector<std::string>> seeds{};
40-        std::optional<int64_t> nPowTargetSpacing{};
41+        int64_t nPowTargetSpacing{10 * 60};
42     };
43 
44     /**

stickies-v commented at 11:21 am on April 12, 2023: contributor

Concept ACK, I’ve seen the need for signet custom block times come up quite often and I think it makes sense.

carnhofdaki commented at 12:50 pm on April 12, 2023: contributor

How does it work in Elements (Blockstream Liquid)? Is the 1-minute block time hardwired there?

I agree with @ajtowns this may cause inconsistencies (and I would add “confusion”). For example nodes that will not set the parameter to the same value as miners will simply refuse the blocks which do not fit their point-of-view. So in the end the parameter would need to be distributed the same way as signetchallenge (which is a once and for all setting) and can not be changed on-the-fly on an already-running custom Signet network with uncountable number of nodes.

stickies-v commented at 2:19 pm on April 12, 2023: contributor

So in the end the parameter would need to be distributed the same way as signetchallenge

I had similar thoughts and think eventually we may want to introduce a more encompassing (and upgradeable) parameter such as -signetconfig in which all signet consensus parameters are encoded (currently just challenge and blocktime), but I think starting with the current modular approach is fine given that it’s signet.

ajtowns commented at 4:34 pm on April 12, 2023: contributor

Configurable consensus parameters seems like a great way to make the network inconsistent in general (in particular, if you get the parameter wrong, you’ll end up marking valid blocks invalid, with various consequences). I don’t know if that is a really fair criticism for signet, -signetchallenge is a consensus param as well.

Two nodes with different signetchallenges will immediately realise they don’t want to talk to each other – at the p2p (and wallet, iirc) level their network MessageStart values will disagree, and at the consensus level they won’t agree on the validity as of the first block after genesis.

If you’re trying to generate many blocks, then regtest is already better as it doesn’t require a baseline proof of work, and simply having multiple regtest configs probably makes more sense. This doesn’t really fit our usecase because we can’t have app defaults that differ between different regtests.

That doesn’t sound like it makes sense; there isn’t a global regtest, so simply by supporting regtest at all you already can handle different regtests… (I would have expected the objection to be that you want this to be shared with random users, and that you don’t want those random users to be able to trigger an unwanted reorg by creating their own regtest blocks)

But if you’re demoing to users what it’s like to use bitcoin, just without the risk of losing real money, then 30s blocks is pretty misleading (by a factor of 20x even without taking variance into account) – not that that’s a bad thing in an of itself (eth and liquid do fine with blocks around that frequency) but the code in bitcoind is for bitcoin, and it’s hard enough to get that right, without adding other random features…

benthecarman force-pushed on Apr 12, 2023

benthecarman commented at 5:55 pm on April 12, 2023: contributor

Pushed up some changes to respond to @stickies-v and @MarcoFalke’s review

benthecarman commented at 6:00 pm on April 12, 2023: contributor

@ajtowns I don’t think I am properly communicating my usecase correctly, but there are lots of concept acks from other developers that run signets as well. People want to use custom signets to have their own test networks, I don’t see why we should disallow having custom block times. The main concern being people don’t copy-paste the param into their conf and have troubles syncing, doesn’t seem like a valid one given that they could do the same for -signetchallenge. I understand that it would cause a failure on block 1 vs block 2016, but normally those blocks are synced near instantly anyways.

Allow configuring target block time for a signet d8434da3c1

in src/chainparams.cpp:32 in 922aa7a5f1 outdated

26@@ -27,6 +27,15 @@ void ReadSigNetArgs(const ArgsManager& args, CChainParams::SigNetOptions& option
27         }
28         options.challenge.emplace(ParseHex(signet_challenge[0]));
29     }
30+    if (const auto signetblocktime{args.GetIntArg("-signetblocktime")}) {
31+        if (!args.IsArgSet("-signetchallenge")) {
32+            throw std::runtime_error(strprintf("%s: -signetblocktime cannot be set without -signetchallenge", __func__));

maflcko commented at 6:19 pm on April 12, 2023:

Any reason to leak implementation details (__func__) to the user when it doesn’t add any value? -signetblocktime cannot be set without -signetchallenge should be self-explanatory?

benthecarman commented at 6:56 pm on April 12, 2023:

done

maflcko commented at 2:37 pm on April 14, 2023:

unrelated nit: If you retouch, you could also remove __func__ for -signetchallenge

benthecarman force-pushed on Apr 12, 2023

benthecarman commented at 7:31 pm on April 12, 2023: contributor

If anyone is interested in testing, I am running this for a custom signet.

0[signet]
1signetchallenge=512102f7561d208dd9ae99bf497273e16f389bdbd6c4742ddb8e6b216e64fa2928ad8f51ae
2signetblocktime=30
3addnode=45.79.52.207:38333
4dnsseed=0

Block explorer: https://mutinynet.com faucet: https://faucet.mutinynet.com

There is also a rapid gossip sync server and some lightning nodes as well

rsafier commented at 9:28 pm on April 12, 2023: none

I made some quick mods to have working docker for this PR: https://github.com/rsafier/bitcoin_signet/tree/carman-blocktime-adjustable

I used it to get my 10Msats.

rsafier commented at 2:06 am on April 15, 2023: none

ACK d8434da3c14ed6723d86ef2cd266008d366e1413 Tested against parameters for Mutinynet above, Plebnet Playground, ZBD signet as a client, synced up no issues. Tested mining on throw away random signet at 1s block time target for 2300 blocks, seems good, didn’t suddenly get harder.

RandyMcMillan commented at 10:48 pm on April 17, 2023: contributor

utACK d8434da

kristapsk commented at 4:33 pm on April 18, 2023: contributor

Concept ACK

achow101 commented at 9:27 pm on April 18, 2023: member

-0

If we want to allow for more configurable signet consensus rules, then I think we need to do it in a more robust way rather than ad-hoc adding options that all users must remember to set if they want to use a particular signet. As these are consensus parameters, forgetting to set the option correctly will result in eventual consensus failure which is generally only resolved by nuking the data directory. This may garner additional and unnecessary issues being opened in this repo which we generally don’t want to deal with.

A key difference between -signetchallenge and the proposed -signetblocktime is that if -signetchallenge is incorrect, no blocks will be downloaded and the signet datadir is still usable for any signet, whereas if -signetblocktime is incorrect, some blocks will be downloaded before a failure condition that leaves the datadir in a bad state.

This brings up a related issue in that we can’t have multiple signets in the same base datadir. All signets use the signet datadir, so any change of parameter may result in a bad chainstate for that signet node.

I think we should instead look at implementing a more robust multiple signet solution that allows for multiple configurable consensus rules in a clean and simple way since clearly that’s desired. Ideally we would have a single parameter that contains all of the rules being configured rather than having multiple for each rule. We should also make datadirs specific to each set of rules, ideally in a deterministic way.

maflcko commented at 1:57 pm on April 19, 2023: member

I guess one can pass all non-default consensus args to a hasher and use that as the network magic, and also use the network magic as the suffix for the signet folder name.

This would also allow to make more consensus args configurable as a non-breaking change in the future. Though, obviously it would be a breaking change now.

ajtowns commented at 4:03 pm on April 20, 2023: contributor

@ajtowns I don’t think I am properly communicating my usecase correctly,

When I asked what you were trying to solve, you didn’t respond to that question at all…

The example listed on the mutinynet blog entry is “The most obvious win here is we can have a channel “confirmed” much more quickly.” – but that isn’t a win in the long term: it’s setting expectations that mainnet will inevitably fall short of, no matter what you do (pay a high fee, do lots of rbf, wait for a lull in the mempool, run your own mining operation, etc).

but there are lots of concept acks from other developers that run signets as well. People want to use custom signets to have their own test networks, I don’t see why we should disallow having custom block times.

There have always been plenty of people who’d like an easy way to create altcoins with faster block times, and that’s a fine thing to do if you’re maintaining your own codebase, but the purpose of including signet in this repo is about making it easier to test bitcoin, not about building your own altcoin with different parameters. How does using a 30s signet do anything but encourage developers to build software that will only work well on altcoins (like liquid and ethereum) with more frequent blocks than bitcoin?

I mean, I understand where you’re coming from – waiting for a block is annoying, and papering over it for a test network is really easy, which just adds to the frustration.

You can get a similar result without changing consensus too, either having the miner pause when it would otherwise mine an empty block, and do a burst when transactions actually come in, or making use of the timewarp bug, and just having blocks occur at whatever rate you want. But both those have the same problem: they’re giving you behaviour that you won’t see on mainnet.

But if you really just want “instant channel setup” you can actually do that in a way that is suitable for mainnet though – just do “zeroconf” channels (https://github.com/lightning/bolts/pull/910), where the faucet opens a channel with the user and immediately pushes X sats to them. You could even open a channel with a balance of 40X, and push X sats to them every day for a month to “reward” people that actually keep their node running…

ajtowns commented at 4:11 pm on April 20, 2023: contributor

If we want to allow for more configurable signet consensus rules, then I think we need to do it in a more robust way

I think the easy, mostly backwards compatible way of doing this would just be to have the signetchallenge contain the data directly, eg making it be the script OP_RETURN PUSHDATA<params> PUSHDATA<actual challenge>. Then you’re still just passing around a hex “challenge” string that completely defines the signet consensus behaviour, and will get different magic when there are different parameters, even for the same mining pubkey. Also old software will immediately choke on it. I haven’t come up with anything that seems like a good reason to vary consensus params though: if you want to test non-bitcoin behaviours, why modify bitcoin to do it?

benthecarman commented at 3:56 am on April 21, 2023: contributor

I haven’t come up with anything that seems like a good reason to vary consensus params though: if you want to test non-bitcoin behaviours, why modify bitcoin to do it?

I just want to open a lightning channel and not need to wait an hour to use it…

ekzyis commented at 4:28 pm on April 24, 2023: none

utACK d8434da3c14ed6723d86ef2cd266008d366e1413

If we want to allow for more configurable signet consensus rules, then I think we need to do it in a more robust way rather than ad-hoc adding options that all users must remember to set if they want to use a particular signet. As these are consensus parameters, forgetting to set the option correctly will result in eventual consensus failure which is generally only resolved by nuking the data directory. This may garner additional and unnecessary issues being opened in this repo which we generally don’t want to deal with.

– #27446 (comment)

I think the concerns are valid. However, I think this won’t happen often so shouldn’t be much of a concern right now. There is only one additional parameter added now which can’t be used on it’s own. So if someone is setting up a signet and does not know about this new parameter, he is probably already using the correct parameters.

If someone is setting up a signet and knows about this new parameter, he will most likely know to set this parameter correctly.

And even if not: I assume throwing away all the signet data isn’t that much of a problem anyway? Wouldn’t this “eventual consensus failure” happen pretty early? If I understood @benthecarman correctly, this will happen in block 2016 (difficulty adjustment period) which seems to be early enough:

The main concern being people don’t copy-paste the param into their conf and have troubles syncing, doesn’t seem like a valid one given that they could do the same for -signetchallenge. I understand that it would cause a failure on block 1 vs block 2016, but normally those blocks are synced near instantly anyways.

– #27446 (comment)

So I think this concern is more relevant if we go further into this direction regarding signet parameters as mentioned:

If we want to allow for more configurable signet consensus rules

Essentially, I agree with this comment:

So in the end the parameter would need to be distributed the same way as signetchallenge

I had similar thoughts and think eventually we may want to introduce a more encompassing (and upgradeable) parameter such as -signetconfig in which all signet consensus parameters are encoded (currently just challenge and blocktime), but I think starting with the current modular approach is fine given that it’s signet.

– #27446 (comment)

if you want to test non-bitcoin behaviours, why modify bitcoin to do it?

I wouldn’t call this change “non-bitcoin behaviour”. It’s essentially just simulating miners consistently getting really lucky

litch commented at 0:55 am on May 3, 2023: contributor

tACK - we’re running this fork in several contexts and it works as expected

TheCharlatan commented at 7:06 pm on May 21, 2023: contributor

Approach NACK.

I get this - wasting time waiting for confirmations is more than annoying. Its effect usually lingers, because you loose context in the meantime and it slows down development in your entire organization. However, I also think supporting options that might lead to clients running disagreeing consensus rules on the same network is dangerous and might annoy developers in fresh ways. I would support the approach laid out by ajtowns in this comment.

benthecarman commented at 7:39 pm on May 21, 2023: contributor

Approach NACK.

I get this - wasting time waiting for confirmations is more than annoying. Its effect usually lingers, because you loose context in the meantime and it slows down development in your entire organization. However, I also think supporting options that might lead to clients running disagreeing consensus rules on the same network is dangerous and might annoy developers in fresh ways. I would support the approach laid out by ajtowns in this comment.

If someone wants to do this, feel free to make it. I think that this approach is much simpler. It is already being used by multiple companies and individuals with no issues.

michaelfolkson commented at 12:02 pm on May 24, 2023: contributor

I think “you should continue to maintain a fork of Core if this adds significant value to you” is a reasonable counter. I did a quick scan of historical pull requests and I couldn’t find any pull requests that were merged to support bitcoin-inquisition specifically beyond the introduction of signet itself.

However, I also think supporting options that might lead to clients running disagreeing consensus rules on the same network is dangerous

We are already in a world of (potentially) disagreeing consensus rules with bitcoin-inquisition. There is a difference though between consensus rules that would never be considered to be merged on mainnet (e.g. this block time configuration) and new consensus rules that could plausibly be activated on mainnet.

dangershony commented at 3:12 pm on June 5, 2023: contributor

Having tried to setup a customs signet I must ACK the concept.

But both those have the same problem: they’re giving you behaviour that you won’t see on mainnet.

You can totally get 30 sec blocks on mainnet, sure its very uncommon but valid none the less.

ajtowns commented at 8:21 pm on July 3, 2023: contributor

I haven’t come up with anything that seems like a good reason to vary consensus params though: if you want to test non-bitcoin behaviours, why modify bitcoin to do it?

I just want to open a lightning channel and not need to wait an hour to use it…

With core lightning:

on the “faucet” side, you need to create a zeroconf channel and fund it. to fund a 0.1 BTC channel with 0.05 BTC sent to the receiver, do: lightning-cli -k --signet fundchannel id=$RECEIVER amount=1000000 feerate=1000perkb announce=true minconf=0 mindepth=0 push_msat=500000000 (note that amount is in sats and push_msat is in msats)
on the receiving side, you need to accept zeroconf channels, which seems like it requires the zeroconf-selective test plugin. run: lightning-cli -k --signet plugin subcommand=start plugin=$HOME/lightning/tests/plugins/zeroconf-selective.py zeroconf-allow=any to make that happen. or change any to the faucet’s node id.

The security concern with doing this in general is that the channel might never confirm; in that case any funds that you received on the channel will disappear – if you only received the initial faucet funds, that is probably fine: it’s no worse than the faucet not working in the first place. (If this were on mainnet, presumably you would have deposited $1500 cash to justify the 0.05 BTC and you’ll want to recover the $1500 cash, but again, it’s the same process to do that whether the channel failed or if they declined to open a channel in the first place)

However if you create invoices for incoming payments, and resolve them prior to the channel being confirmed, that could result in loss of income, and the zeroconf-selective plugin above doesn’t do anything to prevent that. But it’s signet, so it’s easy to just say “YOLO” and not worry; and even on mainnet, if you were in a position to recover the $1500, this may just mean you’ll now be wanting to recover $1600 from the same company/group instead, and is not a meaningfully different problem.

Likewise if you route payments from a zeroconf channel through some other channel, that would allow stealing funds from that other channel – the funds go out on the other channel, and the funds that looked like they were coming in on the zeroconf channel disappear when the channel never confirms. (This is a good reason not to use any above, even on signet).

Fixing those special cases (ie, rejecting incoming funds on a zeroconf channel on the receiver’s side) might be enough for this approach to work on mainnet and avoid hour long waits there too, if you can establish a reasonable level of trust when setting up the channel (the “faucet” wants to actually get the $1500 and not be chargebacked after losing 0.05 BTC; the receiver wants to be able to go to the police to recover the $1500 if the channel doesn’t open).

benthecarman commented at 8:30 pm on July 3, 2023: contributor

@ajtowns I dont think we’re gonna agree on this so whatever. I will continue to maintain this fork for its users.

https://github.com/benthecarman/bitcoin/releases/tag/custom-signet-blocktime

ajtowns commented at 9:50 pm on July 3, 2023: contributor

@ajtowns I dont think we’re gonna agree on this so whatever.

I don’t understand why you’re ignoring an approach that seems to do a better job of getting what you want (instant channels, without even having to wait a minute), and can work on mainnet (so won’t end up developing a UX that’s only conceivably good for demos and doesn’t work with real money), and is already supported by CLN, LND and LDK. If you don’t want to explain, or even just figure “if it’s dumb but it works that’s good enough”, I guess that’s your choice, but it seems like a super unproductive approach.

benthecarman commented at 9:57 pm on July 3, 2023: contributor

The wallet that I am working on supports and uses 0-conf channels. That doesn’t negate the fact that shorter block times are useful.

luke-jr commented at 6:42 pm on July 31, 2023: member

…the purpose of including signet in this repo is about making it easier to test bitcoin, not about building your own altcoin with different parameters. How does using a 30s signet do anything but encourage developers to build software that will only work well on altcoins (like liquid…

Just thought I should point out that Liquid is Bitcoin, not an altcoin.

bitcoin deleted a comment on Jul 31, 2023

chrisguida commented at 0:22 am on August 29, 2023: none

#27446 (comment)

How does using a 30s signet do anything but encourage developers to build software that will only work well on altcoins (like liquid and ethereum) with more frequent blocks than bitcoin?

I don’t understand the above criticism at all.. obviously people using the 30s signet are using it only for development purposes. It helps with development, especially for second-layer protocols and apps, because it vastly reduces the time you need to wait for things to happen.

I don’t think any developer using 30-second block times would be unaware that mainnet has 10-minute block times, and I very much doubt any developers will fail to consider mainnet block times in their apps. If somehow they do, they’ll quickly discover their error as soon as they deploy to mainnet. In this case it’s obviously the fault of the developers themselves, or at most the fault of the signet operators. I don’t see how this could possibly lead to people creating issues in this repo.

But perhaps I’m missing something…

DrahtBot added the label CI failed on Sep 11, 2023

DrahtBot removed the label CI failed on Sep 16, 2023

DrahtBot added the label CI failed on Oct 20, 2023

DrahtBot removed the label CI failed on Oct 24, 2023

DrahtBot added the label CI failed on Oct 25, 2023

cmdruid commented at 1:11 am on November 6, 2023: none

Regtest is not an option if you want to host your own test network for other users to test your software, as anyone can grief the network by generating blocks. Default testnet/signet is too slow for actual testing.

I’d like the ability to host a test network that outside users can use, but not grief, while having fast block times (or even no time restriction at all) so that I can incorporate it into CI/CD for different projects.

This seems like a real obvious tool to have for development of bitcoin infrastructure, and it is kind of crazy the only two options are regtest where everyone has god mode, or you are stuck with normal block times for testing.

I’ll just download and use the mutiny-net fork, which is fine for me, but if people are worried about fragmentation, I think having to use multiple forks of bitcoin for tooling is more dangerous than having multiple params for signet.

DrahtBot requested review from stickies-v on Nov 6, 2023

jsarenik commented at 9:07 am on November 6, 2023: none

@ajtowns wrote:

What’s the actual problem you’re trying to solve here? Would it work to have the signet miner only generate blocks when the mempool has entries, and sit idle when it’s empty, for instance? If you’re trying to generate many blocks, then regtest is already better as it doesn’t require a baseline proof of work, and simply having multiple regtest configs probably makes more sense.

The last comment above makes a point. It really is hard when anyone can spam the regtest with blocks. Could that be somehow prevented? E.g. a signet-mode in regtest (only permissioned keys can mine)? @cmdruid Thanks for comment! IMHO it sheds new light on this.

EDIT: The original link to comment wanted to refer to cmdruid’s comment. Apologies for unrelated link.

achow101 commented at 7:56 pm on January 10, 2024: member

Closing as there is continued disagreement over the approach and no further changes have been made to this PR. This seems to be at a stalemate and unlikely to go anywhere.

achow101 closed this on Jan 10, 2024

starius commented at 4:48 pm on January 31, 2024: none

Approach NACK. I get this - wasting time waiting for confirmations is more than annoying. Its effect usually lingers, because you loose context in the meantime and it slows down development in your entire organization. However, I also think supporting options that might lead to clients running disagreeing consensus rules on the same network is dangerous and might annoy developers in fresh ways. I would support the approach laid out by ajtowns in this comment.

If someone wants to do this, feel free to make it. I think that this approach is much simpler. It is already being used by multiple companies and individuals with no issues.

I want to do this. I implemented the code proposed in this comment and managed to sync with mutinynet. I’m writing tests. Expect a PR soon :)

Allow configuring target block time for a signet #27446

Code Coverage

Reviews

Conflicts