github.com/aakash4dev/cometbft@v0.38.2/CONTRIBUTING.md (about)

     1  # Contributing
     2  
     3  Thank you for your interest in contributing to CometBFT! Before contributing, it
     4  may be helpful to understand the goal of the project. The goal of CometBFT is to
     5  develop a BFT consensus engine robust enough to support permissionless
     6  value-carrying networks. While all contributions are welcome, contributors
     7  should bear this goal in mind in deciding if they should target the main
     8  CometBFT project or a potential fork.
     9  
    10  ## Overview
    11  
    12  When targeting the main CometBFT project, following the processes outlined in
    13  this document will lead to the best chance of landing changes in a release.
    14  
    15  ### Core team responsibility
    16  
    17  The CometBFT core team is responsible for stewarding this
    18  project over time. This means that the core team needs to understand the nature
    19  of, and agree to maintain, all of the changes that land on `main` or a backport
    20  branch. It may cost a few days/weeks' worth of time to _submit_ a
    21  particular change, but _maintaining_ that change over the years has a
    22  much higher cost that the core team will bear.
    23  
    24  ### Ease of reviewing
    25  
    26  The fact that the core team needs to be able to deeply understand the short-,
    27  medium- and long-term consequences of incoming changes means that changes need
    28  to be **easily reviewed**.
    29  
    30  What makes a change easy to review, and more likely to land in an upcoming
    31  release?
    32  
    33  1. **Each pull request must do _one thing_**. It must be very clear what that
    34     one thing is when looking at the pull request title, description, and linked
    35     issues. It must also be very clear what value it ultimately aims to deliver,
    36     and to which user(s). A single pull request that does multiple things, or
    37     without a clear articulation of the problem it attempts to solve, may be
    38     rejected immediately.
    39  
    40  2. **Each pull request must be at most 300 lines of code changes**. Larger
    41     changes must be structured as a series of pull requests of at most 300 lines
    42     of code changes each, each building upon the previous one, all ideally
    43     tracked in a tracking issue.
    44  
    45     If a single PR absolutely has to be larger, it _must_ be structured such that
    46     it can be reviewed commit by commit, with each commit doing _one logical
    47     thing_ (with a good description of what it aims to achieve in the Git
    48     commit), and each commit ideally being no larger than 300 lines of code
    49     changes. Poorly structured pull requests may be rejected immediately with a
    50     request to restructure them.
    51  
    52     This does not necessarily apply to documentation-related changes or
    53     automatically generated code (e.g. generated from Protobuf definitions). But
    54     automatically generated code changes should occur within separate commits, so
    55     they are easily distinguishable from manual code changes.
    56  
    57  ## Workflow
    58  
    59  The following diagram summarizes the general workflow used by the core team to
    60  make changes, with a full description of the workflow below the diagram.
    61  Exceptions to this process will naturally occur (e.g. in the case of urgent
    62  security fixes), but this is rare.
    63  
    64  Each stage of the process is aimed at creating feedback cycles which align
    65  contributors and maintainers to make sure:
    66  
    67  - Contributors don’t waste their time implementing/proposing features which
    68    won't land in `main`.
    69  - Maintainers have the necessary context in order to support and review
    70    contributions.
    71  
    72  ```mermaid
    73  flowchart LR
    74      complexity{Problem\ncomplexity}
    75      issue("New issue\n(Problem articulation\nfor discussion)")
    76      clarity{"Problem +\nsolution clarity"}
    77      rfc("RFC pull request(s)")
    78      rfc_merge("Merge RFC to main")
    79      risk{"Solution\ncomplexity/risk"}
    80      adr("ADR + PoC\npull request(s)")
    81      adr_merge("Merge ADR to main\nand create tracking issue")
    82      pr("Solution\npull request(s)")
    83      merge("Merge to main/backport\nor feature branch")
    84  
    85      complexity --"Low/Moderate/High"--> issue
    86      complexity --Trivial--> pr
    87      issue --> clarity
    88      clarity --High--> risk
    89      clarity --Low--> rfc
    90      rfc --Approved--> rfc_merge
    91      risk --"Moderate/High"--> adr
    92      adr --"ADR accepted by core team"--> adr_merge
    93      adr_merge --> pr
    94      risk --Low--> pr
    95      pr --Approved--> merge
    96  ```
    97  
    98  ### GitHub issues
    99  
   100  All non-trivial work on the code base should be motivated by a [GitHub
   101  Issue][gh-issues]. [Search][search-issues] is a good place to start when looking
   102  for places to contribute. If you would like to work on an issue which already
   103  exists, please indicate so by leaving a comment. If someone else is already
   104  assigned to that issue and you would like to contribute to it or take it over,
   105  please coordinate with the existing assignee(s) and only start work on it once
   106  you have been assigned to it. Unsolicited pull requests relating to issues
   107  assigned to other users may be rejected immediately.
   108  
   109  All new contributions should start with a [GitHub Issue][new-gh-issue]. The
   110  issue helps capture the **problem** being solved and allows for early feedback.
   111  Problems must be captured in terms of the **impact** that they have on specific
   112  users. Once the issue is created the process can proceed in different directions
   113  depending on how well defined the problem and potential solution are. If the
   114  change is simple and well understood, maintainers will indicate their support
   115  with a heartfelt emoji.
   116  
   117  ### Request for comments (RFCs)
   118  
   119  If the issue would benefit from thorough discussion, maintainers may request
   120  that you create a [Request For Comment][rfcs] in the CometBFT repo. Discussion
   121  at the RFC stage will build collective understanding of the dimensions of the
   122  problems and help structure conversations around trade-offs.
   123  
   124  ### Architecture decision records (ADRs)
   125  
   126  When the problem is well understood but the solution leads to
   127  large/complex/risky structural changes to the code base, these changes should be
   128  proposed in the form of an [Architectural Decision Record
   129  (ADR)](./docs/architecture/). The ADR will help build consensus on an overall
   130  strategy to ensure the code base maintains coherence in the larger context. If
   131  you are not comfortable with writing an ADR, you can open a less-formal issue
   132  and the maintainers will help you turn it into an ADR. Sometimes the best way to
   133  demonstrate the value of an ADR is to build a proof-of-concept (PoC) along with
   134  the ADR - in this case, link to the PoC from the ADR PR.
   135  
   136  **How does one pick a number for an new ADR?**
   137  
   138  Find the largest existing ADR number (between those in `./docs/architecture/`
   139  and those that may be open as issues or pull requests) and bump it by 1.
   140  
   141  ### Pull requests
   142  
   143  When the problem as well as proposed solution are well understood and low-risk,
   144  changes should start with a **pull request**.
   145  
   146  Please adhere to the guidelines in the [Ease of reviewing](#ease-of-reviewing)
   147  section above when submitting pull requests.
   148  
   149  ### Draft pull requests
   150  
   151  One can optionally submit a [draft pull request][gh-draft-prs] against `main`,
   152  in which case this signals that work is underway and is not ready for review.
   153  Only users that are familiar with the issue, or those that the author explicitly
   154  requested a review from are expected to write comments at this point. When the
   155  work is ready for feedback, hitting "Ready for Review" will signal to the
   156  maintainers to take a look, and to the rest of the community that feedback is
   157  welcome.
   158  
   159  **The team may opt to ignore unsolicited comments/feedback on draft PRs**, as
   160  having to respond to feedback on work that is not marked as "Ready for Review"
   161  interferes with the process of getting the work to the point that it is ready to
   162  review.
   163  
   164  ## Forking
   165  
   166  Please note that Go requires code to live under absolute paths, which
   167  complicates forking. While my fork lives at
   168  `https://github.com/ebuchman/cometbft`, the code should never exist at
   169  `$GOPATH/src/github.com/ebuchman/cometbft`. Instead, we use `git remote` to add
   170  the fork as a new remote for the original repo,
   171  `$GOPATH/src/github.com/aakash4dev/cometbft`, and do all the work there.
   172  
   173  For instance, to create a fork and work on a branch of it, I would:
   174  
   175  - Create the fork on GitHub, using the fork button.
   176  - Go to the original repo checked out locally (i.e.
   177    `$GOPATH/src/github.com/aakash4dev/cometbft`)
   178  - `git remote rename origin upstream`
   179  - `git remote add origin git@github.com:ebuchman/basecoin.git`
   180  
   181  Now `origin` refers to my fork and `upstream` refers to the CometBFT version. So
   182  I can `git push -u origin main` to update my fork, and make pull requests to
   183  CometBFT from there. Of course, replace `ebuchman` with your git handle.
   184  
   185  To pull in updates from the origin repo, run
   186  
   187  - `git fetch upstream`
   188  - `git rebase upstream/main` (or whatever branch you want)
   189  
   190  ## Dependencies
   191  
   192  We use [Go modules] to manage dependencies.
   193  
   194  That said, the `main` branch of every CometBFT repository should just build with
   195  `go get`, which means they should be kept up-to-date with their dependencies so
   196  we can get away with telling people they can just `go get` our software.
   197  
   198  Since some dependencies are not under our control, a third party may break our
   199  build, in which case we can fall back on `go mod tidy`. Even for dependencies
   200  under our control, go helps us to keep multiple repos in sync as they evolve.
   201  Anything with an executable, such as apps, tools, and the core, should use dep.
   202  
   203  Run `go list -u -m all` to get a list of dependencies that may not be
   204  up-to-date.
   205  
   206  When updating dependencies, please only update the particular dependencies you
   207  need. Instead of running `go get -u=patch`, which will update anything, specify
   208  exactly the dependency you want to update.
   209  
   210  ## Protobuf
   211  
   212  We use [Protocol Buffers] along with [`gogoproto`] to generate code for use
   213  across CometBFT.
   214  
   215  To generate proto stubs, lint, and check protos for breaking changes, you will
   216  need to install [buf] and `gogoproto`. Then, from the root of the repository,
   217  run:
   218  
   219  ```bash
   220  # Lint all of the .proto files
   221  make proto-lint
   222  
   223  # Check if any of your local changes (prior to committing to the Git repository)
   224  # are breaking
   225  make proto-check-breaking
   226  
   227  # Generate Go code from the .proto files
   228  make proto-gen
   229  ```
   230  
   231  To automatically format `.proto` files, you will need [`clang-format`]
   232  installed. Once installed, you can run:
   233  
   234  ```bash
   235  make proto-format
   236  ```
   237  
   238  ### Visual Studio Code
   239  
   240  If you are a VS Code user, you may want to add the following to your
   241  `.vscode/settings.json`:
   242  
   243  ```json
   244  {
   245    "protoc": {
   246      "options": [
   247        "--proto_path=${workspaceRoot}/proto",
   248      ]
   249    }
   250  }
   251  ```
   252  
   253  ## Changelog
   254  
   255  To manage and generate our changelog, we currently use [unclog].
   256  
   257  Every fix, improvement, feature, or breaking change should be made in a
   258  pull-request that includes a file
   259  `.changelog/unreleased/${category}/${issue-or-pr-number}-${description}.md`,
   260  where:
   261  
   262  - `category` is one of `improvements`, `breaking-changes`, `bug-fixes`,
   263    `features` and if multiple apply, create multiple files;
   264  - `description` is a short (4 to 6 word), hyphen separated description of the
   265    fix, starting the component changed; and,
   266  - `issue or PR number` is the CometBFT issue number, if one exists, or the PR
   267    number, otherwise.
   268  
   269  For examples, see the [.changelog](.changelog) folder.
   270  
   271  A feature can also be worked on a feature branch, if its size and/or risk
   272  justifies it (see [below](#branching-model-and-release)).
   273  
   274  ### What does a good changelog entry look like?
   275  
   276  Changelog entries should answer the question: "what is important about this
   277  change for users to know?" or "what problem does this solve for users?". It
   278  should not simply be a reiteration of the title of the associated PR, unless the
   279  title of the PR _very_ clearly explains the benefit of a change to a user.
   280  
   281  Some good examples of changelog entry descriptions:
   282  
   283  ```md
   284  - [consensus] \#1111 Small transaction throughput improvement (approximately
   285    3-5\% from preliminary tests) through refactoring the way we use channels
   286  - [mempool] \#1112 Refactor Go API to be able to easily swap out the current
   287    mempool implementation in CometBFT forks
   288  - [p2p] \#1113 Automatically ban peers when their messages are unsolicited or
   289    are received too frequently
   290  ```
   291  
   292  Some bad examples of changelog entry descriptions:
   293  
   294  ```md
   295  - [consensus] \#1111 Refactor channel usage
   296  - [mempool] \#1112 Make API generic
   297  - [p2p] \#1113 Ban for PEX message abuse
   298  ```
   299  
   300  For more on how to write good changelog entries, see:
   301  
   302  - <https://keepachangelog.com>
   303  - <https://docs.gitlab.com/ee/development/changelog.html#writing-good-changelog-entries>
   304  - <https://depfu.com/blog/what-makes-a-good-changelog>
   305  
   306  ### Changelog entry format
   307  
   308  Changelog entries should be formatted as follows:
   309  
   310  ```md
   311  - [module] \#xxx Some description of the change (@contributor)
   312  ```
   313  
   314  Here, `module` is the part of the code that changed (typically a top-level Go
   315  package), `xxx` is the pull-request number, and `contributor` is the author/s of
   316  the change.
   317  
   318  It's also acceptable for `xxx` to refer to the relevant issue number, but
   319  pull-request numbers are preferred. Note this means pull-requests should be
   320  opened first so the changelog can then be updated with the pull-request's
   321  number. There is no need to include the full link, as this will be added
   322  automatically during release. But please include the backslash and pound, eg.
   323  `\#2313`.
   324  
   325  Changelog entries should be ordered alphabetically according to the `module`,
   326  and numerically according to the pull-request number.
   327  
   328  Changes with multiple classifications should be doubly included (eg. a bug fix
   329  that is also a breaking change should be recorded under both).
   330  
   331  Breaking changes are further subdivided according to the APIs/users they impact.
   332  Any change that affects multiple APIs/users should be recorded multiply - for
   333  instance, a change to the `Blockchain Protocol` that removes a field from the
   334  header should also be recorded under `CLI/RPC/Config` since the field will be
   335  removed from the header in RPC responses as well.
   336  
   337  ## Branching Model and Release
   338  
   339  The main development branch is `main`.
   340  
   341  Every release is maintained in a release branch named `vX.Y.Z`.
   342  
   343  Pending minor releases have long-lived release candidate ("RC") branches. Minor
   344  release changes should be merged to these long-lived RC branches at the same
   345  time that the changes are merged to `main`.
   346  
   347  If a feature's size is big and/or its risk is high, it can be implemented in a
   348  feature branch. While the feature work is in progress, pull requests are open
   349  and squash merged against the feature branch. Branch `main` is periodically
   350  merged (merge commit) into the feature branch, to reduce branch divergence. When
   351  the feature is complete, the feature branch is merged back (merge commit) into
   352  `main`. The moment of the final merge can be carefully chosen so as to land
   353  different features in different releases.
   354  
   355  Note, all pull requests should be squash merged except for merging to a release
   356  branch (named `vX.Y`). This keeps the commit history clean and makes it easy to
   357  reference the pull request where a change was introduced.
   358  
   359  ### Development Procedure
   360  
   361  The latest state of development is on `main`, which must never fail `make test`.
   362  _Never_ force push `main`, unless fixing broken git history (which we rarely do
   363  anyways).
   364  
   365  To begin contributing, create a development branch either on
   366  `github.com/aakash4dev/cometbft`, or your fork (using `git remote add origin`).
   367  
   368  Make changes, and before submitting a pull request, update the changelog to
   369  record your change. Also, run either `git rebase` or `git merge` on top of the
   370  latest `main`. (Since pull requests are squash-merged, either is fine!)
   371  
   372  Update the `UPGRADING.md` if the change you've made is breaking and the
   373  instructions should be in place for a user on how he/she can upgrade its
   374  software (ABCI application, CometBFT blockchain, light client, wallet).
   375  
   376  Sometimes (often!) pull requests get out-of-date with `main`, as other people
   377  merge different pull requests to `main`. It is our convention that pull request
   378  authors are responsible for updating their branches with `main`. (This also
   379  means that you shouldn't update someone else's branch for them; even if it seems
   380  like you're doing them a favor, you may be interfering with their git flow in
   381  some way!)
   382  
   383  #### Merging Pull Requests
   384  
   385  It is also our convention that authors merge their own pull requests, when
   386  possible. External contributors may not have the necessary permissions to do
   387  this, in which case, a member of the core team will merge the pull request once
   388  it's been approved.
   389  
   390  Before merging a pull request:
   391  
   392  - Ensure pull branch is up-to-date with a recent `main` (GitHub won't let you
   393    merge without this!)
   394  - Run `make test` to ensure that all tests pass
   395  - [Squash][git-squash] merge pull request
   396  
   397  #### Pull Requests for Minor Releases
   398  
   399  If your change should be included in a minor release, please also open a PR
   400  against the long-lived minor release candidate branch (e.g., `rc1/v0.33.5`)
   401  _immediately after your change has been merged to main_.
   402  
   403  You can do this by cherry-picking your commit off `main`:
   404  
   405  ```sh
   406  $ git checkout rc1/v0.33.5
   407  $ git checkout -b {new branch name}
   408  $ git cherry-pick {commit SHA from main}
   409  # may need to fix conflicts, and then use git add and git cherry-pick --continue
   410  $ git push origin {new branch name}
   411  ```
   412  
   413  After this, you can open a PR. Please note in the PR body if there were merge
   414  conflicts so that reviewers can be sure to take a thorough look.
   415  
   416  ### Git Commit Style
   417  
   418  We follow the [Go style guide on commit messages][go-git-commit-style]. Write
   419  concise commits that start with the package name and have a description that
   420  finishes the sentence "This change modifies CometBFT to...". For example,
   421  
   422  ```sh
   423  cmd/debug: execute p.Signal only when p is not nil
   424  
   425  [potentially longer description in the body]
   426  
   427  Fixes #nnnn
   428  ```
   429  
   430  Each PR should have one commit once it lands on `main`; this can be accomplished
   431  by using the "squash and merge" button on GitHub. Be sure to edit your commit
   432  message, though!
   433  
   434  ## Testing
   435  
   436  ### Unit tests
   437  
   438  Unit tests are located in `_test.go` files as directed by [the Go testing
   439  package][go-testing]. If you're adding or removing a function, please check
   440  there's a `TestType_Method` test for it.
   441  
   442  Run: `make test`
   443  
   444  ### Integration tests
   445  
   446  Integration tests are also located in `_test.go` files. What differentiates them
   447  is a more complicated setup, which usually involves setting up two or more
   448  components.
   449  
   450  Run: `make test_integrations`
   451  
   452  ### End-to-end tests
   453  
   454  End-to-end tests are used to verify a fully integrated CometBFT network.
   455  
   456  See [README](./test/e2e/README.md) for details.
   457  
   458  Run:
   459  
   460  ```sh
   461  cd test/e2e && \
   462    make && \
   463    ./build/runner -f networks/ci.toml
   464  ```
   465  
   466  ### Fuzz tests (ADVANCED)
   467  
   468  *NOTE: if you're just submitting your first PR, you won't need to touch these
   469  most probably (99.9%)*.
   470  
   471  [Fuzz tests] can be found inside the `./test/fuzz` directory. See
   472  [README.md](./test/fuzz/README.md) for details.
   473  
   474  Run: `cd test/fuzz && make fuzz-{PACKAGE-COMPONENT}`
   475  
   476  ### RPC Testing
   477  
   478  **If you contribute to the RPC endpoints it's important to document your changes
   479  in the [OpenAPI file](./rpc/openapi/openapi.yaml)**.
   480  
   481  [gh-issues]: https://github.com/aakash4dev/cometbft/issues
   482  [search-issues]: https://github.com/aakash4dev/cometbft/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22
   483  [new-gh-issue]: https://github.com/aakash4dev/cometbft/issues/new/choose
   484  [rfcs]: https://github.com/aakash4dev/cometbft/tree/main/docs/rfc
   485  [gh-draft-prs]: https://github.blog/2019-02-14-introducing-draft-pull-requests/
   486  [Go modules]: https://github.com/golang/go/wiki/Modules
   487  [Protocol Buffers]: https://protobuf.dev/
   488  [`gogoproto`]: https://github.com/cosmos/gogoproto
   489  [buf]: https://buf.build/
   490  [`clang-format`]: https://clang.llvm.org/docs/ClangFormat.html
   491  [unclog]: https://github.com/informalsystems/unclog
   492  [git-squash]: https://stackoverflow.com/questions/5189560/squash-my-last-x-commits-together-using-git
   493  [go-git-commit-style]: https://tip.golang.org/doc/contribute.html#commit_messages
   494  [go-testing]: https://golang.org/pkg/testing/
   495  [Fuzz tests]: https://en.wikipedia.org/wiki/Fuzzing