|
| 1 | +# Contributing to rust-miniscript |
| 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 the rust-miniscript |
| 6 | +project. They are substantially based on the contributing guidelines for the |
| 7 | +[rust-bitcoin project](https://github.com/rust-bitcoin/rust-bitcoin/blob/master/CONTRIBUTING.md). |
| 8 | +These are guidelines, not rules. Use your best judgment. |
| 9 | + |
| 10 | +#### Table Of Contents |
| 11 | + |
| 12 | +- [General](#general) |
| 13 | +- [Communication channels](#communication-channels) |
| 14 | +- [Asking questions](#asking-questions) |
| 15 | +- [Getting Started](#getting-started) |
| 16 | + * [Installing Rust](#installing-rust) |
| 17 | + * [Building](#building) |
| 18 | +- [Development Tools](#development-tools) |
| 19 | + * [Building the docs](#building-the-docs) |
| 20 | +- [Contribution workflow](#contribution-workflow) |
| 21 | + * [Preparing PRs](#preparing-prs) |
| 22 | + * [Peer review](#peer-review) |
| 23 | + * [CI and Merging](#ci-and-merging) |
| 24 | + * [Repository maintainers](#repository-maintainers) |
| 25 | + * [Unsafe code](#unsafe-code) |
| 26 | +- [Security](#security) |
| 27 | +- [Testing](#testing) |
| 28 | + * [Unit/Integration tests](#unitintegration-tests) |
| 29 | + * [Benchmarks](#benchmarks) |
| 30 | +- [Going further](#going-further) |
| 31 | + |
| 32 | + |
| 33 | +## General |
| 34 | + |
| 35 | +The Rust Bitcoin project operates an open contributor model where anyone is |
| 36 | +welcome to contribute towards development in the form of peer review, |
| 37 | +documentation, testing and patches. |
| 38 | + |
| 39 | +Anyone is invited to contribute without regard to technical experience, |
| 40 | +"expertise", OSS experience, age, or other concern. However, the development of |
| 41 | +standards & reference implementations demands a high-level of rigor, adversarial |
| 42 | +thinking, thorough testing and risk-minimization. Any bug may cost users real |
| 43 | +money. That being said, we deeply welcome people contributing for the first time |
| 44 | +to an open source project or pick up Rust while contributing. Don't be shy, |
| 45 | +you'll learn. |
| 46 | + |
| 47 | +## Communication channels |
| 48 | + |
| 49 | +Communication about rust-miniscript happens primarily in |
| 50 | +[#bitcoin-rust](https://web.libera.chat/?channel=#bitcoin-rust) IRC chat on |
| 51 | +[Libera](https://libera.chat/) with the logs available at |
| 52 | +<https://gnusha.org/bitcoin-rust/> (starting from Jun 2021 and now on) and |
| 53 | +<https://gnusha.org/rust-bitcoin/> (historical archive before Jun 2021). |
| 54 | + |
| 55 | +Discussion about code base improvements happens in GitHub issues and on pull |
| 56 | +requests. |
| 57 | + |
| 58 | + |
| 59 | +## Asking questions |
| 60 | + |
| 61 | +> **Note:** Please don't file an issue to ask a question. You'll get faster |
| 62 | +> results by using the resources below. |
| 63 | +
|
| 64 | +We have a dedicated developer channel on IRC, #bitcoin-rust@libera.chat where |
| 65 | +you may get helpful advice if you have questions. |
| 66 | + |
| 67 | + |
| 68 | +## Getting Started |
| 69 | + |
| 70 | +### Installing Rust |
| 71 | + |
| 72 | +Rust can be installed using your package manager of choice or [rustup.rs](https://rustup.rs). The |
| 73 | +former way is considered more secure since it typically doesn't involve trust in the CA system. But |
| 74 | +you should be aware that the version of Rust shipped by your distribution might be out of date. |
| 75 | +Generally this isn't a problem for `rust-miniscript` since we support much older versions than the |
| 76 | +current stable one (see MSRV section in [README.md](./README.md)). |
| 77 | + |
| 78 | +### Building |
| 79 | + |
| 80 | +The library can be built and tested using [`cargo`](https://github.com/rust-lang/cargo/): |
| 81 | + |
| 82 | +``` |
| 83 | +git clone git@github.com:rust-bitcoin/rust-miniscript.git |
| 84 | +cd rust-miniscript |
| 85 | +cargo build |
| 86 | +``` |
| 87 | + |
| 88 | +You can run tests with: |
| 89 | + |
| 90 | +``` |
| 91 | +cargo test |
| 92 | +``` |
| 93 | + |
| 94 | +Please refer to the [`cargo` documentation](https://doc.rust-lang.org/stable/cargo/) for more |
| 95 | +detailed instructions. |
| 96 | + |
| 97 | + |
| 98 | +## Development Tools |
| 99 | + |
| 100 | +### Building the docs |
| 101 | + |
| 102 | +We build docs with the nightly toolchain, you may wish to use the following shell alias to check |
| 103 | +your documentation changes build correctly. |
| 104 | + |
| 105 | +``` |
| 106 | +alias build-docs='RUSTDOCFLAGS="--cfg docsrs" cargo +nightly rustdoc --features="$FEATURES" -- -D rustdoc::broken-intra-doc-links' |
| 107 | +``` |
| 108 | + |
| 109 | + |
| 110 | +## Contribution workflow |
| 111 | + |
| 112 | +The codebase is maintained using the "contributor workflow" where everyone |
| 113 | +without exception contributes patch proposals using "pull requests". This |
| 114 | +facilitates social contribution, easy testing and peer review. |
| 115 | + |
| 116 | +To contribute a patch, the workflow is as follows: |
| 117 | + |
| 118 | +1. Fork Repository |
| 119 | +2. Create topic branch |
| 120 | +3. Commit patches |
| 121 | + |
| 122 | +Please keep commits atomic and diffs easy to read. For this reason |
| 123 | +do not mix any formatting fixes or code moves with actual code changes. |
| 124 | +Further, each commit, individually, should compile and pass tests, in order to |
| 125 | +ensure git bisect and other automated tools function properly. |
| 126 | + |
| 127 | +Please cover every new feature with unit tests. |
| 128 | + |
| 129 | +When refactoring, structure your PR to make it easy to review and don't hesitate |
| 130 | +to split it into multiple small, focused PRs. |
| 131 | + |
| 132 | +Commits should cover both the issue fixed and the solution's rationale. |
| 133 | +Please keep these [guidelines](https://chris.beams.io/posts/git-commit/) in mind. |
| 134 | + |
| 135 | + |
| 136 | +## Preparing PRs |
| 137 | + |
| 138 | +The main library development happens in the `master` branch. This branch must |
| 139 | +always compile without errors (using GitHub CI). All external contributions are |
| 140 | +made within PRs into this branch. |
| 141 | + |
| 142 | +Prerequisites that a PR must satisfy for merging into the `master` branch: |
| 143 | +* each commit within a PR must compile and pass unit tests with no errors, with |
| 144 | + every feature combination (including compiling the fuzztests) on some |
| 145 | + reasonably recent compiler (this is partially automated with CI, so the rule |
| 146 | + is that we will not accept commits which do not pass GitHub CI); |
| 147 | +* the tip of any PR branch must also compile and pass tests with no errors on |
| 148 | + MSRV (check [README.md] on current MSRV requirements) and pass fuzz tests on |
| 149 | + nightly rust; |
| 150 | +* contain all necessary tests for the introduced functionality (either as a part of |
| 151 | + commits, or, more preferably, as separate commits, so that it's easy to |
| 152 | + reorder them during review and check that the new tests fail without the new |
| 153 | + code); |
| 154 | +* contain all inline docs for newly introduced API and pass doc tests including |
| 155 | + running `just lint` without any errors or warnings; |
| 156 | +* be based on the recent `master` tip from the original repository at |
| 157 | + <https://github.com/rust-bitcoin/rust-miniscript>. |
| 158 | + |
| 159 | +NB: reviewers may run more complex test/CI scripts, thus, satisfying all the |
| 160 | +requirements above is just a preliminary, but not necessary sufficient step for |
| 161 | +getting the PR accepted as a valid candidate PR for the `master` branch. |
| 162 | + |
| 163 | +High quality commits help us review and merge you contributions. We attempt to |
| 164 | +adhere to the ideas presented in the following two blog posts: |
| 165 | + |
| 166 | +- [How to Write a Git Commit Message](https://cbea.ms/git-commit/) |
| 167 | +- [Write Better Commits, Build Better Projects](https://github.blog/2022-06-30-write-better-commits-build-better-projects/) |
| 168 | + |
| 169 | +### Deprecation and Versioning |
| 170 | + |
| 171 | +Whenever any part of your code wants to mention the version number the code will |
| 172 | +be released in, primarily in deprecation notices, you should use the string |
| 173 | +`TBD` (verbatim), so that the release preparation script can detect the |
| 174 | +change and the correct version number can be filled in preparation of the |
| 175 | +release. |
| 176 | + |
| 177 | +```rust |
| 178 | + #[deprecated(since = "TBD", note = "use `alternative_method()` instead")] |
| 179 | +``` |
| 180 | + |
| 181 | +### Peer review |
| 182 | + |
| 183 | +Anyone may participate in peer review which is expressed by comments in the pull |
| 184 | +request. Typically, reviewers will review the code for obvious errors, as well as |
| 185 | +test out the patch set and opine on the technical merits of the patch. Please, |
| 186 | +first review PR on the conceptual level before focusing on code style or |
| 187 | +grammar fixes. |
| 188 | + |
| 189 | +### CI and Merging |
| 190 | + |
| 191 | +We use GitHub for CI as well to test the final state of each PR. |
| 192 | + |
| 193 | +Also we use a local CI box which runs a large matrix of feature combinations as |
| 194 | +well as testing each patch in a PR. This box is often very backlogged, sometimes |
| 195 | +by multiple days. Please be patient, we will get to merging your PRs when the |
| 196 | +backlog clears. |
| 197 | + |
| 198 | +### Repository maintainers |
| 199 | + |
| 200 | +Like all open source projects our maintainers are busy. Please take it easy on |
| 201 | +them and only bump if you get no response for a week or two. |
| 202 | + |
| 203 | +Pull request merge requirements: |
| 204 | +- all CI test should pass, |
| 205 | +- at least one "accepts"/ACKs from the repository maintainers |
| 206 | +- no reasonable "rejects"/NACKs from anybody who reviewed the code. |
| 207 | + |
| 208 | +Current list of the project maintainers: |
| 209 | + |
| 210 | +- [Andrew Poelstra](https://github.com/apoelstra) |
| 211 | +- [Sanket Kanjalkar](https://github.com/sanket1729) |
| 212 | + |
| 213 | +### Unsafe code |
| 214 | + |
| 215 | +Use of `unsafe` code is prohibited unless there is a unanimous decision among |
| 216 | +library maintainers on the exclusion from this rule. In such cases there is a |
| 217 | +requirement to test unsafe code with sanitizers including Miri. |
| 218 | + |
| 219 | +## Security |
| 220 | + |
| 221 | +Security is the primary focus for this library; disclosure of security |
| 222 | +vulnerabilities helps prevent user loss of funds. If you believe a vulnerability |
| 223 | +may affect other implementations, please disclose this information according to |
| 224 | +the [security guidelines](./SECURITY.md), work on which is currently in progress. |
| 225 | +Before it is completed, feel free to send disclosure to Andrew Poelstra, |
| 226 | +apoelstra@wpsoftware.net, encrypted with his public key from |
| 227 | +<https://www.wpsoftware.net/andrew/andrew.gpg>. |
| 228 | + |
| 229 | + |
| 230 | +## Testing |
| 231 | + |
| 232 | +Related to the security aspect, rust bitcoin developers take testing very |
| 233 | +seriously. Due to the modular nature of the project, writing new test cases is |
| 234 | +easy and good test coverage of the codebase is an important goal. Refactoring |
| 235 | +the project to enable fine-grained unit testing is also an ongoing effort. |
| 236 | + |
| 237 | +Unit and integration tests are available for those interested, along with benchmarks. For project |
| 238 | +developers, especially new contributors looking for something to work on, we do: |
| 239 | + |
| 240 | +- Fuzz testing with [`libfuzzer`](https://github.com/rust-fuzz/libfuzzer) |
| 241 | +- Mutation testing with [`cargo-mutants`](https://github.com/sourcefrog/cargo-mutants) |
| 242 | +- Code verification with [`Kani`](https://github.com/model-checking/kani) |
| 243 | + |
| 244 | +There are always more tests to write and more bugs to find. PRs are extremely welcomed. |
| 245 | +Please consider testing code as a first-class citizen. We definitely do take PRs |
| 246 | +improving and cleaning up test code. |
| 247 | + |
| 248 | +### Unit/Integration tests |
| 249 | + |
| 250 | +Run as for any other Rust project `cargo test --all-features`. |
| 251 | + |
| 252 | +### Benchmarks |
| 253 | + |
| 254 | +We use a custom Rust compiler configuration conditional to guard the benchmark code. To run the |
| 255 | +benchmarks use: `RUSTFLAGS='--cfg=bench' cargo +nightly bench`. |
| 256 | + |
| 257 | + |
| 258 | +## LLMs, GitHub bot accounts, and AI agents |
| 259 | + |
| 260 | +This project does not accept contributions from bot GitHub accounts. All |
| 261 | +PRs that appear to come from such an account will be closed. |
| 262 | + |
| 263 | +Patches created by LLMs and AI agents are also viewed with suspicion unless a |
| 264 | +human has reviewed them. All LLM generated patches MUST have text in the git log |
| 265 | +and in the PR description that indicates the patch was created using an LLM. |
| 266 | +First time contributions by way of LLM generated patches are not welcome. Thanks |
| 267 | +for your time, please be respectful of ours. |
| 268 | + |
| 269 | + |
| 270 | +## Going further |
| 271 | + |
| 272 | +You may be interested in the guide by Jon Atack on |
| 273 | +[How to review Bitcoin Core PRs](https://github.com/jonatack/bitcoin-development/blob/master/how-to-review-bitcoin-core-prs.md) |
| 274 | +and [How to make Bitcoin Core PRs](https://github.com/jonatack/bitcoin-development/blob/master/how-to-make-bitcoin-core-prs.md). |
| 275 | +While there are differences between the projects in terms of context and |
| 276 | +maturity, many of the suggestions offered apply to this project. |
| 277 | + |
| 278 | +Overall, have fun :) |
0 commit comments