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 22 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
6 changes: 4 additions & 2 deletions modules/apps/27-interchain-accounts/host/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ type Keeper struct {

scopedKeeper exported.ScopedKeeper

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

// the address capable of executing a MsgUpdateParams message. Typically, this
// should be the x/gov module account.
Expand All @@ -48,7 +49,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 +70,7 @@ func NewKeeper(
accountKeeper: accountKeeper,
scopedKeeper: scopedKeeper,
msgRouter: msgRouter,
queryRouter: queryRouter,
authority: authority,
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ func (suite *KeeperTestSuite) TestNewKeeper() {
suite.chainA.GetSimApp().AccountKeeper,
suite.chainA.GetSimApp().ScopedICAHostKeeper,
suite.chainA.GetSimApp().MsgServiceRouter(),
suite.chainA.GetSimApp().GRPCQueryRouter(),
suite.chainA.GetSimApp().ICAHostKeeper.GetAuthority(),
)
}, true},
Expand All @@ -161,6 +162,7 @@ func (suite *KeeperTestSuite) TestNewKeeper() {
authkeeper.AccountKeeper{}, // empty account keeper
suite.chainA.GetSimApp().ScopedICAHostKeeper,
suite.chainA.GetSimApp().MsgServiceRouter(),
suite.chainA.GetSimApp().GRPCQueryRouter(),
suite.chainA.GetSimApp().ICAHostKeeper.GetAuthority(),
)
}, false},
Expand All @@ -175,6 +177,7 @@ func (suite *KeeperTestSuite) TestNewKeeper() {
suite.chainA.GetSimApp().AccountKeeper,
suite.chainA.GetSimApp().ScopedICAHostKeeper,
suite.chainA.GetSimApp().MsgServiceRouter(),
suite.chainA.GetSimApp().GRPCQueryRouter(),
"", // authority
)
}, false},
Expand Down
39 changes: 39 additions & 0 deletions modules/apps/27-interchain-accounts/host/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (

sdk "github.com/cosmos/cosmos-sdk/types"

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

"github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/types"
ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
)
Expand All @@ -23,6 +25,43 @@ func NewMsgServerImpl(keeper *Keeper) types.MsgServer {
return &msgServer{Keeper: keeper}
}

// ModuleQuerySafe routes the queries to the keeper's query router if they are module_query_safe.
// This handler doesn't use the signer.
func (m msgServer) ModuleQuerySafe(goCtx context.Context, msg *types.MsgModuleQuerySafe) (*types.MsgModuleQuerySafeResponse, error) {
ctx := sdk.UnwrapSDKContext(goCtx)

responses := make([][]byte, len(msg.Requests))
for i, query := range msg.Requests {
isModuleQuerySafe, err := types.IsModuleQuerySafe(query.Path)
if err != nil {
return nil, err
}
if !isModuleQuerySafe {
return nil, errorsmod.Wrapf(ibcerrors.ErrInvalidRequest, "not module query safe: %s", query.Path)
}

route := m.queryRouter.Route(query.Path)
if route == nil {
return nil, errorsmod.Wrapf(ibcerrors.ErrInvalidRequest, "no route to query: %s", query.Path)
}

res, err := route(ctx, &abci.RequestQuery{
Path: query.Path,
Data: query.Data,
})
if err != nil {
return nil, err
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved
}
if res == nil || res.Value == nil {
return nil, errorsmod.Wrapf(ibcerrors.ErrInvalidRequest, "no response for query: %s", query.Path)
}

responses[i] = res.Value
}

return &types.MsgModuleQuerySafeResponse{Responses: responses}, nil
}

// UpdateParams updates the host submodule's params.
func (m msgServer) UpdateParams(goCtx context.Context, msg *types.MsgUpdateParams) (*types.MsgUpdateParamsResponse, error) {
if m.GetAuthority() != msg.Signer {
Expand Down
147 changes: 147 additions & 0 deletions modules/apps/27-interchain-accounts/host/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,157 @@
package keeper_test

import (
sdk "github.com/cosmos/cosmos-sdk/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"

"github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/keeper"
"github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/types"
transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
)

func (suite *KeeperTestSuite) TestModuleQuerySafe() {
var (
msg *types.MsgModuleQuerySafe
expResponses [][]byte
)
testCases := []struct {
name string
malleate func()
expErr error
}{
{
"success",
func() {
queryBz, err := banktypes.NewQueryBalanceRequest(suite.chainA.SenderAccount.GetAddress(), sdk.DefaultBondDenom).Marshal()
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved
suite.Require().NoError(err)

queryReq := types.QueryRequest{
Path: "/cosmos.bank.v1beta1.Query/Balance",
Data: queryBz,
}

msg = types.NewMsgModuleQuerySafe(suite.chainA.GetSimApp().ICAHostKeeper.GetAuthority(), []*types.QueryRequest{&queryReq})
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved

balance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), sdk.DefaultBondDenom)
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved

expResp := banktypes.QueryBalanceResponse{Balance: &balance}
expRespBz, err := expResp.Marshal()
suite.Require().NoError(err)

expResponses = [][]byte{expRespBz}
},
nil,
},
{
"success: multiple queries",
func() {
queryBz, err := banktypes.NewQueryBalanceRequest(suite.chainA.SenderAccount.GetAddress(), sdk.DefaultBondDenom).Marshal()
srdtrk marked this conversation as resolved.
Show resolved Hide resolved
suite.Require().NoError(err)

queryReq := types.QueryRequest{
Path: "/cosmos.bank.v1beta1.Query/Balance",
Data: queryBz,
}

paramsQuery := stakingtypes.QueryParamsRequest{}
paramsQueryBz, err := paramsQuery.Marshal()
suite.Require().NoError(err)

paramsQueryReq := types.QueryRequest{
Path: "/cosmos.staking.v1beta1.Query/Params",
Data: paramsQueryBz,
}

msg = types.NewMsgModuleQuerySafe(suite.chainA.GetSimApp().ICAHostKeeper.GetAuthority(), []*types.QueryRequest{&queryReq, &paramsQueryReq})

balance := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), sdk.DefaultBondDenom)

expResp := banktypes.QueryBalanceResponse{Balance: &balance}
expRespBz, err := expResp.Marshal()
suite.Require().NoError(err)

params, err := suite.chainA.GetSimApp().StakingKeeper.GetParams(suite.chainA.GetContext())
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved
suite.Require().NoError(err)
expParamsResp := stakingtypes.QueryParamsResponse{Params: params}
expParamsRespBz, err := expParamsResp.Marshal()
suite.Require().NoError(err)

expResponses = [][]byte{expRespBz, expParamsRespBz}
},
nil,
},
{
"failure: not module query safe",
func() {
queryBz, err := banktypes.NewQueryBalanceRequest(suite.chainA.SenderAccount.GetAddress(), sdk.DefaultBondDenom).Marshal()
srdtrk marked this conversation as resolved.
Show resolved Hide resolved
suite.Require().NoError(err)

queryReq := types.QueryRequest{
Path: "/cosmos.bank.v1beta1.Query/Balance",
Data: queryBz,
}

paramsQuery := transfertypes.QueryParamsRequest{}
paramsQueryBz, err := paramsQuery.Marshal()
suite.Require().NoError(err)

paramsQueryReq := types.QueryRequest{
Path: "/ibc.applications.transfer.v1.Query/Params",
Data: paramsQueryBz,
}

msg = types.NewMsgModuleQuerySafe(suite.chainA.GetSimApp().ICAHostKeeper.GetAuthority(), []*types.QueryRequest{&queryReq, &paramsQueryReq})
},
ibcerrors.ErrInvalidRequest,
},
{
"failure: invalid query path",
func() {
queryBz, err := banktypes.NewQueryBalanceRequest(suite.chainA.SenderAccount.GetAddress(), sdk.DefaultBondDenom).Marshal()
suite.Require().NoError(err)

queryReq := types.QueryRequest{
Path: "/cosmos.invalid.Query/Invalid",
Data: queryBz,
}

msg = types.NewMsgModuleQuerySafe(suite.chainA.GetSimApp().ICAHostKeeper.GetAuthority(), []*types.QueryRequest{&queryReq})
},
ibcerrors.ErrInvalidRequest,
},
}

for _, tc := range testCases {
tc := tc

suite.Run(tc.name, func() {
suite.SetupTest()

// reset
msg = nil
expResponses = nil

tc.malleate()

ctx := suite.chainA.GetContext()
msgServer := keeper.NewMsgServerImpl(&suite.chainA.GetSimApp().ICAHostKeeper)
res, err := msgServer.ModuleQuerySafe(ctx, msg)

if tc.expErr == nil {
suite.Require().NoError(err)
suite.Require().NotNil(res)

suite.Require().ElementsMatch(expResponses, res.Responses)
} else {
suite.Require().ErrorIs(err, tc.expErr)
suite.Require().Nil(res)
}
})
}
}

func (suite *KeeperTestSuite) TestUpdateParams() {
testCases := []struct {
name string
Expand Down
Loading
Loading