Permalink
Fetching contributors…
Cannot retrieve contributors at this time
520 lines (362 sloc) 17 KB

Raiden Development Guide

Welcome! This guide serves as the guideline to contributing to the Raiden Network codebase. It's here to help you understand what development practises we use here and what are the requirements for a Pull Request to be opened against Raiden.

Contributing

There are two ways you can contribute to the development. You can either open an Issue or if you have programming abilities open a Pull Request.

Creating an Issue

If you experience a problem while using Raiden or want to request a feature then you should open an issue against the repository. All issues should contain:

For Feature Requests:

  • A description of what you would like to see implemented.
  • An explanation of why you believe this would make a good addition to Raiden.

For Bugs:

  • A short description of the problem.
  • Detailed description of your system, raiden version, geth version, solidity version e.t.c.
  • What was the exact unexpected thing that occured.
  • What you were expecting to happen instead.

Creating a Pull Request

If you have some coding abilities and would like to contribute to the actual codebase of Raiden then you can open a Pull Request(PR) against the repository.

All PRs should be:

  • Self-contained.
  • As short as possible and address a single issue or even a part of an issue. Consider breaking long PRs into smaller ones.

In order for a Pull Request to get merged into the main repository you should have one approved review from one of the core developers of Raiden and also all Continuous Integration tests should be passing and the CI build should be green.

Additionally you need to sign the raiden project CLA (Contributor License Agreement). Our CLA bot will help you with that after you created a pull request. If you or your employer do not hold the whole copyright of the authorship submitted we can not accept your contribution.

Development environment setup

Nix

See nix.rst.

Development external dependencies

These are the required external dependencies for development:

  • Python >=3.6.
  • Ethereum client (Geth recommended).
  • Solidity compiler >=0.4.23.
  • A matrix capable server (Synapse recommended).
  • A compiler tool chain to compile the eliptic curve library.
  • Git for version control.

Get the code

Start by getting the source code

git clone https://github.com/raiden-network/raiden.git
cd raiden

Install system wide dependencies

Archlinux

All the required packages are available in community or extra:

sudo pacman -Sy go-ethereum python python-pip python-virtualenv solidity \
base-devel git
Debian/Ubuntu

To get the Geth client and solidity compiler add Ethereum's PPA repository:

sudo add-apt-repository -y ppa:ethereum/ethereum
sudo apt-get update

Then install the required packages:

sudo apt-get install build-essential git libffi-dev libgmp-dev libssl-dev \
libtool pkg-config python3 python3-pip python3-dev python3-virtualenv \
ethereum solc git

Create the virtual env

It's highly recommended to use a virtual environment for development. This will isolate the python dependencies used by Raiden from your system:

virtualenv env
source ./env/bin/activate

Development dependencies

Install the development dependencies:

pip install -r requirements-dev.txt -c constraints.txt
./tools/install_synapse.sh

Development Guidelines

Testing

To run the tests use pytest

pytest raiden

Tests are split in unit tests and integration tests. The first are faster to execute while the latter test the whole system but are slower to run. To choose which type of tests to run, just use the appropriate folder

pytest raiden/tests/<integration|unit>

Testing on the CI

By default whenever you make a Pull Request the linter tests, smoketests, unit tests and all the integration tests will run. This is in essence our entire test suite and at the time of writting takes around 50 mins assuming travis has enough free jobs for all the parallel jobs to run without any waiting time.

This can take a lot of time and as such there are various labels you can add in the body of a commit in order to change which tests run on Travis. Following is a breakdown of those labels:

  • [skip ci]: This will skip all tests and run nothing. A repository administrator must merge the PR if you do this for the last commit.
  • [ci nightly]: This will run no tests but start a nightly release build.
  • [no ci integration]: This will only run the linter, smoketest and unit tests. It is rather fast.
  • [ci integration-general]: This will run linter, smoke, unit and only the integration tests under the integration/ directory but no other subdirectories. This will include our slowest integration tests at the moment. At the time of writting they take around 38 mins.
  • [ci integration-transfer]: This will run linter, smoke, unit and only the integration tests under the integration/transfer directory. At the time of writting these take around 20 mins.
  • [ci integration-long-running]: This will run linter, smoke, unit and only the integration tests under the integration/long-running directory. At the time of writting these are empty so they take up only the time that is required for setting up a job. Around 2 mins.
  • [ci integration-contracts]: This will run linter, smoke, unit and only the integration tests under the integration/contracts directory. At the time of writting they take around 6 mins.
  • [ci integration-api]: This will run linter, smoke, unit and only the integration tests under the integration/api directory. At the time of writting they take around 17 mins.
  • [ci integration-cli]: This will run linter, smoke, unit and only the integration tests under the integration/cli directory. At the time of writting they take around 5 mins.

Commiting Rules

For an exhaustive guide read this guide. It's all really good advice. Some rules that you should always follow though are:

  • A commit title not exceeding 50 characters
  • A blank line after the title (optional if there is no description)
  • A description of what the commit did (optional if the commit is really small)

Why are these rules important? All tools that consume git repos and show you information treat the first 80 characters as a title. Even Github itself does this. And the git history looks really nice and neat if these simple rules are followed.

Encoding

Addresses should follow "sandwich encoding" so that each point of entry does its own encoding into binary but the core programmatic API accepts only binary. Thus we setup the following rules:

  • Programmatic API should only expect binary and break if it accepts anything else. It should do type checking on its input and provide meaningful error for hex encoded addresses or length mismatch.
  • All other places from which we receive addresses need to do their own encoding from whatever the input encoding is to binary. Such places are: CLI, Rest-API e.t.c.
  • All modules which generate output to the outside world should also encode from binary to whatever the expected output encoding of that module is. Such places are stdout, communication with the ethereum node e.t.c.

Coding Style

In this section we are going to describe the coding rules for contributing to the raiden repository. All code you write should strive to comply with these rules.

General Style

In this section we are going to see style rules that should be followed across all languages.

Line breaks after operators

For long expressions where we break the expression at the operators the line break should come after the operator and not before.

The following should be avoided:

        participant1_amount = (
            participant1_state.deposit
            + participant2_state.transferred_amount
            - participant1_state.transferred_amount
        );

instead it should be:

        participant1_amount = (
            participant1_state.deposit +
            participant2_state.transferred_amount -
            participant1_state.transferred_amount
        );

Python

Raiden is written in Python and we follow the official Python style guide PEP8. It is highly recommended to use the linting tools in order to automatically determine any and all style violations. The customizable part of pylint is at .pylint.rc.

Line Length

Flake8 will warn you for 99 characters which is the hard limit on the max length. Try not to go above it. We also have a soft limit on 80 characters but that is not enforced and is there just to encourage short lines.

Function definitions formatting

The line length must be smaller than 100 characters, so the function definition must be split when this limit is reached. When splitting the function arguments, each must go into its own line, followed by the argument type and a comma, and indented with 8 spaces.

The following is not allowed:

def function_with_many_args(argument1: Type1, argument2: Type2, argument3: Type3) -> ReturnType:
    pass

This must be used instead:

def function_with_many_args(
        argument1: Type1,
        argument2: Type2,
        argument3: Type3,
) -> ReturnType:
    pass

Docstrings

For docstrings we follow PEP 0257.

A single line docstring should be like this:

def a(
        b: B,
        c: C,
) -> D:
    """ Here be docs """
    pass

A multiline docstring should have a short title and then a body. So like this:

def a(
        b: B,
        c: C,
) -> D:
    """ Function Title

    body comes
    here
    """
    pass

The closing quotes should be on their own line. If in doubt consult the PEP.

Usage of single and double quotes

All strings must use single quotes by default.

Bad:

s = "foo"

Good:

s = 'foo'

The only reason to use double quotes is to avoid escaping the single quote in a string. So this is okay:

s = "Augusto's computer is awesome"

Naming Convention

Use descriptive variable names and avoid short abbreviations.

The following is bad:

mgr = Manager()
a = AccountBalanceHolder()
s = RaidenService()

While this is good:

manager = Manager()
balance_holder = AccountBalanceHolder()
service = RaidenService()

We try to follow a consistent naming convention throughout the codebase to make it easy for the reader of the code to understand what is going on. Thus we introduce the following rules:

For addresses:

  • <name>_address_hex for hex encoded addresses
  • <name>_address for binary encoded addresses

Lists of objects:

  • <name>s, e.g. channels for a list Channel object instances.

  • to initialize an empty list use list() instead of []. Note this is only for style consistency's sake and may change in the future as there may be a tiny change in performance.

Mappings/dicts:

If it is a simple one to one mapping

<name>_to_<name>, e.g. tokenaddress_to_taskmanager

If the mapped to object is a list then add an s

<name>_to_<name>s, e.g. tokenaddress_to_taskmanagers = defaultdict(list())

To initialize an empty dict use dict() instead of {}. Note this is only for style consistency's sake and may change in the future as there may be a tiny change in performance.

Class attributes and functions:

All class members should be private by default and they should start with a leading _. Whatever is part of the interface of the class should not have the leading underscore. The public interface of the class is what we should be testing in our tests and it should be the only way other parts of the code use the class through.

Minimal Example:

class Diary(object):

    def __init__(self, entries):
        self._entries = entries

    def entry(index):
        return _entries[index]

NewTypes and type comparisons

For often used types it makes sense to define new types using the typing.NewType function. New type names should be capitalized.

Address = NewType('Address', bytes)

These type definitions can not be used for type comparisons. To make this possible always define an associated alias, which must start with T_.

T_Address = bytes

typing.Optional convention

For typing.Optional we follow the convention that if the argument has a default value of None then we should omit the use of typing.Optional[].

Good Example:

def foo(a: int = None)

Bad Example:

def foo(a: typing.Optional[int] = None)

Solidity

For solidity we generally follow the style guide as shown in the solidity documentation with a few notable exceptions:

Variable Names

All variable name should be in snake case, just like in python. Function names on the other hand should be mixedCase. MixedCase is essentially like CamelCase but with the initial letter being a small letter. This helps us to easily determine which function calls are smart contract calls in the python code side.

function iDoSomething(uint awesome_argument) {
    doSomethingElse();
}

Documentation

Code should be documented. For docstrings the Google conventions are used.

Workflow

When developing a feature, or a bug fix you should always start by writting a test for it, or by modifying existing tests to test for your feature. Once you see that test failing you should implement the feature and confirm that all your new tests pass.

Afterwards you should open a Pull Request from your fork or feature branch against master. You will be given feedback from the core developers of raiden and you should try to incorporate that feedback into your branch. Once you do so and all tests pass your feature/fix will be merged.

Updating documentation/spec

If a PR you are working on is updating something in the REST API, then make sure to also update the respective documentation. The documentation update can also be in a different PR but should be taken care of and linked to the initial PR.

If a PR you are working on is making a protocol change then make sure to also update the specification. The spec update can also be introduced in a different PR but should be taken care of and linked to the initial PR.

Contributing to other people's PRs

If you are a core developer of Raiden with write privileges to the repository then you can add commits or rebase to master any Pull Request by other people.

Let us take this PR as an example. The contributor has everything ready and all is looking good apart from a minor glitch. You can wait until he fixes it himself but you can always help him by contributing to his branch's PR:

git remote add hackaugusto git@github.com:hackaugusto/raiden.git
git fetch hackaugusto
git checkout travis_build

Right now you are working on the contributor's Pull Request. Make sure to coordinate to avoid any conflicts and always warn people beforehand if you are to work on their branch. Once you are done:

git commit -m 'Add my contribution

The PR was missing something. I added it.'
git push hackaugusto travis_build

Congratulations, you have added to someone else's PR!

Integrating Pull Requests

When integrating a successful Pull Request into the codebase we have the option of using either a "Rebase and Merge" or to "Create a Merge commit". Unfortunately in Github the default option is to "Create a Merge commit". This is not our preferred option as in this way we can't be sure that the result of the merge will also have all tests passing, since there may be other patches merged since the PR opened. But there are many PRs which we definitely know won't have any conflicts and for which enforcing rebase would make no sense and only waste our time. As such we provide the option to use both at our own discretion. So the general guidelines are:

  • If there are patches that have been merged to master since the PR was opened, on top of which our current PR may have different behaviour then use Rebase and Merge.
  • If there are patches that have been merged to master since the PR was opened which touch documentation, infrastucture or completely unrelated parts of the code then you can freely use Create a Merge Commit and save the time of rebasing.