Skip to content

Commit e3df58d

Browse files
balajimarabalajimara
committed
updating changes
0 parents  commit e3df58d

File tree

672 files changed

+51555
-0
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

672 files changed

+51555
-0
lines changed

‎CONTRIBUTING.md‎

Lines changed: 388 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,388 @@
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+
* [Upgrading dependencies](#upgrading-dependencies)
25+
* [Unsafe code](#unsafe-code)
26+
* [Policy](#policy)
27+
- [Security](#security)
28+
- [Testing](#testing)
29+
- [Going further](#going-further)
30+
31+
32+
## General
33+
34+
The Rust Bitcoin project operates an open contributor model where anyone is
35+
welcome to contribute towards development in the form of peer review,
36+
documentation, testing and patches.
37+
38+
Anyone is invited to contribute without regard to technical experience,
39+
"expertise", OSS experience, age, or other concern. However, the development of
40+
standards & reference implementations demands a high-level of rigor, adversarial
41+
thinking, thorough testing and risk-minimization. Any bug may cost users real
42+
money. That being said, we deeply welcome people contributing for the first time
43+
to an open source project or pick up Rust while contributing. Don't be shy,
44+
you'll learn.
45+
46+
47+
## Communication channels
48+
49+
Communication about Rust Bitcoin 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+
Major projects are tracked [here](https://github.com/orgs/rust-bitcoin/projects).
59+
Major milestones are tracked [here](https://github.com/rust-bitcoin/rust-bitcoin/milestones).
60+
61+
62+
## Asking questions
63+
64+
> **Note:** Please don't file an issue to ask a question. You'll get faster
65+
> results by using the resources below.
66+
67+
We have a dedicated developer channel on IRC, #[email protected] where
68+
you may get helpful advice if you have questions.
69+
70+
71+
## Contribution workflow
72+
73+
The codebase is maintained using the "contributor workflow" where everyone
74+
without exception contributes patch proposals using "pull requests". This
75+
facilitates social contribution, easy testing and peer review.
76+
77+
To contribute a patch, the workflow is a as follows:
78+
79+
1. Fork Repository
80+
2. Create topic branch
81+
3. Commit patches
82+
83+
Please keep commits should atomic and diffs easy to read. For this reason
84+
do not mix any formatting fixes or code moves with actual code changes.
85+
Further, each commit, individually, should compile and pass tests, in order to
86+
ensure git bisect and other automated tools function properly.
87+
88+
Please cover every new feature with unit tests.
89+
90+
When refactoring, structure your PR to make it easy to review and don't hesitate
91+
to split it into multiple small, focused PRs.
92+
93+
Commits should cover both the issue fixed and the solution's rationale.
94+
Please keep these [guidelines](https://chris.beams.io/posts/git-commit/) in mind.
95+
96+
To facilitate communication with other contributors, the project is making use
97+
of GitHub's "assignee" field. First check that no one is assigned and then
98+
comment suggesting that you're working on it. If someone is already assigned,
99+
don't hesitate to ask if the assigned party or previous commenters are still
100+
working on it if it has been awhile.
101+
102+
103+
## Preparing PRs
104+
105+
The main library development happens in the `master` branch. This branch must
106+
always compile without errors (using GitHub CI). All external contributions are
107+
made within PRs into this branch.
108+
109+
Prerequisites that a PR must satisfy for merging into the `master` branch:
110+
* each commit within a PR must compile and pass unit tests with no errors, with
111+
every feature combination (including compiling the fuzztests) on some
112+
reasonably recent compiler (this is partially automated with CI, so the rule
113+
is that we will not accept commits which do not pass GitHub CI);
114+
* the tip of any PR branch must also compile and pass tests with no errors on
115+
MSRV (check [README.md] on current MSRV requirements) and pass fuzz tests on
116+
nightly rust;
117+
* contain all necessary tests for the introduced functional (either as a part of
118+
commits, or, more preferably, as separate commits, so that it's easy to
119+
reorder them during review and check that the new tests fail without the new
120+
code);
121+
* contain all inline docs for newly introduced API and pass doc tests;
122+
* be based on the recent `master` tip from the original repository at
123+
<https://github.com/rust-bitcoin/rust-bitcoin>.
124+
125+
NB: reviewers may run more complex test/CI scripts, thus, satisfying all the
126+
requirements above is just a preliminary, but not necessary sufficient step for
127+
getting the PR accepted as a valid candidate PR for the `master` branch.
128+
129+
PR authors may also find it useful to run the following script locally in order
130+
to check that each of the commits within the PR satisfies the requirements
131+
above, before submitting the PR to review:
132+
```shell script
133+
RUSTUP_TOOLCHAIN=1.41.1 ./contrib/test.sh
134+
```
135+
Please replace the value in `RUSTUP_TOOLCHAIN=1.41.1` with the current MSRV from
136+
[README.md].
137+
138+
NB: Please keep in mind that the script above replaces `Cargo.lock` file, which
139+
is necessary to support current MSRV, incompatible with `stable` and newer cargo
140+
versions.
141+
142+
### Peer review
143+
144+
Anyone may participate in peer review which is expressed by comments in the pull
145+
request. Typically, reviewers will review the code for obvious errors, as well as
146+
test out the patch set and opine on the technical merits of the patch. Please,
147+
first review PR on the conceptual level before focusing on code style or
148+
grammar fixes.
149+
150+
### Repository maintainers
151+
152+
Pull request merge requirements:
153+
- all CI test should pass,
154+
- at least two "accepts"/ACKs from the repository maintainers (see "refactor carve-out").
155+
- no reasonable "rejects"/NACKs from anybody who reviewed the code.
156+
157+
Current list of the project maintainers:
158+
159+
- [Andrew Poelstra](https://github.com/apoelstra)
160+
- [Steven Roose](https://github.com/stevenroose)
161+
- [Matt Corallo](https://github.com/TheBlueMatt)
162+
- [Elichai Turkel](https://github.com/elichai)
163+
- [Sanket Kanjalkar](https://github.com/sanket1729)
164+
- [Martin Habovštiak](https://github.com/Kixunil)
165+
- [Riccardo Casatta](https://github.com/RCasatta)
166+
- [Tobin Harding](https://github.com/tcharding)
167+
168+
#### Refactor carve-out
169+
170+
The repository is going through heavy refactoring and "trivial" API redesign
171+
(eg, rename `Foo::empty` to `Foo::new`) as we push towards API stabilization. As
172+
such reviewers are either bored or overloaded with notifications, hence we have
173+
created a carve out to the 2-ACK rule.
174+
175+
A PR may be considered for merge if it has a single ACK and has sat open for at
176+
least two weeks with no comments, questions, or NACKs.
177+
178+
#### One ACK carve-out
179+
180+
We reserve the right to merge PRs with a single ACK [0], at any time, if they match
181+
any of the following conditions:
182+
183+
1. PR only touches CI i.e, only changes any of the `test.sh` scripts and/or
184+
stuff in `.github/workflows`.
185+
2. Non-content changing documentation fixes i.e., grammar/typos, spelling, full
186+
stops, capital letters. Any change with more substance must still get two
187+
ACKs.
188+
3. Code moves that do not change the API e.g., moving error types to a private
189+
submodule and re-exporting them from the original module. Must not include
190+
any code changes except to import paths. This rule is more restrictive than
191+
the refactor carve-out. It requires absolutely no change to the public API.
192+
193+
[0] - Obviously author and ACK'er must not be the same person.
194+
195+
## Coding conventions
196+
197+
Library reflects Bitcoin Core approach whenever possible.
198+
199+
### Naming conventions
200+
201+
Naming of data structures/enums and their fields/variants must follow names used
202+
in Bitcoin Core, with the following exceptions:
203+
- the case should follow Rust standards (i.e. PascalCase for types and
204+
snake_case for fields and variants);
205+
- omit `C`-prefixes.
206+
207+
### Upgrading dependencies
208+
209+
If your change requires a dependency to be upgraded you must do the following:
210+
211+
1. Modify `Cargo.toml`
212+
2. Copy `Cargo-minimal.lock` to `Cargo.lock`
213+
3. Trigger cargo to update the required entries in the lock file - use `--precise` using the minimum version number that works
214+
4. Test your change
215+
5. Copy `Cargo.lock` to `Cargo-minimal.lock`
216+
6. Update `Cargo-recent.lock` if it is also behind
217+
7. Commit both lock files together with `Cargo.toml` and your code changes
218+
219+
### Unsafe code
220+
221+
Use of `unsafe` code is prohibited unless there is a unanimous decision among
222+
library maintainers on the exclusion from this rule. In such cases there is a
223+
requirement to test unsafe code with sanitizers including Miri.
224+
225+
226+
### Policy
227+
228+
We have various `rust-bitcoin` specific coding styles and conventions that are
229+
grouped here loosely under the term 'policy'. These are things we try to adhere
230+
to but that you should not need to worry too much about if you are a new
231+
contributor. Think of this as a place to collect group knowledge that exists in
232+
the various PRs over the last few years.
233+
234+
#### Import statements
235+
236+
We use the following style for import statements, see
237+
(https://github.com/rust-bitcoin/rust-bitcoin/discussions/2088) for the discussion that led to this.
238+
239+
```rust
240+
241+
// Modules first, as they are part of the project's structure.
242+
pub mod aa_this;
243+
mod bb_private;
244+
pub mod cc_that;
245+
246+
// Private imports, rustfmt will sort and merge them correctly.
247+
use crate::aa_this::{This, That};
248+
use crate::bb_that;
249+
250+
// Public re-exports.
251+
#[rustfmt::skip] // Keeps public re-exports separate, because of this we have to sort manually.
252+
pub use {
253+
crate::aa_aa_this,
254+
crate::bb_bb::That,
255+
}
256+
```
257+
258+
#### Errors
259+
260+
Return as much context as possible with errors e.g., if an error was encountered parsing a string
261+
include the string in the returned error type. If a function consumes costly-to-compute input
262+
(allocations are also considered costly) it should return the input back in the error type.
263+
264+
More specifically an error should
265+
266+
- be `non_exhaustive` unless we _really_ never want to change it.
267+
- have private fields unless we are very confident they won't change.
268+
- derive `Debug, Clone, PartialEq, Eq` (and `Copy` iff not `non_exhaustive`).
269+
- implement Display using `write_err!()` macro if a variant contains an inner error source.
270+
- have `Error` suffix
271+
- implement `std::error::Error` if they are public (feature gated on "std").
272+
273+
```rust
274+
/// Documentation for the `Error` type.
275+
#[derive(Debug, Clone, PartialEq, Eq)]
276+
#[non_exhaustive] // Add liberally; if the error type may ever have new variants added.
277+
pub enum Error {
278+
/// Documentation for variant A.
279+
A,
280+
/// Documentation for variant B.
281+
B,
282+
}
283+
```
284+
285+
286+
#### Rustdocs
287+
288+
Be liberal with references to BIPs or other documentation; the aim is that devs can learn about
289+
Bitcoin by hacking on this codebase as opposed to having to learn about Bitcoin first and then start
290+
hacking on this codebase. Consider the following format, not all sections will be required for all types.
291+
292+
293+
```rust
294+
/// The Bitcoin foobar.
295+
///
296+
/// Contains all the data used when passing a foobar around the Bitcoin network.
297+
///
298+
/// <details>
299+
/// <summary>FooBar Original Design</summary>
300+
///
301+
/// The foobar was introduced in Bitcoin x.y.z to increase the amount of foo in bar.
302+
///
303+
/// </details>
304+
///
305+
/// ### Relevant BIPs
306+
///
307+
/// * [BIP X - FooBar in Bitcoin](https://github.com/bitcoin/bips/blob/master/bip-0000.mediawiki)
308+
pub struct FooBar {
309+
/// The version in use.
310+
pub version: Version
311+
}
312+
```
313+
314+
Do use rustdoc subheadings. Do put an empty newline below each heading e.g.,
315+
316+
```rust
317+
impl FooBar {
318+
/// Constructs a `FooBar` from a [`Baz`].
319+
///
320+
/// # Errors
321+
///
322+
/// Returns an error if `Baz` is not ...
323+
///
324+
/// # Panics
325+
///
326+
/// If the `Baz`, converted to a `usize`, is out of bounds.
327+
pub fn from_baz(baz: Baz) -> Result<Self, Error> {
328+
...
329+
}
330+
}
331+
```
332+
333+
Add Panics section if any input to the function can trigger a panic.
334+
335+
Generally we prefer to have non-panicking APIs but it is impractical in some cases. If you're not
336+
sure, feel free to ask. If we determine panicking is more practical it must be documented. Internal
337+
panics that could theoretically occur because of bugs in our code must not be documented.
338+
339+
340+
#### Attributes
341+
342+
- `#[track_caller]`: Used on functions that panic on invalid arguments
343+
(see https://rustc-dev-guide.rust-lang.org/backend/implicit-caller-location.html)
344+
345+
- `#[cfg(rust_v_1_60)]`: Used to guard code that should only be built in if the toolchain is
346+
compatible. These configuration conditionals are set at build time in `bitcoin/build.rs`. New
347+
version attributes may be added as needed.
348+
349+
350+
#### Licensing
351+
352+
We use SPDX license tags, all files should start with
353+
354+
```
355+
// SPDX-License-Identifier: CC0-1.0
356+
```
357+
358+
## Security
359+
360+
Security is the primary focus for this library; disclosure of security
361+
vulnerabilities helps prevent user loss of funds. If you believe a vulnerability
362+
may affect other implementations, please disclose this information according to
363+
the [security guidelines](./SECURITY.md), work on which is currently in progress.
364+
Before it is completed, feel free to send disclosure to Andrew Poelstra,
365+
[email protected], encrypted with his public key from
366+
<https://www.wpsoftware.net/andrew/andrew.gpg>.
367+
368+
369+
## Testing
370+
371+
Related to the security aspect, rust bitcoin developers take testing very
372+
seriously. Due to the modular nature of the project, writing new test cases is
373+
easy and good test coverage of the codebase is an important goal. Refactoring
374+
the project to enable fine-grained unit testing is also an ongoing effort.
375+
376+
Various methods of testing are in use (e.g. fuzzing, mutation), please see
377+
the [readme](./README.md) for more information.
378+
379+
380+
## Going further
381+
382+
You may be interested in the guide by Jon Atack on
383+
[How to review Bitcoin Core PRs](https://github.com/jonatack/bitcoin-development/blob/master/how-to-review-bitcoin-core-prs.md)
384+
and [How to make Bitcoin Core PRs](https://github.com/jonatack/bitcoin-development/blob/master/how-to-make-bitcoin-core-prs.md).
385+
While there are differences between the projects in terms of context and
386+
maturity, many of the suggestions offered apply to this project.
387+
388+
Overall, have fun :)

0 commit comments

Comments
 (0)