Last updated: 12 July 2020
How to Contribute Pull Requests to Bitcoin Core

BEFORE YOU BEGIN

This guide builds on the foundation laid by How to Review Pull Requests in Bitcoin Core.

Remember that testing issues and reviewing are the most effective ways you can contribute as a new contributor (and will teach you more about the codebase than opening PRs).

As Steve Lee of Square Crypto wrote to me recently: "If literally all a developer did was review hard but important PRs it'd be incredibly valuable."

Don't be in a hurry to add to the growing pile of open PRs.

A good rule of thumb is to review 5-15 PRs (or solve or test 5-15 open issues) for every PR you make.

HOW TO CONTRIBUTE PULL REQUESTS TO BITCOIN CORE

When you do start contributing PRs, here are some guidelines:

Aim for quality over quantity and a balance between deep work and quick wins.

Read and know the Bitcoin Core Contributing Guide and Developer Notes.

What to do

Choose your contribution carefully to be sure it is desired by the maintainers and reviewers; without their approval, you risk squandering your time and social capital.

The best places to look for good things to do as a new contributor are:

Good ideas may also be found while doing PR reviews and in #bitcoin-core-dev IRC discussions when maintainers or experienced contributors mention nice-to-haves.

Things to do and fix can also be found by searching for TODO and FIXME comments in the codebase. An easy way to do this is to run git grep "TODO\|FIXME" on the command line from root. Keep in mind that these comments can sometimes be out of date.

Priorities

Focus on user problems, actual bugs, and "used, but untested" methods that affect outcomes and need tests. Bitcoin Core is a large, legacy codebase and there are inconsistencies, but it's good to keep a healthy perspective on priorities. An item might seem fun to contribute, but before opening that PR, consider the review time you're asking for. Because of your PR, is a regression or CVE getting past a lack of review elsewhere?

Try to solve a clearly defined issue with a clear, minimal change.

Avoid making trivial refactoring, style cleanup, or spelling PRs; these consume valuable contributor and maintainer time, often for little gain. Code churn can cause hidden new bugs and blurs the code history and git blame. Any code refactoring needs substantial motivation, advantages, or simplifications.

Do not begin by trying to change consensus code; it is difficult and dangerous territory. The goal of Bitcoin Core is to maintain the correct consensus on the Bitcoin network — all else is secondary.

As a general guideline, here are priorities in decreasing order: fixing bugs, improving security/test coverage, addressing real use cases, doing less/removing code (only when doing nothing isn't better), doing more/adding code.

Gather information

Gather context about the change you have in mind. Has it already been proposed in the past? Do issues or PRs about it already exist? (They probably do). Sleuth a bit. Search the git log -S history. Look at the git blame of the code in question and git show the commits. Check the GitHub file history. You'll see who understands that part of the codebase and can be contacted for questions or review.

It may save you a lot of time to consult with relevant people on IRC before tackling a difficult project in potentially the wrong way, or one that has already been tried and turned out to be a dead end for reasons that may not be apparent to you.

I sometimes seek Concept ACKs for larger changes at IRC meetings (example) as a topic request and discussion.

Write test coverage

Test coverage is essential; don't hesitate to write any missing unit, functional, or fuzz tests, or improve the benchmarks.

A Gregory Maxwell warning to test contributors: "Overly exact tests have greatly diminished value if their exactitude turns them into hyper-active something changed detectors, when they would otherwise be more useful as something isn't right detectors if constructed more in terms of invariants instead of strict behaviour."

Write documentation

Documentation is important, e.g. high-level documentation of how things work and interact, clear and accurate code docs, whether a function has a good description and Doxygen documentation, test logging (both info and debug), and so on.

Add release notes when appropriate. RPC-related release notes should refer to the RPC help for details instead of substituting for full documentation, for example: "Please refer to the RPC help of getbalances for details."

To avoid potential merge conflicts, release notes are usually added in a separate markdown file in the /doc directory named release-notes-<PR_NUMBER>.md, not directly in doc/release-notes.md.

In general, PRs that intelligently improve documentation and tests in a well thought-out way tend to be well-received.

Hygienic commits

Make sure each individual commit in your PR is hygienic. Every commit must compile successfully on its own without warnings, errors, regressions, or test failures.

This means compiling your PRs with --enable-debug passed to your build configuration. Run ./configure --help | grep -A1 --enable-debug for more info.

Set up Travis CI on your own GitHub Bitcoin repository so that when you push a branch or commit, the linter and Travis CI (continuous integration) tests run. It can be a good idea to verify that they are all green on your GitHub repository before filing a PR. Unfortunately, Travis CI is also sometimes slow and times out frequently — you may have to look at the logs of failing jobs to distinguish between real issues and when you just need to restart the job.

When it works, https://bitcoinbuilds.org runs more quickly than Travis CI (but for the moment it tests fewer platforms), so don't hesitate to consult it for quicker feedback when you push a PR or changes.

Do it well

Don't rush. Give your work time to mature, revisit it with fresh perspective, and iterate on it a few times before putting it out there and asking for others' time. It might be a good rule of thumb to write several WIP PRs for each one that you do file.

Put the time in to make your PRs as easy to understand and review as possible: Code, tests, documentation, good commit messages, relevant references, concise explanations.

Timing

Consider also the timing of what is going on in the project in terms of priorities and releases as well as your current status. If you already have a few PRs open it may be best to close them or concentrate on having them merged before adding yet another PR to the review stack. The idea of limiting open PRs to 5 per contributor is occasionally raised; consider staying well under that.

Give some thought to PR size and scope. Many contributors err on the side of making their PRs too large and time-consuming to review. It is often better to keep your PRs short and focused to make them easier and safer to review. This may require splitting your work into several consecutive PRs.

PR descriptions

As described in the Bitcoin Core contributing guide, prefix your PR title with the relevant area (e.g. doc, test, rpc, wallet).

PR descriptions are important. They "sell" your PR yet are often either too terse or too verbose. Write the essentials, then take the time to make your PR description brief, clear, even pithy.

Never put @-prefixed usernames in commits and PR descriptions; the latter because usernames in the description are copied into the merge commit by the merge script. This can cause endless annoying notifications for those concerned. An update to the merge script now warns maintainers if a merge message contains a @username, but it's better not to add them to begin with.

Don't rely on markdown in your PR description for it to make sense, especially strike-through formatting. The Bitcoin Core merge script will copy your PR description to git history in the merge commit but any formatting will be lost.

Say why

PR descriptions need to explain why. Summarizing "what" can be good too, but "why" is essential to review because if the why doesn't make sense then the what probably doesn't matter. Sometimes things seem self-apparent, but when in doubt no one was ever hurt by a little more concise explanation.

Say how to review and test your PR

It's a smart habit to give reviewers tips in your PR description on how to review and test your changes: "Here's how to review this".

Rebasing

It essential to be comfortable with frequently rebasing your commits to reorganise or squash them. Rebasing is frequently done to keep your commits logical, separate, and easier to review. Even better, hygienic — each standing on their own without introducing regressions.

For simple edits of existing commits, don't hesitate to squash your commits before pushing, without waiting for the maintainers to ask you to do so.

You can sign your commits using the OpenTimestamps Git Integration. It's easy to setup.

Remember that every time you comment on your PR after pushing, you're sending notifications to the people who previously reviewed or commented on it. Respect their time.

Getting review

Learn how to get people to review your work when needed. It's a skill. Review is scarce and valuable -- it's the bottleneck.

The best ways to get review are organic: Making sure your work is interesting, desired, and done well. Giving back by helping others and giving review. Giving more than you take or ask for; adding value as best as you can while figuring things out on your own. It's a long and sometimes frustrating path but a rewarding one.

While waiting for review, the very best thing you can do is give review to others. Karma is a thing — and reviewing/testing PRs and issues builds your credibility and understanding of the codebase and the process.

Persistence can pay off. Sometimes a PR succeeds on the second or third try if the first ones didn't see enough review and if there are valid reasons to continue.

In this case, one idea seen in practice is to recap various ACKs from the previous PRs, with GitHub usernames, to rope in support for the new PR. If you do, be sure to do it in a comment — not in a commit message and not in the PR description.

Ask yourself how others will see your PR. Try to only file PRs that people will be motivated to review and that are worth merging, for both Bitcoin and your track record. Your PRs will receive more attention from the contributors and maintainers and be merged more quickly.

Scale up your PR contributions gradually in line with your ability and understanding of the project, process, and codebase.

Keep reviewing

Continue to do more reviewing and testing than adding PRs to the backlog. Reviewing 5 to 15 PRs for every PR you open is a good range.

If you are a good reviewer and begin building a reputation and appreciation for your help in moving the project forward, maintainers and other contributors may reciprocate by reviewing your work more quickly as well. See these two articles for more:

Happy contributing!

Cheers,

Jon Atack