diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 335e6572c19..381bb7eaae9 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -43,7 +43,7 @@ write a little note why. - [ ] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#pr-targeting)). - [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. -- [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/main/docs/docs/building-modules/10-structure.md). +- [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/main/docs/docs/building-modules/10-structure.md) and [Go style guide](../docs/dev/go-style-guide.md). - [ ] Wrote unit and integration [tests](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#testing). - [ ] Updated relevant documentation (`docs/`) or specification (`x//spec/`). - [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code). diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 292a924d06b..b380474ee92 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,275 +1,65 @@ -# Contributing +# Contributing to ibc-go -- [Contributing](#contributing) - - [Architecture Decision Records (ADR)](#architecture-decision-records-adr) - - [Pull Requests](#pull-requests) - - [Process for reviewing PRs](#process-for-reviewing-prs) - - [Updating Documentation](#updating-documentation) - - [Forking](#forking) - - [Dependencies](#dependencies) - - [Protobuf](#protobuf) - - [Testing](#testing) - - [Branching Model and Release](#branching-model-and-release) - - [PR Targeting](#pr-targeting) - - [Development Procedure](#development-procedure) - - [Pull Merge Procedure](#pull-merge-procedure) - - [Release Procedure](#release-procedure) - - [Point Release Procedure](#point-release-procedure) +Thank you for considering making contributions to ibc-go! 🎉👍 + +## Code of conduct -Thank you for considering making contributions to ibc-go! +This project and everyone participating in it is governed by ibc-go's [code of conduct](./CODE_OF_CONDUCT.md). By participating, you are expected to uphold this code -Contributing to this repo can mean many things such as participating in -discussion or proposing code changes. To ensure a smooth workflow for all -contributors, the general procedure for contributing has been established: +## How can I contribute? -1. Either [open](https://github.com/cosmos/ibc-go/issues/new/choose) or - [find](https://github.com/cosmos/ibc-go/issues) an issue you'd like to help with -2. Participate in thoughtful discussion on that issue -3. If you would like to contribute: - 1. If the issue is a proposal, ensure that the proposal has been accepted - 2. Ensure that nobody else has already begun working on this issue. If they have, - make sure to contact them to collaborate - 3. If nobody has been assigned for the issue and you would like to work on it, - make a comment on the issue to inform the community of your intentions - to begin work - 4. Follow standard Github best practices: fork the repo, branch from the - HEAD of `main`, make some commits, and submit a PR to `main` - - For core developers working within the ibc-go repo, to ensure a clear - ownership of branches, branches must be named with the convention - `{moniker}/{issue#}-branch-name` - 5. Be sure to submit the PR in `Draft` mode submit your PR early, even if - it's incomplete as this indicates to the community you're working on - something and allows them to provide comments early in the development process - 6. When the code is complete it can be marked `Ready for Review` - 7. Be sure to include a relevant change log entry in the `Unreleased` section - of `CHANGELOG.md` (see file for log format) +Contributing to this repository can mean many things such as participating in discussions or proposing code changes. To ensure a smooth workflow for all contributors, the general procedure for contributing has been established: -Note that for very small or blatantly obvious problems (such as typos) it is -not required to an open issue to submit a PR, but be aware that for more complex -problems/features, if a PR is opened before an adequate design discussion has -taken place in a github issue, that PR runs a high likelihood of being rejected. +### Reporting bugs -Other notes: +If you find that something is not working as expected, please open an issue using the [bug report template](https://github.com/cosmos/ibc-go/blob/main/.github/ISSUE_TEMPLATE/bug-report.md) and provide as much information possible: how can the bug be reproduced? What's the expected behavior? What version is affected? -- Looking for a good place to start contributing? How about checking out some - [good first issues](https://github.com/cosmos/ibc-go/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22) -- Please make sure to run `make format` before every commit - the easiest way - to do this is have your editor run it for you upon saving a file. Additionally - please ensure that your code is lint compliant by running `golangci-lint run`. +### Proposing improvements or new features -## Architecture Decision Records (ADR) +New features or improvements should be written in an issue using the [new feature template](https://github.com/cosmos/ibc-go/blob/main/.github/ISSUE_TEMPLATE/feature-request.md). Please include in the issue as many details as possible: what use case(s) would this new feature or improvement enable? Why are those use cases important or helpful? what user group would benefit? The team will evaluate and engage with you in a discussion of the proposal, which could have different outcomes: -When proposing an architecture decision for the ibc-go, please create an [ADR](./docs/architecture/README.md) -so further discussions can be made. We are following this process so all involved parties are in -agreement before any party begins coding the proposed implementation. If you would like to see some examples -of how these are written refer to [Cosmos SDK ADRs](https://github.com/cosmos/cosmos-sdk/tree/master/docs/architecture) +- the core ibc-go team deciding to implement this feature and adding it to their planning, +- agreeing to support external contributors to implement it with the goal of merging it eventually in ibc-go, +- discarding the suggestion if deemed not aligned with the objectives of ibc-go; +- or proposing (in the case of applications or light clients) to be developed and maintained in a separate repository. -## Pull Requests +Please check out also our [Request For Maintainership](./MAINTAINERSHIP.md) process, which contains information relevant to this. -To accommodate review process we suggest that PRs are categorically broken up. -Ideally each PR addresses only a single issue. Additionally, as much as possible -code refactoring and cleanup should be submitted as a separate PRs from bugfixes/feature-additions. +### Architecture Decision Records (ADR) -### Process for reviewing PRs +When proposing an architecture decision for the ibc-go, please create an [ADR](./docs/architecture/README.md) so further discussions can be made. We are following this process so all involved parties are in agreement before any party begins coding the proposed implementation. Please use the [ADR template](./docs/architecture/adr-template.md) to scaffold any new ADR. If you would like to see some examples of how these are written refer to ibc-go's [ADRs](./docs/architecture/). Solidified designs that will be implemented in ibc-go (and do not have a spec). They should document the architecture that will be built. Most design feedback should be gathered before the initial draft of the ADR. ADR's can/should be written for any design decisions we make which may be changed at some point in the future. -All PRs require an approval from at least one CODEOWNER before merge. PRs which cause signficant changes require two approvals from CODEOWNERS. When reviewing PRs please use the following review explanations: +### Participating in discussions -- `LGTM` without an explicit approval means that the changes look good, but you haven't pulled down the code, run tests locally and thoroughly reviewed it. -- `Approval` through the GH UI means that you understand the code, documentation/spec is updated in the right places, you have pulled down and tested the code locally. In addition: - - You must also think through anything which ought to be included but is not - - You must think through whether any added code could be partially combined (DRYed) with existing code - - You must think through any potential security issues or incentive-compatibility flaws introduced by the changes - - Naming must be consistent with conventions and the rest of the codebase - - Code must live in a reasonable location, considering dependency structures (e.g. not importing testing modules in production code, or including example code modules in production code). - - if you approve of the PR, you are responsible for fixing any of the issues mentioned here and more -- If you sat down with the PR submitter and did a pairing review please note that in the `Approval`, or your PR comments. -- If you are only making "surface level" reviews, submit any notes as `Comments` without adding a review. +New features or improvements are sometimes also debated in [discussions](https://github.com/cosmos/ibc-go/discussions). Sharing feedback or ideas there is very helpful for us. high level discussions that may get a lot of comments on a variety of different aspects, design aspects still being considered. -### Commit Messages +### Submitting pull requests -Commit messages should be [conventional](https://www.conventionalcommits.org/en/v1.0.0/). +Unless you feel confident your change will be accepted (trivial bug fixes, code cleanup, etc) you should first create an issue to discuss your change with us. This lets us all discuss the design and proposed implementation of your change, which helps ensure your time is well spent and that your contribution will be accepted. -If opening a PR, include the proposed commit message in the PR description. +Looking for a good place to start contributing? The issue tracker is always the first place to go. Issues are triaged to categorize them: -The commit message type should be one of: +- Check out some [`good first issue`s](https://github.com/cosmos/ibc-go/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22). These are issues whose scope of work should be pretty clearly specified and they are best suited for developers new to ibc-go (i.e. no deep knowledge of Cosmos SDK or ibc-go is required). For example, some of these issues may involve improving the logging, emitting new events or removing unused code. +- Or pick up a [`help wanted`](https://github.com/cosmos/ibc-go/issues?q=is%3Aopen+is%3Aissue+label%3A%22help+wanted%22) issue. These issues should be a bit more involved than the good first issues and the developer working on them would benefit from some familiarity already with the codebase. These types of issues may involve adding new (or extending the functionality of existing) gRPC endpoints, bumping the version of Cosmos SDK or Tendermint or fixing bugs. -* `feat` / `feature` for feature work. -* `bug` / `fix` for bug fixes. -* `imp` / `improvements` for improvements. -* `doc` / `docs` / `documentation` for any documentation changes. -* `test` / `e2e` for addition or improvements of unit, integration and e2e tests or their corresponding infrastructure. -* `deprecated` for deprecation changes. -* `deps` / `build` for changes to dependencies. -* `chore` / `misc` / `nit` for any miscellaneous changes that don't fit into another category. +If you would like to contribute, follow this process: -**Note**: If any change is breaking, the following format must be used: -* `type` + `(api)!` for api breaking changes, e.g. `fix(api)!: api breaking fix` -* `type` + `(statemachine)!` for state machine breaking changes, e.g. `fix(statemachine)!: state machine breaking fix` +1. If the issue is a proposal, ensure that the proposal has been accepted. +2. Ensure that nobody else has already begun working on this issue. If they have, make sure to contact them to collaborate. +3. If nobody has been assigned for the issue and you would like to work on it, comment on the issue to inform the community of your intentions to begin work. Then we will be able to assign the issue to you, making it visible for others that this issue is being tackled. If you end up not creating a pull request for this issue, please comment on the issue as well, so that it can be assigned to somebody else. +4. Follow standard GitHub best practices: fork the repo, branch from the HEAD of `main`, make some commits, and submit a PR to `main`. For core developers working within the ibc-go repo, branches must be named with the convention `{moniker}/{issue#}-branch-name` to ensure a clear ownership of branches. +5. Feel free to submit the pull request in `Draft` mode, even if the work is not complete, as this indicates to the community you are working on something and allows them to provide comments early in the development process. +6. When the code is complete it can be marked `Ready for Review`. +7. Be sure to include a relevant changelog entry in the [Commit Message / Changelog Entry section of pull request description](https://github.com/cosmos/ibc-go/blob/main/.github/PULL_REQUEST_TEMPLATE.md#commit-message--changelog-entry) so that we can add changelog entry when merging the pull request. Please follow the [Conventional Commits specification](https://www.conventionalcommits.org/en/v1.0.0/) and use one of the commit types mentioned in the [Commit messages section of the pull request guidelines](./docs/dev/pull-requests.md#commit-messages). -**`api` breaking changes take precedence over `statemachine` breaking changes.** +Please make sure to check out our [Pull request guidelines](./docs/dev/pull-requests.md) for more information. -### Updating Documentation +## Relevant development docs -If you open a PR on ibc-go, it is mandatory to update the relevant documentation in /docs. - -## Forking - -Please note that Go requires code to live under absolute paths, which complicates forking. -While my fork lives at `https://github.com/colin-axner/ibc-go`, -the code should never exist at `$GOPATH/src/github.com/colin-axner/ibc-go`. -Instead, we use `git remote` to add the fork as a new remote for the original repo, -`$GOPATH/src/github.com/cosmos/ibc-go`, and do all the work there. - -For instance, to create a fork and work on a branch of it, I would: - -- Create the fork on github, using the fork button. -- Go to the original repo checked out locally (i.e. `$GOPATH/src/github.com/cosmos/ibc-go`) -- `git remote add fork git@github.com:colin-axner/ibc-go.git` - -Now `fork` refers to my fork and `origin` refers to the ibc-go version. -So I can `git push -u fork main` to update my fork, and make pull requests to ibc-go from there. -Of course, replace `colin-axner` with your git handle. - -To pull in updates from the origin repo, run - -- `git fetch origin` -- `git rebase origin/main` (or whatever branch you want) - -Please don't make Pull Requests from `main`. - -## Dependencies - -We use [Go 1.14 Modules](https://github.com/golang/go/wiki/Modules) to manage -dependency versions. - -The main branch of every Cosmos repository should just build with `go get`, -which means they should be kept up-to-date with their dependencies, so we can -get away with telling people they can just `go get` our software. - -Since some dependencies are not under our control, a third party may break our -build, in which case we can fall back on `go mod tidy -v`. - -## Protobuf - -We use [Protocol Buffers](https://developers.google.com/protocol-buffers) along with [gogoproto](https://github.com/gogo/protobuf) to generate code for use in ibc-go. - -For determinstic behavior around Protobuf tooling, everything is containerized using Docker. Make sure to have Docker installed on your machine, or head to [Docker's website](https://docs.docker.com/get-docker/) to install it. - -For formatting code in `.proto` files, you can run `make proto-format` command. - -For linting and checking breaking changes, we use [buf](https://buf.build/). You can use the commands `make proto-lint` and `make proto-check-breaking` to respectively lint your proto files and check for breaking changes. - -To generate the protobuf stubs, you can run `make proto-gen`. - -We also added the `make proto-all` command to run all the above commands sequentially. - -In order for imports to properly compile in your IDE, you may need to manually set your protobuf path in your IDE's workspace settings/config. - -For example, in vscode your `.vscode/settings.json` should look like: - -``` -{ - "protoc": { - "options": [ - "--proto_path=${workspaceRoot}/proto", - "--proto_path=${workspaceRoot}/third_party/proto" - ] - } -} -``` - -## Testing - -All go tests in ibc-go can be ran by running `make test`. - -When testing a function under a variety of different inputs, we prefer to use -[table driven tests](https://github.com/golang/go/wiki/TableDrivenTests). - -All tests should use the testing package. Please see the testing package [README](./testing/README.md) for more information. - - -## Branching Model and Release - -User-facing repos should adhere to the trunk based development branching model: https://trunkbaseddevelopment.com/. - -ibc-go utilizes [semantic versioning](https://semver.org/). - -### PR Targeting - -Ensure that you base and target your PR on the `main` branch. - -All development should be targeted against `main`. Bug fixes which are required for outstanding releases should be backported if the CODEOWNERS decide it is applicable. - -### Development Procedure - -- the latest state of development is on `main` -- `main` must never fail `make test` -- no `--force` onto `main` (except when reverting a broken commit, which should seldom happen) -- create a development branch either on github.com/cosmos/ibc-go, or your fork (using `git remote add fork`) -- before submitting a pull request, begin `git rebase` on top of `main` - -### Pull Merge Procedure - -- ensure all github requirements pass -- squash and merge pull request - -### Release Procedure - -- Start on `main` -- Create the release candidate branch `rc/v*` (going forward known as **RC**) - and ensure it's protected against pushing from anyone except the release - manager/coordinator - - **no PRs targeting this branch should be merged unless exceptional circumstances arise** -- On the `RC` branch, prepare a new version section in the `CHANGELOG.md` - - All links must be link-ified: `$ python ./scripts/linkify_changelog.py CHANGELOG.md` -- Run external relayer tests against the prepared RC -- If errors are found during the relayer testing, commit the fixes to `main` - and create a new `RC` branch (making sure to increment the `rcN`) -- After relayer testing has successfully completed, create the release branch - (`release/vX.XX.X`) from the `RC` branch -- Create a PR to `main` to incorporate the `CHANGELOG.md` updates -- Tag the release (use `git tag -a`) and create a release in Github -- Delete the `RC` branches - -### Point Release Procedure - -At the moment, only a single major release will be supported, so all point releases will be based -off of that release. - -In order to alleviate the burden for a single person to have to cherry-pick and handle merge conflicts -of all desired backporting PRs to a point release, we instead maintain a living backport branch, where -all desired features and bug fixes are merged into as separate PRs. - -Example: - -Current release is `v1.0.2`. We then maintain a (living) branch `release/v1.0.x`, given x as -the next patch release number (currently `1.0.3`) for the `1.0` release series. As bugs are fixed -and PRs are merged into `main`, if a contributor wishes the PR to be released into the -`v1.0.x` point release, the contributor must: - -1. Add `1.0.x-backport` label -2. Pull latest changes on the desired `release/v1.0.x` branch -3. Create a 2nd PR merging the respective PR into `release/v1.0.x` -4. Update the PR's description and ensure it contains the following information: - - **[Impact]** Explanation of how the bug affects users or developers. - - **[Test Case]** section with detailed instructions on how to reproduce the bug. - - **[Regression Potential]** section with a discussion how regressions are most likely to manifest, or might - manifest even if it's unlikely, as a result of the change. **It is assumed that any backport PR is - well-tested before it is merged in and has an overall low risk of regression**. This section should discuss - the potential for state breaking changes to occur such as through out-of-gas errors. - -It is the PR's author's responsibility to fix merge conflicts, update changelog entries, and -ensure CI passes. If a PR originates from an external contributor, it may be a core team member's -responsibility to perform this process instead of the original author. -Lastly, it is core team's responsibility to ensure that the PR meets all the backport criteria. - -Finally, when a point release is ready to be made: - -1. Checkout `release/v1.0.x` branch -2. Ensure changelog entries are verified -3. Add release version date to the changelog -4. Push release branch along with the annotated tag: **git tag -a** -5. Create a PR into `main` containing ONLY `CHANGELOG.md` updates - -Note, although we aim to support only a single release at a time, the process stated above could be -used for multiple previous versions. +- [Project structure](./docs/dev/project-structure.md) +- [Develoment setup](./docs/dev/development-setup.md) +- [Go style guide](./docs/dev/go-style-guide.md) +- [Documentation guidelines](./docs/DOCS_GUIDELINES.md) +- [Writing tests](./testing/README.md) +- [Pull request guidelines](./docs/dev/pull-requests.md) +- [Release process](./docs/dev/release-management.md) \ No newline at end of file diff --git a/README.md b/README.md index 34adacf6484..51de14ad8a7 100644 --- a/README.md +++ b/README.md @@ -84,9 +84,23 @@ For questions and support please use the `developers` channel in the [Cosmos Net To receive announcements of new releases or other technical updates, please join the [Telegram group that we administer](https://t.me/ibc_is_expansive). -## Contribution Guidelines & Requests for Maintainership +## Contributing -We have detailed documents for contributors wishing to [contribute code to ibc-go](https://github.com/cosmos/ibc-go/blob/main/CONTRIBUTING.md) or [submit a feature for maintainership](./MAINTAINERSHIP.md) in the `ibc-go` codebase. Please note that all maintainers of and contributors to the codebase are subject to the [Code of Conduct](https://github.com/cosmos/ibc-go/blob/main/CODE_OF_CONDUCT.md). +If you're interested in contributing to ibc-go, please take a look at the [contributing guidelines](./CONTRIBUTING.md). We welcome and appreciate community contributions! + +This project adheres to ibc-go's [code of conduct](./CODE_OF_CONDUCT.md). By participating, you are expected to uphold this code. + +To help contributors understand which issues are good to pick up, we have the following two categories: +- Issues with the label [`good first issue`](https://github.com/cosmos/ibc-go/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22) should be pretty well defined and are best suited for developers new to ibc-go. +- Issues with the label [`help wanted`](https://github.com/cosmos/ibc-go/issues?q=is%3Aopen+is%3Aissue+label%3A%22help+wanted%22) are a bit more involved and they usually require some familiarity already with the codebase. + +If you are interested in working on an issue, please comment on it; then we will be able to assign it to you. We will be happy to answer any questions you may have and help you out while you work on the issue. + +If you have any general questions or feedback, please reach out to us in the [IBC Gang Discord server](https://discord.com/channels/955868717269516318/955883113484013578). + +## Request for maintainership + +We have a document that describes the process for [submitting a feature for maintainership](./MAINTAINERSHIP.md) in the ibc-go codebase. ## Security diff --git a/docs/DOCS_GUIDELINES.md b/docs/DOCS_GUIDELINES.md new file mode 100644 index 00000000000..f22c8804468 --- /dev/null +++ b/docs/DOCS_GUIDELINES.md @@ -0,0 +1,40 @@ +# Documentation guidelines + +## Best practices + +- Check the spelling and grammar, even if you have to copy and paste from an external source. +- Use simple sentences. Easy-to-read sentences mean the reader can quickly use the guidance you share. +- Try to express your thoughts in a concise and clean way. +- Either Leave a space or use a `-` between the acronyms ADR and ICS and the corresponding number (e.g. ADR 008 or ADR-008, and ICS 27 or ICS-27). +- Don't overuse `code` format when writing in plain English. +- Follow Google developer documentation [style guide](https://developers.google.com/style). +- Check the meaning of words in Microsoft's [A-Z word list and term collections](https://docs.microsoft.com/en-us/style-guide/a-z-word-list-term-collections/term-collections/accessibility-terms) (use the search input!). +- We recommend using RFC keywords in user documentation (lowercase). The RFC keywords are: "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL. They are to be interpreted as described in [RFC 2119](https://datatracker.ietf.org/doc/html/rfc2119). + +### Links + +**NOTE:** Strongly consider the existing links (both within this directory and to the website docs) when moving or deleting files. + +Relative links should be used nearly everywhere, due to versioning. Note that in case of page reshuffling, you must update all links references. + +### Code snippets + +Code snippets can be included in the documentation using normal Markdown code blocks. For example: + +```md + ```go + func() {} + ``` +``` + +It is also possible to include code snippets from GitHub files by referencing the files directly (and the line numbers if needed). For example: + +```md + ```go reference + https://github.com/cosmos/ibc-go/blob/v5.0.0/modules/core/04-channel/keeper/handshake.go#L18-L65 + ``` +``` + +## Technical writing course + +Google provides a free [course](https://developers.google.com/tech-writing/overview) for technical writing. diff --git a/docs/dev/development-setup.md b/docs/dev/development-setup.md new file mode 100644 index 00000000000..e0df0d23953 --- /dev/null +++ b/docs/dev/development-setup.md @@ -0,0 +1,60 @@ +# Development setup + +## Dependencies + +We use [Go 1.14 Modules](https://github.com/golang/go/wiki/Modules) to manage dependency versions. + +The main branch of every Cosmos repository should just build with `go get`, which means they should be kept up-to-date with their dependencies, so we can get away with telling people they can just `go get` our software. + +Since some dependencies are not under our control, a third party may break our build, in which case we can fall back on `go mod tidy -v`. + +Other helpful commands: + +- `go get` to add a new go module (including if the existing go module is being semantic version bumped, i.e. my/module/v1 -> my/module/v2). +- `go get -u` to update an existing dependency. +- `go mod tidy` to update dependencies in `go.sum`. + +## Protobuf + +We use [Protocol Buffers](https://developers.google.com/protocol-buffers) along with [buf](https://docs.buf.build/introduction) and [gogoproto](https://github.com/gogo/protobuf) to generate code for use in ibc-go. + +For determinstic behavior around protobuf tooling, everything is containerized using Docker. Make sure to have Docker installed on your machine, or head to [Docker's website](https://docs.docker.com/get-docker/) to install it. + +For formatting code in `.proto` files, you can run the `make proto-format` command. + +For linting and checking breaking changes, we also use [buf](https://buf.build/). You can use the commands `make proto-lint` and `make proto-check-breaking` to respectively lint your proto files and check for breaking changes. + +To generate the protobuf stubs, you can run `make proto-gen`. + +We also added the `make proto-all` command to run the above commands (`proto-format`, `proto-lint` and `proto-gen`) sequentially. + +To update third-party protobuf dependencies, you can run `make proto-update-deps`. This requires `buf` to be installed in the local development environment (see [`buf`s installation documentation](https://docs.buf.build/installation) for more details). + +For generating or updating the swagger file that documents the URLs of the RESTful API that exposes the gRPC endpoints over HTTP, you can run the `proto-swagger-gen` command. + +It reads protobuf service definitions and generates a reverse-proxy server which translates a RESTful HTTP API into gRPC. + +## Developing and testing + +- The latest state of development is on `main`. +- Build the `simd` test chain binary with `make build`. +- `main` must never fail `make test`. +- No `--force` onto `main` (except when reverting a broken commit, which should seldom happen). +- Create a development branch either on `github.com/cosmos/ibc-go`, or your fork (using `git remote add fork`). +- Before submitting a pull request, begin `git rebase` on top of `main`. + +All Go tests in ibc-go can be ran by running `make test`. + +Please make sure to run `make format` before every commit - the easiest way to do this is have your editor run it for you upon saving a file. Additionally please ensure that your code is lint compliant by running `make lint-fix` (requires `golangci-lint`). + +When testing a function under a variety of different inputs, we prefer to use [table driven tests](https://github.com/golang/go/wiki/TableDrivenTests). + +All unit tests should use the testing package. Please see the testing package [README](../../testing/README.md) for more information. + +Test coverage is continuously deployed at https://app.codecov.io/github/cosmos/ibc-go. PRs that improve test coverage are welcome, but in general the test coverage should be used as a guidance for finding API use cases that are not covered by tests. We don't recommend adding tests that only improve coverage but not actually test a meaning use case. + +## Documentation + +- If you open a PR on ibc-go, it is mandatory to update the relevant documentation in `/docs`. +- Generate the folder `docs/.vuepress/dist` with all the static files for the documentation site with `make build-docs`. +- Run the documentation site locally with `make view-docs`. diff --git a/docs/dev/go-style-guide.md b/docs/dev/go-style-guide.md new file mode 100644 index 00000000000..f481c4bcd32 --- /dev/null +++ b/docs/dev/go-style-guide.md @@ -0,0 +1,115 @@ + +# Go style guide + +In order to keep our code looking good with lots of programmers working on it, it helps to have a "style guide", so all the code generally looks quite similar. This doesn't mean there is only one "right way" to write code, or even that this standard is better than your style. But if we agree to a number of stylistic practices, it makes it much easier to read and modify new code. Please feel free to make suggestions if there's something you would like to add or modify. + +We expect all contributors to be familiar with [Effective Go](https://golang.org/doc/effective_go.html) (and it's recommended reading for all Go programmers anyways). Additionally, we generally agree with the suggestions in [Uber's style guide](https://github.com/uber-go/guide/blob/master/style.md) and use that as a starting point. + +## Code Structure + +Perhaps more key for code readability than good commenting is having the right structure. As a rule of thumb, try to write in a logical order of importance, taking a little time to think how to order and divide the code such that someone could scroll down and understand the functionality of it just as well as you do. A loose example of such order would be: + +- Constants, global and package-level variables. +- Main struct definition. +- Options (only if they are seen as critical to the struct else they should be placed in another file). +- Initialization/start and stop of the service functions. +- Public functions (in order of most important). +- Private/helper functions. +- Auxiliary structs and function (can also be above private functions or in a separate file). + +## General + +- Use `gofumpt` to format all code upon saving it (or run `make format`). +- Think about documentation, and try to leave godoc comments, when it will help new developers. +- Every package should have a high level doc.go file to describe the purpose of that package, its main functions, and any other relevant information. +- Applications (e.g. clis/servers) should panic on unexpected unrecoverable errors and print a stack trace. + +## Comments + +- Use a space after the comment deliminter (ex. `// your comment`). +- Many comments are not sentences. These should begin with a lower case letter and end without a period. +- Conversely, sentences in comments should be sentenced-cased and end with a period. +- Comments should explain _why_ something is being done rather than _what_ the code is doing. For example: + + The comments in + + ``` + // assign a variable foo + f := foo + // assign f to b + b := f + ``` + + have little value, but the following is more useful: + + ``` + f := foo + // we copy the variable f because we want to preserve the state at time of initialization + b := f + ``` + +## Linting + +- Run `make lint-fix` to fix any linting errors. + +## Various + +- Functions that return functions should have the suffix `Fn`. +- Names should not [stutter](https://blog.golang.org/package-names). For example, a struct generally shouldn’t have a field named after itself; e.g., this shouldn't occur: + + ``` golang + type middleware struct { + middleware Middleware + } + ``` + +- Acronyms are all capitalized, like "RPC", "gRPC", "API". "MyID", rather than "MyId". +- Whenever it is safe to use Go's built-in `error` instantiation functions (as opposed to Cosmos SDK's error instantiation functions), prefer `errors.New()` instead of `fmt.Errorf()` unless you're actually using the format feature with arguments. + +## Importing libraries + +- Use [goimports](https://godoc.org/golang.org/x/tools/cmd/goimports). +- Separate imports into blocks: one for the standard lib, one for external libs and one for application libs. For example: + + ```golang + import ( + // standard library imports + "fmt" + "testing" + + // external library imports + "github.com/stretchr/testify/require" + abci "github.com/tendermint/tendermint/abci/types" + + // ibc-go library imports + "github.com/cosmos/ibc-go/modules/core/23-commitment/types" + ) + ``` + +## Dependencies + +- Dependencies should be pinned by a release tag, or specific commit, to avoid breaking `go get` when external dependencies are updated. +- Refer to the [contributing](./development-setup.md#dependencies) document for more details. + +## Testing + +- Make use of table driven testing where possible and not-cumbersome. Read [this blog post](https://dave.cheney.net/2013/06/09/writing-table-driven-tests-in-go) for more information. See the [tests](https://github.com/cosmos/ibc-go/blob/f24f41ea8a61fe87f6becab94e84de08c8aa9381/modules/apps/transfer/keeper/msg_server_test.go#L11) for [`Transfer`](https://github.com/cosmos/ibc-go/blob/f24f41ea8a61fe87f6becab94e84de08c8aa9381/modules/apps/transfer/keeper/msg_server.go#L15) for an example. +- Make use of Testify [assert](https://godoc.org/github.com/stretchr/testify/assert) and [require](https://godoc.org/github.com/stretchr/testify/require). +- When using mocks, it is recommended to use Testify [mock](https://pkg.go.dev/github.com/stretchr/testify/mock) along with [Mockery](https://github.com/vektra/mockery) for autogeneration. + +## Errors + +- Ensure that errors are concise, clear and traceable. +- Depending on the context, use either `cosmossdk.io/errors` or `stdlib` error packages. +- For wrapping errors, use `fmt.Errorf()` with `%w`. +- Panic is appropriate when an internal invariant of a system is broken, while all other cases (in particular, incorrect or invalid usage) should return errors. +- Error messages should be formatted as following: + + ```golang + sdkerrors.Wrapf( + , + "expected %s, got %s", + , + + ) + ``` \ No newline at end of file diff --git a/docs/dev/project-structure.md b/docs/dev/project-structure.md new file mode 100644 index 00000000000..c207b927079 --- /dev/null +++ b/docs/dev/project-structure.md @@ -0,0 +1,46 @@ +# Project structure + +If you're not familiar with the overall module structure from the SDK modules, please check this [document](https://github.com/cosmos/cosmos-sdk/blob/main/docs/docs/building-modules/10-structure.md) as prerequisite reading. + +Every Interchain Standard (ICS) has been developed in its own package. The development team separated the IBC TAO (Transport, Authentication, Ordering) ICS specifications from the IBC application level specification. The following sections describe the architecture of the most relevant directories that comprise this repository. + +## `modules` + +This folder contains implementations for the IBC TAO (`core`), IBC applications (`apps`) and light clients (`light-clients`). + +### `core` + +- `02-client`: This package is an implementation for Cosmos SDK-based chains of [ICS 02](https://github.com/cosmos/ibc/tree/main/spec/core/ics-002-client-semantics). This implementation defines the types and methods needed to operate light clients tracking other chain's consensus state. +- `03-connection`: This package is an implementation for Cosmos SDK-based chains of [ICS 03](https://github.com/cosmos/ibc/tree/main/spec/core/ics-003-connection-semantics). This implementation defines the types and methods necessary to perform connection handshake between two chains. +- `04-channel`: This package is an implementation for Cosmos SDK-based chains of [ICS 04](https://github.com/cosmos/ibc/tree/main/spec/core/ics-004-channel-and-packet-semantics). This implementation defines the types and methods necessary to perform channel handshake between two chains and ensure correct packet sending flow. +- `05-port`: This package is an implementation for Cosmos SDK-based chains of [ICS 05](https://github.com/cosmos/ibc/tree/main/spec/core/ics-005-port-allocation). This implements the port allocation system by which modules can bind to uniquely named ports. +- `23-commitment`: This package is an implementation for Cosmos SDK-based chains of [ICS 23](https://github.com/cosmos/ibc/tree/main/spec/core/ics-023-vector-commitments). This implementation defines the functions required to prove inclusion or non-inclusion of particular values at particular paths in state. +- `24-host`: This package is an implementation for Cosmos SDK-based chains of [ICS 24](https://github.com/cosmos/ibc/tree/main/spec/core/ics-024-host-requirements). + +### `apps` + +- `transfer`: This is the Cosmos SDK implementation of the [ICS 20](https://github.com/cosmos/ibc/tree/main/spec/app/ics-020-fungible-token-transfer) protocol, which enables cross-chain fungible token transfers. For more information, read the [module's docs](../apps/transfer/overview.md) +- `27-interchain-accounts`: This is the Cosmos SDK implementation of the [ICS 27](https://github.com/cosmos/ibc/tree/main/spec/app/ics-027-interchain-accounts) protocol, which enables cross-chain account management built upon IBC. For more information, read the [module's documentation](../apps/interchain-accounts/overview.md). +- `29-fee`: This is the Cosmos SDK implementation of the [ICS 29](https://github.com/cosmos/ibc/tree/main/spec/app/ics-029-fee-payment) middleware, which handles packet incentivisation and fee distribution on top of any ICS application protocol, enabling fee payment to relayer operators. For more information, read the [module's documentation](../middleware/ics29-fee/overview.md). + +### `light-clients` + +- `06-solomachine`: This package implement the types for the Solo Machine light client specified in [ICS 06](https://github.com/cosmos/ibc/tree/main/spec/client/ics-006-solo-machine-client). +- `07-tendermint`: This package implement the types for the Tendermint consensus light client as specified in [ICS 07](https://github.com/cosmos/ibc/tree/main/spec/client/ics-007-tendermint-client). + +## `proto` + +This folder contains all the Protobuf files used for + +- common message type definitions, +- message type definitions related to genesis state, +- `Query` service and related message type definitions, +- `Msg` service and related message type definitions. + +## `testing` + +This package contains the implementation of the testing package used in unit and integration tests. Please read the [package's documentation](../../testing/README.md) for more information. + +## `e2e` + +This folder contains all the e2e tests of ibc-go. Please read the [module's documentation](../../e2e/README.md) for more information. diff --git a/docs/dev/pull-requests.md b/docs/dev/pull-requests.md new file mode 100644 index 00000000000..53a5bc0811e --- /dev/null +++ b/docs/dev/pull-requests.md @@ -0,0 +1,67 @@ +# Pull request guidelines + +> To accommodate the review process we suggest that PRs are categorically broken up. Ideally each PR addresses only a single issue and does not introduce unrelated changes. Additionally, as much as possible code refactoring and cleanup should be submitted as separate PRs from bug fixes and feature additions. + +If the PR is the result of a related GitHub issue, please include `closes: #` in the PR’s description in order to auto-close the related issue once the PR is merged. This will also link the issue and the PR together so that if anyone looks at either in the future, they won’t have any problem trying to find the corresponding issue/PR as it will be recorded in the sidebar. + +If the PR is not the result of an existing issue and it fixes a bug, please provide a detailed description of the bug. For feature addtions, we recommend opening an issue first and have it discussed and agreed upon, before working on it and opening a PR. + +If possible, [tick the "Allow edits from maintainers" box](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork) when opening your PR from your fork of ibc-go. This allows us to directly make minor edits / refactors and speeds up the merging process. + +If you open a PR on ibc-go, it is mandatory to update the relevant documentation in `/docs`. + +## Pull request targeting + +Ensure that you base and target your PR on the either the `main` branch or the corresponding feature branch where a large body of work is being implemented. Please make sure that the PR is made from a branch different than either `main` or the corresponding feature branch. + +All development should be then targeted against `main` or the feature branch. Bug fixes which are required for outstanding releases should be backported if the CODEOWNERS decide it is applicable. + +## Commit Messages + +Commit messages should follow the [Conventional Commits specification](https://www.conventionalcommits.org/en/v1.0.0/). + +When opening a PR, include the proposed commit message in the PR description. + +The commit message type should be one of: + +- `feat` / `feature` for feature work. +- `bug` / `fix` for bug fixes. +- `imp` / `improvements` for improvements. +- `doc` / `docs` / `documentation` for any documentation changes. +- `test` / `e2e` for addition or improvements of unit, integration and e2e tests or their corresponding infrastructure. +- `deprecated` for deprecation changes. +- `deps` / `build` for changes to dependencies. +- `chore` / `misc` / `nit` for any miscellaneous changes that don't fit into another category. + +**Note**: If any change is breaking, the following format must be used: + +- `type` + `(api)!` for api breaking changes, e.g. `fix(api)!: api breaking fix` +- `type` + `(statemachine)!` for state machine breaking changes, e.g. `fix(statemachine)!: state machine breaking fix` + +**`api` breaking changes take precedence over `statemachine` breaking changes.** + +## Pull request review process + +All PRs require an approval from at least one CODEOWNER before merge. PRs which cause significant changes require two approvals from CODEOWNERS. When reviewing PRs please use the following review guidelines: + +- `Approval` through the GitHub UI with the following comments: + - `Concept ACK` means that you agree with the overall proposed concept, but have neither reviewed the code nor tested it. + - `LGTM` means the above and besides you have superficially reviewed the code without considering how logic affects other parts the codebase. + - `utACK` (aka. `Untested ACK`) means the above and besides have thoroughly reviewed the code and considered the safety of logic changes, but have not tested it. + - `Tested ACK` means the above and besides you have tested the code. +- If you are only making "surface level" reviews, submit any notes as `Comments` without submitting an approval. + +A thorough review means that: +- You understand the code and make sure that documentation is updated in the right places. +- You must also think through anything which ought to be included but is not. +- You must think through whether any added code could be partially combined (DRYed) with existing code. +- You must think through any potential security issues or incentive-compatibility flaws introduced by the changes. +- Naming must be consistent with conventions and the rest of the codebase. +- Code must live in a reasonable location, considering dependency structures (e.g. not importing testing modules in production code, or including example code modules in production code). + +## Pull request merge procedure + +- Ensure pull request branch is rebased on target branch. +- Ensure all GitHub requirements pass. +- Set the changelog entry in the commit message for the pull request. +- Squash and merge pull request. diff --git a/docs/dev/release-management.md b/docs/dev/release-management.md new file mode 100644 index 00000000000..8f1f36e082c --- /dev/null +++ b/docs/dev/release-management.md @@ -0,0 +1,78 @@ +# Tagging a release + +## New major release branch + +Pre-requisites for creating a release branch for a new major version: + +1. Bump [Go package version](https://github.com/cosmos/ibc-go/blob/main/go.mod#L3). +2. Change all imports. For example: if the next major version is `v3`, then change all imports starting with `github.com/cosmos/ibc-go/v2` to `github.com/cosmos/ibc-go/v3`). + +Once the above pre-requisites are satified: + +1. Start on `main`. +2. Create the release branch (`release/vX.XX.X`). For example: `release/v3.0.x`. + +## New minor release branch + +1. Start on the latest release branch in the same major release line. For example: the latest release branch in the `v3` release line is `v3.2.x`. +2. Create branch from the release branch. For example: create branch `release/v3.3.x` from `v3.2.x`. + +Post-requisites for both new major and minor release branches: + +1. Add branch protection rules to new release branch. +2. Add backport task to [`mergify.yml`](https://github.com/cosmos/ibc-go/blob/main/.github/mergify.yml). +3. Create label for backport (e.g.`backport-to-v3.0.x`). + +## Point release procedure + +In order to alleviate the burden for a single person to have to cherry-pick and handle merge conflicts of all desired backporting PRs to a point release, we instead maintain a living backport branch, where all desired features and bug fixes are merged into as separate PRs. + +### Example + +Current release is `v1.0.2`. We then maintain a (living) branch `release/v1.0.x`, given `x` as the next patch release number (currently `v1.0.3`) for the `v1.0` release series. As bugs are fixed and PRs are merged into `main`, if a contributor wishes the PR to be released into the `v1.0.x` point release, the contributor must: + +1. Add the `backport-to-v1.0x` label to the PR. +2. Once the PR is merged, the Mergify GitHub application will automatically copy the changes into another branch and open a new PR agains the desired `release/v1.0.x` branch. +3. If the following has not been discussed in the original PR, then update the backport PR's description and ensure it contains the following information: + - **[Impact]** explanation of how the bug affects users or developers. + - **[Test Case]** section with detailed instructions on how to reproduce the bug. + - **[Regression Potential]** section with a discussion how regressions are most likely to manifest, or might manifest even if it's unlikely, as a result of the change. **It is assumed that any backport PR is well-tested before it is merged in and has an overall low risk of regression**. This section should discuss the potential for state breaking changes to occur such as through out-of-gas errors. + +It is the PR's author's responsibility to fix merge conflicts, update changelog entries, and ensure CI passes. If a PR originates from an external contributor, it may be a core team member's responsibility to perform this process instead of the original author. Lastly, it is core team's responsibility to ensure that the PR meets all the backport criteria. + +Finally, when a point release is ready to be made: + +1. Checkout the release branch (e.g. `release/v1.0.x`). +2. In `CHANGELOG.md`: + - Ensure changelog entries are verified. + - Remove any sections of the changelog that do not have any entries (e.g. if the release does not have any bug fixes, then remove the section). + - Remove the `[Unreleased]` title. + - Add release version and date of release. +3. Create release in GitHub: + - Select the correct target branch (e.g. `release/v1.0.x`). + - Choose a tag (e.g. `v1.0.3`). + - Write release notes. + - Check the `This is a pre-release` checkbox if needed (this applies for alpha, beta and release candidates). + +### Post-release procedure + +- Update [`CHANGELOG.md`](../../CHANGELOG.md) in `main` (remove from the `[Unreleased]` section any items that are part of the release).` +- Put back the `[Unreleased]` section in the release branch (e.g. `release/v1.0.x`) with clean sections for each of the types of changelog entries, so that entries will be added for the PRs that are backported for the next release. +- Update [version matrix](../../RELEASES.md#version-matrix) in `RELEASES.md`: add the new release and remove any tags that might not be recommended anymore. + +Additionally, for the first point release of a new major or minor release branch: + +- Update the table of supported release lines (and End of Life dates) in [`RELEASES.md`](../../RELEASES.md): add the new release line and remove any release lines that might have become discontinued. +- Update the [list of supported release lines in README.md](../../RELEASES.md#releases), if necessary. +- Update the [e2e compatibility test matrices](https://github.com/cosmos/ibc-go/tree/main/.github/compatibility-test-matrices): add the tag for the new release and remove any tags that might not be recommended anymore. +- Update the manual [e2e `simd`](https://github.com/cosmos/ibc-go/blob/main/.github/workflows/e2e-manual-simd.yaml) and [e2e `icad`](https://github.com/cosmos/ibc-go/blob/main/.github/workflows/e2e-manual-icad.yaml) test workflows: + - Add the new release and the new `icad` tag. + - Remove any tags that might not be recommended anymore. +- Bump ibc-go version in [cosmos/interchain-accounts-demo repository](https://github.com/cosmos/interchain-accounts-demo) and create a tag. +- Open a PR to `main` updating the docs site: + - Add new release branch to [`docs/versions`](../versions) file. + - Add `label` and `key` to `versions` array in [`config.js`](https://github.com/cosmos/ibc-go/blob/main/docs/.vuepress/config.js#L33). +- After changes to docs site are deployed, check [ibc.cosmos.network](https://ibc.cosmos.network) is updated. +- Open issue in [SDK tutorials repo](https://github.com/cosmos/sdk-tutorials) to update tutorials to the released version of ibc-go. + +See [this PR](https://github.com/cosmos/ibc-go/pull/2919) for an example of the involved changes. \ No newline at end of file diff --git a/testing/README.md b/testing/README.md index 000245bd02d..b3f869d7855 100644 --- a/testing/README.md +++ b/testing/README.md @@ -74,7 +74,7 @@ func (app *SimApp) GetBaseApp() *baseapp.BaseApp { } // GetStakingKeeper implements the TestingApp interface. -func (app *SimApp) GetStakingKeeper() stakingkeeper.Keeper { +func (app *SimApp) GetStakingKeeper() ibctestingtypes.Keeper { return app.StakingKeeper }