Constrain constant values to a single location in code

luke-jr commented at 3:03 am on June 28, 2015: member

paveljanik commented at 9:56 am on June 28, 2015: contributor

No wallet build fails with:

CXX libbitcoin_server_a-init.o init.cpp: In function ‘std::string HelpMessage(HelpMessageMode)’: init.cpp:363:156: error: ‘DEFAULT_WALLET_DBLOGSIZE’ was not declared in this scope init.cpp:368:121: error: ‘DEFAULT_FLUSHWALLET’ was not declared in this scope init.cpp:394:128: error: ‘DEFAULT_WALLET_PRIVDB’ was not declared in this scope

jgarzik commented at 2:07 pm on June 28, 2015: contributor

concept ACK.

seemed like the REST constant should go in a rest, not rpc, file

petertodd commented at 1:43 am on June 30, 2015: contributor

concept ACK

luke-jr commented at 4:31 am on July 1, 2015: member

@jgarzik I put it in rpcserver.h because the -rest option is used in rpcserver.cpp, which handles all HTTP requests. There is no rest.h - do you really want me to make one just for this?

luke-jr commented at 4:31 am on July 1, 2015: member

@paveljanik Fixed.

in src/main.h: in 9241201f76 outdated

89@@ -90,6 +90,13 @@ static const unsigned int DATABASE_WRITE_INTERVAL = 60 * 60;
90 static const unsigned int DATABASE_FLUSH_INTERVAL = 24 * 60 * 60;
91 /** Maximum length of reject messages. */
92 static const unsigned int MAX_REJECT_MESSAGE_LENGTH = 111;
93+static const unsigned int DEFAULT_LIMITFREERELAY = 15;
94+static const bool DEFAULT_RELAYPRIORITY = true;
95+
96+static const bool TXINDEX_DEFAULT = false;

Diapolo commented at 6:03 am on July 1, 2015:

Why did you call this TXINDEX_DEFAULT instead of DEFAULT_TXINDEX?

luke-jr commented at 6:08 am on July 1, 2015:

Dunno, changed for consistency.

in src/script/standard.cpp: in 9241201f76 outdated

53@@ -53,7 +54,7 @@ bool Solver(const CScript& scriptPubKey, txnouttype& typeRet, vector<vector<unsi
54         mTemplates.insert(make_pair(TX_MULTISIG, CScript() << OP_SMALLINTEGER << OP_PUBKEYS << OP_SMALLINTEGER << OP_CHECKMULTISIG));
55 
56         // Empty, provably prunable, data-carrying output
57-        if (GetBoolArg("-datacarrier", true))

Diapolo commented at 6:04 am on July 1, 2015:

Why is this removed and now defaults just to true? Edit: Ah okay, you moved that to another file…

luke-jr force-pushed on Jul 1, 2015

laanwj added the label Refactoring on Jul 2, 2015

in src/util.h: in 09c3b82f97 outdated

47@@ -48,6 +48,13 @@ extern bool fLogIPs;
48 extern volatile bool fReopenDebugLog;
49 extern CTranslationInterface translationInterface;
50 
51+static const char BITCOIN_CONF_FILENAME[] = "bitcoin.conf";

laanwj commented at 4:55 pm on July 20, 2015:

Declaring the constant and its value in a header won’t work well for data structures that require a larger amount of memory or indirected pointers, such as strings, structs, objects etc. In worst case this causes errors in some compilers, in best case this causes unnecessary memory use.

laanwj commented at 12:06 pm on August 3, 2015:

So please move the content of the variable and BITCOIN_PID_FILENAME to util.cpp Apart from that I’d like to merge this

laanwj commented at 4:56 pm on July 20, 2015: member

Concept ACK

laanwj commented at 3:41 pm on August 20, 2015: member

Needs rebase + string nit addresed

dcousens commented at 10:14 pm on August 20, 2015: contributor

concept ACK

jgarzik commented at 10:19 am on October 1, 2015: contributor

concept ACK - as @laanwj says it appears merge-ready once rebased + nits addressed

Bugfix: RPC: blockchain: Display correct defaults in help for verifychain method caa3d42f06

Bugfix: Describe dblogsize option correctly (it refers to the wallet database, not memory pool) 420a82f1ae

Bugfix: If genproclimit is omitted to RPC setgenerate, don't change it; also show correct default in getmininginfo 5f9260f458

Constrain constant values to a single location in code d93613adea

Bugfix: Omit wallet-related options from -help when wallet is not supported 851b6e016f

Bugfix: Default -uiplatform is not actually the platform this build was compiled on

Also hides the option for non-GUI builds.

29266b64e8

luke-jr force-pushed on Oct 1, 2015

luke-jr commented at 11:44 pm on October 1, 2015: member

Rebased, addressed nit, added equivalent commit for -uiplatform. (Travis passes.)

in src/init.cpp: in 29266b64e8

497         strUsage += HelpMessageOpt("-rootcertificates=<file>", _("Set SSL root certificates for payment request (default: -system-)"));
498-        strUsage += HelpMessageOpt("-splash", _("Show splash screen on startup (default: 1)"));
499-        if (showDebug) {
500-            strUsage += HelpMessageOpt("-uiplatform", "Select platform to customize UI for (one of windows, macosx, other; default: platform compiled on)");
501+        strUsage += HelpMessageOpt("-splash", strprintf(_("Show splash screen on startup (default: %u)"), DEFAULT_SPLASHSCREEN));
502+        if (showDebug && !uiInterface.DefaultUIPlatform.empty()) {

laanwj commented at 8:23 am on October 2, 2015:

I don’t think we should use uiInterface here, nor put this uiplatform id as a variable on the UI interface (as bitcoind has no business with it). I’d just check for mode == HMM_BITCOIN_QT, like for the other options.

laanwj commented at 9:42 am on October 2, 2015:

Hmm I understand now, you want to show the actual default here. Ok then it makes sense. We should never have moved the UI-specific options back to init, but that’s not your fault.

dcousens commented at 11:38 pm on October 4, 2015: contributor

utACK, however I didn’t cross verify the constants.

MarcoFalke commented at 2:28 pm on October 5, 2015: member

utACK 29266b64e8d17baee18d42a93a8f1a790ad858ca.

MarcoFalke commented at 6:08 pm on October 13, 2015: member

Does this need more reviewers or can it be merged as is? (Any further merge conflicts would only retrigger review, so let’s try to finish with this PR.)

MarcoFalke commented at 8:35 pm on October 20, 2015: member

Needs rebase (caused by #6235)

dcousens commented at 1:52 am on October 21, 2015: contributor

@luke-jr this needs rebase @laanwj anything holding back from merge?

sipa commented at 9:16 pm on November 28, 2015: member

Fixed by #6961.

sipa closed this on Nov 28, 2015

MarcoFalke commented at 9:19 pm on November 28, 2015: member

@luke-jr are you going to rebase (3/3) 29266b6?

MarcoFalke locked this on Sep 8, 2021

Constrain constant values to a single location in code #6349