|
| 1 | +# Contributing to rust-bitcoin |
| 2 | + |
| 3 | +:+1::tada: First off, thanks for taking the time to contribute! :tada::+1: |
| 4 | + |
| 5 | +The following is a set of guidelines for contributing to Rust Bitcoin |
| 6 | +implementation and other Rust Bitcoin-related projects, which are hosted in the |
| 7 | +[Rust Bitcoin Community](https://github.com/rust-bitcoin) on GitHub. These are |
| 8 | +mostly guidelines, not rules. Use your best judgment, and feel free to propose |
| 9 | +changes to this document in a pull request. |
| 10 | + |
| 11 | +#### Table Of Contents |
| 12 | + |
| 13 | +- [General](#general) |
| 14 | +- [Communication channels](#communication-channels) |
| 15 | +- [Asking questions](#asking-questions) |
| 16 | +- [Contribution workflow](#contribution-workflow) |
| 17 | + * [Preparing PRs](#preparing-prs) |
| 18 | + * [Peer review](#peer-review) |
| 19 | + * [Repository maintainers](#repository-maintainers) |
| 20 | +- [Coding conventions](#coding-conventions) |
| 21 | + * [Formatting](#formatting) |
| 22 | + * [MSRV](#msrv) |
| 23 | + * [Naming conventions](#naming-conventions) |
| 24 | + * [Unsafe code](#unsafe-code) |
| 25 | +- [Security](#security) |
| 26 | +- [Testing](#testing) |
| 27 | +- [Going further](#going-further) |
| 28 | + |
| 29 | + |
| 30 | +## General |
| 31 | + |
| 32 | +The Rust Bitcoin project operates an open contributor model where anyone is |
| 33 | +welcome to contribute towards development in the form of peer review, |
| 34 | +documentation, testing and patches. |
| 35 | + |
| 36 | +Anyone is invited to contribute without regard to technical experience, |
| 37 | +"expertise", OSS experience, age, or other concern. However, the development of |
| 38 | +standards & reference implementations demands a high-level of rigor, adversarial |
| 39 | +thinking, thorough testing and risk-minimization. Any bug may cost users real |
| 40 | +money. That being said, we deeply welcome people contributing for the first time |
| 41 | +to an open source project or pick up Rust while contributing. Don't be shy, |
| 42 | +you'll learn. |
| 43 | + |
| 44 | + |
| 45 | +## Communication channels |
| 46 | + |
| 47 | +Communication about Rust Bitcoin happens primarily in |
| 48 | +[#bitcoin-rust](https://web.libera.chat/?channel=#bitcoin-rust) IRC chat on |
| 49 | +[Libera](https://libera.chat/) with the logs available at |
| 50 | +<https://gnusha.org/bitcoin-rust/> (starting from Jun 2021 and now on) and |
| 51 | +<https://gnusha.org/rust-bitcoin/> (historical archive before Jun 2021). |
| 52 | + |
| 53 | +Discussion about code base improvements happens in GitHub issues and on pull |
| 54 | +requests. |
| 55 | + |
| 56 | +Major projects are tracked [here](https://github.com/orgs/rust-bitcoin/projects). |
| 57 | +Major milestones are tracked [here](https://github.com/rust-bitcoin/rust-bitcoin/milestones). |
| 58 | + |
| 59 | + |
| 60 | +## Asking questions |
| 61 | + |
| 62 | +> **Note:** Please don't file an issue to ask a question. You'll get faster |
| 63 | +> results by using the resources below. |
| 64 | +
|
| 65 | +We have a dedicated developer channel on IRC, # [email protected] where |
| 66 | +you may get helpful advice if you have questions. |
| 67 | + |
| 68 | + |
| 69 | +## Contribution workflow |
| 70 | + |
| 71 | +The codebase is maintained using the "contributor workflow" where everyone |
| 72 | +without exception contributes patch proposals using "pull requests". This |
| 73 | +facilitates social contribution, easy testing and peer review. |
| 74 | + |
| 75 | +To contribute a patch, the workflow is a as follows: |
| 76 | + |
| 77 | +1. Fork Repository |
| 78 | +2. Create topic branch |
| 79 | +3. Commit patches |
| 80 | + |
| 81 | +Please keep commits should atomic and diffs easy to read. For this reason |
| 82 | +do not mix any formatting fixes or code moves with actual code changes. |
| 83 | +Further, each commit, individually, should compile and pass tests, in order to |
| 84 | +ensure git bisect and other automated tools function properly. |
| 85 | + |
| 86 | +Please cover every new feature with unit tests. |
| 87 | + |
| 88 | +When refactoring, structure your PR to make it easy to review and don't hesitate |
| 89 | +to split it into multiple small, focused PRs. |
| 90 | + |
| 91 | +Commits should cover both the issue fixed and the solution's rationale. |
| 92 | +Please keep these [guidelines](https://chris.beams.io/posts/git-commit/) in mind. |
| 93 | + |
| 94 | +To facilitate communication with other contributors, the project is making use |
| 95 | +of GitHub's "assignee" field. First check that no one is assigned and then |
| 96 | +comment suggesting that you're working on it. If someone is already assigned, |
| 97 | +don't hesitate to ask if the assigned party or previous commenters are still |
| 98 | +working on it if it has been awhile. |
| 99 | + |
| 100 | +## Preparing PRs |
| 101 | + |
| 102 | +The main library development happens in the `master` branch. This branch must |
| 103 | +always compile without errors (using GitHub CI). All external contributions are |
| 104 | +made within PRs into this branch. |
| 105 | + |
| 106 | +Prerequisites that a PR must satisfy for merging into the `master` branch: |
| 107 | +* each commit within a PR must compile and pass unit tests with no errors, with |
| 108 | + every feature combination (including compiling the fuzztests) on some |
| 109 | + reasonably recent compiler (this is partially automated with CI, so the rule |
| 110 | + is that we will not accept commits which do not pass GitHub CI); |
| 111 | +* the tip of any PR branch must also compile and pass tests with no errors on |
| 112 | + MSRV (check [README.md] on current MSRV requirements) and pass fuzz tests on |
| 113 | + nightly rust; |
| 114 | +* contain all necessary tests for the introduced functional (either as a part of |
| 115 | + commits, or, more preferably, as separate commits, so that it's easy to |
| 116 | + reorder them during review and check that the new tests fail without the new |
| 117 | + code); |
| 118 | +* contain all inline docs for newly introduced API and pass doc tests; |
| 119 | +* be based on the recent `master` tip from the original repository at |
| 120 | + <https://github.com/rust-bitcoin/rust-bitcoin>. |
| 121 | + |
| 122 | +NB: reviewers may run more complex test/CI scripts, thus, satisfying all the |
| 123 | +requirements above is just a preliminary, but not necessary sufficient step for |
| 124 | +getting the PR accepted as a valid candidate PR for the `master` branch. |
| 125 | + |
| 126 | +PR authors may also find it useful to run the following script locally in order |
| 127 | +to check that each of the commits within the PR satisfies the requirements |
| 128 | +above, before submitting the PR to review: |
| 129 | +```shell script |
| 130 | +BITCOIN_MSRV=1.29.0 ./contrib/ci.sh |
| 131 | +``` |
| 132 | +Please replace the value in `BITCOIN_MSRV=1.29.0` with the current MSRV from |
| 133 | +[README.md]. |
| 134 | + |
| 135 | +NB: Please keep in mind that the script above replaces `Cargo.lock` file, which |
| 136 | +is necessary to support current MSRV, incompatible with `stable` and newer cargo |
| 137 | +versions. |
| 138 | + |
| 139 | +### Peer review |
| 140 | + |
| 141 | +Anyone may participate in peer review which is expressed by comments in the pull |
| 142 | +request. Typically, reviewers will review the code for obvious errors, as well as |
| 143 | +test out the patch set and opine on the technical merits of the patch. Please, |
| 144 | +first review PR on the conceptual level before focusing on code style or |
| 145 | +grammar fixes. |
| 146 | + |
| 147 | +### Repository maintainers |
| 148 | + |
| 149 | +Pull request merge requirements: |
| 150 | +- all CI test should pass, |
| 151 | +- at least two "accepts"/ACKs from the repository maintainers |
| 152 | +- no reasonable "rejects"/NACKs from anybody who reviewed the code. |
| 153 | + |
| 154 | +Current list of the project maintainers: |
| 155 | + |
| 156 | +- [Andrew Poelstra](https://github.com/apoelstra) |
| 157 | +- [Steven Roose](https://github.com/stevenroose) |
| 158 | +- [Maxim Orlovsky](https://github.com/dr-orlovsky) |
| 159 | +- [Matt Corallo](https://github.com/TheBlueMatt) |
| 160 | +- [Elichai Turkel](https://github.com/elichai) |
| 161 | +- [Sebastian Geisler](https://github.com/sgeisler) |
| 162 | +- [Sanket Kanjalkar](https://github.com/sanket1729) |
| 163 | + |
| 164 | + |
| 165 | +## Coding conventions |
| 166 | + |
| 167 | +Library reflects Bitcoin Core approach whenever possible. |
| 168 | + |
| 169 | +### Formatting |
| 170 | + |
| 171 | +The repository currently does not use `rustfmt`. |
| 172 | + |
| 173 | +New changes may format the code with `rustfmt`, but they should not re-format |
| 174 | +any existing code for maintaining diff size small, keeping `git blame` intact and |
| 175 | +reduce review time. Repository maintainers may not review PRs introducing large |
| 176 | +blocks of re-formatted code. |
| 177 | + |
| 178 | +You may check the [discussion on the formatting](https://github.com/rust-bitcoin/rust-bitcoin/issues/172) |
| 179 | +and [how it is planned to coordinate it with crate refactoring](https://github.com/rust-bitcoin/rust-bitcoin/pull/525) |
| 180 | + |
| 181 | +For the new code it is recommended to follow style of the existing codebase and |
| 182 | +avoid any end-line space characters. |
| 183 | + |
| 184 | +### MSRV |
| 185 | + |
| 186 | +The Minimal Supported Rust Version (MSRV) is 1.29; it is enforced by our CI. |
| 187 | +Later we plan to increase MSRV to support Rust 2018 and you are welcome to check |
| 188 | +the [tracking issue](https://github.com/rust-bitcoin/rust-bitcoin/issues/510). |
| 189 | + |
| 190 | +### Naming conventions |
| 191 | + |
| 192 | +Naming of data structures/enums and their fields/variants must follow names used |
| 193 | +in Bitcoin Core, with the following exceptions: |
| 194 | +- the case should follow Rust standards (i.e. PascalCase for types and |
| 195 | + snake_case for fields and variants); |
| 196 | +- omit `C`-prefixes. |
| 197 | + |
| 198 | +### Unsafe code |
| 199 | + |
| 200 | +Use of `unsafe` code is prohibited unless there is a unanimous decision among |
| 201 | +library maintainers on the exclusion from this rule. In such cases there is a |
| 202 | +requirement to test unsafe code with sanitizers including Miri. |
| 203 | + |
| 204 | + |
| 205 | +## Security |
| 206 | + |
| 207 | +Security is the primary focus for this library; disclosure of security |
| 208 | +vulnerabilities helps prevent user loss of funds. If you believe a vulnerability |
| 209 | +may affect other implementations, please disclose this information according to |
| 210 | +the [security guidelines](./SECURITY.md), work on which is currently in progress. |
| 211 | +Before it is completed, feel free to send disclosure to Andrew Poelstra, |
| 212 | +[email protected], encrypted with his public key from |
| 213 | +<https://www.wpsoftware.net/andrew/andrew.gpg>. |
| 214 | + |
| 215 | + |
| 216 | +## Testing |
| 217 | + |
| 218 | +Related to the security aspect, rust bitcoin developers take testing very |
| 219 | +seriously. Due to the modular nature of the project, writing new test cases is |
| 220 | +easy and good test coverage of the codebase is an important goal. Refactoring |
| 221 | +the project to enable fine-grained unit testing is also an ongoing effort. |
| 222 | + |
| 223 | +Fuzzing is heavily encouraged: feel free to add related material under `fuzz/` |
| 224 | + |
| 225 | +Mutation testing is planned; any contributions helping with that are highly |
| 226 | +welcome! |
| 227 | + |
| 228 | + |
| 229 | +## Going further |
| 230 | + |
| 231 | +You may be interested in the guide by Jon Atack on |
| 232 | +[How to review Bitcoin Core PRs](https://github.com/jonatack/bitcoin-development/blob/master/how-to-review-bitcoin-core-prs.md) |
| 233 | +and [How to make Bitcoin Core PRs](https://github.com/jonatack/bitcoin-development/blob/master/how-to-make-bitcoin-core-prs.md). |
| 234 | +While there are differences between the projects in terms of context and |
| 235 | +maturity, many of the suggestions offered apply to this project. |
| 236 | + |
| 237 | +Overall, have fun :) |
0 commit comments