Skip to content

Commit

Permalink
feat: allow module safe queries in ICA (#5785)
Browse files Browse the repository at this point in the history
* imp: initial modification of tx.proto

* imp: ran 'make proto-all'

* fix: compiler errors

* imp: added query router interface

* imp: added queryRouter to icahost keeper

* imp: improved proto definitions

* imp: ran 'make proto-all'

* imp: added sdk.Msg helpers

* feat: basic implementation

* style: improved field names

* imp: ran 'make proto-all'

* fix: compiler errors

* imp: ran gofumpt

* feat: tests passing

* feat: tests improved

* test: removed unneeded code

* imp: improved 'IsModuleSafe' function

* imp: added IsModuleQuerySafe to msg_server

* imp: added more test cases

* fix: callbacks compiler

* fix: non determinancy issues

* fix: added query msg to codec

* imp: whitelist logic added

* e2e: new test added

* fix: new test

* fix: test

* fix: e2e

* fix: e2e

* imp(e2e): added the QueryTxsByEvents function

* fix: e2e

* e2e: lint fix

* fix: e2e

* e2e: debug

* fix: e2e

* test: debug helpers

* debug

* test: added codec_test case

* imp: additional test case

* imp: added important unit test

* r4r

* e2e: debug

* imp: added logs

* fix: e2e

* fix: e2e

* fix: e2e

* imp: added height to proto response

* imp: ran 'make proto-all'

* imp: added height

* e2e: updated e2e to test height

* imp: review suggestions

* e2e: remove unneeded log

* refactor: refactored 'ExtractValueFromEvents'

* e2e: review item

* imp: review item

* nit: review item

* docs: added godocs

* test: unit test for mqsWhitelist added

* imp: added logging

* style: rename to allow list

* add changelog

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
  • Loading branch information
srdtrk and crodriguezvega authored Mar 27, 2024
1 parent e7c74b0 commit eecfa5c
Show file tree
Hide file tree
Showing 21 changed files with 1,355 additions and 47 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Features

* (apps/27-interchain-accounts) [\#5785](https://github.com/cosmos/ibc-go/pull/5785) Introduce a new tx message that ICA host submodule can use to query the chain (only those marked with `module_query_safe`) and write the responses to the acknowledgement.

### Bug Fixes

## [v8.1.0](https://github.com/cosmos/ibc-go/releases/tag/v8.1.0) - 2024-01-31
Expand Down
161 changes: 161 additions & 0 deletions e2e/tests/interchain_accounts/query_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
//go:build !test_e2e

package interchainaccounts

import (
"context"
"encoding/hex"
"encoding/json"
"testing"
"time"

"github.com/cosmos/gogoproto/proto"
"github.com/strangelove-ventures/interchaintest/v8/testutil"
testifysuite "github.com/stretchr/testify/suite"

sdk "github.com/cosmos/cosmos-sdk/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"

"github.com/cosmos/ibc-go/e2e/testsuite"
"github.com/cosmos/ibc-go/e2e/testvalues"
controllertypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/controller/types"
icahosttypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/types"
icatypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/types"
channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
ibctesting "github.com/cosmos/ibc-go/v8/testing"
)

func TestInterchainAccountsQueryTestSuite(t *testing.T) {
testifysuite.Run(t, new(InterchainAccountsQueryTestSuite))
}

type InterchainAccountsQueryTestSuite struct {
testsuite.E2ETestSuite
}

func (s *InterchainAccountsQueryTestSuite) TestInterchainAccountsQuery() {
t := s.T()
ctx := context.TODO()

// setup relayers and connection-0 between two chains
// channel-0 is a transfer channel but it will not be used in this test case
relayer, _ := s.SetupChainsRelayerAndChannel(ctx, nil)
chainA, chainB := s.GetChains()

// setup 2 accounts: controller account on chain A, a second chain B account.
// host account will be created when the ICA is registered
controllerAccount := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount)
controllerAddress := controllerAccount.FormattedAddress()
chainBAccount := s.CreateUserOnChainB(ctx, testvalues.StartingTokenAmount)
var hostAccount string

t.Run("broadcast MsgRegisterInterchainAccount", func(t *testing.T) {
// explicitly set the version string because we don't want to use incentivized channels.
version := icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version, channeltypes.UNORDERED)

txResp := s.BroadcastMessages(ctx, chainA, controllerAccount, msgRegisterAccount)
s.AssertTxSuccess(txResp)
})

t.Run("start relayer", func(t *testing.T) {
s.StartRelayer(relayer)
})

t.Run("verify interchain account", func(t *testing.T) {
var err error
hostAccount, err = s.QueryInterchainAccount(ctx, chainA, controllerAddress, ibctesting.FirstConnectionID)
s.Require().NoError(err)
s.Require().NotZero(len(hostAccount))

channels, err := relayer.GetChannels(ctx, s.GetRelayerExecReporter(), chainA.Config().ChainID)
s.Require().NoError(err)
s.Require().Len(channels, 2)
})

t.Run("query via interchain account", func(t *testing.T) {
// the host account need not be funded
t.Run("broadcast query packet", func(t *testing.T) {
balanceQuery := banktypes.NewQueryBalanceRequest(chainBAccount.Address(), chainB.Config().Denom)
queryBz, err := balanceQuery.Marshal()
s.Require().NoError(err)

queryMsg := icahosttypes.NewMsgModuleQuerySafe(hostAccount, []*icahosttypes.QueryRequest{
{
Path: "/cosmos.bank.v1beta1.Query/Balance",
Data: queryBz,
},
})

cdc := testsuite.Codec()
bz, err := icatypes.SerializeCosmosTx(cdc, []proto.Message{queryMsg}, icatypes.EncodingProtobuf)
s.Require().NoError(err)

packetData := icatypes.InterchainAccountPacketData{
Type: icatypes.EXECUTE_TX,
Data: bz,
Memo: "e2e",
}

icaQueryMsg := controllertypes.NewMsgSendTx(controllerAddress, ibctesting.FirstConnectionID, uint64(time.Hour.Nanoseconds()), packetData)

txResp := s.BroadcastMessages(ctx, chainA, controllerAccount, icaQueryMsg)
s.AssertTxSuccess(txResp)

s.Require().NoError(testutil.WaitForBlocks(ctx, 10, chainA, chainB))
})

t.Run("verify query response", func(t *testing.T) {
var expQueryHeight uint64

ack := &channeltypes.Acknowledgement_Result{}
t.Run("retrieve acknowledgement", func(t *testing.T) {
txSearchRes, err := s.QueryTxsByEvents(
ctx, chainB, 1, 1,
"message.action='/ibc.core.channel.v1.MsgRecvPacket'", "",
)
s.Require().NoError(err)
s.Require().Len(txSearchRes.Txs, 1)

expQueryHeight = uint64(txSearchRes.Txs[0].Height)

ackHexValue, isFound := s.ExtractValueFromEvents(
txSearchRes.Txs[0].Events,
channeltypes.EventTypeWriteAck,
channeltypes.AttributeKeyAckHex,
)
s.Require().True(isFound)
s.Require().NotEmpty(ackHexValue)

ackBz, err := hex.DecodeString(ackHexValue)
s.Require().NoError(err)

err = json.Unmarshal(ackBz, ack)
s.Require().NoError(err)
})

icaAck := &sdk.TxMsgData{}
t.Run("unmarshal ica response", func(t *testing.T) {
err := proto.Unmarshal(ack.Result, icaAck)
s.Require().NoError(err)
s.Require().Len(icaAck.GetMsgResponses(), 1)
})

queryTxResp := &icahosttypes.MsgModuleQuerySafeResponse{}
t.Run("unmarshal MsgModuleQuerySafeResponse", func(t *testing.T) {
err := proto.Unmarshal(icaAck.MsgResponses[0].Value, queryTxResp)
s.Require().NoError(err)
s.Require().Len(queryTxResp.Responses, 1)
s.Require().Equal(expQueryHeight, queryTxResp.Height)
})

balanceResp := &banktypes.QueryBalanceResponse{}
t.Run("unmarshal and verify bank query response", func(t *testing.T) {
err := proto.Unmarshal(queryTxResp.Responses[0], balanceResp)
s.Require().NoError(err)
s.Require().Equal(chainB.Config().Denom, balanceResp.Balance.Denom)
s.Require().Equal(testvalues.StartingTokenAmount, balanceResp.Balance.Amount.Int64())
})
})
})
}
2 changes: 2 additions & 0 deletions e2e/testsuite/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/module/testutil"
txtypes "github.com/cosmos/cosmos-sdk/types/tx"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
"github.com/cosmos/cosmos-sdk/x/authz"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
Expand Down Expand Up @@ -88,6 +89,7 @@ func codecAndEncodingConfig() (*codec.ProtoCodec, simappparams.EncodingConfig) {
grouptypes.RegisterInterfaces(cfg.InterfaceRegistry)
proposaltypes.RegisterInterfaces(cfg.InterfaceRegistry)
authz.RegisterInterfaces(cfg.InterfaceRegistry)
txtypes.RegisterInterfaces(cfg.InterfaceRegistry)

cdc := codec.NewProtoCodec(cfg.InterfaceRegistry)
return cdc, cfg
Expand Down
60 changes: 60 additions & 0 deletions e2e/testsuite/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,16 @@ import (
sdkmath "cosmossdk.io/math"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/client/tx"
sdk "github.com/cosmos/cosmos-sdk/types"
signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing"
authtx "github.com/cosmos/cosmos-sdk/x/auth/tx"
govtypesv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
govtypesv1beta1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"

abci "github.com/cometbft/cometbft/abci/types"

"github.com/cosmos/ibc-go/e2e/testsuite/sanitize"
"github.com/cosmos/ibc-go/e2e/testvalues"
feetypes "github.com/cosmos/ibc-go/v8/modules/apps/29-fee/types"
Expand Down Expand Up @@ -297,3 +300,60 @@ func (s *E2ETestSuite) PruneAcknowledgements(
msg := channeltypes.NewMsgPruneAcknowledgements(portID, channelID, limit, user.FormattedAddress())
return s.BroadcastMessages(ctx, chain, user, msg)
}

// QueryTxsByEvents runs the QueryTxsByEvents command on the given chain.
// https://github.com/cosmos/cosmos-sdk/blob/65ab2530cc654fd9e252b124ed24cbaa18023b2b/x/auth/client/cli/query.go#L33
func (*E2ETestSuite) QueryTxsByEvents(
ctx context.Context, chain ibc.Chain,
page, limit int, query, orderBy string,
) (*sdk.SearchTxsResult, error) {
cosmosChain, ok := chain.(*cosmos.CosmosChain)
if !ok {
return nil, fmt.Errorf("QueryTxsByEvents must be passed a cosmos.CosmosChain")
}

cmd := []string{"txs", "--query", query}
if orderBy != "" {
cmd = append(cmd, "--order_by", orderBy)
}
if page != 0 {
cmd = append(cmd, "--"+flags.FlagPage, strconv.Itoa(page))
}
if limit != 0 {
cmd = append(cmd, "--"+flags.FlagLimit, strconv.Itoa(limit))
}

stdout, _, err := cosmosChain.GetNode().ExecQuery(ctx, cmd...)
if err != nil {
return nil, err
}

result := &sdk.SearchTxsResult{}
err = Codec().UnmarshalJSON(stdout, result)
if err != nil {
return nil, err
}

return result, nil
}

// ExtractValueFromEvents extracts the value of an attribute from a list of events.
// If the attribute is not found, the function returns an empty string and false.
// If the attribute is found, the function returns the value and true.
func (*E2ETestSuite) ExtractValueFromEvents(events []abci.Event, eventType, attrKey string) (string, bool) {
for _, event := range events {
if event.Type != eventType {
continue
}

for _, attr := range event.Attributes {
if attr.Key != attrKey {
continue
}

return attr.Value, true
}
}

return "", false
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,8 @@ import (
func (k Keeper) GetAppMetadata(ctx sdk.Context, portID, channelID string) (icatypes.Metadata, error) {
return k.getAppMetadata(ctx, portID, channelID)
}

// NewModuleQuerySafeAllowList is a wrapper around newModuleQuerySafeAllowList to allow the function to be directly called in tests.
func NewModuleQuerySafeAllowList() []string {
return newModuleQuerySafeAllowList()
}
53 changes: 51 additions & 2 deletions modules/apps/27-interchain-accounts/host/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ import (
"fmt"
"strings"

gogoproto "github.com/cosmos/gogoproto/proto"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/reflect/protoreflect"

msgv1 "cosmossdk.io/api/cosmos/msg/v1"
queryv1 "cosmossdk.io/api/cosmos/query/v1"
errorsmod "cosmossdk.io/errors"
"cosmossdk.io/log"
storetypes "cosmossdk.io/store/types"
Expand Down Expand Up @@ -36,7 +42,11 @@ type Keeper struct {

scopedKeeper exported.ScopedKeeper

msgRouter icatypes.MessageRouter
msgRouter icatypes.MessageRouter
queryRouter icatypes.QueryRouter

// mqsAllowList is a list of all module safe query paths
mqsAllowList []string

// the address capable of executing a MsgUpdateParams message. Typically, this
// should be the x/gov module account.
Expand All @@ -48,7 +58,7 @@ func NewKeeper(
cdc codec.Codec, key storetypes.StoreKey, legacySubspace icatypes.ParamSubspace,
ics4Wrapper porttypes.ICS4Wrapper, channelKeeper icatypes.ChannelKeeper, portKeeper icatypes.PortKeeper,
accountKeeper icatypes.AccountKeeper, scopedKeeper exported.ScopedKeeper, msgRouter icatypes.MessageRouter,
authority string,
queryRouter icatypes.QueryRouter, authority string,
) Keeper {
// ensure ibc interchain accounts module account is set
if addr := accountKeeper.GetModuleAddress(icatypes.ModuleName); addr == nil {
Expand All @@ -69,6 +79,8 @@ func NewKeeper(
accountKeeper: accountKeeper,
scopedKeeper: scopedKeeper,
msgRouter: msgRouter,
queryRouter: queryRouter,
mqsAllowList: newModuleQuerySafeAllowList(),
authority: authority,
}
}
Expand Down Expand Up @@ -261,3 +273,40 @@ func (k Keeper) SetParams(ctx sdk.Context, params types.Params) {
bz := k.cdc.MustMarshal(&params)
store.Set([]byte(types.ParamsKey), bz)
}

// newModuleQuerySafeAllowList returns a list of all query paths labeled with module_query_safe in the proto files.
func newModuleQuerySafeAllowList() []string {
protoFiles, err := gogoproto.MergedRegistry()
if err != nil {
panic(err)
}

allowList := []string{}
protoFiles.RangeFiles(func(fd protoreflect.FileDescriptor) bool {
for i := 0; i < fd.Services().Len(); i++ {
// Get the service descriptor
sd := fd.Services().Get(i)

// Skip services that are annotated with the "cosmos.msg.v1.service" option.
if ext := proto.GetExtension(sd.Options(), msgv1.E_Service); ext != nil && ext.(bool) {
continue
}

for j := 0; j < sd.Methods().Len(); j++ {
// Get the method descriptor
md := sd.Methods().Get(j)

// Skip methods that are not annotated with the "cosmos.query.v1.module_query_safe" option.
if ext := proto.GetExtension(md.Options(), queryv1.E_ModuleQuerySafe); ext == nil || !ext.(bool) {
continue
}

// Add the method to the whitelist
allowList = append(allowList, fmt.Sprintf("/%s/%s", sd.FullName(), md.Name()))
}
}
return true
})

return allowList
}
Loading

0 comments on commit eecfa5c

Please sign in to comment.