From ca37ecd7c50480f81f0bf4efa2f340aa2a245219 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 1 Jun 2023 09:05:32 +0200 Subject: [PATCH] imp: transfer total escrow follow ups (#3558) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * transfer (total escrow): add documentation and migration docs (#3509) * add docs for total escrow feature * revert change * fix tag * Update docs/migrations/v7-to-v7_1.md --------- Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> * transfer (total escrow): some more review comments (#3519) * some more review comments * Rename pathAndEscrowAMount to pathAndEscrowAmount --------- Co-authored-by: DimitrisJim * transfer (total escrow): add invariant for total escrow (#3506) * add invariant for total escrow * address review comment * refactor: simplify logic for asserting invariant * fix: use safeAdd instead of append * Update modules/apps/transfer/keeper/keeper.go Co-authored-by: Damian Nolan --------- Co-authored-by: Colin Axnér <25233464+colin-axner@users.noreply.github.com> Co-authored-by: Damian Nolan * imp: do not store total escrow when amount is zero (#3585) * do not store 0 escrow amout * adapt success test * Update modules/apps/transfer/keeper/keeper.go Co-authored-by: Jim Fasarakis-Hilliard * Update modules/apps/transfer/keeper/keeper.go Co-authored-by: Damian Nolan * add comments --------- Co-authored-by: Jim Fasarakis-Hilliard Co-authored-by: Damian Nolan * add feature release for total escrow state entry to conditionally query the endpoint in e2e (#3605) * fix total escrow cli documentation * Apply suggestions from code review Co-authored-by: Jim Fasarakis-Hilliard Co-authored-by: Damian Nolan * address review comments * docs: adr 011 total escrow state entry (#3641) * docs: add adr 011 for total escrow state entry * indentation * code formatting * Apply suggestions from code review Co-authored-by: Damian Nolan * Apply suggestions from code review Co-authored-by: Charly --------- Co-authored-by: Damian Nolan Co-authored-by: Charly --------- Co-authored-by: Carlos Rodriguez Co-authored-by: DimitrisJim Co-authored-by: Damian Nolan Co-authored-by: Charly (cherry picked from commit 6f628d9ff048309648e3cbba3afb1414dcb49f51) # Conflicts: # docs/architecture/README.md # docs/migrations/v7-to-v7_1.md # e2e/testconfig/testconfig.go # e2e/tests/transfer/base_test.go --- docs/.vuepress/config.js | 5 + docs/apps/transfer/authorizations.md | 4 + docs/apps/transfer/client.md | 66 +++ docs/architecture/README.md | 10 + ...r-011-transfer-total-escrow-state-entry.md | 145 +++++ docs/migrations/v7-to-v7_1.md | 12 +- e2e/testconfig/testconfig.go | 536 ++++++++++++++++++ e2e/tests/transfer/base_test.go | 473 ++++++++++++++++ modules/apps/transfer/client/cli/query.go | 4 +- modules/apps/transfer/keeper/genesis_test.go | 6 +- modules/apps/transfer/keeper/invariants.go | 51 ++ .../apps/transfer/keeper/invariants_test.go | 71 +++ modules/apps/transfer/keeper/keeper.go | 22 +- modules/apps/transfer/keeper/keeper_test.go | 20 +- modules/apps/transfer/keeper/relay_test.go | 6 +- modules/apps/transfer/module.go | 4 +- .../ibc/applications/transfer/v1/query.proto | 1 + 17 files changed, 1414 insertions(+), 22 deletions(-) create mode 100644 docs/apps/transfer/client.md create mode 100644 docs/architecture/adr-011-transfer-total-escrow-state-entry.md create mode 100644 e2e/testconfig/testconfig.go create mode 100644 e2e/tests/transfer/base_test.go create mode 100644 modules/apps/transfer/keeper/invariants.go create mode 100644 modules/apps/transfer/keeper/invariants_test.go diff --git a/docs/.vuepress/config.js b/docs/.vuepress/config.js index 93244dbd5bd..4d3a1357c63 100644 --- a/docs/.vuepress/config.js +++ b/docs/.vuepress/config.js @@ -335,6 +335,11 @@ module.exports = { directory: false, path: "/apps/transfer/authorizations.html", }, + { + title: "Client", + directory: false, + path: "/apps/transfer/client.html", + }, ], }, ], diff --git a/docs/apps/transfer/authorizations.md b/docs/apps/transfer/authorizations.md index fc5c3cc89a8..ea18133b095 100644 --- a/docs/apps/transfer/authorizations.md +++ b/docs/apps/transfer/authorizations.md @@ -1,3 +1,7 @@ + + # `TransferAuthorization` `TransferAuthorization` implements the `Authorization` interface for `ibc.applications.transfer.v1.MsgTransfer`. It allows a granter to grant a grantee the privilege to submit `MsgTransfer` on its behalf. Please see the [Cosmos SDK docs](https://docs.cosmos.network/v0.47/modules/authz) for more details on granting privileges via the `x/authz` module. diff --git a/docs/apps/transfer/client.md b/docs/apps/transfer/client.md new file mode 100644 index 00000000000..515fad57d5c --- /dev/null +++ b/docs/apps/transfer/client.md @@ -0,0 +1,66 @@ + + +# Client + +## CLI + +A user can query and interact with the `transfer` module using the CLI. Use the `--help` flag to discover the available commands: + +### Query + +The `query` commands allow users to query `transfer` state. + +```shell +simd query ibc-transfer --help +``` + +#### `total-escrow` + +The `total-escrow` command allows users to query the total amount in escrow for a particular coin denomination regardless of the transfer channel from where the coins were sent out. + +```shell +simd query ibc-transfer total-escrow [denom] [flags] +``` + +Example: + +```shell +simd query ibc-transfer total-escrow samoleans +``` + +Example Output: + +```shell +amount: "100" +``` + +## gRPC + +A user can query the `transfer` module using gRPC endpoints. + +### `TotalEscrowForDenom` + +The `TotalEscrowForDenom` endpoint allows users to query the total amount in escrow for a particular coin denomination regardless of the transfer channel from where the coins were sent out. + +```shell +ibc.applications.transfer.v1.Query/TotalEscrowForDenom +``` + +Example: + +```shell +grpcurl -plaintext \ + -d '{"denom":"samoleans"}' \ + localhost:9090 \ + ibc.applications.transfer.v1.Query/TotalEscrowForDenom +``` + +Example output: + +```shell +{ + "amount": "100" +} +``` \ No newline at end of file diff --git a/docs/architecture/README.md b/docs/architecture/README.md index edfbea4f0c8..2c15dcf772e 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -31,6 +31,16 @@ To suggest an ADR, please make use of the [ADR template](./adr-template.md) prov | [002](./adr-002-go-module-versioning.md) | Go module versioning | Accepted | | [003](./adr-003-ics27-acknowledgement.md) | ICS27 acknowledgement format | Accepted | | [004](./adr-004-ics29-lock-fee-module.md) | ICS29 module locking upon escrow out of balance | Accepted | +<<<<<<< HEAD +======= +| [005](./adr-005-consensus-height-events.md) | `UpdateClient` events - `ClientState` consensus heights | Accepted | +| [006](./adr-006-02-client-refactor.md) | ICS02 client refactor | Accepted | +| [007](./adr-007-solomachine-signbytes.md) | ICS06 Solo machine sign bytes | +| [008](./adr-008-app-caller-cbs/adr-008-app-caller-cbs.md) | Callback to IBC ACtors | Accepted | +| [009](./adr-009-v6-ics27-msgserver.md) | ICS27 message server addition | Accepted | +| [010](./adr-010-light-clients-as-sdk-modules.md) | IBC light clients as SDK modules | Accepted | +| [011](./adr-011-transfer-total-escrow-state-entry.md) | ICS20 state entry for total amount of tokens in escrow | Accepted | +>>>>>>> 6f628d9f (imp: transfer total escrow follow ups (#3558)) | [015](./adr-015-ibc-packet-receiver.md) | IBC Packet Routing | Accepted | | [025](./adr-025-ibc-passive-channels.md) | IBC passive channels | Deprecated | | [026](./adr-026-ibc-client-recovery-mechanisms.md) | IBC client recovery mechansisms | Accepted | diff --git a/docs/architecture/adr-011-transfer-total-escrow-state-entry.md b/docs/architecture/adr-011-transfer-total-escrow-state-entry.md new file mode 100644 index 00000000000..18241965386 --- /dev/null +++ b/docs/architecture/adr-011-transfer-total-escrow-state-entry.md @@ -0,0 +1,145 @@ +# ADR 011: ICS-20 transfer state entry for total amount of tokens in escrow + +## Changelog + +* 2023-05-24: Initial draft + +## Status + +Accepted and applied in v7.1 of ibc-go + +## Context + +Every ICS-20 transfer channel has its own escrow bank account. This account is used to lock tokens that are transferred out of a chain that acts as the source of the tokens (i.e. when the tokens being transferred have not returned to the originating chain). This design makes it easy to query the balance of the escrow accounts and find out the total amount of tokens in escrow in a particular channel. However, there are use cases where it would be useful to determine the total escrowed amount of a given denomination across all channels where those tokens have been transferred out. + +For example: assuming that there are three channels between Cosmos Hub to Osmosis and 10 ATOM have been transferred from the Cosmos Hub to Osmosis on each of those channels, then we would like to know that 30 ATOM have been transferred (i.e. are locked in the escrow accounts of each channel) without needing to iterate over each escrow account to add up the balances of each. + +For a sample use case where this feature would be useful, please refer to Osmosis' rate limiting use case described in [#2664](https://github.com/cosmos/ibc-go/issues/2664). + +## Decision + +### State entry denom -> amount + +The total amount of tokens in escrow (across all transfer channels) for a given denomination is stored in state in an entry keyed by the denomination: `totalEscrowForDenom/{denom}`. + +### Panic if amount is negative + +If a negative amount is ever attempted to be stored, then the keeper function will panic: + +```go +if coin.Amount.IsNegative() { + panic(fmt.Sprintf("amount cannot be negative: %s", coin.Amount)) +} +``` + +### Delete state entry if amount is zero + +When setting the amount for a particular denomination, the value might be zero if all tokens that were transferred out of the chain have been transferred back. If this happens, then the state entry for this particular denomination will be deleted, since Cosmos SDK's `x/bank` module prunes any non-zero balances: + +```go +if coin.Amount.IsZero() { + store.Delete(key) // delete the key since Cosmos SDK x/bank module will prune any non-zero balances + return +} +``` + +### Bundle escrow/unescrow with setting state entry + +Two new functions are implemented that bundle together the operations of escrowing/unescrowing and setting the total escrow amount in state, since these operations need to be executed together. + +For escrowing tokens: + +```go +// escrowToken will send the given token from the provided sender to the escrow address. It will also +// update the total escrowed amount by adding the escrowed token to the current total escrow. +func (k Keeper) escrowToken(ctx sdk.Context, sender, escrowAddress sdk.AccAddress, token sdk.Coin) error { + if err := k.bankKeeper.SendCoins(ctx, sender, escrowAddress, sdk.NewCoins(token)); err != nil { + // failure is expected for insufficient balances + return err + } + + // track the total amount in escrow keyed by denomination to allow for efficient iteration + currentTotalEscrow := k.GetTotalEscrowForDenom(ctx, token.GetDenom()) + newTotalEscrow := currentTotalEscrow.Add(token) + k.SetTotalEscrowForDenom(ctx, newTotalEscrow) + + return nil +} +``` + +For unescrowing tokens: + +```go +// unescrowToken will send the given token from the escrow address to the provided receiver. It will also +// update the total escrow by deducting the unescrowed token from the current total escrow. +func (k Keeper) unescrowToken(ctx sdk.Context, escrowAddress, receiver sdk.AccAddress, token sdk.Coin) error { + if err := k.bankKeeper.SendCoins(ctx, escrowAddress, receiver, sdk.NewCoins(token)); err != nil { + // NOTE: this error is only expected to occur given an unexpected bug or a malicious + // counterparty module. The bug may occur in bank or any part of the code that allows + // the escrow address to be drained. A malicious counterparty module could drain the + // escrow address by allowing more tokens to be sent back then were escrowed. + return errorsmod.Wrap(err, "unable to unescrow tokens, this may be caused by a malicious counterparty module or a bug: please open an issue on counterparty module") + } + + // track the total amount in escrow keyed by denomination to allow for efficient iteration + currentTotalEscrow := k.GetTotalEscrowForDenom(ctx, token.GetDenom()) + newTotalEscrow := currentTotalEscrow.Sub(token) + k.SetTotalEscrowForDenom(ctx, newTotalEscrow) + + return nil +} +``` + +When tokens need to be escrowed in `sendTransfer`, then `escrowToken` is called; when tokens need to be unescrowed on execution of the `OnRecvPacket`, `OnAcknowledgementPacket` or `OnTimeoutPacket` callbacks, then `unescrowToken` is called. + +### gRPC query endpoint and CLI to retrieve amount + +A gRPC query endpoint is added so that it is possible to retrieve the total amount for a given denomination: + +```proto +// TotalEscrowForDenom returns the total amount of tokens in escrow based on the denom. +rpc TotalEscrowForDenom(QueryTotalEscrowForDenomRequest) returns (QueryTotalEscrowForDenomResponse) { + option (google.api.http).get = "/ibc/apps/transfer/v1/denoms/{denom=**}/total_escrow"; +} + +// QueryTotalEscrowForDenomRequest is the request type for TotalEscrowForDenom RPC method. +message QueryTotalEscrowForDenomRequest { + string denom = 1; +} + +// QueryTotalEscrowForDenomResponse is the response type for TotalEscrowForDenom RPC method. +message QueryTotalEscrowForDenomResponse { + cosmos.base.v1beta1.Coin amount = 1 [(gogoproto.nullable) = false]; +} +``` + +And a CLI query is also available to retrieve the total amount via the command line: + +```shell +query ibc-transfer total-escrow [denom] +``` + +## Consequences + +### Positive + +* Possibility to retrieve the total amount of a particular denomination in escrow across all transfer channels without iteration. + +### Negative + +No notable consequences + +### Neutral + +* A new entry is added to state for every denomination that is transferred out of the chain. + +## References + +Issues: + +* [#2664](https://github.com/cosmos/ibc-go/issues/2664) + +PRs: + +* [#3019](https://github.com/cosmos/ibc-go/pull/3019) +* [#3558](https://github.com/cosmos/ibc-go/pull/3558) \ No newline at end of file diff --git a/docs/migrations/v7-to-v7_1.md b/docs/migrations/v7-to-v7_1.md index da2bf051921..659572d105a 100644 --- a/docs/migrations/v7-to-v7_1.md +++ b/docs/migrations/v7-to-v7_1.md @@ -14,15 +14,21 @@ There are four sections based on the four potential user groups of this document ## Chains +### 09-localhost migration + In the previous release of ibc-go, the localhost `v1` light client module was deprecated and removed. The ibc-go `v7.1.0` release introduces `v2` of the 09-localhost light client module. +<<<<<<< HEAD An [automatic migration handler](https://github.com/cosmos/ibc-go/blob/09-localhost/modules/core/module.go#L133-L145) is configured in the core IBC module to set the localhost `ClientState` and sentintel `ConnectionEnd` in state. +======= +An [automatic migration handler](https://github.com/cosmos/ibc-go/blob/release/v7.1.x/modules/core/module.go#L127-L145) is configured in the core IBC module to set the localhost `ClientState` and sentinel `ConnectionEnd` in state. +>>>>>>> 6f628d9f (imp: transfer total escrow follow ups (#3558)) In order to use the 09-localhost client chains must update the `AllowedClients` parameter in the 02-client submodule of core IBC. This can be configured directly in the application upgrade handler or alternatively updated via the legacy governance parameter change proposal. We __strongly__ recommend chains to perform this action so that intra-ledger communication can be carried out using the familiar IBC interfaces. -See the upgrade handler code sample provided below or [follow this link](https://github.com/cosmos/ibc-go/blob/09-localhost/testing/simapp/upgrades/upgrades.go#L85) for the upgrade handler used by the ibc-go simapp. +See the upgrade handler code sample provided below or [follow this link](https://github.com/cosmos/ibc-go/blob/release/v7.1.x/testing/simapp/upgrades/upgrades.go#L85) for the upgrade handler used by the ibc-go simapp. ```go func CreateV7LocalhostUpgradeHandler( @@ -41,7 +47,9 @@ func CreateV7LocalhostUpgradeHandler( } ``` -[For more information please refer to the 09-localhost light client module documentation](../ibc/light-clients/localhost/overview.md). +### Transfer migration + +An automatic migration handler (TODO: add link after backport to v7.1.x is merged) is configured in the transfer module to set the total amount in escrow for all denominations of coins that have been sent out. For each denomination a state entry is added with the total amount of coins in escrow regardless of the channel from which they were transferred. ## IBC Apps diff --git a/e2e/testconfig/testconfig.go b/e2e/testconfig/testconfig.go new file mode 100644 index 00000000000..f5b015ab7f1 --- /dev/null +++ b/e2e/testconfig/testconfig.go @@ -0,0 +1,536 @@ +package testconfig + +import ( + "encoding/json" + "fmt" + "os" + "path" + "strings" + + tmjson "github.com/cometbft/cometbft/libs/json" + tmtypes "github.com/cometbft/cometbft/types" + "github.com/cosmos/cosmos-sdk/codec" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/module/testutil" + genutiltypes "github.com/cosmos/cosmos-sdk/x/genutil/types" + govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" + govv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" + govv1beta1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" + "github.com/strangelove-ventures/interchaintest/v7/ibc" + interchaintestutil "github.com/strangelove-ventures/interchaintest/v7/testutil" + "gopkg.in/yaml.v2" + + "github.com/cosmos/ibc-go/e2e/relayer" + "github.com/cosmos/ibc-go/e2e/testvalues" +) + +const ( + // ChainImageEnv specifies the image that the chains will use. If left unspecified, it will + // default to being determined based on the specified binary. E.g. ghcr.io/cosmos/ibc-go-simd + ChainImageEnv = "CHAIN_IMAGE" + // ChainATagEnv specifies the tag that Chain A will use. + ChainATagEnv = "CHAIN_A_TAG" + // ChainBTagEnv specifies the tag that Chain B will use. If unspecified + // the value will default to the same value as Chain A. + ChainBTagEnv = "CHAIN_B_TAG" + // RelayerTagEnv specifies the relayer version. Defaults to "main" + RelayerTagEnv = "RELAYER_TAG" + // RelayerTypeEnv specifies the type of relayer that should be used. + RelayerTypeEnv = "RELAYER_TYPE" + // ChainBinaryEnv binary is the binary that will be used for both chains. + ChainBinaryEnv = "CHAIN_BINARY" + // ChainUpgradeTagEnv specifies the upgrade version tag + ChainUpgradeTagEnv = "CHAIN_UPGRADE_TAG" + // ChainUpgradePlanEnv specifies the upgrade plan name + ChainUpgradePlanEnv = "CHAIN_UPGRADE_PLAN" + // E2EConfigFilePathEnv allows you to specify a custom path for the config file to be used. + E2EConfigFilePathEnv = "E2E_CONFIG_PATH" + + // defaultBinary is the default binary that will be used by the chains. + defaultBinary = "simd" + // defaultRlyTag is the tag that will be used if no relayer tag is specified. + // all images are here https://github.com/cosmos/relayer/pkgs/container/relayer/versions + defaultRlyTag = "latest" // "andrew-tendermint_v0.37" // "v2.2.0" + // defaultChainTag is the tag that will be used for the chains if none is specified. + defaultChainTag = "main" + // defaultRelayerType is the default relayer that will be used if none is specified. + defaultRelayerType = relayer.Rly + // defaultConfigFileName is the default filename for the config file that can be used to configure + // e2e tests. See sample.config.yaml as an example for what this should look like. + defaultConfigFileName = ".ibc-go-e2e-config.yaml" + + // icadBinary is the binary for interchain-accounts-demo repository. + icadBinary = "icad" +) + +func getChainImage(binary string) string { + if binary == "" { + binary = defaultBinary + } + return fmt.Sprintf("ghcr.io/cosmos/ibc-go-%s", binary) +} + +// TestConfig holds configuration used throughout the different e2e tests. +type TestConfig struct { + // ChainConfigs holds configuration values related to the chains used in the tests. + ChainConfigs []ChainConfig `yaml:"chains"` + // RelayerConfig holds configuration for the relayer to be used. + RelayerConfig relayer.Config `yaml:"relayer"` + // UpgradeConfig holds values used only for the upgrade tests. + UpgradeConfig UpgradeConfig `yaml:"upgrade"` + // CometBFTConfig holds values for configuring CometBFT. + CometBFTConfig CometBFTConfig `yaml:"cometbft"` + // DebugConfig holds configuration for miscellaneous options. + DebugConfig DebugConfig `yaml:"debug"` +} + +// GetChainNumValidators returns the number of validators for the specific chain index. +// default 1 +func (tc TestConfig) GetChainNumValidators(idx int) int { + if tc.ChainConfigs[idx].NumValidators > 0 { + return tc.ChainConfigs[idx].NumValidators + } + return 1 +} + +// GetChainNumFullNodes returns the number of full nodes for the specific chain index. +// default 0 +func (tc TestConfig) GetChainNumFullNodes(idx int) int { + if tc.ChainConfigs[idx].NumFullNodes > 0 { + return tc.ChainConfigs[idx].NumFullNodes + } + return 0 +} + +// GetChainAID returns the chain-id for chain A. +func (tc TestConfig) GetChainAID() string { + if tc.ChainConfigs[0].ChainID != "" { + return tc.ChainConfigs[0].ChainID + } + return "chain-a" +} + +// GetChainBID returns the chain-id for chain B. +func (tc TestConfig) GetChainBID() string { + if tc.ChainConfigs[1].ChainID != "" { + return tc.ChainConfigs[1].ChainID + } + return "chain-b" +} + +// UpgradeConfig holds values relevant to upgrade tests. +type UpgradeConfig struct { + PlanName string `yaml:"planName"` + Tag string `yaml:"tag"` +} + +// ChainConfig holds information about an individual chain used in the tests. +type ChainConfig struct { + ChainID string `yaml:"chainId"` + Image string `yaml:"image"` + Tag string `yaml:"tag"` + Binary string `yaml:"binary"` + NumValidators int `yaml:"numValidators"` + NumFullNodes int `yaml:"numFullNodes"` +} + +type CometBFTConfig struct { + LogLevel string `yaml:"logLevel"` +} + +type DebugConfig struct { + // DumpLogs forces the logs to be collected before removing test containers. + DumpLogs bool `yaml:"dumpLogs"` +} + +// LoadConfig attempts to load a atest configuration from the default file path. +// if any environment variables are specified, they will take precedence over the individual configuration +// options. +func LoadConfig() TestConfig { + fileTc, foundFile := fromFile() + if !foundFile { + return fromEnv() + } + return applyEnvironmentVariableOverrides(fileTc) +} + +// fromFile returns a TestConfig from a json file and a boolean indicating if the file was found. +func fromFile() (TestConfig, bool) { + var tc TestConfig + bz, err := os.ReadFile(getConfigFilePath()) + if err != nil { + return TestConfig{}, false + } + + if err := yaml.Unmarshal(bz, &tc); err != nil { + panic(err) + } + + return tc, true +} + +// applyEnvironmentVariableOverrides applies all environment variable changes to the config +// loaded from a file. +func applyEnvironmentVariableOverrides(fromFile TestConfig) TestConfig { + envTc := fromEnv() + + if os.Getenv(ChainATagEnv) != "" { + fromFile.ChainConfigs[0].Tag = envTc.ChainConfigs[0].Tag + } + + if os.Getenv(ChainBTagEnv) != "" { + fromFile.ChainConfigs[1].Tag = envTc.ChainConfigs[1].Tag + } + + if os.Getenv(ChainBinaryEnv) != "" { + for i := range fromFile.ChainConfigs { + fromFile.ChainConfigs[i].Binary = envTc.ChainConfigs[i].Binary + } + } + + if os.Getenv(ChainImageEnv) != "" { + for i := range fromFile.ChainConfigs { + fromFile.ChainConfigs[i].Image = envTc.ChainConfigs[i].Image + } + } + + if os.Getenv(RelayerTagEnv) != "" { + fromFile.RelayerConfig.Tag = envTc.RelayerConfig.Tag + } + + if os.Getenv(RelayerTypeEnv) != "" { + fromFile.RelayerConfig.Type = envTc.RelayerConfig.Type + } + + if os.Getenv(ChainUpgradePlanEnv) != "" { + fromFile.UpgradeConfig.PlanName = envTc.UpgradeConfig.PlanName + } + + if os.Getenv(ChainUpgradeTagEnv) != "" { + fromFile.UpgradeConfig.Tag = envTc.UpgradeConfig.Tag + } + + return fromFile +} + +// fromEnv returns a TestConfig constructed from environment variables. +func fromEnv() TestConfig { + return TestConfig{ + ChainConfigs: getChainConfigsFromEnv(), + UpgradeConfig: getUpgradePlanConfigFromEnv(), + RelayerConfig: getRelayerConfigFromEnv(), + } +} + +// getChainConfigsFromEnv returns the chain configs from environment variables. +func getChainConfigsFromEnv() []ChainConfig { + chainBinary, ok := os.LookupEnv(ChainBinaryEnv) + if !ok { + chainBinary = defaultBinary + } + + chainATag, ok := os.LookupEnv(ChainATagEnv) + if !ok { + chainATag = defaultChainTag + } + + chainBTag, ok := os.LookupEnv(ChainBTagEnv) + if !ok { + chainBTag = chainATag + } + + chainAImage := getChainImage(chainBinary) + specifiedChainImage, ok := os.LookupEnv(ChainImageEnv) + if ok { + chainAImage = specifiedChainImage + } + + chainBImage := chainAImage + return []ChainConfig{ + { + Image: chainAImage, + Tag: chainATag, + Binary: chainBinary, + }, + { + Image: chainBImage, + Tag: chainBTag, + Binary: chainBinary, + }, + } +} + +// getConfigFilePath returns the absolute path where the e2e config file should be. +func getConfigFilePath() string { + if absoluteConfigPath := os.Getenv(E2EConfigFilePathEnv); absoluteConfigPath != "" { + return absoluteConfigPath + } + + homeDir, err := os.UserHomeDir() + if err != nil { + panic(err) + } + return path.Join(homeDir, defaultConfigFileName) +} + +// getRelayerConfigFromEnv returns the RelayerConfig from present environment variables. +func getRelayerConfigFromEnv() relayer.Config { + relayerType := strings.TrimSpace(os.Getenv(RelayerTypeEnv)) + if relayerType == "" { + relayerType = defaultRelayerType + } + + rlyTag := strings.TrimSpace(os.Getenv(RelayerTagEnv)) + if rlyTag == "" { + if relayerType == relayer.Rly { + rlyTag = defaultRlyTag + } + if relayerType == relayer.Hermes { + // TODO: set default hermes version + } + } + return relayer.Config{ + Tag: rlyTag, + Type: relayerType, + } +} + +// getUpgradePlanConfigFromEnv returns the upgrade config from environment variables. +func getUpgradePlanConfigFromEnv() UpgradeConfig { + upgradeTag, ok := os.LookupEnv(ChainUpgradeTagEnv) + if !ok { + upgradeTag = "" + } + + upgradePlan, ok := os.LookupEnv(ChainUpgradePlanEnv) + if !ok { + upgradePlan = "" + } + return UpgradeConfig{ + PlanName: upgradePlan, + Tag: upgradeTag, + } +} + +func GetChainATag() string { + return LoadConfig().ChainConfigs[0].Tag +} + +func GetChainBTag() string { + if chainBTag := LoadConfig().ChainConfigs[1].Tag; chainBTag != "" { + return chainBTag + } + return GetChainATag() +} + +// IsCI returns true if the tests are running in CI, false is returned +// if the tests are running locally. +// Note: github actions passes a CI env value of true by default to all runners. +func IsCI() bool { + return strings.ToLower(os.Getenv("CI")) == "true" +} + +// ChainOptions stores chain configurations for the chains that will be +// created for the tests. They can be modified by passing ChainOptionConfiguration +// to E2ETestSuite.GetChains. +type ChainOptions struct { + ChainAConfig *ibc.ChainConfig + ChainBConfig *ibc.ChainConfig +} + +// ChainOptionConfiguration enables arbitrary configuration of ChainOptions. +type ChainOptionConfiguration func(options *ChainOptions) + +// DefaultChainOptions returns the default configuration for the chains. +// These options can be configured by passing configuration functions to E2ETestSuite.GetChains. +func DefaultChainOptions() ChainOptions { + tc := LoadConfig() + + chainACfg := newDefaultSimappConfig(tc.ChainConfigs[0], "simapp-a", tc.GetChainAID(), "atoma", tc.CometBFTConfig) + chainBCfg := newDefaultSimappConfig(tc.ChainConfigs[1], "simapp-b", tc.GetChainBID(), "atomb", tc.CometBFTConfig) + return ChainOptions{ + ChainAConfig: &chainACfg, + ChainBConfig: &chainBCfg, + } +} + +// newDefaultSimappConfig creates an ibc configuration for simd. +func newDefaultSimappConfig(cc ChainConfig, name, chainID, denom string, cometCfg CometBFTConfig) ibc.ChainConfig { + configFileOverrides := make(map[string]any) + tmTomlOverrides := make(interchaintestutil.Toml) + + tmTomlOverrides["log_level"] = cometCfg.LogLevel // change to debug in ~/.ibc-go-e2e-config.json to increase cometbft logging. + configFileOverrides["config/config.toml"] = tmTomlOverrides + + useNewGenesisCommand := cc.Binary == icadBinary && testvalues.IcadNewGenesisCommandsFeatureReleases.IsSupported(cc.Tag) + + return ibc.ChainConfig{ + Type: "cosmos", + Name: name, + ChainID: chainID, + Images: []ibc.DockerImage{ + { + Repository: cc.Image, + Version: cc.Tag, + }, + }, + Bin: cc.Binary, + Bech32Prefix: "cosmos", + CoinType: fmt.Sprint(sdk.GetConfig().GetCoinType()), + Denom: denom, + GasPrices: fmt.Sprintf("0.00%s", denom), + GasAdjustment: 1.3, + TrustingPeriod: "508h", + NoHostMount: false, + ModifyGenesis: getGenesisModificationFunction(cc), + ConfigFileOverrides: configFileOverrides, + UsingNewGenesisCommand: useNewGenesisCommand, + } +} + +// getGenesisModificationFunction returns a genesis modification function that handles the GenesisState type +// correctly depending on if the govv1beta1 gov module is used or if govv1 is being used. +func getGenesisModificationFunction(cc ChainConfig) func(ibc.ChainConfig, []byte) ([]byte, error) { + binary := cc.Binary + version := cc.Tag + + simdSupportsGovV1Genesis := binary == defaultBinary && testvalues.GovGenesisFeatureReleases.IsSupported(version) + icadSupportsGovV1Genesis := testvalues.IcadGovGenesisFeatureReleases.IsSupported(version) + + if simdSupportsGovV1Genesis || icadSupportsGovV1Genesis { + return defaultGovv1ModifyGenesis() + } + + return defaultGovv1Beta1ModifyGenesis() +} + +// defaultGovv1ModifyGenesis will only modify governance params to ensure the voting period and minimum deposit +// are functional for e2e testing purposes. +func defaultGovv1ModifyGenesis() func(ibc.ChainConfig, []byte) ([]byte, error) { + return func(chainConfig ibc.ChainConfig, genbz []byte) ([]byte, error) { + genDoc, err := tmtypes.GenesisDocFromJSON(genbz) + if err != nil { + return nil, fmt.Errorf("failed to unmarshal genesis bytes into genesis doc: %w", err) + } + + var appState genutiltypes.AppMap + if err := json.Unmarshal(genDoc.AppState, &appState); err != nil { + return nil, fmt.Errorf("failed to unmarshal genesis bytes into app state: %w", err) + } + + govGenBz, err := modifyGovAppState(chainConfig, appState[govtypes.ModuleName]) + if err != nil { + return nil, err + } + + appState[govtypes.ModuleName] = govGenBz + + genDoc.AppState, err = json.Marshal(appState) + if err != nil { + return nil, err + } + + bz, err := tmjson.MarshalIndent(genDoc, "", " ") + if err != nil { + return nil, err + } + + return bz, nil + } +} + +// defaultGovv1Beta1ModifyGenesis will only modify governance params to ensure the voting period and minimum deposit +// // are functional for e2e testing purposes. +func defaultGovv1Beta1ModifyGenesis() func(ibc.ChainConfig, []byte) ([]byte, error) { + const appStateKey = "app_state" + return func(chainConfig ibc.ChainConfig, genbz []byte) ([]byte, error) { + genesisDocMap := map[string]interface{}{} + err := json.Unmarshal(genbz, &genesisDocMap) + if err != nil { + return nil, fmt.Errorf("failed to unmarshal genesis bytes into genesis doc: %w", err) + } + + appStateMap, ok := genesisDocMap[appStateKey].(map[string]interface{}) + if !ok { + return nil, fmt.Errorf("failed to extract to app_state") + } + + govModuleBytes, err := json.Marshal(appStateMap[govtypes.ModuleName]) + if err != nil { + return nil, fmt.Errorf("failed to extract gov genesis bytes: %s", err) + } + + govModuleGenesisBytes, err := modifyGovv1Beta1AppState(chainConfig, govModuleBytes) + if err != nil { + return nil, err + } + + govModuleGenesisMap := map[string]interface{}{} + err = json.Unmarshal(govModuleGenesisBytes, &govModuleGenesisMap) + if err != nil { + return nil, fmt.Errorf("failed to unmarshal gov genesis bytes into map: %w", err) + } + + appStateMap[govtypes.ModuleName] = govModuleGenesisMap + genesisDocMap[appStateKey] = appStateMap + + finalGenesisDocBytes, err := json.MarshalIndent(genesisDocMap, "", " ") + if err != nil { + return nil, err + } + + return finalGenesisDocBytes, nil + } +} + +// modifyGovAppState takes the existing gov app state and marshals it to a govv1 GenesisState. +func modifyGovAppState(chainConfig ibc.ChainConfig, govAppState []byte) ([]byte, error) { + cfg := testutil.MakeTestEncodingConfig() + + cdc := codec.NewProtoCodec(cfg.InterfaceRegistry) + govv1.RegisterInterfaces(cfg.InterfaceRegistry) + + govGenesisState := &govv1.GenesisState{} + + if err := cdc.UnmarshalJSON(govAppState, govGenesisState); err != nil { + return nil, fmt.Errorf("failed to unmarshal genesis bytes into gov genesis state: %w", err) + } + + if govGenesisState.Params == nil { + govGenesisState.Params = &govv1.Params{} + } + + govGenesisState.Params.MinDeposit = sdk.NewCoins(sdk.NewCoin(chainConfig.Denom, govv1beta1.DefaultMinDepositTokens)) + vp := testvalues.VotingPeriod + govGenesisState.Params.VotingPeriod = &vp + + govGenBz, err := cdc.MarshalJSON(govGenesisState) + if err != nil { + return nil, fmt.Errorf("failed to marshal gov genesis state: %w", err) + } + + return govGenBz, nil +} + +// modifyGovv1Beta1AppState takes the existing gov app state and marshals it to a govv1beta1 GenesisState. +func modifyGovv1Beta1AppState(chainConfig ibc.ChainConfig, govAppState []byte) ([]byte, error) { + cfg := testutil.MakeTestEncodingConfig() + + cdc := codec.NewProtoCodec(cfg.InterfaceRegistry) + govv1beta1.RegisterInterfaces(cfg.InterfaceRegistry) + + govGenesisState := &govv1beta1.GenesisState{} + if err := cdc.UnmarshalJSON(govAppState, govGenesisState); err != nil { + return nil, fmt.Errorf("failed to unmarshal genesis bytes into govv1beta1 genesis state: %w", err) + } + + govGenesisState.DepositParams.MinDeposit = sdk.NewCoins(sdk.NewCoin(chainConfig.Denom, govv1beta1.DefaultMinDepositTokens)) + govGenesisState.VotingParams.VotingPeriod = testvalues.VotingPeriod + + govGenBz, err := cdc.MarshalJSON(govGenesisState) + if err != nil { + return nil, fmt.Errorf("failed to marshal gov genesis state: %w", err) + } + + return govGenBz, nil +} diff --git a/e2e/tests/transfer/base_test.go b/e2e/tests/transfer/base_test.go new file mode 100644 index 00000000000..901c5e9b1a8 --- /dev/null +++ b/e2e/tests/transfer/base_test.go @@ -0,0 +1,473 @@ +package transfer + +import ( + "context" + "testing" + "time" + + "cosmossdk.io/math" + sdk "github.com/cosmos/cosmos-sdk/types" + govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" + paramsproposaltypes "github.com/cosmos/cosmos-sdk/x/params/types/proposal" + transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" + + "github.com/strangelove-ventures/interchaintest/v7/ibc" + test "github.com/strangelove-ventures/interchaintest/v7/testutil" + "github.com/stretchr/testify/suite" + + "github.com/cosmos/ibc-go/e2e/testsuite" + "github.com/cosmos/ibc-go/e2e/testvalues" + ibctesting "github.com/cosmos/ibc-go/v7/testing" +) + +func TestTransferTestSuite(t *testing.T) { + suite.Run(t, new(TransferTestSuite)) +} + +type TransferTestSuite struct { + testsuite.E2ETestSuite +} + +// QueryTransferSendEnabledParam queries the on-chain send enabled param for the transfer module +func (s *TransferTestSuite) QueryTransferParams(ctx context.Context, chain ibc.Chain) transfertypes.Params { + queryClient := s.GetChainGRCPClients(chain).TransferQueryClient + res, err := queryClient.Params(ctx, &transfertypes.QueryParamsRequest{}) + s.Require().NoError(err) + return *res.Params +} + +// TestMsgTransfer_Succeeds_Nonincentivized will test sending successful IBC transfers from chainA to chainB. +// The transfer will occur over a basic transfer channel (non incentivized) and both native and non-native tokens +// will be sent forwards and backwards in the IBC transfer timeline (both chains will act as source and receiver chains). +func (s *TransferTestSuite) TestMsgTransfer_Succeeds_Nonincentivized() { + t := s.T() + ctx := context.TODO() + + relayer, channelA := s.SetupChainsRelayerAndChannel(ctx, s.TransferChannelOptions()) + chainA, chainB := s.GetChains() + + chainADenom := chainA.Config().Denom + + chainAWallet := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount) + chainAAddress := chainAWallet.FormattedAddress() + + chainBWallet := s.CreateUserOnChainB(ctx, testvalues.StartingTokenAmount) + chainBAddress := chainBWallet.FormattedAddress() + + s.Require().NoError(test.WaitForBlocks(ctx, 1, chainA, chainB), "failed to wait for blocks") + + chainAVersion := chainA.Config().Images[0].Version + chainBVersion := chainB.Config().Images[0].Version + + t.Run("native IBC token transfer from chainA to chainB, sender is source of tokens", func(t *testing.T) { + transferTxResp := s.Transfer(ctx, chainA, chainAWallet, channelA.PortID, channelA.ChannelID, testvalues.DefaultTransferAmount(chainADenom), chainAAddress, chainBAddress, s.GetTimeoutHeight(ctx, chainB), 0, "") + s.AssertTxSuccess(transferTxResp) + }) + + t.Run("tokens are escrowed", func(t *testing.T) { + actualBalance, err := s.GetChainANativeBalance(ctx, chainAWallet) + s.Require().NoError(err) + + expected := testvalues.StartingTokenAmount - testvalues.IBCTransferAmount + s.Require().Equal(expected, actualBalance) + + if testvalues.TotalEscrowFeatureReleases.IsSupported(chainAVersion) { + actualTotalEscrow, err := s.QueryTotalEscrowForDenom(ctx, chainA, chainADenom) + s.Require().NoError(err) + + expectedTotalEscrow := sdk.NewCoin(chainADenom, math.NewInt(testvalues.IBCTransferAmount)) + s.Require().Equal(expectedTotalEscrow, actualTotalEscrow) + } + }) + + t.Run("start relayer", func(t *testing.T) { + s.StartRelayer(relayer) + }) + + chainBIBCToken := testsuite.GetIBCToken(chainADenom, channelA.Counterparty.PortID, channelA.Counterparty.ChannelID) + + t.Run("packets are relayed", func(t *testing.T) { + s.AssertPacketRelayed(ctx, chainA, channelA.PortID, channelA.ChannelID, 1) + + actualBalance, err := chainB.GetBalance(ctx, chainBAddress, chainBIBCToken.IBCDenom()) + s.Require().NoError(err) + + expected := testvalues.IBCTransferAmount + s.Require().Equal(expected, actualBalance) + }) + + t.Run("non-native IBC token transfer from chainB to chainA, receiver is source of tokens", func(t *testing.T) { + transferTxResp := s.Transfer(ctx, chainB, chainBWallet, channelA.Counterparty.PortID, channelA.Counterparty.ChannelID, testvalues.DefaultTransferAmount(chainBIBCToken.IBCDenom()), chainBAddress, chainAAddress, s.GetTimeoutHeight(ctx, chainA), 0, "") + s.AssertTxSuccess(transferTxResp) + }) + + t.Run("tokens are escrowed", func(t *testing.T) { + actualBalance, err := chainB.GetBalance(ctx, chainBAddress, chainBIBCToken.IBCDenom()) + s.Require().NoError(err) + + s.Require().Equal(int64(0), actualBalance) + + if testvalues.TotalEscrowFeatureReleases.IsSupported(chainBVersion) { + actualTotalEscrow, err := s.QueryTotalEscrowForDenom(ctx, chainB, chainBIBCToken.IBCDenom()) + s.Require().NoError(err) + s.Require().Equal(sdk.NewCoin(chainBIBCToken.IBCDenom(), sdk.NewInt(0)), actualTotalEscrow) // total escrow is zero because sending chain is not source for tokens + } + }) + + s.Require().NoError(test.WaitForBlocks(ctx, 5, chainA, chainB), "failed to wait for blocks") + + t.Run("packets are relayed", func(t *testing.T) { + s.AssertPacketRelayed(ctx, chainB, channelA.Counterparty.PortID, channelA.Counterparty.ChannelID, 1) + + actualBalance, err := s.GetChainANativeBalance(ctx, chainAWallet) + s.Require().NoError(err) + + expected := testvalues.StartingTokenAmount + s.Require().Equal(expected, actualBalance) + }) + + if testvalues.TotalEscrowFeatureReleases.IsSupported(chainAVersion) { + t.Run("tokens are un-escrowed", func(t *testing.T) { + actualTotalEscrow, err := s.QueryTotalEscrowForDenom(ctx, chainA, chainADenom) + s.Require().NoError(err) + s.Require().Equal(sdk.NewCoin(chainADenom, sdk.NewInt(0)), actualTotalEscrow) // total escrow is zero because tokens have come back + }) + } +} + +// TestMsgTransfer_Fails_InvalidAddress attempts to send an IBC transfer to an invalid address and ensures +// that the tokens on the sending chain are unescrowed. +func (s *TransferTestSuite) TestMsgTransfer_Fails_InvalidAddress() { + t := s.T() + ctx := context.TODO() + + relayer, channelA := s.SetupChainsRelayerAndChannel(ctx, s.TransferChannelOptions()) + chainA, chainB := s.GetChains() + + chainADenom := chainA.Config().Denom + + chainAWallet := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount) + chainAAddress := chainAWallet.FormattedAddress() + + s.Require().NoError(test.WaitForBlocks(ctx, 1, chainA, chainB), "failed to wait for blocks") + + t.Run("native IBC token transfer from chainA to invalid address", func(t *testing.T) { + transferTxResp := s.Transfer(ctx, chainA, chainAWallet, channelA.PortID, channelA.ChannelID, testvalues.DefaultTransferAmount(chainADenom), chainAAddress, testvalues.InvalidAddress, s.GetTimeoutHeight(ctx, chainB), 0, "") + s.AssertTxSuccess(transferTxResp) + }) + + t.Run("tokens are escrowed", func(t *testing.T) { + actualBalance, err := s.GetChainANativeBalance(ctx, chainAWallet) + s.Require().NoError(err) + + expected := testvalues.StartingTokenAmount - testvalues.IBCTransferAmount + s.Require().Equal(expected, actualBalance) + }) + + t.Run("start relayer", func(t *testing.T) { + s.StartRelayer(relayer) + }) + + t.Run("packets are relayed", func(t *testing.T) { + s.AssertPacketRelayed(ctx, chainA, channelA.PortID, channelA.ChannelID, 1) + }) + + t.Run("token transfer amount unescrowed", func(t *testing.T) { + actualBalance, err := s.GetChainANativeBalance(ctx, chainAWallet) + s.Require().NoError(err) + + expected := testvalues.StartingTokenAmount + s.Require().Equal(expected, actualBalance) + }) +} + +func (s *TransferTestSuite) TestMsgTransfer_Timeout_Nonincentivized() { + t := s.T() + ctx := context.TODO() + + relayer, channelA := s.SetupChainsRelayerAndChannel(ctx, s.TransferChannelOptions()) + chainA, _ := s.GetChains() + chainAWallet := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount) + chainBWallet := s.CreateUserOnChainB(ctx, testvalues.StartingTokenAmount) + + chainBWalletAmount := ibc.WalletAmount{ + Address: chainBWallet.FormattedAddress(), // destination address + Denom: chainA.Config().Denom, + Amount: testvalues.IBCTransferAmount, + } + + t.Run("IBC transfer packet timesout", func(t *testing.T) { + tx, err := chainA.SendIBCTransfer(ctx, channelA.ChannelID, chainAWallet.KeyName(), chainBWalletAmount, ibc.TransferOptions{Timeout: testvalues.ImmediatelyTimeout()}) + s.Require().NoError(err) + s.Require().NoError(tx.Validate(), "source ibc transfer tx is invalid") + time.Sleep(time.Nanosecond * 1) // want it to timeout immediately + }) + + t.Run("tokens are escrowed", func(t *testing.T) { + actualBalance, err := s.GetChainANativeBalance(ctx, chainAWallet) + s.Require().NoError(err) + + expected := testvalues.StartingTokenAmount - testvalues.IBCTransferAmount + s.Require().Equal(expected, actualBalance) + }) + + t.Run("start relayer", func(t *testing.T) { + s.StartRelayer(relayer) + }) + + t.Run("ensure escrowed tokens have been refunded to sender due to timeout", func(t *testing.T) { + // ensure destination address did not receive any tokens + bal, err := s.GetChainBNativeBalance(ctx, chainBWallet) + s.Require().NoError(err) + s.Require().Equal(testvalues.StartingTokenAmount, bal) + + // ensure that the sender address has been successfully refunded the full amount + bal, err = s.GetChainANativeBalance(ctx, chainAWallet) + s.Require().NoError(err) + s.Require().Equal(testvalues.StartingTokenAmount, bal) + }) +} + +// TestSendEnabledParam tests changing ics20 SendEnabled parameter +func (s *TransferTestSuite) TestSendEnabledParam() { + t := s.T() + ctx := context.TODO() + + _, channelA := s.SetupChainsRelayerAndChannel(ctx, s.TransferChannelOptions()) + chainA, chainB := s.GetChains() + + chainADenom := chainA.Config().Denom + + chainAWallet := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount) + chainAAddress := chainAWallet.FormattedAddress() + + chainBWallet := s.CreateUserOnChainB(ctx, testvalues.StartingTokenAmount) + chainBAddress := chainBWallet.FormattedAddress() + + chainAVersion := chainA.Config().Images[0].Version + isSelfManagingParams := testvalues.TransferSelfParamsFeatureReleases.IsSupported(chainAVersion) + + govModuleAddress, err := s.QueryModuleAccountAddress(ctx, govtypes.ModuleName, chainA) + s.Require().NoError(err) + s.Require().NotNil(govModuleAddress) + + s.Require().NoError(test.WaitForBlocks(ctx, 1, chainA, chainB), "failed to wait for blocks") + + t.Run("ensure transfer sending is enabled", func(t *testing.T) { + enabled := s.QueryTransferParams(ctx, chainA).SendEnabled + s.Require().True(enabled) + }) + + t.Run("ensure packets can be sent", func(t *testing.T) { + transferTxResp := s.Transfer(ctx, chainA, chainAWallet, channelA.PortID, channelA.ChannelID, testvalues.DefaultTransferAmount(chainADenom), chainAAddress, chainBAddress, s.GetTimeoutHeight(ctx, chainB), 0, "") + s.AssertTxSuccess(transferTxResp) + }) + + t.Run("change send enabled parameter to disabled", func(t *testing.T) { + if isSelfManagingParams { + msg := transfertypes.NewMsgUpdateParams(govModuleAddress.String(), transfertypes.NewParams(false, true)) + s.ExecuteGovProposalV1(ctx, msg, chainA, chainAWallet, 1) + } else { + changes := []paramsproposaltypes.ParamChange{ + paramsproposaltypes.NewParamChange(transfertypes.StoreKey, string(transfertypes.KeySendEnabled), "false"), + } + + proposal := paramsproposaltypes.NewParameterChangeProposal(ibctesting.Title, ibctesting.Description, changes) + s.ExecuteGovProposal(ctx, chainA, chainAWallet, proposal) + } + }) + + t.Run("ensure transfer params are disabled", func(t *testing.T) { + enabled := s.QueryTransferParams(ctx, chainA).SendEnabled + s.Require().False(enabled) + }) + + t.Run("ensure ics20 transfer fails", func(t *testing.T) { + transferTxResp := s.Transfer(ctx, chainA, chainAWallet, channelA.PortID, channelA.ChannelID, testvalues.DefaultTransferAmount(chainADenom), chainAAddress, chainBAddress, s.GetTimeoutHeight(ctx, chainB), 0, "") + s.AssertTxFailure(transferTxResp, transfertypes.ErrSendDisabled) + }) +} + +// TestReceiveEnabledParam tests changing ics20 ReceiveEnabled parameter +func (s *TransferTestSuite) TestReceiveEnabledParam() { + t := s.T() + ctx := context.TODO() + + relayer, channelA := s.SetupChainsRelayerAndChannel(ctx, s.TransferChannelOptions()) + chainA, chainB := s.GetChains() + + chainAWallet := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount) + chainBWallet := s.CreateUserOnChainB(ctx, testvalues.StartingTokenAmount) + + var ( + chainBDenom = chainB.Config().Denom + chainAIBCToken = testsuite.GetIBCToken(chainBDenom, channelA.PortID, channelA.ChannelID) // IBC token sent to chainA + + chainAAddress = chainAWallet.FormattedAddress() + chainBAddress = chainBWallet.FormattedAddress() + ) + + chainAVersion := chainA.Config().Images[0].Version + isSelfManagingParams := testvalues.TransferSelfParamsFeatureReleases.IsSupported(chainAVersion) + + govModuleAddress, err := s.QueryModuleAccountAddress(ctx, govtypes.ModuleName, chainA) + s.Require().NoError(err) + s.Require().NotNil(govModuleAddress) + + s.Require().NoError(test.WaitForBlocks(ctx, 1, chainA, chainB), "failed to wait for blocks") + + t.Run("ensure transfer receive is enabled", func(t *testing.T) { + enabled := s.QueryTransferParams(ctx, chainA).ReceiveEnabled + s.Require().True(enabled) + }) + + t.Run("ensure packets can be received, send from chainB to chainA", func(t *testing.T) { + t.Run("send from chainB to chainA", func(t *testing.T) { + transferTxResp := s.Transfer(ctx, chainB, chainBWallet, channelA.Counterparty.PortID, channelA.Counterparty.ChannelID, testvalues.DefaultTransferAmount(chainBDenom), chainBAddress, chainAAddress, s.GetTimeoutHeight(ctx, chainA), 0, "") + s.AssertTxSuccess(transferTxResp) + }) + + t.Run("tokens are escrowed", func(t *testing.T) { + actualBalance, err := s.GetChainBNativeBalance(ctx, chainBWallet) + s.Require().NoError(err) + + expected := testvalues.StartingTokenAmount - testvalues.IBCTransferAmount + s.Require().Equal(expected, actualBalance) + }) + + t.Run("start relayer", func(t *testing.T) { + s.StartRelayer(relayer) + }) + + t.Run("packets are relayed", func(t *testing.T) { + s.AssertPacketRelayed(ctx, chainA, channelA.Counterparty.PortID, channelA.Counterparty.ChannelID, 1) + + actualBalance, err := chainA.GetBalance(ctx, chainAAddress, chainAIBCToken.IBCDenom()) + s.Require().NoError(err) + + expected := testvalues.IBCTransferAmount + s.Require().Equal(expected, actualBalance) + }) + + t.Run("stop relayer", func(t *testing.T) { + s.StopRelayer(ctx, relayer) + }) + }) + + t.Run("change receive enabled parameter to disabled ", func(t *testing.T) { + if isSelfManagingParams { + msg := transfertypes.NewMsgUpdateParams(govModuleAddress.String(), transfertypes.NewParams(false, false)) + s.ExecuteGovProposalV1(ctx, msg, chainA, chainAWallet, 1) + } else { + changes := []paramsproposaltypes.ParamChange{ + paramsproposaltypes.NewParamChange(transfertypes.StoreKey, string(transfertypes.KeyReceiveEnabled), "false"), + } + + proposal := paramsproposaltypes.NewParameterChangeProposal(ibctesting.Title, ibctesting.Description, changes) + s.ExecuteGovProposal(ctx, chainA, chainAWallet, proposal) + } + }) + + t.Run("ensure transfer params are disabled", func(t *testing.T) { + enabled := s.QueryTransferParams(ctx, chainA).ReceiveEnabled + s.Require().False(enabled) + }) + + t.Run("ensure ics20 transfer fails", func(t *testing.T) { + t.Run("send from chainB to chainA", func(t *testing.T) { + transferTxResp := s.Transfer(ctx, chainB, chainBWallet, channelA.Counterparty.PortID, channelA.Counterparty.ChannelID, testvalues.DefaultTransferAmount(chainBDenom), chainBAddress, chainAAddress, s.GetTimeoutHeight(ctx, chainA), 0, "") + s.AssertTxSuccess(transferTxResp) + }) + + t.Run("tokens are escrowed", func(t *testing.T) { + actualBalance, err := s.GetChainBNativeBalance(ctx, chainBWallet) + s.Require().NoError(err) + + expected := testvalues.StartingTokenAmount - (testvalues.IBCTransferAmount * 2) // second send + s.Require().Equal(expected, actualBalance) + }) + + t.Run("start relayer", func(t *testing.T) { + s.StartRelayer(relayer) + }) + + t.Run("tokens are unescrowed in failed acknowledgement", func(t *testing.T) { + actualBalance, err := s.GetChainBNativeBalance(ctx, chainBWallet) + s.Require().NoError(err) + + expected := testvalues.StartingTokenAmount - testvalues.IBCTransferAmount // only first send marked + s.Require().Equal(expected, actualBalance) + }) + }) +} + +// This can be used to test sending with a transfer packet with a memo given different combinations of +// ibc-go versions. +// +// TestMsgTransfer_WithMemo will test sending IBC transfers from chainA to chainB +// If the chains contain a version of FungibleTokenPacketData with memo, both send and receive should succeed. +// If one of the chains contains a version of FungibleTokenPacketData without memo, then receiving a packet with +// memo should fail in that chain +func (s *TransferTestSuite) TestMsgTransfer_WithMemo() { + t := s.T() + ctx := context.TODO() + + relayer, channelA := s.SetupChainsRelayerAndChannel(ctx, s.TransferChannelOptions()) + chainA, chainB := s.GetChains() + + chainADenom := chainA.Config().Denom + + chainAWallet := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount) + chainAAddress := chainAWallet.FormattedAddress() + + chainBWallet := s.CreateUserOnChainB(ctx, testvalues.StartingTokenAmount) + chainBAddress := chainBWallet.FormattedAddress() + + s.Require().NoError(test.WaitForBlocks(ctx, 1, chainA, chainB), "failed to wait for blocks") + + chainAVersion := chainA.Config().Images[0].Version + chainBVersion := chainB.Config().Images[0].Version + + t.Run("IBC token transfer with memo from chainA to chainB", func(t *testing.T) { + transferTxResp := s.Transfer(ctx, chainA, chainAWallet, channelA.PortID, channelA.ChannelID, testvalues.DefaultTransferAmount(chainADenom), chainAAddress, chainBAddress, s.GetTimeoutHeight(ctx, chainB), 0, "memo") + + if testvalues.MemoFeatureReleases.IsSupported(chainAVersion) { + s.AssertTxSuccess(transferTxResp) + } else { + s.Require().Equal(uint32(2), transferTxResp.Code) + s.Require().Contains(transferTxResp.RawLog, "errUnknownField") + } + }) + + if !testvalues.MemoFeatureReleases.IsSupported(chainAVersion) { + // transfer not sent, end test + return + } + + t.Run("tokens are escrowed", func(t *testing.T) { + actualBalance, err := s.GetChainANativeBalance(ctx, chainAWallet) + s.Require().NoError(err) + + expected := testvalues.StartingTokenAmount - testvalues.IBCTransferAmount + s.Require().Equal(expected, actualBalance) + }) + + t.Run("start relayer", func(t *testing.T) { + s.StartRelayer(relayer) + }) + + chainBIBCToken := testsuite.GetIBCToken(chainADenom, channelA.Counterparty.PortID, channelA.Counterparty.ChannelID) + + t.Run("packets relayed", func(t *testing.T) { + s.AssertPacketRelayed(ctx, chainA, channelA.PortID, channelA.ChannelID, 1) + + actualBalance, err := chainB.GetBalance(ctx, chainBAddress, chainBIBCToken.IBCDenom()) + s.Require().NoError(err) + + if testvalues.MemoFeatureReleases.IsSupported(chainBVersion) { + s.Require().Equal(testvalues.IBCTransferAmount, actualBalance) + } else { + s.Require().Equal(int64(0), actualBalance) + } + }) +} diff --git a/modules/apps/transfer/client/cli/query.go b/modules/apps/transfer/client/cli/query.go index d8d90b9dfee..8ebab138c66 100644 --- a/modules/apps/transfer/client/cli/query.go +++ b/modules/apps/transfer/client/cli/query.go @@ -171,10 +171,10 @@ func GetCmdQueryDenomHash() *cobra.Command { // GetCmdQueryTotalEscrowForDenom defines the command to query the total amount of tokens in escrow for a denom func GetCmdQueryTotalEscrowForDenom() *cobra.Command { cmd := &cobra.Command{ - Use: "token-escrow [denom]", + Use: "total-escrow [denom]", Short: "Query the total amount of tokens in escrow for a denom", Long: "Query the total amount of tokens in escrow for a denom", - Example: fmt.Sprintf("%s query ibc-transfer token-escrow uosmo", version.AppName), + Example: fmt.Sprintf("%s query ibc-transfer total-escrow uosmo", version.AppName), Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { clientCtx, err := client.GetClientQueryContext(cmd) diff --git a/modules/apps/transfer/keeper/genesis_test.go b/modules/apps/transfer/keeper/genesis_test.go index 0ea92efcf52..99c25eab242 100644 --- a/modules/apps/transfer/keeper/genesis_test.go +++ b/modules/apps/transfer/keeper/genesis_test.go @@ -29,16 +29,16 @@ func (suite *KeeperTestSuite) TestGenesis() { } ) - for _, pathAndEscrowMount := range pathsAndEscrowAmounts { + for _, pathAndEscrowAmount := range pathsAndEscrowAmounts { denomTrace := types.DenomTrace{ BaseDenom: "uatom", - Path: pathAndEscrowMount.path, + Path: pathAndEscrowAmount.path, } traces = append(types.Traces{denomTrace}, traces...) suite.chainA.GetSimApp().TransferKeeper.SetDenomTrace(suite.chainA.GetContext(), denomTrace) denom := denomTrace.IBCDenom() - amount, ok := math.NewIntFromString(pathAndEscrowMount.escrow) + amount, ok := math.NewIntFromString(pathAndEscrowAmount.escrow) suite.Require().True(ok) escrows = append(sdk.NewCoins(sdk.NewCoin(denom, amount)), escrows...) suite.chainA.GetSimApp().TransferKeeper.SetTotalEscrowForDenom(suite.chainA.GetContext(), sdk.NewCoin(denom, amount)) diff --git a/modules/apps/transfer/keeper/invariants.go b/modules/apps/transfer/keeper/invariants.go new file mode 100644 index 00000000000..93e5aa36259 --- /dev/null +++ b/modules/apps/transfer/keeper/invariants.go @@ -0,0 +1,51 @@ +package keeper + +import ( + "fmt" + + sdk "github.com/cosmos/cosmos-sdk/types" + + "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" +) + +// RegisterInvariants registers all transfer invariants +func RegisterInvariants(ir sdk.InvariantRegistry, k *Keeper) { + ir.RegisterRoute(types.ModuleName, "total-escrow-per-denom", + TotalEscrowPerDenomInvariants(k)) +} + +// AllInvariants runs all invariants of the transfer module. +func AllInvariants(k *Keeper) sdk.Invariant { + return func(ctx sdk.Context) (string, bool) { + return TotalEscrowPerDenomInvariants(k)(ctx) + } +} + +// TotalEscrowPerDenomInvariants checks that the total amount escrowed for +// each denom is not smaller than the amount stored in the state entry. +func TotalEscrowPerDenomInvariants(k *Keeper) sdk.Invariant { + return func(ctx sdk.Context) (string, bool) { + var actualTotalEscrowed sdk.Coins + + expectedTotalEscrowed := k.GetAllTotalEscrowed(ctx) + + portID := k.GetPort(ctx) + transferChannels := k.channelKeeper.GetAllChannelsWithPortPrefix(ctx, portID) + for _, channel := range transferChannels { + escrowAddress := types.GetEscrowAddress(portID, channel.ChannelId) + escrowBalances := k.bankKeeper.GetAllBalances(ctx, escrowAddress) + + actualTotalEscrowed = actualTotalEscrowed.Add(escrowBalances...) + } + + // the actual escrowed amount must be greater than or equal to the expected amount for all denominations + if !actualTotalEscrowed.IsAllGTE(expectedTotalEscrowed) { + return sdk.FormatInvariant( + types.ModuleName, + "total escrow per denom invariance", + fmt.Sprintf("found denom(s) with total escrow amount lower than expected:\nactual total escrowed: %s\nexpected total escrowed: %s", actualTotalEscrowed, expectedTotalEscrowed)), true + } + + return "", false + } +} diff --git a/modules/apps/transfer/keeper/invariants_test.go b/modules/apps/transfer/keeper/invariants_test.go new file mode 100644 index 00000000000..972060ca6b7 --- /dev/null +++ b/modules/apps/transfer/keeper/invariants_test.go @@ -0,0 +1,71 @@ +package keeper_test + +import ( + "cosmossdk.io/math" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/ibc-go/v7/modules/apps/transfer/keeper" + "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" +) + +func (suite *KeeperTestSuite) TestTotalEscrowPerDenomInvariant() { + testCases := []struct { + name string + malleate func() + expPass bool + }{ + { + "success", + func() {}, + true, + }, + { + "fails with broken invariant", + func() { + // set amount for denom higher than actual value in escrow + amount := math.NewInt(200) + suite.chainA.GetSimApp().TransferKeeper.SetTotalEscrowForDenom(suite.chainA.GetContext(), sdk.NewCoin(sdk.DefaultBondDenom, amount)) + }, + false, + }, + } + + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + suite.SetupTest() // reset + path := NewTransferPath(suite.chainA, suite.chainB) + suite.coordinator.Setup(path) + + amount := math.NewInt(100) + + // send coins from chain A to chain B so that we have them in escrow + coin := sdk.NewCoin(sdk.DefaultBondDenom, amount) + msg := types.NewMsgTransfer( + path.EndpointA.ChannelConfig.PortID, + path.EndpointA.ChannelID, + coin, + suite.chainA.SenderAccount.GetAddress().String(), + suite.chainB.SenderAccount.GetAddress().String(), + suite.chainA.GetTimeoutHeight(), 0, "", + ) + + res, err := suite.chainA.SendMsgs(msg) + suite.Require().NoError(err) + suite.Require().NotNil(res) + + tc.malleate() + + out, broken := keeper.TotalEscrowPerDenomInvariants(&suite.chainA.GetSimApp().TransferKeeper)(suite.chainA.GetContext()) + + if tc.expPass { + suite.Require().False(broken) + suite.Require().Empty(out) + } else { + suite.Require().True(broken) + suite.Require().NotEmpty(out) + } + }) + } +} diff --git a/modules/apps/transfer/keeper/keeper.go b/modules/apps/transfer/keeper/keeper.go index 08f75cf105f..736bb11298a 100644 --- a/modules/apps/transfer/keeper/keeper.go +++ b/modules/apps/transfer/keeper/keeper.go @@ -151,7 +151,7 @@ func (k Keeper) IterateDenomTraces(ctx sdk.Context, cb func(denomTrace types.Den func (k Keeper) GetTotalEscrowForDenom(ctx sdk.Context, denom string) sdk.Coin { store := ctx.KVStore(k.storeKey) bz := store.Get(types.TotalEscrowForDenomKey(denom)) - if bz == nil { + if len(bz) == 0 { return sdk.NewCoin(denom, sdk.ZeroInt()) } @@ -162,21 +162,30 @@ func (k Keeper) GetTotalEscrowForDenom(ctx sdk.Context, denom string) sdk.Coin { } // SetTotalEscrowForDenom stores the total amount of source chain tokens that are in escrow. +// Amount is stored in state if and only if it is not equal to zero. The function will panic +// if the amount is negative. func (k Keeper) SetTotalEscrowForDenom(ctx sdk.Context, coin sdk.Coin) { if coin.Amount.IsNegative() { panic(fmt.Sprintf("amount cannot be negative: %s", coin.Amount)) } store := ctx.KVStore(k.storeKey) + key := types.TotalEscrowForDenomKey(coin.Denom) + + if coin.Amount.IsZero() { + store.Delete(key) // delete the key since Cosmos SDK x/bank module will prune any non-zero balances + return + } + bz := k.cdc.MustMarshal(&sdk.IntProto{Int: coin.Amount}) - store.Set(types.TotalEscrowForDenomKey(coin.Denom), bz) + store.Set(key, bz) } // GetAllTotalEscrowed returns the escrow information for all the denominations. func (k Keeper) GetAllTotalEscrowed(ctx sdk.Context) sdk.Coins { var escrows sdk.Coins k.IterateTokensInEscrow(ctx, []byte(types.KeyTotalEscrowPrefix), func(denomEscrow sdk.Coin) bool { - escrows = append(escrows, denomEscrow) + escrows = escrows.Add(denomEscrow) return false }) @@ -192,12 +201,7 @@ func (k Keeper) IterateTokensInEscrow(ctx sdk.Context, prefix []byte, cb func(de defer sdk.LogDeferred(ctx.Logger(), func() error { return iterator.Close() }) for ; iterator.Valid(); iterator.Next() { - keySplit := strings.Split(string(iterator.Key()), "/") - if len(keySplit) < 2 { - continue // key doesn't conform to expected format - } - - denom := strings.Join(keySplit[1:], "/") + denom := strings.TrimPrefix(string(iterator.Key()), fmt.Sprintf("%s/", types.KeyTotalEscrowPrefix)) if strings.TrimSpace(denom) == "" { continue // denom is empty } diff --git a/modules/apps/transfer/keeper/keeper_test.go b/modules/apps/transfer/keeper/keeper_test.go index a620a0938a8..f0a746108cc 100644 --- a/modules/apps/transfer/keeper/keeper_test.go +++ b/modules/apps/transfer/keeper/keeper_test.go @@ -63,7 +63,7 @@ func (suite *KeeperTestSuite) TestSetGetTotalEscrowForDenom() { expPass bool }{ { - "success: with 0 escrow amount", + "success: with non-zero escrow amount", func() {}, true, }, @@ -74,6 +74,13 @@ func (suite *KeeperTestSuite) TestSetGetTotalEscrowForDenom() { }, true, }, + { + "success: escrow amount 0 is not stored", + func() { + expAmount = math.ZeroInt() + }, + true, + }, { "failure: setter panics with negative escrow amount", func() { @@ -88,7 +95,7 @@ func (suite *KeeperTestSuite) TestSetGetTotalEscrowForDenom() { suite.Run(tc.name, func() { suite.SetupTest() // reset - expAmount = math.ZeroInt() + expAmount = math.NewInt(100) ctx := suite.chainA.GetContext() tc.malleate() @@ -97,6 +104,15 @@ func (suite *KeeperTestSuite) TestSetGetTotalEscrowForDenom() { suite.chainA.GetSimApp().TransferKeeper.SetTotalEscrowForDenom(ctx, sdk.NewCoin(denom, expAmount)) total := suite.chainA.GetSimApp().TransferKeeper.GetTotalEscrowForDenom(ctx, denom) suite.Require().Equal(expAmount, total.Amount) + + storeKey := suite.chainA.GetSimApp().GetKey(types.ModuleName) + store := ctx.KVStore(storeKey) + key := types.TotalEscrowForDenomKey(denom) + if expAmount.IsZero() { + suite.Require().False(store.Has(key)) + } else { + suite.Require().True(store.Has(key)) + } } else { suite.Require().PanicsWithError("negative coin amount: -1", func() { suite.chainA.GetSimApp().TransferKeeper.SetTotalEscrowForDenom(ctx, sdk.NewCoin(denom, expAmount)) diff --git a/modules/apps/transfer/keeper/relay_test.go b/modules/apps/transfer/keeper/relay_test.go index c3c5b826f17..e5eeb9a8ef6 100644 --- a/modules/apps/transfer/keeper/relay_test.go +++ b/modules/apps/transfer/keeper/relay_test.go @@ -384,8 +384,10 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { err = suite.chainB.GetSimApp().TransferKeeper.OnRecvPacket(suite.chainB.GetContext(), packet, data) // check total amount in escrow of received token denom on receiving chain - var denom string - var totalEscrow sdk.Coin + var ( + denom string + totalEscrow sdk.Coin + ) if tc.recvIsSource { denom = sdk.DefaultBondDenom } else { diff --git a/modules/apps/transfer/module.go b/modules/apps/transfer/module.go index c278498e590..132f0534e74 100644 --- a/modules/apps/transfer/module.go +++ b/modules/apps/transfer/module.go @@ -94,8 +94,8 @@ func NewAppModule(k keeper.Keeper) AppModule { } // RegisterInvariants implements the AppModule interface -func (AppModule) RegisterInvariants(ir sdk.InvariantRegistry) { - // TODO +func (am AppModule) RegisterInvariants(ir sdk.InvariantRegistry) { + keeper.RegisterInvariants(ir, &am.keeper) } // RegisterServices registers module services. diff --git a/proto/ibc/applications/transfer/v1/query.proto b/proto/ibc/applications/transfer/v1/query.proto index 9a41aa847b3..ff56cc30c8d 100644 --- a/proto/ibc/applications/transfer/v1/query.proto +++ b/proto/ibc/applications/transfer/v1/query.proto @@ -114,6 +114,7 @@ message QueryEscrowAddressResponse { message QueryTotalEscrowForDenomRequest { string denom = 1; } + // QueryTotalEscrowForDenomResponse is the response type for TotalEscrowForDenom RPC method. message QueryTotalEscrowForDenomResponse { cosmos.base.v1beta1.Coin amount = 1 [(gogoproto.nullable) = false];