Add -requirestandard command line option

petertodd commented at 3:52 pm on June 8, 2015: contributor

Previousy there was no way to enable IsStandard() in testnet or regtest, making testing related to standardness difficult. This new option lets you (optionally) enable IsStandard() during testing, better approximating the behavior of mainnet.

Previousy there was no way to enable IsStandard() in testnet or regtest,
making testing related to standardness difficult. This new option lets
you (optionally) enable IsStandard() during testing, better
approximating the behavior of mainnet.

757a1aa2a9

in src/init.cpp: in 757a1aa2a9

393@@ -394,6 +394,7 @@ std::string HelpMessage(HelpMessageMode mode)
394     }
395     strUsage += HelpMessageOpt("-shrinkdebugfile", _("Shrink debug.log file on client startup (default: 1 when no -debug)"));
396     strUsage += HelpMessageOpt("-testnet", _("Use the test network"));
397+    strUsage += HelpMessageOpt("-requirestandard", _("Require transactions to be standard even in testnet/regtest"));

Diapolo commented at 9:11 pm on June 8, 2015:

Nit: Can you please add the (default: %d) here and set it to 0 after the string?

petertodd commented at 9:16 pm on June 8, 2015:

To be clear, the way -requirestandard works is that it can only turn on IsStandard() - it can’t turn it off. Turning off IsStandard() is problematic on mainnet due to DoS attacks, and this simplistic implementation doesn’t have any code to prevent those DoS attacks. (like @luke-jr’s -acceptnonstdtxn patch does)

With the above in mind, would you still say the -help text should have the default thing? I think that’d give the user the wrong impression that passing -requirestandard=0 in mainnet would turn off IsStandard()

Diapolo commented at 9:20 pm on June 8, 2015:

Perhaps you should somehow disallow setting it to false on mainnet or only allow setting it on testnet/regtest then? Or extend the help message explaining that fact? Dunno what is best, but an option without a default looks strange IMHO.

petertodd commented at 9:24 pm on June 8, 2015:

I didn’t want to make setting it on mainnet cause any issues, as it’s annoying to have scripts and bitcoin.conf files where changing from mainnet to testnet requires more than a single flag to be set; I think the help text as-is gives the impression sufficiently. After all, the fact that it doesn’t have the default to me at least implies it’s an option that only does something when set, and that setting it to =0 off is equivalent to not specifying it at all.

luke-jr commented at 9:14 pm on June 8, 2015: member

This option has historically been called -acceptnonstdtxn; best to keep compatible with that. It should also work on mainnet.

luke-jr commented at 10:45 pm on June 8, 2015: member

If the scope is to be limited to just non-mainnet, I suggest something like:

0if (chainparams.RequireStandard() && GetArg("-acceptnonstdtxn", false))
1  return InitError(_("acceptnonstdtxn is not currently supported for %s chain", chainparams.strNetworkID));

laanwj added the label TX fees and policy on Jun 9, 2015

laanwj commented at 12:19 pm on June 9, 2015: member

Concept ACK.

I think using the same option for mainnet and testnet makes it too easy to accidentally disable it on mainnnet, though (blame that on using the same conf file for testnet and mainnet…).

petertodd commented at 2:49 pm on June 9, 2015: contributor

I’d be more inclined to use the name -acceptnonstdtxn if we were planning on actually adding a way to disable IsStandard() on mainnet; if not I’m with @laanwj

luke-jr commented at 6:43 pm on June 9, 2015: member

@petertodd Policy is not something the dev team ought to be deciding, so a way to easily disable IsStandard on mainnet is only reasonable. @laanwj Maybe we should include ~/.bitcoin/testnet3/bitcoin.conf on top of the main config?

petertodd commented at 9:35 pm on June 9, 2015: contributor

@luke-jr It’s a fair point, although we need some code to make that somewhat safe, e.g. re: excess sigops attacks. Also IIRC you can still add enough hash ops/etc. to make a block that takes excessively long to verify.

luke-jr commented at 9:38 pm on June 9, 2015: member

@petertodd My branch already adds that. I suggest just forbidding it here (see previous code snippet) to limit the scope of complexity in this PR, though.

jgarzik commented at 4:52 am on June 10, 2015: contributor

concept ACK

laanwj commented at 3:43 pm on June 12, 2015: member

@laanwj Maybe we should include ~/.bitcoin/testnet3/bitcoin.conf on top of the main config?

The location of the configuration is primarily driven by the -conf option. This configuration file can specify the data directory, as well as -testnet, -regtest. Thus reading another configuration file depending on the network would make the option processing even more complex. In retrospect, a (sub)config file per network may have been better, but let’s leave it as it is.

You can accomplish the same if you switch the network, instead of with -testnet on the command line, by loading a different configuration file using -conf=bitcoin-testnet.conf, where that file has testnet=1 as well as your testnet-specific parameters.

petertodd commented at 2:34 pm on July 3, 2015: contributor

Closing as Luke-jr’s #6329 was merged instead.

petertodd closed this on Jul 3, 2015

DrahtBot locked this on Sep 8, 2021

Add -requirestandard command line option #6255