Merge branch 'main' into charly/remove_ref_allow_update

.github/ISSUE_TEMPLATE/epic-tracker.md

@@ -0,0 +1,64 @@
+---
+name: Epic Tracker
+about: Create an issue to track feature epic progress
+
+---
+
+<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺ 
+v                            ✰  Thanks for opening an issue! ✰    
+v    Before smashing the submit button please review the template.
+v    Word of caution: poorly thought-out proposals may be rejected 
+v                     without deliberation 
+☺ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >  -->
+
+## Requirements document
+
+<!-- Link to requirements document -->
+
+## IBC spec
+
+<!-- Link to specification -->
+
+## ADRs
+
+<!-- Links to ADRs related to this epic -->
+
+## Milestones
+
+<!-- Links to alpha, beta, RC milestones -->
+
+## Implementation issues
+
+<!-- Links to specific issues, thematically/logically grouped -->
+
+## QA scenarios
+
+<!-- Lists of manual QA tests that need be performed -->
+
+## Automated e2e tests
+
+<!-- List of automated e2e tests that need be added to CI -->
+
+## Pre-releases
+
+<!-- Links to alpha, beta, RC tags/releases -->
+
+## Checklist
+
+<!-- Remove any items that are not applicable. -->
+
+- [ ] Internal audit(s)
+- [ ] External audit(s)
+- [ ] Documentation
+- [ ] Swagger
+- [ ] Integration with relayers:
+  - [ ] Hermes
+  - [ ] Rly
+
+____
+
+#### For Admin Use
+
+- [ ] Not duplicate issue
+- [ ] Appropriate labels applied
+- [ ] Appropriate contributors tagged/assigned

.github/ISSUE_TEMPLATE/release-tracker.md

@@ -0,0 +1,65 @@
+---
+name: Release tracker
+about: Create an issue to track release progress
+
+---
+
+<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺ 
+v                            ✰  Thanks for opening an issue! ✰    
+v    Before smashing the submit button please review the template.
+v    Word of caution: poorly thought-out proposals may be rejected 
+v                     without deliberation 
+☺ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >  -->
+
+## Milestones
+
+<!-- Links to alpha, beta, RC or final milestones -->
+
+### IBC spec compatibility
+
+<!-- Version of the IBC spec that this release is compatible with -->
+
+### QA
+
+#### Backwards compatibility
+
+<!-- List of tests that need be performed with previous
+versions of ibc-go to guarantee that no regression is introduced -->
+
+- [ ] Fungible token transfers over a non-incentivised channel works with a counterparty chain running each previous major version.
+- [ ] Fungible token transfers over an incentivised channel works with a counterparty chain running each previous major version that supports ICS-29 Fee Middleware.
+- [ ] Interchain Accounts over a non-incentivised channel works with a counterparty chain running each previous major version that supports ICS-27 Interchain Accounts over non-incentivised channels.
+- [ ] Interchain Accounts over an incentivised channel works with a counterparty chain running each previous major version that supports ICS-27 Interchain Accounts over incentivised channels.
+
+### Migration 
+
+<!-- Link to migration document -->
+
+## Checklist
+
+<!-- Remove any items that are not applicable. -->
+
+- [ ] Bump [go package version](https://github.com/cosmos/ibc-go/blob/main/go.mod#L3).
+- [ ] Change all imports starting with `github.com/cosmos/ibc-go/v{x}` to `github.com/cosmos/ibc-go/v{x+1}`.
+- [ ] Branch off main to create release branch in the form  of `release/vx.y.z`.
+- [ ] Add branch protection rules to new release branch.
+- [ ] Add backport task to [`mergify.yml`](https://github.com/cosmos/ibc-go/blob/main/.github/mergify.yml)
+- [ ] Upgrade ibc-go version in [ibctest](https://github.com/strangelove-ventures/ibctest).
+- [ ] Check Swagger is up-to-date.
+
+## Post-release checklist
+
+- [ ] Update [`CHANGELOG.md`](https://github.com/cosmos/ibc-go/blob/main/CHANGELOG.md)
+- [ ] Update the table of supported release lines (and End of Life dates) in [`RELEASES.md`](https://github.com/cosmos/ibc-go/blob/main/RELEASES.md).
+- [ ] Update docs site:
+  - [ ] Add new release branch to [`docs/versions`](https://github.com/cosmos/ibc-go/blob/main/docs/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.
+
+____
+
+#### For Admin Use
+
+- [ ] Not duplicate issue
+- [ ] Appropriate labels applied
+- [ ] Appropriate contributors tagged/assigned

.github/workflows/e2e-fork.yml

@@ -1,14 +1,16 @@
 name: Tests / E2E Fork
 on:
+  workflow_dispatch:
   pull_request:
     branches:
       - main
     paths-ignore:
       - docs/**
+
 jobs:
   # dynamically build a matrix of test/test suite pairs to run
   build-test-matrix:
-    if: ${{ github.event.pull_request.head.repo.fork }}
+    if: ${{ github.event.pull_request.head.repo.fork || github.actor == 'dependabot[bot]' || github.event_name == 'workflow_dispatch' }}
     runs-on: ubuntu-latest
     outputs:
       matrix: ${{ steps.set-matrix.outputs.matrix }}
@@ -24,7 +26,7 @@ jobs:
     env:
       SIMD_TAG: latest
       SIMD_IMAGE: ibc-go-simd-e2e
-    if: ${{ github.event.pull_request.head.repo.fork }}
+    if: ${{ github.event.pull_request.head.repo.fork || github.actor == 'dependabot[bot]' || github.event_name == 'workflow_dispatch' }}
     needs:
       - build-test-matrix
     runs-on: ubuntu-latest

.github/workflows/e2e.yaml

@@ -1,5 +1,6 @@
 name: Tests / E2E
 on:
+  workflow_dispatch:
   pull_request:
   push:
     branches:
@@ -13,7 +14,7 @@ env:
 
 jobs:
   docker-build:
-    if: ${{ !github.event.pull_request.head.repo.fork }}
+    if: ${{ !github.event.pull_request.head.repo.fork && github.actor != 'dependabot[bot]' }}
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v3
@@ -40,7 +41,7 @@ jobs:
 
   # dynamically build a matrix of test/test suite pairs to run
   build-test-matrix:
-    if: ${{ !github.event.pull_request.head.repo.fork }}
+    if: ${{ !github.event.pull_request.head.repo.fork && github.actor != 'dependabot[bot]' }}
     runs-on: ubuntu-latest
     outputs:
       matrix: ${{ steps.set-matrix.outputs.matrix }}
@@ -56,7 +57,7 @@ jobs:
   # the tag of the image will differ if this is a PR or the branch is being merged into main.
   # we store the tag as an environment variable and use it in the E2E tests to determine the tag.
   determine-image-tag:
-    if: ${{ !github.event.pull_request.head.repo.fork }}
+    if: ${{ !github.event.pull_request.head.repo.fork && github.actor != 'dependabot[bot]' }}
     runs-on: ubuntu-latest
     outputs:
       simd-tag: ${{ steps.get-tag.outputs.simd-tag }}
@@ -73,7 +74,7 @@ jobs:
 
 
   e2e:
-    if: ${{ !github.event.pull_request.head.repo.fork }}
+    if: ${{ !github.event.pull_request.head.repo.fork && github.actor != 'dependabot[bot]' }}
     runs-on: ubuntu-latest
     needs:
       - build-test-matrix

CHANGELOG.md

@@ -60,6 +60,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Improvements
 
+* (app/20-transfer) [\#1680](https://github.com/cosmos/ibc-go/pull/1680) Adds migration to correct any malformed trace path information of tokens with denoms that contains slashes. The transfer module consensus version has been bumped to 2.
+* (app/20-transfer) [\#1730](https://github.com/cosmos/ibc-go/pull/1730) parse the ics20 denomination provided via a packet using the channel identifier format specified by ibc-go.
 * (cleanup) [\#1335](https://github.com/cosmos/ibc-go/pull/1335/) `gofumpt -w -l .` to standardize the code layout more strictly than `go fmt ./...`
 * (middleware) [\#1022](https://github.com/cosmos/ibc-go/pull/1022) Add `GetAppVersion` to the ICS4Wrapper interface. This function should be used by IBC applications to obtain their own version since the version set in the channel structure may be wrapped many times by middleware. 
 * (modules/core/04-channel) [\#1232](https://github.com/cosmos/ibc-go/pull/1232) Updating params on `NewPacketId` and moving to bottom of file.
@@ -100,6 +102,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (transfer) [\#1414](https://github.com/cosmos/ibc-go/pull/1414) Emitting Sender address from `fungible_token_packet` events in `OnRecvPacket` and `OnAcknowledgementPacket`.
 * (modules/core/04-channel) [\#1464](https://github.com/cosmos/ibc-go/pull/1464) Emit a channel close event when an ordered channel is closed.
 * (modules/light-clients/07-tendermint) [\#1118](https://github.com/cosmos/ibc-go/pull/1118) Deprecating `AllowUpdateAfterExpiry` and `AllowUpdateAfterMisbehaviour`. See ADR-026 for context.
+* (modules/light-clients/07-tendermint) [\#1713](https://github.com/cosmos/ibc-go/pull/1713) Allow client upgrade proposals to update `TrustingPeriod`. See ADR-026 for context.
 
 ### Features
 

docs/architecture/adr-026-ibc-client-recovery-mechanisms.md

@@ -7,6 +7,7 @@
 - 2021/01/15: Revision to support substitute clients for unfreezing
 - 2021/05/20: Revision to simplify consensus state copying, remove initial height
 - 2022/04/08: Revision to deprecate AllowUpdateAfterExpiry and AllowUpdateAfterMisbehaviour
+- 2022/07/15: Revision to allow updating of TrustingPeriod
 
 ## Status
 
@@ -51,6 +52,9 @@ We elect not to deal with chains which have actually halted, which is necessaril
 
     Previously, `AllowUpdateAfterExpiry` and `AllowUpdateAfterMisbehaviour` were used to signal the recovery options for an expired or frozen client, and governance proposals were not allowed to overwrite the client if these parameters were set to false. However, this has now been deprecated because a code migration can overwrite the client and consensus states regardless of the value of these parameters. If governance would vote to overwrite a client or consensus state, it is likely that governance would also be willing to perform a code migration to do the same.
 
+    In addition, `TrustingPeriod` was initally not allowed to be updated by a client upgrade proposal. However, due to the number of situations experienced in production where the `TrustingPeriod` of a client should be allowed to be updated because of ie: initial misconfiguration for a canonical channel, governance should be allowed to update this client parameter. 
+
+    Note that this should NOT be lightly updated, as there may be a gap in time between when misbehaviour has occured and when the evidence of misbehaviour is submitted. For example, if the `UnbondingPeriod` is 2 weeks and the `TrustingPeriod` has also been set to two weeks, a validator could wait until right before `UnbondingPeriod` finishes, submit false information, then unbond and exit without being slashed for misbehaviour. Therefore, we recommend that the trusting period for the 07-tendermint client be set to 2/3 of the `UnbondingPeriod`.
 
 Note that clients frozen due to misbehaviour must wait for the evidence to expire to avoid becoming refrozen. 
 

docs/ibc/upgrades/developer-guide.md

@@ -36,7 +36,7 @@ Developers must ensure that the new client adopts all of the new Client paramete
 
 Upgrades must adhere to the IBC Security Model. IBC does not rely on the assumption of honest relayers for correctness. Thus users should not have to rely on relayers to maintain client correctness and security (though honest relayers must exist to maintain relayer liveness). While relayers may choose any set of client parameters while creating a new `ClientState`, this still holds under the security model since users can always choose a relayer-created client that suits their security and correctness needs or create a Client with their desired parameters if no such client exists.
 
-However, when upgrading an existing client, one must keep in mind that there are already many users who depend on this client's particular parameters. We cannot give the upgrading relayer free choice over these parameters once they have already been chosen. This would violate the security model since users who rely on the client would have to rely on the upgrading relayer to maintain the same level of security. Thus, developers must make sure that their upgrade mechanism allows clients to upgrade the chain-specified parameters whenever a chain upgrade changes these parameters (examples in the Tendermint client include `UnbondingPeriod`, `ChainID`, `UpgradePath`, etc.), while ensuring that the relayer submitting the `UpgradeClientMsg` cannot alter the client-chosen parameters that the users are relying upon (examples in Tendermint client include `TrustingPeriod`, `TrustLevel`, `MaxClockDrift`, etc).
+However, when upgrading an existing client, one must keep in mind that there are already many users who depend on this client's particular parameters. We cannot give the upgrading relayer free choice over these parameters once they have already been chosen. This would violate the security model since users who rely on the client would have to rely on the upgrading relayer to maintain the same level of security. Thus, developers must make sure that their upgrade mechanism allows clients to upgrade the chain-specified parameters whenever a chain upgrade changes these parameters (examples in the Tendermint client include `UnbondingPeriod`, `TrustingPeriod`, `ChainID`, `UpgradePath`, etc.), while ensuring that the relayer submitting the `UpgradeClientMsg` cannot alter the client-chosen parameters that the users are relying upon (examples in Tendermint client include `TrustLevel`, `MaxClockDrift`, etc).
 
 Developers should maintain the distinction between Client parameters that are uniform across every valid light client of a chain (chain-chosen parameters), and Client parameters that are customizable by each individual client (client-chosen parameters); since this distinction is necessary to implement the `ZeroCustomFields` method in the `ClientState` interface:
 

docs/migrations/support-denoms-with-slashes.md

@@ -9,7 +9,7 @@ There are four sections based on the four potential user groups of this document
 - Relayers
 - IBC Light Clients
 
-This document is necessary when chains are upgrading from a version that does not support base denoms with slashes (e.g. v3.0.0) to a version that does (e.g. v3.1.0). All versions of ibc-go smaller than v1.5.0 for the v1.x release line, v2.3.0 for the v2.x release line, and v3.1.0 for the v3.x release line do *NOT** support IBC token transfers of coins whose base denoms contain slashes. Therefore the in-place of genesis migration described in this document are required when upgrading.
+This document is necessary when chains are upgrading from a version that does not support base denoms with slashes (e.g. v3.0.0) to a version that does (e.g. v3.2.0). All versions of ibc-go smaller than v1.5.0 for the v1.x release line, v2.3.0 for the v2.x release line, and v3.1.0 for the v3.x release line do **NOT** support IBC token transfers of coins whose base denoms contain slashes. Therefore the in-place of genesis migration described in this document are required when upgrading.
 
 If a chain receives coins of a base denom with slashes before it upgrades to supporting it, the receive may pass however the trace information will be incorrect.
 
@@ -28,41 +28,17 @@ The transfer module will now support slashes in base denoms, so we must iterate
 ### Upgrade Proposal
 
 ```go
-// Here the upgrade name is the upgrade name set by the chain
-app.UpgradeKeeper.SetUpgradeHandler("supportSlashedDenomsUpgrade",
+app.UpgradeKeeper.SetUpgradeHandler("MigrateTraces",
     func(ctx sdk.Context, _ upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) {
-        // list of traces that must replace the old traces in store
-        var newTraces []ibctransfertypes.DenomTrace
-        app.TransferKeeper.IterateDenomTraces(ctx,
-            func(dt ibctransfertypes.DenomTrace) bool {
-                // check if the new way of splitting FullDenom
-                // into Trace and BaseDenom passes validation and
-                // is the same as the current DenomTrace.
-                // If it isn't then store the new DenomTrace in the list of new traces.
-                newTrace := ibctransfertypes.ParseDenomTrace(dt.GetFullDenomPath())
-                if err := newTrace.Validate(); err == nil && !equalTraces(newTrace, dt) {
-                    newTraces = append(newTraces, newTrace)
-                }
-
-                return false
-            })
-
-        // replace the outdated traces with the new trace information
-        for _, nt := range newTraces {
-            app.TransferKeeper.SetDenomTrace(ctx, nt)
-        }
-
+        // transfer module consensus version has been bumped to 2
         return app.mm.RunMigrations(ctx, app.configurator, fromVM)
     })
-
-func equalTraces(dtA, dtB ibctransfertypes.DenomTrace) bool {
-    return dtA.BaseDenom == dtB.BaseDenom && dtA.Path == dtB.Path
-}
+
 ```
 
 This is only necessary if there are denom traces in the store with incorrect trace information from previously received coins that had a slash in the base denom. However, it is recommended that any chain upgrading to support base denominations with slashes runs this code for safety.
 
-For a more detailed sample, please check out the code changes in [this pull request](https://github.com/cosmos/ibc-go/pull/1527).
+For a more detailed sample, please check out the code changes in [this pull request](https://github.com/cosmos/ibc-go/pull/1680).
 
 ### Genesis Migration
 

docs/migrations/v3-to-v4.md

@@ -18,7 +18,27 @@ No genesis or in-place migrations required when upgrading from v1 or v2 of ibc-g
 
 ## Chains
 
-- No relevant changes were made in this release.
+### Migration to fix support for base denoms with slashes
+
+As part of [v1.5.0](https://github.com/cosmos/ibc-go/releases/tag/v1.5.0), [v2.3.0](https://github.com/cosmos/ibc-go/releases/tag/v2.3.0) and [v3.1.0](https://github.com/cosmos/ibc-go/releases/tag/v3.1.0) some [migration handler code sample was documented](https://github.com/cosmos/ibc-go/blob/main/docs/migrations/support-denoms-with-slashes.md#upgrade-proposal) that needs to run in order to correct the trace information of coins transferred using ICS20 whose base denom contains slashes.
+
+Based on feedback from the community we add now an improved solution to run the same migration that does not require copying a large piece of code over from the migration document, but instead requires only adding a one-line upgrade handler.
+
+If the chain will migrate to supporting base denoms with slashes, it must set the appropriate params during the execution of the upgrade handler in `app.go`: 
+```go
+app.UpgradeKeeper.SetUpgradeHandler("MigrateTraces",
+    func(ctx sdk.Context, _ upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) {
+        // transfer module consensus version has been bumped to 2
+        return app.mm.RunMigrations(ctx, app.configurator, fromVM)
+    })
+
+```
+
+If a chain receives coins of a base denom with slashes before it upgrades to supporting it, the receive may pass however the trace information will be incorrect.
+
+E.g. If a base denom of `testcoin/testcoin/testcoin` is sent to a chain that does not support slashes in the base denom, the receive will be successful. However, the trace information stored on the receiving chain will be: `Trace: "transfer/{channel-id}/testcoin/testcoin", BaseDenom: "testcoin"`.
+
+This incorrect trace information must be corrected when the chain does upgrade to fully supporting denominations with slashes.
 
 ## IBC Apps