Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: allow module safe queries in ICA #5785

Merged
merged 72 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from 67 commits
Commits
Show all changes
72 commits
Select commit Hold shift + click to select a range
3224eb1
imp: initial modification of tx.proto
srdtrk Jan 29, 2024
6e1d4de
imp: ran 'make proto-all'
srdtrk Jan 29, 2024
d2ce96f
fix: compiler errors
srdtrk Jan 29, 2024
47d8fb1
imp: added query router interface
srdtrk Jan 29, 2024
47fdeed
imp: added queryRouter to icahost keeper
srdtrk Jan 29, 2024
adea94e
imp: improved proto definitions
srdtrk Jan 29, 2024
4c9ee5e
imp: ran 'make proto-all'
srdtrk Jan 29, 2024
4846f80
imp: added sdk.Msg helpers
srdtrk Jan 29, 2024
ea72aed
feat: basic implementation
srdtrk Jan 29, 2024
cf6db81
style: improved field names
srdtrk Jan 29, 2024
322734e
imp: ran 'make proto-all'
srdtrk Jan 29, 2024
0195490
fix: compiler errors
srdtrk Jan 29, 2024
915d515
imp: ran gofumpt
srdtrk Jan 29, 2024
5665d04
feat: tests passing
srdtrk Jan 30, 2024
a8fa43a
feat: tests improved
srdtrk Jan 30, 2024
330bd24
test: removed unneeded code
srdtrk Jan 30, 2024
f4ac337
imp: improved 'IsModuleSafe' function
srdtrk Jan 31, 2024
67a1870
imp: added IsModuleQuerySafe to msg_server
srdtrk Jan 31, 2024
e1853fb
imp: added more test cases
srdtrk Jan 31, 2024
578d006
fix: callbacks compiler
srdtrk Jan 31, 2024
d29250b
Merge branch 'main' into serdar/focus-ica-icq
srdtrk Jan 31, 2024
2298e31
Merge branch 'main' into serdar/focus-ica-icq
srdtrk Mar 11, 2024
53cb24e
fix: non determinancy issues
srdtrk Mar 11, 2024
cf010ee
fix: added query msg to codec
srdtrk Mar 11, 2024
8782d84
Merge branch 'main' into serdar/focus-ica-icq
srdtrk Mar 14, 2024
fe541be
imp: whitelist logic added
srdtrk Mar 14, 2024
a75d1be
Merge branch 'main' into serdar/focus-ica-icq
srdtrk Mar 15, 2024
6ef6445
Merge branch 'main' into serdar/focus-ica-icq
srdtrk Mar 18, 2024
4f9dff7
e2e: new test added
srdtrk Mar 18, 2024
3b7f49b
Merge branch 'main' into serdar/focus-ica-icq
srdtrk Mar 18, 2024
75e3fdf
fix: new test
srdtrk Mar 18, 2024
79acd69
fix: test
srdtrk Mar 18, 2024
0243af9
Merge branch 'main' into serdar/focus-ica-icq
srdtrk Mar 18, 2024
8a60e62
fix: e2e
srdtrk Mar 19, 2024
98e850d
fix: e2e
srdtrk Mar 19, 2024
678c98d
imp(e2e): added the QueryTxsByEvents function
srdtrk Mar 20, 2024
49acca4
fix: e2e
srdtrk Mar 20, 2024
78917a1
e2e: lint fix
srdtrk Mar 20, 2024
0b726ee
Merge branch 'main' into serdar/focus-ica-icq
srdtrk Mar 20, 2024
62d7358
fix: e2e
srdtrk Mar 20, 2024
22d0fc8
e2e: debug
srdtrk Mar 20, 2024
f61f0c6
fix: e2e
srdtrk Mar 20, 2024
3f24164
test: debug helpers
srdtrk Mar 20, 2024
d31dbbb
debug
srdtrk Mar 20, 2024
dfbef77
test: added codec_test case
srdtrk Mar 20, 2024
a1597a9
imp: additional test case
srdtrk Mar 20, 2024
85038bf
imp: added important unit test
srdtrk Mar 20, 2024
4fa3c28
r4r
srdtrk Mar 20, 2024
91a985a
e2e: debug
srdtrk Mar 20, 2024
197c645
imp: added logs
srdtrk Mar 20, 2024
8dc0dd6
fix: e2e
srdtrk Mar 20, 2024
660e088
fix: e2e
srdtrk Mar 20, 2024
b971791
fix: e2e
srdtrk Mar 20, 2024
4bc7934
Merge branch 'main' into serdar/focus-ica-icq
srdtrk Mar 24, 2024
1569f76
imp: added height to proto response
srdtrk Mar 24, 2024
2a553e7
imp: ran 'make proto-all'
srdtrk Mar 24, 2024
0b031b7
imp: added height
srdtrk Mar 24, 2024
b746644
e2e: updated e2e to test height
srdtrk Mar 24, 2024
3a55334
imp: review suggestions
srdtrk Mar 25, 2024
4878ea7
e2e: remove unneeded log
srdtrk Mar 25, 2024
8ab8f77
refactor: refactored 'ExtractValueFromEvents'
srdtrk Mar 25, 2024
78e21c9
Merge branch 'main' into serdar/focus-ica-icq
srdtrk Mar 26, 2024
779848b
e2e: review item
srdtrk Mar 26, 2024
f2fb96c
imp: review item
srdtrk Mar 26, 2024
15a1ccd
nit: review item
srdtrk Mar 26, 2024
d76ff53
docs: added godocs
srdtrk Mar 26, 2024
4083edc
test: unit test for mqsWhitelist added
srdtrk Mar 26, 2024
17531fa
imp: added logging
srdtrk Mar 26, 2024
b86b3a4
style: rename to allow list
srdtrk Mar 26, 2024
a854551
Merge branch 'main' into serdar/focus-ica-icq
crodriguezvega Mar 26, 2024
72561cc
add changelog
crodriguezvega Mar 26, 2024
ec6df04
Merge branch 'main' into serdar/focus-ica-icq
crodriguezvega Mar 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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) {
srdtrk marked this conversation as resolved.
Show resolved Hide resolved
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)
}

// NewModuleQuerySafeWhitelist is a wrapper around newModuleQuerySafeWhitelist to allow the function to be directly called in tests.
func NewModuleQuerySafeWhitelist() []string {
return newModuleQuerySafeWhitelist()
}
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

// whitelist of module safe queries
mqsWhitelist []string
srdtrk marked this conversation as resolved.
Show resolved Hide resolved
damiannolan marked this conversation as resolved.
Show resolved Hide resolved

// 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,
srdtrk marked this conversation as resolved.
Show resolved Hide resolved
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,
mqsWhitelist: newModuleQuerySafeWhitelist(),
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)
}

// newModuleQuerySafeWhitelist returns a list of all query paths labeled with module_query_safe in the proto files.
func newModuleQuerySafeWhitelist() []string {
srdtrk marked this conversation as resolved.
Show resolved Hide resolved
protoFiles, err := gogoproto.MergedRegistry()
if err != nil {
panic(err)
}

whitelist := []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
whitelist = append(whitelist, fmt.Sprintf("/%s/%s", sd.FullName(), md.Name()))
}
}
return true
})

return whitelist
}
Loading
Loading