Skip to content

Instantly share code, notes, and snippets.

@tarrenj

tarrenj/DRAFTdeveloper-notes.md

Last active September 27, 2018 23:56
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save tarrenj/36a14f8099c73a331f5b9ad420f28c2a to your computer and use it in GitHub Desktop.
Save tarrenj/36a14f8099c73a331f5b9ad420f28c2a to your computer and use it in GitHub Desktop.
Download ZIP
Raw
DRAFTdeveloper-notes.md

Development Process

All repositories at the ZenCashOfficial GitHub org are held to the same standards outlined in this section.

We practice "Enforced Gitflow via Protected Branches". The important parts are:

  • All work must be done in feature branches and merged to development.
  • Only development can be merged to master.
  • Developers are not allowed to merge their own pull requests.
  • Pull requests must recive at least one TestedACK before merging.
  • All version numbers must follow Semantic Versioning (semver_bash is recommended).

Protected branch rules:

  • Only repository or organizational admins may push to master.
  • Merges to master must have at least two approvals.
  • Approvals are dismissed after additional commits are made.
  • In some cases, merges to development must also have at least 1 approval.

Additional notes:

  • All protocol level changes must have their specifications submitted as a proposal, and that proposal must be accepted before being merged into development.
  • All Horizen team members are required to use signed commits. Community members are not required to sign commits, but are encouraged to do so.
  • A well documented issue should be created before beginning work in a feature branch.
  • All pull requests must build as submitted.

Example Workflow

An example workflow could be:

  1. Create a well documented issue or new feature with user story.
  2. Create a feature branch or fork and implement the feature or fix.
  3. Perform a test build and verify functionality of the feature/fix.
  4. Submit a pull request to development, include documentation on how the feature/fix was tested.
  5. That pull request is approved by at least two other parties, including at least one TestedACK.
  6. The issue is closed, or the label "Included in next release" is added.
  7. A pull request is submitted to merge development into master for the next release.
  8. That pull request receives two approvals, and is merged into master as the next release.
  9. The issue is closed if needed.

Releases

NOTE: The release prcess is in progress.
      Various updates to the referenced doc/ files are required before it can be followed 

NOTE: The requirements for releasing a hotfix are different, see: doc/hotfix-process.md

Version numbers

Testing and required updates.

In addition to specific testing of the updated code, the following must be performed before a release is made:

All testcases must be run with:

  • qa/zcash/full-test-suite.py
  • qa/zen/full-test-suite.sh
  • qa/pull-tester/rpc-tests.sh

The relevant documentation must be updated:

  • If applicable copyright headers must be updated. See contrib/devtools/fix-copyright-headers.py

  • If applicable, tanslations must be updated. See contrib/devtools/update-translations.py

  • If applicable, manpages must be regenerated. See contrib/devtools/gen-manpages.sh

  • If applicable, Linux binaries produced by gitian must be verified to only contain allowed gcc, glibc, and libstdc++ version symbols. See contrib/devtools/symbol-check.py

  • Update the changelog using keep-a-changelog

  • Update the version numbers using semver_bash

After those have been performed, follow the release process outlined within doc/release-process.md

Coding

Various coding styles have been used during the history of the codebase, and the result is not very consistent. However, we are 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:

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;
    }
}
}

zen folder and namespace

All Horizen-specific files should reside under a zen subfolder and Horizen-specific C++ classes must be declared within a zen namespace.

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:

/**
 * ... 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:

/**
 * 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:

int var; //!< Detailed description after the member

Also OK:

///
/// ... text ...
///
bool function2(int arg1, const char *arg2)

Not OK (used plenty in the current source, but not picked up):

//
// ... 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

Test suite

the pyblake2 and zmq packages are required to run the test suite.

pip install pyblake2 zmq

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 Horizen" 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

Horizen 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 9033 (19033 for testnet).

  • 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 18231 for connections and services them.

  • horizen-miner : Generates Horizen (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.

  • NIT - Short for "nit pick", used during reviews to indicate minor, non-blocking, issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment