Coding ==================== Various coding styles have been used during the history of the codebase, and the result is not very consistent. However, we're now trying to converge to a single style, so please use it in new code. Old code will be converted gradually. - Basic rules specified in src/.clang-format. Use a recent clang-format-3.5 to format automatically. - Braces on new lines for namespaces, classes, functions, methods. - Braces on the same line for everything else. - 4 space indentation (no tabs) for every block except namespaces. - No indentation for public/protected/private or for namespaces. - No extra spaces inside parenthesis; don't do ( this ) - No space after function names; one space after if, for and while. Block style example: ```c++ namespace foo { class Class { bool Function(char* psz, int n) { // Comment summarising what this section of code does for (int i = 0; i < n; i++) { // When something fails, return early if (!Something()) return false; ... } // Success return is usually at the end return true; } } } ``` Doxygen comments ----------------- To facilitate the generation of documentation, use doxygen-compatible comment blocks for functions, methods and fields. For example, to describe a function use: ```c++ /** * ... text ... * @param[in] arg1 A description * @param[in] arg2 Another argument description * @pre Precondition for function... */ bool function(int arg1, const char *arg2) ``` A complete list of `@xxx` commands can be found at http://www.stack.nl/~dimitri/doxygen/manual/commands.html. As Doxygen recognizes the comments by the delimiters (`/**` and `*/` in this case), you don't *need* to provide any commands for a comment to be valid; just a description text is fine. To describe a class use the same construct above the class definition: ```c++ /** * Alerts are for notifying old versions if they become too obsolete and * need to upgrade. The message is displayed in the status bar. * @see GetWarnings() */ class CAlert { ``` To describe a member or variable use: ```c++ int var; //!< Detailed description after the member ``` Also OK: ```c++ /// /// ... text ... /// bool function2(int arg1, const char *arg2) ``` Not OK (used plenty in the current source, but not picked up): ```c++ // // ... text ... // ``` A full list of comment syntaxes picked up by doxygen can be found at http://www.stack.nl/~dimitri/doxygen/manual/docblocks.html, but if possible use one of the above styles. Development tips and tricks --------------------------- **compiling for debugging** Run configure with the --enable-debug option, then make. Or run configure with CXXFLAGS="-g -ggdb -O0" or whatever debug flags you need. **debug.log** If the code is behaving strangely, take a look in the debug.log file in the data directory; error and debugging messages are written there. The -debug=... command-line option controls debugging; running with just -debug or -debug=1 will turn on all categories (and give you a very large debug.log file). **testnet and regtest modes** Run with the -testnet option to run with "play zcash" on the test network, if you are testing multi-machine code that needs to operate across the internet. If you are testing something that can run on one machine, run with the -regtest option. In regression test mode, blocks can be created on-demand; see qa/rpc-tests/ for tests that run in -regtest mode. **DEBUG_LOCKORDER** Zcash is a multithreaded application, and deadlocks or other multithreading bugs can be very difficult to track down. Compiling with -DDEBUG_LOCKORDER (configure CXXFLAGS="-DDEBUG_LOCKORDER -g") inserts run-time checks to keep track of which locks are held, and adds warnings to the debug.log file if inconsistencies are detected. Locking/mutex usage notes ------------------------- The code is multi-threaded, and uses mutexes and the LOCK/TRY_LOCK macros to protect data structures. Deadlocks due to inconsistent lock ordering (thread 1 locks cs_main and then cs_wallet, while thread 2 locks them in the opposite order: result, deadlock as each waits for the other to release its lock) are a problem. Compile with -DDEBUG_LOCKORDER to get lock order inconsistencies reported in the debug.log file. Re-architecting the core code so there are better-defined interfaces between the various components is a goal, with any necessary locking done by the components (e.g. see the self-contained CKeyStore class and its cs_KeyStore lock for example). Threads ------- - ThreadScriptCheck : Verifies block scripts. - ThreadImport : Loads blocks from blk*.dat files or bootstrap.dat. - StartNode : Starts other threads. - ThreadDNSAddressSeed : Loads addresses of peers from the DNS. - ThreadMapPort : Universal plug-and-play startup/shutdown - ThreadSocketHandler : Sends/Receives data from peers on port 8233. - ThreadOpenAddedConnections : Opens network connections to added nodes. - ThreadOpenConnections : Initiates new connections to peers. - ThreadMessageHandler : Higher-level message handling (sending and receiving). - DumpAddresses : Dumps IP addresses of nodes to peers.dat. - ThreadFlushWalletDB : Close the wallet.dat file if it hasn't been used in 500ms. - ThreadRPCServer : Remote procedure call handler, listens on port 8232 for connections and services them. - ZcashMiner : Generates zcash (if wallet is enabled). - Shutdown : Does an orderly shutdown of everything. Pull Request Terminology ------------------------ Concept ACK - Agree with the idea and overall direction, but have neither reviewed nor tested the code changes. utACK (untested ACK) - Reviewed and agree with the code changes but haven't actually tested them. Tested ACK - Reviewed the code changes and have verified the functionality or bug fix. ACK - A loose ACK can be confusing. It's best to avoid them unless it's a documentation/comment only change in which case there is nothing to test/verify; therefore the tested/untested distinction is not there. NACK - Disagree with the code changes/concept. Should be accompanied by an explanation.