From 038cbce6d9652332abc356ddfe11fa9e97e5a04d Mon Sep 17 00:00:00 2001 From: Nicolas Lara Date: Wed, 26 Jul 2023 20:22:36 +0200 Subject: [PATCH] Use query routes in async_icq (#67) * better gas management * better test * lint * lint (cherry picked from commit 3a9d46a7c2a42e03951f3e88d324d505c1e53435) # Conflicts: # modules/async-icq/keeper/relay_test.go --- modules/async-icq/keeper/keeper.go | 7 +- modules/async-icq/keeper/relay.go | 16 ++++- modules/async-icq/keeper/relay_test.go | 93 +++++++++++++++++++++++++ modules/async-icq/testing/simapp/app.go | 2 +- 4 files changed, 113 insertions(+), 5 deletions(-) diff --git a/modules/async-icq/keeper/keeper.go b/modules/async-icq/keeper/keeper.go index 63770444..281611ec 100644 --- a/modules/async-icq/keeper/keeper.go +++ b/modules/async-icq/keeper/keeper.go @@ -6,6 +6,7 @@ import ( "github.com/cosmos/ibc-apps/modules/async-icq/v4/types" "github.com/tendermint/tendermint/libs/log" + "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/codec" storetypes "github.com/cosmos/cosmos-sdk/store/types" sdk "github.com/cosmos/cosmos-sdk/types" @@ -28,14 +29,14 @@ type Keeper struct { scopedKeeper capabilitykeeper.ScopedKeeper - querier sdk.Queryable + queryRouter *baseapp.GRPCQueryRouter } // NewKeeper creates a new interchain query Keeper instance func NewKeeper( cdc codec.BinaryCodec, key storetypes.StoreKey, paramSpace paramtypes.Subspace, ics4Wrapper types.ICS4Wrapper, channelKeeper types.ChannelKeeper, portKeeper types.PortKeeper, - scopedKeeper capabilitykeeper.ScopedKeeper, querier sdk.Queryable, + scopedKeeper capabilitykeeper.ScopedKeeper, queryRouter *baseapp.GRPCQueryRouter, ) Keeper { // set KeyTable if it has not already been set if !paramSpace.HasKeyTable() { @@ -50,7 +51,7 @@ func NewKeeper( channelKeeper: channelKeeper, portKeeper: portKeeper, scopedKeeper: scopedKeeper, - querier: querier, + queryRouter: queryRouter, } } diff --git a/modules/async-icq/keeper/relay.go b/modules/async-icq/keeper/relay.go index 6e106402..4e5d65f9 100644 --- a/modules/async-icq/keeper/relay.go +++ b/modules/async-icq/keeper/relay.go @@ -41,9 +41,23 @@ func (k Keeper) executeQuery(ctx sdk.Context, reqs []abci.RequestQuery) ([]byte, return nil, err } - resp := k.querier.Query(req) + route := k.queryRouter.Route(req.Path) + if route == nil { + return nil, errors.Wrapf(sdkerrors.ErrUnauthorized, "no route found for: %s", req.Path) + } + + resp, err := route(ctx, abci.RequestQuery{ + Data: req.Data, + Path: req.Path, + }) + if err != nil { + return nil, err + } + // Remove non-deterministic fields from response resps[i] = abci.ResponseQuery{ + // Codespace is not currently part of consensus, but it will probablyy be added in the future + // Codespace: resp.Codespace, Code: resp.Code, Index: resp.Index, Key: resp.Key, diff --git a/modules/async-icq/keeper/relay_test.go b/modules/async-icq/keeper/relay_test.go index 0f3e4ffe..e77594b8 100644 --- a/modules/async-icq/keeper/relay_test.go +++ b/modules/async-icq/keeper/relay_test.go @@ -1,12 +1,21 @@ package keeper_test import ( +<<<<<<< HEAD "github.com/cosmos/ibc-apps/modules/async-icq/v4/testing/simapp" "github.com/cosmos/ibc-apps/modules/async-icq/v4/types" abcitypes "github.com/tendermint/tendermint/abci/types" +======= + "fmt" + "github.com/cosmos/ibc-apps/modules/async-icq/v7/testing/simapp" + "github.com/cosmos/ibc-apps/modules/async-icq/v7/types" +>>>>>>> 3a9d46a (Use query routes in async_icq (#67)) + + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/query" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + minttypes "github.com/cosmos/cosmos-sdk/x/mint/types" clienttypes "github.com/cosmos/ibc-go/v4/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/v4/modules/core/04-channel/types" @@ -180,3 +189,87 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { }) } } + +func (suite *KeeperTestSuite) TestOutOfGasOnSlowQueries() { + path := NewICQPath(suite.chainA, suite.chainB) + suite.coordinator.SetupConnections(path) + + err := SetupICQPath(path) + suite.Require().NoError(err) + + q := banktypes.QueryAllBalancesRequest{ + Address: suite.chainB.SenderAccount.GetAddress().String(), + Pagination: &query.PageRequest{ + Offset: 0, + Limit: 100_000_000, + }, + } + reqs := []abcitypes.RequestQuery{ + { + Path: "/cosmos.bank.v1beta1.Query/AllBalances", + Data: simapp.GetSimApp(suite.chainB).AppCodec().MustMarshal(&q), + }, + } + data, err := types.SerializeCosmosQuery(reqs) + suite.Require().NoError(err) + + icqPacketData := types.InterchainQueryPacketData{ + Data: data, + } + packetData := icqPacketData.GetBytes() + + params := types.NewParams(true, []string{"/cosmos.bank.v1beta1.Query/AllBalances"}) + simapp.GetSimApp(suite.chainB).ICQKeeper.SetParams(suite.chainB.GetContext(), params) + + packet := channeltypes.NewPacket( + packetData, + suite.chainA.SenderAccount.GetSequence(), + path.EndpointA.ChannelConfig.PortID, + path.EndpointA.ChannelID, + path.EndpointB.ChannelConfig.PortID, + path.EndpointB.ChannelID, + clienttypes.NewHeight(1, 100), + 0, + ) + + ctx := suite.chainB.GetContext() + ctx = ctx.WithGasMeter(sdk.NewGasMeter(2000)) + // enough gas for this small query, but not for the larger one. This one should work + _, err = simapp.GetSimApp(suite.chainB).ICQKeeper.OnRecvPacket(ctx, packet) + suite.Require().NoError(err) + + // fund account with 10_000 denoms + ctx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) + for i := 0; i < 10_000; i++ { + denom := fmt.Sprintf("denom%d", i) + err = simapp.GetSimApp(suite.chainB).BankKeeper.MintCoins(ctx, minttypes.ModuleName, sdk.NewCoins(sdk.NewInt64Coin(denom, 10))) + suite.Require().NoError(err) + err = simapp.GetSimApp(suite.chainB).BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, suite.chainB.SenderAccount.GetAddress(), sdk.NewCoins(sdk.NewInt64Coin(denom, 10))) + suite.Require().NoError(err) + + } + + // We need to call NextBlock() so that the context is committed. This doesn't matter as much anymore, + // but previous versions didn't pass the context to the query, so the test would've picked the previous + // block's data + suite.chainB.NextBlock() + ctx = suite.chainB.GetContext() + + packet = channeltypes.NewPacket( + packetData, + suite.chainA.SenderAccount.GetSequence(), + path.EndpointA.ChannelConfig.PortID, + path.EndpointA.ChannelID, + path.EndpointB.ChannelConfig.PortID, + path.EndpointB.ChannelID, + clienttypes.NewHeight(1, 100), + 0, + ) + // + + // and this one should panic + suite.Assert().Panics(func() { + ctx = ctx.WithGasMeter(sdk.NewGasMeter(2000)) + _, _ = simapp.GetSimApp(suite.chainB).ICQKeeper.OnRecvPacket(ctx, packet) + }, "out of gas") +} diff --git a/modules/async-icq/testing/simapp/app.go b/modules/async-icq/testing/simapp/app.go index 76cd0c2c..68392c9a 100644 --- a/modules/async-icq/testing/simapp/app.go +++ b/modules/async-icq/testing/simapp/app.go @@ -333,7 +333,7 @@ func NewSimApp( app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper, scopedICQKeeper, - app.BaseApp, // may be replaced + app.BaseApp.GRPCQueryRouter(), ) // Create IBC Router