/ CONTRIBUTING.md
CONTRIBUTING.md
1 Contributing to Bitcoin Core 2 ============================ 3 4 The Bitcoin Core project operates an open contributor model where anyone is 5 welcome to contribute towards development in the form of peer review, testing 6 and patches. This document explains the practical process and guidelines for 7 contributing. 8 9 First, in terms of structure, there is no particular concept of "Bitcoin Core 10 developers" in the sense of privileged people. Open source often naturally 11 revolves around a meritocracy where contributors earn trust from the developer 12 community over time. Nevertheless, some hierarchy is necessary for practical 13 purposes. As such, there are repository maintainers who are responsible for 14 merging pull requests, the [release cycle](/doc/release-process.md), and 15 moderation. 16 17 Getting Started 18 --------------- 19 20 New contributors are very welcome and needed. 21 22 Reviewing and testing is highly valued and the most effective way you can contribute 23 as a new contributor. It also will teach you much more about the code and 24 process than opening pull requests. Please refer to the [peer review](#peer-review) 25 section below. 26 27 Before you start contributing, familiarize yourself with the Bitcoin Core build 28 system and tests. Refer to the documentation in the repository on how to build 29 Bitcoin Core and how to run the unit tests, functional tests, and fuzz tests. 30 31 There are many open issues of varying difficulty waiting to be fixed. 32 If you're looking for somewhere to start contributing, check out the 33 [good first issue](https://github.com/bitcoin/bitcoin/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22) 34 list or changes that are 35 [up for grabs](https://github.com/bitcoin/bitcoin/issues?utf8=%E2%9C%93&q=label%3A%22Up+for+grabs%22). 36 Some of them might no longer be applicable. So if you are interested, but 37 unsure, you might want to leave a comment on the issue first. 38 39 You may also participate in the [Bitcoin Core PR Review Club](https://bitcoincore.reviews/). 40 41 ### Good First Issue Label 42 43 The purpose of the `good first issue` label is to highlight which issues are 44 suitable for a new contributor without a deep understanding of the codebase. 45 46 However, good first issues can be solved by anyone. If they remain unsolved 47 for a longer time, a frequent contributor might address them. 48 49 You do not need to request permission to start working on an issue. However, 50 you are encouraged to leave a comment if you are planning to work on it. This 51 will help other contributors monitor which issues are actively being addressed 52 and is also an effective way to request assistance if and when you need it. 53 54 Communication Channels 55 ---------------------- 56 57 Most communication about Bitcoin Core development happens on IRC, in the 58 `#bitcoin-core-dev` channel on Libera Chat. The easiest way to participate on IRC is 59 with the web client, [web.libera.chat](https://web.libera.chat/#bitcoin-core-dev). Chat 60 history logs can be found 61 on [https://www.erisian.com.au/bitcoin-core-dev/](https://www.erisian.com.au/bitcoin-core-dev/) 62 and [https://gnusha.org/bitcoin-core-dev/](https://gnusha.org/bitcoin-core-dev/). 63 64 Discussion about codebase improvements happens in GitHub issues and pull 65 requests. 66 67 The developer 68 [mailing list](https://groups.google.com/g/bitcoindev) 69 should be used to discuss complicated or controversial consensus or P2P protocol changes before working on 70 a patch set. 71 Archives can be found on [https://gnusha.org/pi/bitcoindev/](https://gnusha.org/pi/bitcoindev/). 72 73 74 Contributor Workflow 75 -------------------- 76 77 The codebase is maintained using the "contributor workflow" where everyone 78 without exception contributes patch proposals using "pull requests" (PRs). This 79 facilitates social contribution, easy testing and peer review. 80 81 To contribute a patch, the workflow is as follows: 82 83 1. Fork repository ([only for the first time](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/fork-a-repo)) 84 1. Create topic branch 85 1. Commit patches 86 87 For GUI-related issues or pull requests, the https://github.com/bitcoin-core/gui repository should be used. 88 For all other issues and pull requests, the https://github.com/bitcoin/bitcoin node repository should be used. 89 90 The master branch for all monotree repositories is identical. 91 92 As a rule of thumb, everything that only modifies `src/qt` is a GUI-only pull 93 request. However: 94 95 * For global refactoring or other transversal changes the node repository 96 should be used. 97 * For GUI-related build system changes, the node repository should be used 98 because the change needs review by the build systems reviewers. 99 * Changes in `src/interfaces` need to go to the node repository because they 100 might affect other components like the wallet. 101 102 For large GUI changes that include build system and interface changes, it is 103 recommended to first open a pull request against the GUI repository. When there 104 is agreement to proceed with the changes, a pull request with the build system 105 and interfaces changes can be submitted to the node repository. 106 107 The project coding conventions in the [developer notes](doc/developer-notes.md) 108 must be followed. 109 110 ### Committing Patches 111 112 In general, [commits should be atomic](https://en.wikipedia.org/wiki/Atomic_commit#Atomic_commit_convention) 113 and diffs should be easy to read. For this reason, do not mix any formatting 114 fixes or code moves with actual code changes. 115 116 Make sure each individual commit is hygienic: that it builds successfully on its 117 own without warnings, errors, regressions, or test failures. 118 This means tests must be updated in the same commit that changes the behavior. 119 120 Commit messages should be verbose by default consisting of a short subject line 121 (50 chars max), a blank line and detailed explanatory text as separate 122 paragraph(s), unless the title alone is self-explanatory (like "Correct typo 123 in init.cpp") in which case a single title line is sufficient. Commit messages should be 124 helpful to people reading your code in the future, so explain the reasoning for 125 your decisions. Further explanation [here](https://cbea.ms/git-commit/). 126 127 If a particular commit references another issue, please add the reference. For 128 example: `refs #1234` or `fixes #4321`. Using the `fixes` or `closes` keywords 129 will cause the corresponding issue to be closed when the pull request is merged. 130 131 Commit messages should never contain any `@` mentions (usernames prefixed with "@"). 132 133 Please refer to the [Git manual](https://git-scm.com/doc) for more information 134 about Git. 135 136 - Push changes to your fork 137 - Create pull request 138 139 ### Creating the Pull Request 140 141 The title of the pull request should be prefixed by the component or area that 142 the pull request affects. Valid areas as: 143 144 - `consensus` for changes to consensus critical code 145 - `doc` for changes to the documentation 146 - `qt` or `gui` for changes to bitcoin-qt 147 - `log` for changes to log messages 148 - `mining` for changes to the mining code 149 - `net` or `p2p` for changes to the peer-to-peer network code 150 - `refactor` for structural changes that do not change behavior 151 - `rpc`, `rest` or `zmq` for changes to the RPC, REST or ZMQ APIs 152 - `contrib` or `cli` for changes to the scripts and tools 153 - `test`, `qa` or `ci` for changes to the unit tests, QA tests or CI code 154 - `util` or `lib` for changes to the utils or libraries 155 - `wallet` for changes to the wallet code 156 - `build` for changes to CMake 157 - `guix` for changes to the GUIX reproducible builds 158 159 Examples: 160 161 consensus: Add new opcode for BIP-XXXX OP_CHECKAWESOMESIG 162 net: Automatically create onion service, listen on Tor 163 qt: Add feed bump button 164 log: Fix typo in log message 165 166 The body of the pull request should contain sufficient description of *what* the 167 patch does, and even more importantly, *why*, with justification and reasoning. 168 You should include references to any discussions (for example, other issues or 169 mailing list discussions). 170 171 The description for a new pull request should not contain any `@` mentions. The 172 PR description will be included in the commit message when the PR is merged and 173 any users mentioned in the description will be annoyingly notified each time a 174 fork of Bitcoin Core copies the merge. Instead, make any username mentions in a 175 subsequent comment to the PR. 176 177 ### Translation changes 178 179 Note that translations should not be submitted as pull requests. Please see 180 [Translation Process](https://github.com/bitcoin/bitcoin/blob/master/doc/translation_process.md) 181 for more information on helping with translations. 182 183 ### Work in Progress Changes and Requests for Comments 184 185 If a pull request is not to be considered for merging (yet), please 186 prefix the title with [WIP] or use [Tasks Lists](https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax#task-lists) 187 in the body of the pull request to indicate tasks are pending. 188 189 ### Address Feedback 190 191 At this stage, one should expect comments and review from other contributors. You 192 can add more commits to your pull request by committing them locally and pushing 193 to your fork. 194 195 You are expected to reply to any review comments before your pull request is 196 merged. You may update the code or reject the feedback if you do not agree with 197 it, but you should express so in a reply. If there is outstanding feedback and 198 you are not actively working on it, your pull request may be closed. 199 200 Please refer to the [peer review](#peer-review) section below for more details. 201 202 ### Squashing Commits 203 204 If your pull request contains fixup commits (commits that change the same line of code repeatedly) or too fine-grained 205 commits, you may be asked to [squash](https://git-scm.com/docs/git-rebase#_interactive_mode) your commits 206 before it will be reviewed. The basic squashing workflow is shown below. 207 208 git checkout your_branch_name 209 git rebase -i HEAD~n 210 # n is normally the number of commits in the pull request. 211 # Set commits (except the one in the first line) from 'pick' to 'squash', save and quit. 212 # On the next screen, edit/refine commit messages. 213 # Save and quit. 214 git push -f # (force push to GitHub) 215 216 Please update the resulting commit message, if needed. It should read as a 217 coherent message. In most cases, this means not just listing the interim 218 commits. 219 220 If your change contains a merge commit, the above workflow may not work and you 221 will need to remove the merge commit first. See the next section for details on 222 how to rebase. 223 224 Please refrain from creating several pull requests for the same change. 225 Use the pull request that is already open (or was created earlier) to amend 226 changes. This preserves the discussion and review that happened earlier for 227 the respective change set. 228 229 The length of time required for peer review is unpredictable and will vary from 230 pull request to pull request. 231 232 ### Rebasing Changes 233 234 When a pull request conflicts with the target branch, you may be asked to rebase it on top of the current target branch. 235 236 git fetch https://github.com/bitcoin/bitcoin # Fetch the latest upstream commit 237 git rebase FETCH_HEAD # Rebuild commits on top of the new base 238 239 This project aims to have a clean git history, where code changes are only made in non-merge commits. This simplifies 240 auditability because merge commits can be assumed to not contain arbitrary code changes. Merge commits should be signed, 241 and the resulting git tree hash must be deterministic and reproducible. The script in 242 [/contrib/verify-commits](/contrib/verify-commits) checks that. 243 244 After a rebase, reviewers are encouraged to sign off on the force push. This should be relatively straightforward with 245 the `git range-diff` tool explained in the [productivity 246 notes](/doc/productivity.md#diff-the-diffs-with-git-range-diff). To avoid needless review churn, maintainers will 247 generally merge pull requests that received the most review attention first. 248 249 Pull Request Philosophy 250 ----------------------- 251 252 Patchsets should always be focused. For example, a pull request could add a 253 feature, fix a bug, or refactor code; but not a mixture. Please also avoid super 254 pull requests which attempt to do too much, are overly large, or overly complex 255 as this makes review difficult. 256 257 258 ### Features 259 260 When adding a new feature, thought must be given to the long term technical debt 261 and maintenance that feature may require after inclusion. Before proposing a new 262 feature that will require maintenance, please consider if you are willing to 263 maintain it (including bug fixing). If features get orphaned with no maintainer 264 in the future, they may be removed by the Repository Maintainer. 265 266 267 ### Refactoring 268 269 Refactoring is a necessary part of any software project's evolution. The 270 following guidelines cover refactoring pull requests for the project. 271 272 There are three categories of refactoring: code-only moves, code style fixes, and 273 code refactoring. In general, refactoring pull requests should not mix these 274 three kinds of activities in order to make refactoring pull requests easy to 275 review and uncontroversial. In all cases, refactoring PRs must not change the 276 behaviour of code within the pull request (bugs must be preserved as is). 277 278 Project maintainers aim for a quick turnaround on refactoring pull requests, so 279 where possible keep them short, uncomplex and easy to verify. 280 281 Pull requests that refactor the code should not be made by new contributors. It 282 requires a certain level of experience to know where the code belongs to and to 283 understand the full ramification (including rebase effort of open pull requests). 284 285 Trivial pull requests or pull requests that refactor the code with no clear 286 benefits may be immediately closed by the maintainers to reduce unnecessary 287 workload on reviewing. 288 289 290 "Decision Making" Process 291 ------------------------- 292 293 The following applies to code changes to the Bitcoin Core project (and related 294 projects such as libsecp256k1), and is not to be confused with overall Bitcoin 295 Network Protocol consensus changes. 296 297 Whether a pull request is merged into Bitcoin Core rests with the project merge 298 maintainers. 299 300 Maintainers will take into consideration if a patch is in line with the general 301 principles of the project; meets the minimum standards for inclusion; and will 302 judge the general consensus of contributors. 303 304 In general, all pull requests must: 305 306 - Have a clear use case, fix a demonstrable bug or serve the greater good of 307 the project (for example refactoring for modularisation); 308 - Be well peer-reviewed; 309 - Have unit tests, functional tests, and fuzz tests, where appropriate; 310 - Follow code style guidelines ([C++](doc/developer-notes.md), [functional tests](test/functional/README.md)); 311 - Not break the existing test suite; 312 - Where bugs are fixed, where possible, there should be unit tests 313 demonstrating the bug and also proving the fix. This helps prevent regression. 314 - Change relevant comments and documentation when behaviour of code changes. 315 316 Patches that change Bitcoin consensus rules are considerably more involved than 317 normal because they affect the entire ecosystem and so must be preceded by 318 extensive mailing list discussions and have a numbered BIP. While each case will 319 be different, one should be prepared to expend more time and effort than for 320 other kinds of patches because of increased peer review and consensus building 321 requirements. 322 323 324 ### Peer Review 325 326 Anyone may participate in peer review which is expressed by comments in the pull 327 request. Typically reviewers will review the code for obvious errors, as well as 328 test out the patch set and opine on the technical merits of the patch. Project 329 maintainers take into account the peer review when determining if there is 330 consensus to merge a pull request (remember that discussions may have been 331 spread out over GitHub, mailing list and IRC discussions). 332 333 Code review is a burdensome but important part of the development process, and 334 as such, certain types of pull requests are rejected. In general, if the 335 **improvements** do not warrant the **review effort** required, the PR has a 336 high chance of being rejected. It is up to the PR author to convince the 337 reviewers that the changes warrant the review effort, and if reviewers are 338 "Concept NACK'ing" the PR, the author may need to present arguments and/or do 339 research backing their suggested changes. 340 341 #### Conceptual Review 342 343 A review can be a conceptual review, where the reviewer leaves a comment 344 * `Concept (N)ACK`, meaning "I do (not) agree with the general goal of this pull 345 request", 346 * `Approach (N)ACK`, meaning `Concept ACK`, but "I do (not) agree with the 347 approach of this change". 348 349 A `NACK` needs to include a rationale why the change is not worthwhile. 350 NACKs without accompanying reasoning may be disregarded. 351 352 #### Code Review 353 354 After conceptual agreement on the change, code review can be provided. A review 355 begins with `ACK BRANCH_COMMIT`, where `BRANCH_COMMIT` is the top of the PR 356 branch, followed by a description of how the reviewer did the review. The 357 following language is used within pull request comments: 358 359 - "I have tested the code", involving change-specific manual testing in 360 addition to running the unit, functional, or fuzz tests, and in case it is 361 not obvious how the manual testing was done, it should be described; 362 - "I have not tested the code, but I have reviewed it and it looks 363 OK, I agree it can be merged"; 364 - A "nit" refers to a trivial, often non-blocking issue. 365 366 Project maintainers reserve the right to weigh the opinions of peer reviewers 367 using common sense judgement and may also weigh based on merit. Reviewers that 368 have demonstrated a deeper commitment and understanding of the project over time 369 or who have clear domain expertise may naturally have more weight, as one would 370 expect in all walks of life. 371 372 Where a patch set affects consensus-critical code, the bar will be much 373 higher in terms of discussion and peer review requirements, keeping in mind that 374 mistakes could be very costly to the wider community. This includes refactoring 375 of consensus-critical code. 376 377 Where a patch set proposes to change the Bitcoin consensus, it must have been 378 discussed extensively on the mailing list and IRC, be accompanied by a widely 379 discussed BIP and have a generally widely perceived technical consensus of being 380 a worthwhile change based on the judgement of the maintainers. 381 382 ### Finding Reviewers 383 384 As most reviewers are themselves developers with their own projects, the review 385 process can be quite lengthy, and some amount of patience is required. If you find 386 that you've been waiting for a pull request to be given attention for several 387 months, there may be a number of reasons for this, some of which you can do something 388 about: 389 390 - It may be because of a feature freeze due to an upcoming release. During this time, 391 only bug fixes are taken into consideration. If your pull request is a new feature, 392 it will not be prioritized until after the release. Wait for the release. 393 - It may be because the changes you are suggesting do not appeal to people. Rather than 394 nits and critique, which require effort and means they care enough to spend time on your 395 contribution, thundering silence is a good sign of widespread (mild) dislike of a given change 396 (because people don't assume *others* won't actually like the proposal). Don't take 397 that personally, though! Instead, take another critical look at what you are suggesting 398 and see if it: changes too much, is too broad, doesn't adhere to the 399 [developer notes](doc/developer-notes.md), is dangerous or insecure, is messily written, etc. 400 Identify and address any of the issues you find. Then ask e.g. on IRC if someone could give 401 their opinion on the concept itself. 402 - It may be because your code is too complex for all but a few people, and those people 403 may not have realized your pull request even exists. A great way to find people who 404 are qualified and care about the code you are touching is the 405 [Git Blame feature](https://docs.github.com/en/repositories/working-with-files/using-files/viewing-and-understanding-files). Simply 406 look up who last modified the code you are changing and see if you can find 407 them and give them a nudge. Don't be incessant about the nudging, though. 408 - Finally, if all else fails, ask on IRC or elsewhere for someone to give your pull request 409 a look. If you think you've been waiting for an unreasonably long time (say, 410 more than a month) for no particular reason (a few lines changed, etc.), 411 this is totally fine. Try to return the favor when someone else is asking 412 for feedback on their code, and the universe balances out. 413 - Remember that the best thing you can do while waiting is give review to others! 414 415 416 Backporting 417 ----------- 418 419 Security and bug fixes can be backported from `master` to release 420 branches. 421 Maintainers will do backports in batches and 422 use the proper `Needs backport (...)` labels 423 when needed (the original author does not need to worry about it). 424 425 A backport should contain the following metadata in the commit body: 426 427 ``` 428 Github-Pull: #<PR number> 429 Rebased-From: <commit hash of the original commit> 430 ``` 431 432 Have a look at [an example backport PR]( 433 https://github.com/bitcoin/bitcoin/pull/16189). 434 435 Also see the [backport.py script]( 436 https://github.com/bitcoin-core/bitcoin-maintainer-tools#backport). 437 438 Copyright 439 --------- 440 441 By contributing to this repository, you agree to license your work under the 442 MIT license unless specified otherwise in `contrib/debian/copyright` or at 443 the top of the file itself. Any work contributed where you are not the original 444 author must contain its license header with the original author(s) and source.