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

fix: query handlers are ignoring accumulate flag #7831

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 12 additions & 4 deletions modules/core/02-client/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,10 @@ func (q *queryServer) ClientStates(ctx context.Context, req *types.QueryClientSt
return false, err
}

identifiedClient := types.NewIdentifiedClientState(clientID, clientState)
clientStates = append(clientStates, identifiedClient)
if accumulate {
identifiedClient := types.NewIdentifiedClientState(clientID, clientState)
clientStates = append(clientStates, identifiedClient)
}
return true, nil
})
if err != nil {
Expand Down Expand Up @@ -184,7 +186,10 @@ func (q *queryServer) ConsensusStates(ctx context.Context, req *types.QueryConse
return false, err
}

consensusStates = append(consensusStates, types.NewConsensusStateWithHeight(height, consensusState))
if accumulate {
consensusStates = append(consensusStates, types.NewConsensusStateWithHeight(height, consensusState))
}

return true, nil
})
if err != nil {
Expand Down Expand Up @@ -221,7 +226,10 @@ func (q *queryServer) ConsensusStateHeights(ctx context.Context, req *types.Quer
return false, err
}

consensusStateHeights = append(consensusStateHeights, height)
if accumulate {
consensusStateHeights = append(consensusStateHeights, height)
}

return true, nil
})
if err != nil {
Expand Down
84 changes: 83 additions & 1 deletion modules/core/02-client/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ func (suite *KeeperTestSuite) TestQueryClientStates() {
var (
req *types.QueryClientStatesRequest
expClientStates = types.IdentifiedClientStates{}
expCount = 0
)

testCases := []struct {
Expand All @@ -121,13 +122,15 @@ func (suite *KeeperTestSuite) TestQueryClientStates() {
"req is nil",
func() {
req = nil
expCount = 0
},
status.Error(codes.InvalidArgument, "empty request"),
},
{
"empty pagination",
func() {
expClientStates = nil
expCount = 0
req = &types.QueryClientStatesRequest{}
},
nil,
Expand Down Expand Up @@ -155,6 +158,31 @@ func (suite *KeeperTestSuite) TestQueryClientStates() {
CountTotal: true,
},
}
expCount = 2
},
nil,
},
{
"success request with pagination limit",
charymalloju marked this conversation as resolved.
Show resolved Hide resolved
func() {
path1 := ibctesting.NewPath(suite.chainA, suite.chainB)
path1.SetupClients()

path2 := ibctesting.NewPath(suite.chainA, suite.chainB)
path2.SetupClients()

clientStateA1 := path1.EndpointA.GetClientState()

idcs := types.NewIdentifiedClientState(path1.EndpointA.ClientID, clientStateA1)

expClientStates = types.IdentifiedClientStates{idcs}.Sort()
req = &types.QueryClientStatesRequest{
Pagination: &query.PageRequest{
Limit: 1,
CountTotal: true,
},
}
expCount = 2
},
nil,
},
Expand All @@ -174,7 +202,8 @@ func (suite *KeeperTestSuite) TestQueryClientStates() {
suite.Require().NoError(err)
suite.Require().NotNil(res)
suite.Require().Equal(expClientStates.Sort(), res.ClientStates)
suite.Require().Equal(len(expClientStates), int(res.Pagination.Total))
suite.Require().Equal(expCount, int(res.Pagination.Total))

} else {
suite.Require().Error(err)
suite.Require().ErrorIs(err, tc.expErr)
Expand Down Expand Up @@ -370,6 +399,36 @@ func (suite *KeeperTestSuite) TestQueryConsensusStates() {
},
nil,
},
{
"success with pagination limit",
func() {
path := ibctesting.NewPath(suite.chainA, suite.chainB)
path.SetupClients()

height1, ok := path.EndpointA.GetClientLatestHeight().(types.Height)
suite.Require().True(ok)
expConsensusStates = []types.ConsensusStateWithHeight{
types.NewConsensusStateWithHeight(
height1,
path.EndpointA.GetConsensusState(height1),
),
}

err := path.EndpointA.UpdateClient()
suite.Require().NoError(err)
_, ok = path.EndpointA.GetClientLatestHeight().(types.Height)
suite.Require().True(ok)

req = &types.QueryConsensusStatesRequest{
ClientId: path.EndpointA.ClientID,
Pagination: &query.PageRequest{
Limit: 1,
CountTotal: true,
},
}
},
nil,
},
{
"invalid client identifier",
func() {
Expand Down Expand Up @@ -465,6 +524,29 @@ func (suite *KeeperTestSuite) TestQueryConsensusStateHeights() {
},
nil,
},
{
"success: pagination with limit",
func() {
path := ibctesting.NewPath(suite.chainA, suite.chainB)
path.SetupClients()

expConsensusStateHeights = []types.Height{path.EndpointA.GetClientLatestHeight().(types.Height)}

err := path.EndpointA.UpdateClient()
suite.Require().NoError(err)

// expConsensusStateHeights = append(expConsensusStateHeights, path.EndpointA.GetClientLatestHeight().(types.Height))

req = &types.QueryConsensusStateHeightsRequest{
ClientId: path.EndpointA.ClientID,
Pagination: &query.PageRequest{
Limit: 1,
CountTotal: true,
},
}
},
nil,
},
{
"invalid client identifier",
func() {
Expand Down
7 changes: 5 additions & 2 deletions modules/core/04-channel/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,11 @@ func (q *queryServer) ConnectionChannels(ctx context.Context, req *types.QueryCo
return false, err
}

identifiedChannel := types.NewIdentifiedChannel(portID, channelID, result)
channels = append(channels, &identifiedChannel)
if accumulate {
identifiedChannel := types.NewIdentifiedChannel(portID, channelID, result)
channels = append(channels, &identifiedChannel)
}

return true, nil
})
if err != nil {
Expand Down
41 changes: 41 additions & 0 deletions modules/core/04-channel/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,47 @@ func (suite *KeeperTestSuite) TestQueryConnectionChannels() {
},
nil,
},
{
"success with pagination limit",
func() {
path := ibctesting.NewPath(suite.chainA, suite.chainB)
path.Setup()
// channel0 on first connection on chainA
counterparty0 := types.Counterparty{
PortId: path.EndpointB.ChannelConfig.PortID,
ChannelId: path.EndpointB.ChannelID,
}

// path1 creates a second channel on first connection on chainA
path1 := ibctesting.NewPath(suite.chainA, suite.chainB)
path1.SetChannelOrdered()
path1.EndpointA.ClientID = path.EndpointA.ClientID
path1.EndpointB.ClientID = path.EndpointB.ClientID
path1.EndpointA.ConnectionID = path.EndpointA.ConnectionID
path1.EndpointB.ConnectionID = path.EndpointB.ConnectionID

suite.coordinator.CreateMockChannels(path1)

channel0 := types.NewChannel(
types.OPEN, types.UNORDERED,
counterparty0, []string{path.EndpointA.ConnectionID}, path.EndpointA.ChannelConfig.Version,
)

idCh0 := types.NewIdentifiedChannel(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, channel0)

expChannels = []*types.IdentifiedChannel{&idCh0}

req = &types.QueryConnectionChannelsRequest{
Connection: path.EndpointA.ConnectionID,
Pagination: &query.PageRequest{
Key: nil,
Limit: 1,
CountTotal: true,
},
}
},
nil,
},
}

for _, tc := range testCases {
Expand Down