Skip to content

Commit

Permalink
imp: deny selected client types from VerifyMembership rpc (#5871)
Browse files Browse the repository at this point in the history
* chore: add client status check to verify membership rpc

* imp: deny selected client types from VerifyMembership rpc

(cherry picked from commit 4f14cfd)

# Conflicts:
#	modules/core/02-client/keeper/grpc_query.go
#	modules/core/02-client/keeper/grpc_query_test.go
  • Loading branch information
damiannolan authored and mergify[bot] committed Feb 21, 2024
1 parent 57fcdb9 commit 1291bc4
Show file tree
Hide file tree
Showing 2 changed files with 249 additions and 0 deletions.
72 changes: 72 additions & 0 deletions modules/core/02-client/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"fmt"
"slices"
"sort"
"strings"

Expand Down Expand Up @@ -328,3 +329,74 @@ func (k Keeper) UpgradedConsensusState(c context.Context, req *types.QueryUpgrad
UpgradedConsensusState: protoAny,
}, nil
}
<<<<<<< HEAD

Check failure on line 332 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / build (arm64)

syntax error: non-declaration statement outside function body

Check failure on line 332 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / build (amd64)

syntax error: non-declaration statement outside function body

Check failure on line 332 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (03)

syntax error: non-declaration statement outside function body

Check failure on line 332 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (03)

syntax error: non-declaration statement outside function body

Check failure on line 332 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (03)

syntax error: non-declaration statement outside function body

Check failure on line 332 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (03)

syntax error: non-declaration statement outside function body

Check failure on line 332 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (00)

syntax error: non-declaration statement outside function body

Check failure on line 332 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (02)

syntax error: non-declaration statement outside function body

Check failure on line 332 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (02)

syntax error: non-declaration statement outside function body

Check failure on line 332 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (02)

syntax error: non-declaration statement outside function body

Check failure on line 332 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (02)

syntax error: non-declaration statement outside function body

Check failure on line 332 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (01)

syntax error: non-declaration statement outside function body

Check failure on line 332 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (01)

syntax error: non-declaration statement outside function body

Check failure on line 332 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (01)

syntax error: non-declaration statement outside function body

Check failure on line 332 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / lint

syntax error: non-declaration statement outside function body

Check failure on line 332 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / lint

expected declaration, found '<<' (typecheck)

Check failure on line 332 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / lint

syntax error: non-declaration statement outside function body
=======

// VerifyMembership implements the Query/VerifyMembership gRPC method
// NOTE: Any state changes made within this handler are discarded by leveraging a cached context. Gas is consumed for underlying state access.
// This gRPC method is intended to be used within the context of the state machine and delegates to light clients to verify proofs.
func (k Keeper) VerifyMembership(c context.Context, req *types.QueryVerifyMembershipRequest) (*types.QueryVerifyMembershipResponse, error) {
if req == nil {
return nil, status.Error(codes.InvalidArgument, "empty request")
}

if err := host.ClientIdentifierValidator(req.ClientId); err != nil {
return nil, status.Error(codes.InvalidArgument, err.Error())
}

clientType, _, err := types.ParseClientIdentifier(req.ClientId)
if err != nil {
return nil, status.Error(codes.InvalidArgument, err.Error())
}

denyClients := []string{exported.Localhost, exported.Solomachine}
if slices.Contains(denyClients, clientType) {
return nil, status.Error(codes.InvalidArgument, errorsmod.Wrapf(types.ErrInvalidClientType, "verify membership is disabled for client types %s", denyClients).Error())
}

if len(req.Proof) == 0 {
return nil, status.Error(codes.InvalidArgument, "empty proof")
}

if req.ProofHeight.IsZero() {
return nil, status.Error(codes.InvalidArgument, "proof height must be non-zero")
}

if req.MerklePath.Empty() {
return nil, status.Error(codes.InvalidArgument, "empty merkle path")
}

if len(req.Value) == 0 {
return nil, status.Error(codes.InvalidArgument, "empty value")
}

ctx := sdk.UnwrapSDKContext(c)
// cache the context to ensure clientState.VerifyMembership does not change state
cachedCtx, _ := ctx.CacheContext()

// make sure we charge the higher level context even on panic
defer func() {
ctx.GasMeter().ConsumeGas(cachedCtx.GasMeter().GasConsumed(), "verify membership query")
}()

clientState, found := k.GetClientState(cachedCtx, req.ClientId)
if !found {
return nil, status.Error(codes.NotFound, errorsmod.Wrap(types.ErrClientNotFound, req.ClientId).Error())
}

if clientStatus := k.GetClientStatus(ctx, clientState, req.ClientId); clientStatus != exported.Active {
return nil, status.Error(codes.FailedPrecondition, errorsmod.Wrapf(types.ErrClientNotActive, "cannot verify membership using client (%s) with status %s", req.ClientId, clientStatus).Error())
}

if err := clientState.VerifyMembership(cachedCtx, k.ClientStore(cachedCtx, req.ClientId), k.cdc, req.ProofHeight, req.TimeDelay, req.BlockDelay, req.Proof, req.MerklePath, req.Value); err != nil {
k.Logger(ctx).Debug("proof verification failed", "key", req.MerklePath, "error", err)
return &types.QueryVerifyMembershipResponse{
Success: false,
}, nil
}

return &types.QueryVerifyMembershipResponse{
Success: true,
}, nil
}
>>>>>>> 4f14cfd8 (imp: deny selected client types from VerifyMembership rpc (#5871))

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / build (arm64)

syntax error: non-declaration statement outside function body

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / build (arm64)

invalid character U+0023 '#'

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / build (amd64)

syntax error: non-declaration statement outside function body

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / build (amd64)

invalid character U+0023 '#'

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (03)

syntax error: non-declaration statement outside function body

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (03)

invalid character U+0023 '#'

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (03)

syntax error: non-declaration statement outside function body

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (03)

invalid character U+0023 '#'

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (03)

syntax error: non-declaration statement outside function body

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (03)

invalid character U+0023 '#'

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (00)

syntax error: non-declaration statement outside function body

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (00)

invalid character U+0023 '#'

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (02)

syntax error: non-declaration statement outside function body

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (02)

invalid character U+0023 '#'

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (02)

syntax error: non-declaration statement outside function body

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (02)

invalid character U+0023 '#'

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (02)

syntax error: non-declaration statement outside function body

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (02)

invalid character U+0023 '#'

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (01)

syntax error: non-declaration statement outside function body

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (01)

invalid character U+0023 '#'

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (01)

syntax error: non-declaration statement outside function body

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (01)

invalid character U+0023 '#'

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (01)

syntax error: non-declaration statement outside function body

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / tests (01)

invalid character U+0023 '#'

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / lint

syntax error: non-declaration statement outside function body

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / lint

invalid character U+0023 '#' (typecheck)

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / lint

illegal character U+0023 '#' (typecheck)

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / lint

syntax error: non-declaration statement outside function body

Check failure on line 402 in modules/core/02-client/keeper/grpc_query.go

View workflow job for this annotation

GitHub Actions / lint

invalid character U+0023 '#') (typecheck)
177 changes: 177 additions & 0 deletions modules/core/02-client/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -659,3 +659,180 @@ func (suite *KeeperTestSuite) TestQueryClientParams() {
res, _ := suite.chainA.QueryServer.ClientParams(ctx, &types.QueryClientParamsRequest{})
suite.Require().Equal(&expParams, res.Params)
}
<<<<<<< HEAD

Check failure on line 662 in modules/core/02-client/keeper/grpc_query_test.go

View workflow job for this annotation

GitHub Actions / tests (01)

expected declaration, found '<<'
=======

func (suite *KeeperTestSuite) TestQueryVerifyMembershipProof() {
var (
path *ibctesting.Path
req *types.QueryVerifyMembershipRequest
)

testCases := []struct {
name string
malleate func()
expError error
}{
{
"success",
func() {
channel := path.EndpointB.GetChannel()
bz, err := suite.chainB.Codec.Marshal(&channel)
suite.Require().NoError(err)

channelProof, proofHeight := path.EndpointB.QueryProof(host.ChannelKey(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID))

merklePath := commitmenttypes.NewMerklePath(host.ChannelPath(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID))
merklePath, err = commitmenttypes.ApplyPrefix(suite.chainB.GetPrefix(), merklePath)
suite.Require().NoError(err)

req = &types.QueryVerifyMembershipRequest{
ClientId: path.EndpointA.ClientID,
Proof: channelProof,
ProofHeight: proofHeight,
MerklePath: merklePath,
Value: bz,
}
},
nil,
},
{
"req is nil",
func() {
req = nil
},
errors.New("empty request"),
},
{
"invalid client ID",
func() {
req = &types.QueryVerifyMembershipRequest{
ClientId: "//invalid_id",
}
},
host.ErrInvalidID,
},
{
"localhost client ID is denied",
func() {
req = &types.QueryVerifyMembershipRequest{
ClientId: exported.LocalhostClientID,
}
},
types.ErrInvalidClientType,
},
{
"solomachine client ID is denied",
func() {
req = &types.QueryVerifyMembershipRequest{
ClientId: types.FormatClientIdentifier(exported.Solomachine, 1),
}
},
types.ErrInvalidClientType,
},
{
"empty proof",
func() {
req = &types.QueryVerifyMembershipRequest{
ClientId: ibctesting.FirstClientID,
Proof: []byte{},
}
},
errors.New("empty proof"),
},
{
"invalid proof height",
func() {
req = &types.QueryVerifyMembershipRequest{
ClientId: ibctesting.FirstClientID,
Proof: []byte{0x01},
ProofHeight: types.ZeroHeight(),
}
},
errors.New("proof height must be non-zero"),
},
{
"empty merkle path",
func() {
req = &types.QueryVerifyMembershipRequest{
ClientId: ibctesting.FirstClientID,
Proof: []byte{0x01},
ProofHeight: types.NewHeight(1, 100),
}
},
errors.New("empty merkle path"),
},
{
"empty value",
func() {
req = &types.QueryVerifyMembershipRequest{
ClientId: ibctesting.FirstClientID,
Proof: []byte{0x01},
ProofHeight: types.NewHeight(1, 100),
MerklePath: commitmenttypes.NewMerklePath("/ibc", host.ChannelPath(mock.PortID, ibctesting.FirstChannelID)),
}
},
errors.New("empty value"),
},
{
"client not found",
func() {
req = &types.QueryVerifyMembershipRequest{
ClientId: types.FormatClientIdentifier(exported.Tendermint, 100), // use a sequence which hasn't been created yet
Proof: []byte{0x01},
ProofHeight: types.NewHeight(1, 100),
MerklePath: commitmenttypes.NewMerklePath("/ibc", host.ChannelPath(mock.PortID, ibctesting.FirstChannelID)),
Value: []byte{0x01},
}
},
types.ErrClientNotFound,
},
{
"client not active",
func() {
params := types.NewParams("") // disable all clients
suite.chainA.GetSimApp().GetIBCKeeper().ClientKeeper.SetParams(suite.chainA.GetContext(), params)

req = &types.QueryVerifyMembershipRequest{
ClientId: path.EndpointA.ClientID,
Proof: []byte{0x01},
ProofHeight: types.NewHeight(1, 100),
MerklePath: commitmenttypes.NewMerklePath("/ibc", host.ChannelPath(mock.PortID, ibctesting.FirstChannelID)),
Value: []byte{0x01},
}
},
types.ErrClientNotActive,
},
}

for _, tc := range testCases {
tc := tc
suite.Run(tc.name, func() {
suite.SetupTest() // reset

path = ibctesting.NewPath(suite.chainA, suite.chainB)
path.Setup()

tc.malleate()

ctx := suite.chainA.GetContext()
initialGas := ctx.GasMeter().GasConsumed()
res, err := suite.chainA.QueryServer.VerifyMembership(ctx, req)

expPass := tc.expError == nil
if expPass {
suite.Require().NoError(err)
suite.Require().True(res.Success, "failed to verify membership proof")

gasConsumed := ctx.GasMeter().GasConsumed()
suite.Require().Greater(gasConsumed, initialGas, "gas consumed should be greater than initial gas")
} else {
suite.Require().ErrorContains(err, tc.expError.Error())

gasConsumed := ctx.GasMeter().GasConsumed()
suite.Require().GreaterOrEqual(gasConsumed, initialGas, "gas consumed should be greater than or equal to initial gas")
}
})
}
}
>>>>>>> 4f14cfd8 (imp: deny selected client types from VerifyMembership rpc (#5871))

0 comments on commit 1291bc4

Please sign in to comment.