Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: readme revamp - contributing #1999

Closed
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
270 changes: 40 additions & 230 deletions CONTRIBUTING.md

Large diffs are not rendered by default.

14 changes: 14 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,20 @@ In the table below
|Middleware enabling the recovery of tokens sent to unsupported addresses.|[recovery](https://github.com/evmos/evmos/tree/main/x/recovery)|`middleware`|
|Middleware that limits the in or out flow of an asset in a certain time period to minimise the risks of cross chain token transfers.|[IBC-rate-limiting](https://github.com/osmosis-labs/osmosis/pull/2339)|`middleware`|

## Contributing

If you're interested in contributing to ibc-go, please take a look at the [contributing guidelines](./CONTRIBUTING.md) and the style guide. We welcome and appreciate community contributions!
colin-axner marked this conversation as resolved.
Show resolved Hide resolved

This project adheres to ibc-go's [code of conduct](./CODE_OF_CONDUCT.md). By participating, you are expected to uphold this code.

To facilitate contributors the task of choosing what issues to pick up, we have the following two categories:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this wording a little hard to read. Maybe

To assist contributors in deciding which tasks to choose, we have added the following two issue labels:

Or something like that?

- 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- 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.
- 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 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.

Teams develpoing an app, middleware or light client that would like to get in touch with us, please reach out to us in the `interchain` channel of the [IBC Gang Discord server](https://discord.com/channels/955868717269516318/955883113484013578) and we can discuss further.

## Support

We have active, helpful communities on Discord and Telegram.
Expand Down
85 changes: 85 additions & 0 deletions docs/dev/development-setup.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
# Development setup

## 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)

## 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 the `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.

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.

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"
]
}
}
```

## 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`.

If you open a PR on ibc-go, it is mandatory to update the relevant documentation in `/docs`.

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`).

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.

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

- 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`.
74 changes: 74 additions & 0 deletions docs/dev/go-style-guide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@

### Go style guide
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took this from Tendermint and trimmed it down a bit. Feel free to comment on things that should and should not be here.


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
- 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
- Msgs/Events
- 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.

## 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
}
```

* In comments, use "iff" to mean, "if and only if".
* Acronyms are all capitalized, like "RPC", "gRPC", "API". "MyID", rather than "MyId".
* 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.

## 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.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.
- Make use of [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.
- Use stdlib errors package.
- 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.
46 changes: 46 additions & 0 deletions docs/dev/project-structure.md
Original file line number Diff line number Diff line change
@@ -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.
39 changes: 39 additions & 0 deletions docs/dev/pull-requests.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# 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: #<issue number>` in the PR’s description in order to auto-close the related issue once the PR is merged. This will also link the ticket and the PR together so that if anyone looks at either in the future, they won’t have any issue trying to find the corresponding ticket/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.

Commit messages must follow the [Conventional Commits specification](https://www.conventionalcommits.org/en/v1.0.0/). This will help us to eventually move to automatic generation of changelogs.

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 `main` branch. Please make sure that the PR is made from a branch different than `main`.

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.

## 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 explanations:

- `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 GitHub 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.

## Pull request merge procedure

- Ensure all GitHub requirements pass.
- Squash and merge pull request.
Loading