Merge branch 'main' into damian/rename-counterparty-payee-rpc

CHANGELOG.md

@@ -51,6 +51,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 ### State Machine Breaking
 
 ### Improvements
+
 * (cleanup) [\#1335](https://github.com/cosmos/ibc-go/pull/1335/) `gofumpt -w -l .` to standardize the code layout more strictly than `go fmt ./...`
 * (transfer) [\#1342](https://github.com/cosmos/ibc-go/pull/1342) `DenomTrace` grpc now takes in either an `ibc denom` or a `hash` instead of only accepting a `hash`.
 * (modules/core/keeper) [\#1284](https://github.com/cosmos/ibc-go/pull/1284) Add sanity check for the keepers passed into `ibckeeper.NewKeeper`. `ibckeeper.NewKeeper` now panics if any of the keepers passed in is empty.
@@ -62,6 +63,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (app/29-fee) [\#1341](https://github.com/cosmos/ibc-go/pull/1341) Check if the fee module is locked and if the fee module is enabled before refunding all fees
 * (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.
 
 ### Features
 
@@ -72,6 +74,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (apps/29-fee) [\#1225](https://github.com/cosmos/ibc-go/pull/1225) Adding Query/FeeEnabledChannel and Query/FeeEnabledChannels with CLIs to ICS29 fee middleware.
 * (modules/apps/29-fee) [\#1230](https://github.com/cosmos/ibc-go/pull/1230) Adding CLI command for getting incentivized packets for a specific channel-id. 
 * (modules/apps/transfer) [\#1416](https://github.com/cosmos/ibc-go/pull/1416) Adding gRPC endpoint for getting an escrow account for a given port-id and channel-id. 
+* (modules/apps/27-interchain-accounts) [\#1512](https://github.com/cosmos/ibc-go/pull/1512) Allowing ICA modules to handle all message types with "*".
 
 ### Bug Fixes
 

docs/apps/interchain-accounts/parameters.md

@@ -47,4 +47,12 @@ For example, a Cosmos SDK based chain that elects to provide hosted Interchain A
     "host_enabled": true,
     "allow_messages": ["/cosmos.staking.v1beta1.MsgDelegate", "/cosmos.gov.v1beta1.MsgVote"]
 }
+```
+There is also a special wildcard `"*"` message type which allows any type of message to be executed by the interchain account. This must be the only message in the `allow_messages` array.
+
+```
+"params": {
+    "host_enabled": true,
+    "allow_messages": ["*"]
+}
 ```

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

@@ -6,6 +6,7 @@
 - 2020/08/06: Revisions per review & to reference version
 - 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
 
 ## Status
 
@@ -35,21 +36,20 @@ Two-thirds of the validator set (the quorum for governance, module participation
 We elect not to deal with chains which have actually halted, which is necessarily Byzantine behaviour and in which case token recovery is not likely possible anyways (in-flight packets cannot be timed-out, but the relative impact of that is minor).
 
 1. Require Tendermint light clients (ICS 07) to be created with the following additional flags
-    1. `allow_governance_override_after_expiry` (boolean, default false)
+    1. `allow_update_after_expiry` (boolean, default true). Note that this flag has been deprecated, it remains to signal intent but checks against this value will not be enforced.
 1. Require Tendermint light clients (ICS 07) to expose the following additional internal query functions
     1. `Expired() boolean`, which returns whether or not the client has passed the trusting period since the last update (in which case no headers can be validated)
 1. Require Tendermint light clients (ICS 07) & solo machine clients (ICS 06) to be created with the following additional flags
-    1. `allow_governance_override_after_misbehaviour` (boolean, default false)
+    1. `allow_update_after_misbehaviour` (boolean, default true). Note that this flag has been deprecated, it remains to signal intent but checks against this value will not be enforced.
 1. Require Tendermint light clients (ICS 07) to expose the following additional state mutation functions
     1. `Unfreeze()`, which unfreezes a light client after misbehaviour and clears any frozen height previously set
 1. Add a new governance proposal type, `ClientUpdateProposal`, in the `x/ibc` module
     1. Extend the base `Proposal` with two client identifiers (`string`). 
     1. The first client identifier is the proposed client to be updated. This client must be either frozen or expired.
     1. The second client is a substitute client. It carries all the state for the client which may be updated. It must have identitical client and chain parameters to the client which may be updated (except for latest height, frozen height, and chain-id). It should be continually updated during the voting period. 
-    1. If this governance proposal passes, the client on trial will be updated to the latest state of the substitute, if and only if:
-        1. `allow_governance_override_after_expiry` is true and the client has expired (`Expired()` returns true)
-        1. `allow_governance_override_after_misbehaviour` is true and the client has been frozen (`Frozen()` returns true)
-            1. In this case, additionally, the client is unfrozen by calling `Unfreeze()`
+    1. If this governance proposal passes, the client on trial will be updated to the latest state of the substitute.
+
+    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 willing to perform a code migration to do the same.
 
 
 Note that clients frozen due to misbehaviour must wait for the evidence to expire to avoid becoming refrozen. 
@@ -62,7 +62,6 @@ This ADR does not address planned upgrades, which are handled separately as per
 
 - Establishes a mechanism for client recovery in the case of expiry
 - Establishes a mechanism for client recovery in the case of misbehaviour
-- Clients can elect to disallow this recovery mechanism if they do not wish to allow for it
 - Constructing an ClientUpdate Proposal is as difficult as creating a new client
 
 ### Negative

docs/ibc/proposals.md

@@ -46,7 +46,6 @@ See also the relevant documentation: [ADR-026, IBC client recovery mechanisms](.
 
 ### Preconditions 
 - The chain is updated with ibc-go >= v1.1.0.
-- Recovery parameters are set to `true` for the Tendermint light client (this determines if a governance proposal can be used). If the recovery parameters are set to `false`, recovery will require custom migration code.
 - The client identifier of an active client for the same counterparty chain.
 - The governance deposit.
 
@@ -67,7 +66,7 @@ Check if the client is attached to the expected `chain-id`. For example, for an
 }
 ```
 
-The client is attached to the expected Akash `chain-id` and the recovery parameters (`allow_update_after_expiry` and `allow_update_after_misbehaviour`) are set to `true`.
+The client is attached to the expected Akash `chain-id`. Note that although the parameters (`allow_update_after_expiry` and `allow_update_after_misbehaviour`) exist to signal intent, these parameters have been deprecated and will not enforce any checks on the revival of client. See ADR-026 for more context on this deprecation.
 
 ### Step 2
 

docs/ibc/proto-docs.md

@@ -5019,8 +5019,8 @@ and a possible frozen height.
 | `latest_height` | [ibc.core.client.v1.Height](#ibc.core.client.v1.Height) |  | Latest height the client was updated to |
 | `proof_specs` | [ics23.ProofSpec](#ics23.ProofSpec) | repeated | Proof specifications used in verifying counterparty state |
 | `upgrade_path` | [string](#string) | repeated | Path at which next upgraded client will be committed. Each element corresponds to the key for a single CommitmentProof in the chained proof. NOTE: ClientState must stored under `{upgradePath}/{upgradeHeight}/clientState` ConsensusState must be stored under `{upgradepath}/{upgradeHeight}/consensusState` For SDK chains using the default upgrade module, upgrade_path should be []string{"upgrade", "upgradedIBCState"}` |
-| `allow_update_after_expiry` | [bool](#bool) |  | This flag, when set to true, will allow governance to recover a client which has expired |
-| `allow_update_after_misbehaviour` | [bool](#bool) |  | This flag, when set to true, will allow governance to unfreeze a client whose chain has experienced a misbehaviour event |
+| `allow_update_after_expiry` | [bool](#bool) |  | **Deprecated.** allow_update_after_expiry is deprecated |
+| `allow_update_after_misbehaviour` | [bool](#bool) |  | **Deprecated.** allow_update_after_misbehaviour is deprecated |
 
 
 

modules/apps/27-interchain-accounts/host/keeper/relay_test.go

@@ -29,6 +29,45 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
 		malleate func()
 		expPass  bool
 	}{
+		{
+			"interchain account successfully executes an arbitrary message type using the * (allow all message types) param",
+			func() {
+				interchainAccountAddr, found := suite.chainB.GetSimApp().ICAHostKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID)
+				suite.Require().True(found)
+
+				// Populate the gov keeper in advance with an active proposal
+				testProposal := &govtypes.TextProposal{
+					Title:       "IBC Gov Proposal",
+					Description: "tokens for all!",
+				}
+
+				proposal, err := govtypes.NewProposal(testProposal, govtypes.DefaultStartingProposalID, time.Now(), time.Now().Add(time.Hour))
+				suite.Require().NoError(err)
+
+				suite.chainB.GetSimApp().GovKeeper.SetProposal(suite.chainB.GetContext(), proposal)
+				suite.chainB.GetSimApp().GovKeeper.ActivateVotingPeriod(suite.chainB.GetContext(), proposal)
+
+				msg := &govtypes.MsgVote{
+					ProposalId: govtypes.DefaultStartingProposalID,
+					Voter:      interchainAccountAddr,
+					Option:     govtypes.OptionYes,
+				}
+
+				data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []sdk.Msg{msg})
+				suite.Require().NoError(err)
+
+				icaPacketData := icatypes.InterchainAccountPacketData{
+					Type: icatypes.EXECUTE_TX,
+					Data: data,
+				}
+
+				packetData = icaPacketData.GetBytes()
+
+				params := types.NewParams(true, []string{"*"})
+				suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
+			},
+			true,
+		},
 		{
 			"interchain account successfully executes banktypes.MsgSend",
 			func() {

modules/apps/27-interchain-accounts/host/types/keys.go

@@ -14,6 +14,11 @@ const (
 
 // ContainsMsgType returns true if the sdk.Msg TypeURL is present in allowMsgs, otherwise false
 func ContainsMsgType(allowMsgs []string, msg sdk.Msg) bool {
+	// check that wildcard * option for allowing all message types is the only string in the array, if so, return true
+	if len(allowMsgs) == 1 && allowMsgs[0] == "*" {
+		return true
+	}
+
 	for _, v := range allowMsgs {
 		if v == sdk.MsgTypeURL(msg) {
 			return true

modules/light-clients/07-tendermint/types/proposal_handle.go

@@ -12,18 +12,17 @@ import (
 )
 
 // CheckSubstituteAndUpdateState will try to update the client with the state of the
-// substitute if and only if the proposal passes and one of the following conditions are
-// satisfied:
-//	1) AllowUpdateAfterMisbehaviour and Status() == Frozen
-// 	2) AllowUpdateAfterExpiry=true and Status() == Expired
+// substitute.
+//
+// AllowUpdateAfterMisbehaviour and AllowUpdateAfterExpiry have been deprecated.
+// Please see ADR 026 for more information.
 //
 // The following must always be true:
 //	- The substitute client is the same type as the subject client
 //	- The subject and substitute client states match in all parameters (expect frozen height, latest height, and chain-id)
 //
 // In case 1) before updating the client, the client will be unfrozen by resetting
-// the FrozenHeight to the zero Height. If a client is frozen and AllowUpdateAfterMisbehaviour
-// is set to true, the client will be unexpired even if AllowUpdateAfterExpiry is set to false.
+// the FrozenHeight to the zero Height.
 func (cs ClientState) CheckSubstituteAndUpdateState(
 	ctx sdk.Context, cdc codec.BinaryCodec, subjectClientStore,
 	substituteClientStore sdk.KVStore, substituteClient exported.ClientState,
@@ -39,23 +38,9 @@ func (cs ClientState) CheckSubstituteAndUpdateState(
 		return nil, sdkerrors.Wrap(clienttypes.ErrInvalidSubstitute, "subject client state does not match substitute client state")
 	}
 
-	switch cs.Status(ctx, subjectClientStore, cdc) {
-
-	case exported.Frozen:
-		if !cs.AllowUpdateAfterMisbehaviour {
-			return nil, sdkerrors.Wrap(clienttypes.ErrUpdateClientFailed, "client is not allowed to be unfrozen")
-		}
-
+	if cs.Status(ctx, subjectClientStore, cdc) == exported.Frozen {
 		// unfreeze the client
 		cs.FrozenHeight = clienttypes.ZeroHeight()
-
-	case exported.Expired:
-		if !cs.AllowUpdateAfterExpiry {
-			return nil, sdkerrors.Wrap(clienttypes.ErrUpdateClientFailed, "client is not allowed to be unexpired")
-		}
-
-	default:
-		return nil, sdkerrors.Wrap(clienttypes.ErrUpdateClientFailed, "client cannot be updated with proposal")
 	}
 
 	// copy consensus states and processed time from substitute to subject
@@ -101,6 +86,11 @@ func IsMatchingClientState(subject, substitute ClientState) bool {
 	substitute.FrozenHeight = clienttypes.ZeroHeight()
 	subject.ChainId = ""
 	substitute.ChainId = ""
+	// sets both sets of flags to true as these flags have been DEPRECATED, see ADR-026 for more information
+	subject.AllowUpdateAfterExpiry = true
+	substitute.AllowUpdateAfterExpiry = true
+	subject.AllowUpdateAfterMisbehaviour = true
+	substitute.AllowUpdateAfterMisbehaviour = true
 
 	return reflect.DeepEqual(subject, substitute)
 }