diff --git a/internal/validate/validate_test.go b/internal/validate/validate_test.go index ce49cf42475..8f543ccfeb7 100644 --- a/internal/validate/validate_test.go +++ b/internal/validate/validate_test.go @@ -5,6 +5,8 @@ import ( "testing" "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "github.com/cosmos/ibc-go/v9/internal/validate" ) @@ -18,35 +20,37 @@ func TestGRPCRequest(t *testing.T) { msg string portID string channelID string - expPass bool + expErr error }{ { "success", validID, validID, - true, + nil, }, { "invalid portID", invalidID, validID, - false, + status.Error(codes.InvalidArgument, "identifier cannot be blank: invalid identifier"), }, { "invalid channelID", validID, invalidID, - false, + status.Error(codes.InvalidArgument, "identifier cannot be blank: invalid identifier"), }, } for _, tc := range testCases { t.Run(fmt.Sprintf("Case %s", tc.msg), func(t *testing.T) { err := validate.GRPCRequest(tc.portID, tc.channelID) - if tc.expPass { + + if tc.expErr == nil { require.NoError(t, err, tc.msg) } else { require.Error(t, err, tc.msg) + require.EqualError(t, err, tc.expErr.Error(), tc.msg) } }) } diff --git a/modules/core/03-connection/keeper/grpc_query_test.go b/modules/core/03-connection/keeper/grpc_query_test.go index 925c877ff35..a2349aa0d5b 100644 --- a/modules/core/03-connection/keeper/grpc_query_test.go +++ b/modules/core/03-connection/keeper/grpc_query_test.go @@ -3,11 +3,17 @@ package keeper_test import ( "fmt" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + errorsmod "cosmossdk.io/errors" + "github.com/cosmos/cosmos-sdk/types/query" clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types" "github.com/cosmos/ibc-go/v9/modules/core/03-connection/keeper" "github.com/cosmos/ibc-go/v9/modules/core/03-connection/types" + host "github.com/cosmos/ibc-go/v9/modules/core/24-host" "github.com/cosmos/ibc-go/v9/modules/core/exported" ibctesting "github.com/cosmos/ibc-go/v9/testing" ) @@ -21,21 +27,21 @@ func (suite *KeeperTestSuite) TestQueryConnection() { testCases := []struct { msg string malleate func() - expPass bool + expErr error }{ { "empty request", func() { req = nil }, - false, + status.Error(codes.InvalidArgument, "empty request"), }, { "invalid connectionID", func() { req = &types.QueryConnectionRequest{} }, - false, + status.Error(codes.InvalidArgument, errorsmod.Wrap(host.ErrInvalidID, "identifier cannot be blank").Error()), }, { "connection not found", @@ -44,7 +50,10 @@ func (suite *KeeperTestSuite) TestQueryConnection() { ConnectionId: ibctesting.InvalidID, } }, - false, + status.Error( + codes.NotFound, + errorsmod.Wrap(types.ErrConnectionNotFound, "IDisInvalid").Error(), + ), }, { "success", @@ -62,7 +71,7 @@ func (suite *KeeperTestSuite) TestQueryConnection() { ConnectionId: path.EndpointA.ConnectionID, } }, - true, + nil, }, } @@ -78,12 +87,13 @@ func (suite *KeeperTestSuite) TestQueryConnection() { queryServer := keeper.NewQueryServer(suite.chainA.App.GetIBCKeeper().ConnectionKeeper) res, err := queryServer.Connection(ctx, req) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) suite.Require().NotNil(res) suite.Require().Equal(&expConnection, res.Connection) } else { suite.Require().Error(err) + suite.Require().ErrorIs(err, tc.expErr) } }) } @@ -104,21 +114,21 @@ func (suite *KeeperTestSuite) TestQueryConnections() { testCases := []struct { msg string malleate func() - expPass bool + expErr error }{ { "empty request", func() { req = nil }, - false, + status.Error(codes.InvalidArgument, "empty request"), }, { "empty pagination", func() { req = &types.QueryConnectionsRequest{} }, - true, + nil, }, { "success", @@ -155,7 +165,7 @@ func (suite *KeeperTestSuite) TestQueryConnections() { }, } }, - true, + nil, }, } @@ -171,12 +181,13 @@ func (suite *KeeperTestSuite) TestQueryConnections() { queryServer := keeper.NewQueryServer(suite.chainA.App.GetIBCKeeper().ConnectionKeeper) res, err := queryServer.Connections(ctx, req) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) suite.Require().NotNil(res) suite.Require().Equal(expConnections, res.Connections) } else { suite.Require().Error(err) + suite.Require().ErrorIs(err, tc.expErr) } }) } @@ -191,21 +202,21 @@ func (suite *KeeperTestSuite) TestQueryClientConnections() { testCases := []struct { msg string malleate func() - expPass bool + expErr error }{ { "empty request", func() { req = nil }, - false, + status.Error(codes.InvalidArgument, "empty request"), }, { "invalid connectionID", func() { req = &types.QueryClientConnectionsRequest{} }, - false, + status.Error(codes.InvalidArgument, errorsmod.Wrap(host.ErrInvalidID, "identifier cannot be blank").Error()), }, { "connection not found", @@ -214,7 +225,10 @@ func (suite *KeeperTestSuite) TestQueryClientConnections() { ClientId: ibctesting.InvalidID, } }, - false, + status.Error( + codes.NotFound, + errorsmod.Wrap(types.ErrClientConnectionPathsNotFound, "IDisInvalid").Error(), + ), }, { "success", @@ -236,7 +250,7 @@ func (suite *KeeperTestSuite) TestQueryClientConnections() { ClientId: path1.EndpointA.ClientID, } }, - true, + nil, }, } @@ -252,12 +266,13 @@ func (suite *KeeperTestSuite) TestQueryClientConnections() { queryServer := keeper.NewQueryServer(suite.chainA.App.GetIBCKeeper().ConnectionKeeper) res, err := queryServer.ClientConnections(ctx, req) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) suite.Require().NotNil(res) suite.Require().Equal(expPaths, res.ConnectionPaths) } else { suite.Require().Error(err) + suite.Require().ErrorIs(err, tc.expErr) } }) } @@ -272,14 +287,14 @@ func (suite *KeeperTestSuite) TestQueryConnectionClientState() { testCases := []struct { msg string malleate func() - expPass bool + expErr error }{ { "empty request", func() { req = nil }, - false, + status.Error(codes.InvalidArgument, "empty request"), }, { "invalid connection ID", @@ -288,7 +303,7 @@ func (suite *KeeperTestSuite) TestQueryConnectionClientState() { ConnectionId: "", } }, - false, + status.Error(codes.InvalidArgument, errorsmod.Wrap(host.ErrInvalidID, "identifier cannot be blank").Error()), }, { "connection not found", @@ -297,7 +312,10 @@ func (suite *KeeperTestSuite) TestQueryConnectionClientState() { ConnectionId: "test-connection-id", } }, - false, + status.Error( + codes.NotFound, + errorsmod.Wrap(types.ErrConnectionNotFound, "connection-id: test-connection-id").Error(), + ), }, { "client state not found", @@ -311,7 +329,10 @@ func (suite *KeeperTestSuite) TestQueryConnectionClientState() { req = &types.QueryConnectionClientStateRequest{ ConnectionId: path.EndpointA.ConnectionID, } - }, false, + }, status.Error( + codes.NotFound, + errorsmod.Wrap(clienttypes.ErrClientNotFound, "client-id: ").Error(), + ), }, { "success", @@ -326,7 +347,7 @@ func (suite *KeeperTestSuite) TestQueryConnectionClientState() { ConnectionId: path.EndpointA.ConnectionID, } }, - true, + nil, }, } @@ -342,7 +363,7 @@ func (suite *KeeperTestSuite) TestQueryConnectionClientState() { queryServer := keeper.NewQueryServer(suite.chainA.App.GetIBCKeeper().ConnectionKeeper) res, err := queryServer.ConnectionClientState(ctx, req) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) suite.Require().NotNil(res) suite.Require().Equal(&expIdentifiedClientState, res.IdentifiedClientState) @@ -352,6 +373,7 @@ func (suite *KeeperTestSuite) TestQueryConnectionClientState() { suite.Require().NotNil(cachedValue) } else { suite.Require().Error(err) + suite.Require().ErrorIs(err, tc.expErr) } }) } @@ -367,14 +389,14 @@ func (suite *KeeperTestSuite) TestQueryConnectionConsensusState() { testCases := []struct { msg string malleate func() - expPass bool + expErr error }{ { "empty request", func() { req = nil }, - false, + status.Error(codes.InvalidArgument, "empty request"), }, { "invalid connection ID", @@ -385,7 +407,7 @@ func (suite *KeeperTestSuite) TestQueryConnectionConsensusState() { RevisionHeight: 1, } }, - false, + status.Error(codes.InvalidArgument, errorsmod.Wrap(host.ErrInvalidID, "identifier cannot be blank").Error()), }, { "connection not found", @@ -396,7 +418,10 @@ func (suite *KeeperTestSuite) TestQueryConnectionConsensusState() { RevisionHeight: 1, } }, - false, + status.Error( + codes.NotFound, + errorsmod.Wrap(types.ErrConnectionNotFound, "connection-id: test-connection-id").Error(), + ), }, { "consensus state not found", @@ -409,7 +434,10 @@ func (suite *KeeperTestSuite) TestQueryConnectionConsensusState() { RevisionNumber: 0, RevisionHeight: uint64(suite.chainA.GetContext().BlockHeight()), // use current height } - }, false, + }, status.Error( + codes.NotFound, + errorsmod.Wrap(clienttypes.ErrConsensusStateNotFound, "client-id: 07-tendermint-0").Error(), + ), }, { "success", @@ -429,7 +457,7 @@ func (suite *KeeperTestSuite) TestQueryConnectionConsensusState() { RevisionHeight: clientHeight.GetRevisionHeight(), } }, - true, + nil, }, } @@ -445,7 +473,7 @@ func (suite *KeeperTestSuite) TestQueryConnectionConsensusState() { queryServer := keeper.NewQueryServer(suite.chainA.App.GetIBCKeeper().ConnectionKeeper) res, err := queryServer.ConnectionConsensusState(ctx, req) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) suite.Require().NotNil(res) consensusState, err := clienttypes.UnpackConsensusState(res.ConsensusState) @@ -458,6 +486,7 @@ func (suite *KeeperTestSuite) TestQueryConnectionConsensusState() { suite.Require().NotNil(cachedValue) } else { suite.Require().Error(err) + suite.Require().ErrorIs(err, tc.expErr) } }) } diff --git a/modules/core/03-connection/keeper/handshake_test.go b/modules/core/03-connection/keeper/handshake_test.go index e233c68fbe3..34a101d87e0 100644 --- a/modules/core/03-connection/keeper/handshake_test.go +++ b/modules/core/03-connection/keeper/handshake_test.go @@ -3,7 +3,11 @@ package keeper_test import ( "time" + errorsmod "cosmossdk.io/errors" + + clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types" "github.com/cosmos/ibc-go/v9/modules/core/03-connection/types" + commitmenttypes "github.com/cosmos/ibc-go/v9/modules/core/23-commitment/types" host "github.com/cosmos/ibc-go/v9/modules/core/24-host" ibctesting "github.com/cosmos/ibc-go/v9/testing" ) @@ -22,30 +26,30 @@ func (suite *KeeperTestSuite) TestConnOpenInit() { testCases := []struct { msg string malleate func() - expPass bool + expErr error }{ {"success", func() { - }, true}, + }, nil}, {"success with empty counterparty identifier", func() { emptyConnBID = true - }, true}, + }, nil}, {"success with non empty version", func() { version = types.GetCompatibleVersions()[0] - }, true}, + }, nil}, {"success with non zero delayPeriod", func() { delayPeriod = uint64(time.Hour.Nanoseconds()) - }, true}, + }, nil}, {"invalid version", func() { version = &types.Version{} - }, false}, + }, errorsmod.Wrap(types.ErrInvalidVersion, "version is not supported")}, {"couldn't add connection to client", func() { // set path.EndpointA.ClientID to invalid client identifier path.EndpointA.ClientID = "clientidentifier" - }, false}, + }, errorsmod.Wrap(clienttypes.ErrClientNotActive, "client (clientidentifier) status is Unauthorized")}, { - msg: "unauthorized client", - expPass: false, + msg: "unauthorized client", + expErr: errorsmod.Wrap(clienttypes.ErrClientNotActive, "client (07-tendermint-0) status is Unauthorized"), malleate: func() { expErrorMsgSubstring = "status is Unauthorized" // remove client from allowed list @@ -75,13 +79,14 @@ func (suite *KeeperTestSuite) TestConnOpenInit() { connectionID, err := suite.chainA.App.GetIBCKeeper().ConnectionKeeper.ConnOpenInit(suite.chainA.GetContext(), path.EndpointA.ClientID, counterparty, version, delayPeriod) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) suite.Require().Equal(types.FormatConnectionIdentifier(0), connectionID) } else { suite.Require().Error(err) suite.Contains(err.Error(), expErrorMsgSubstring) suite.Require().Equal("", connectionID) + suite.Require().ErrorIs(err, tc.expErr) } }) } @@ -99,12 +104,12 @@ func (suite *KeeperTestSuite) TestConnOpenTry() { testCases := []struct { msg string malleate func() - expPass bool + expErr error }{ {"success", func() { err := path.EndpointA.ConnOpenInit() suite.Require().NoError(err) - }, true}, + }, nil}, {"success with delay period", func() { err := path.EndpointA.ConnOpenInit() suite.Require().NoError(err) @@ -118,23 +123,23 @@ func (suite *KeeperTestSuite) TestConnOpenTry() { suite.coordinator.CommitBlock(suite.chainA) err = path.EndpointB.UpdateClient() suite.Require().NoError(err) - }, true}, + }, nil}, {"counterparty versions is empty", func() { err := path.EndpointA.ConnOpenInit() suite.Require().NoError(err) versions = nil - }, false}, + }, errorsmod.Wrap(types.ErrVersionNegotiationFailed, "failed to find a matching counterparty version ([]) from the supported version list ([identifier:\"1\" features:\"ORDER_ORDERED\" features:\"ORDER_UNORDERED\" ])")}, {"counterparty versions don't have a match", func() { err := path.EndpointA.ConnOpenInit() suite.Require().NoError(err) version := types.NewVersion("0.0", nil) versions = []*types.Version{version} - }, false}, + }, errorsmod.Wrap(types.ErrVersionNegotiationFailed, "failed to find a matching counterparty version ([identifier:\"0.0\" ]) from the supported version list ([identifier:\"1\" features:\"ORDER_ORDERED\" features:\"ORDER_UNORDERED\" ])")}, {"connection state verification failed", func() { // chainA connection not created - }, false}, + }, errorsmod.Wrap(commitmenttypes.ErrInvalidProof, "failed connection state verification for client (07-tendermint-0): commitment proof must be existence proof. got: int at index &{1374402732384}")}, } for _, tc := range testCases { @@ -163,12 +168,13 @@ func (suite *KeeperTestSuite) TestConnOpenTry() { versions, initProof, proofHeight, ) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) suite.Require().Equal(types.FormatConnectionIdentifier(0), connectionID) } else { suite.Require().Error(err) suite.Require().Equal("", connectionID) + suite.Require().ErrorIs(err, tc.expErr) } }) } @@ -185,7 +191,7 @@ func (suite *KeeperTestSuite) TestConnOpenAck() { testCases := []struct { msg string malleate func() - expPass bool + expErr error }{ {"success", func() { err := path.EndpointA.ConnOpenInit() @@ -193,10 +199,10 @@ func (suite *KeeperTestSuite) TestConnOpenAck() { err = path.EndpointB.ConnOpenTry() suite.Require().NoError(err) - }, true}, + }, nil}, {"connection not found", func() { // connections are never created - }, false}, + }, errorsmod.Wrap(types.ErrConnectionNotFound, "")}, {"invalid counterparty connection ID", func() { err := path.EndpointA.ConnOpenInit() suite.Require().NoError(err) @@ -213,7 +219,7 @@ func (suite *KeeperTestSuite) TestConnOpenAck() { err = path.EndpointB.UpdateClient() suite.Require().NoError(err) - }, false}, + }, errorsmod.Wrap(commitmenttypes.ErrInvalidProof, "failed connection state verification for client (07-tendermint-0): commitment proof must be existence proof. got: int at index &{1374412614704}")}, {"connection state is not INIT", func() { // connection state is already OPEN on chainA err := path.EndpointA.ConnOpenInit() @@ -224,7 +230,7 @@ func (suite *KeeperTestSuite) TestConnOpenAck() { err = path.EndpointA.ConnOpenAck() suite.Require().NoError(err) - }, false}, + }, errorsmod.Wrap(types.ErrInvalidConnectionState, "connection state is not INIT (got STATE_OPEN)")}, {"connection is in INIT but the proposed version is invalid", func() { // chainA is in INIT, chainB is in TRYOPEN err := path.EndpointA.ConnOpenInit() @@ -234,7 +240,7 @@ func (suite *KeeperTestSuite) TestConnOpenAck() { suite.Require().NoError(err) version = types.NewVersion("2.0", nil) - }, false}, + }, errorsmod.Wrap(types.ErrInvalidConnectionState, "the counterparty selected version identifier:\"2.0\" is not supported by versions selected on INIT")}, {"incompatible IBC versions", func() { err := path.EndpointA.ConnOpenInit() suite.Require().NoError(err) @@ -244,7 +250,7 @@ func (suite *KeeperTestSuite) TestConnOpenAck() { // set version to a non-compatible version version = types.NewVersion("2.0", nil) - }, false}, + }, errorsmod.Wrap(types.ErrInvalidConnectionState, "the counterparty selected version identifier:\"2.0\" is not supported by versions selected on INIT")}, {"empty version", func() { err := path.EndpointA.ConnOpenInit() suite.Require().NoError(err) @@ -253,7 +259,7 @@ func (suite *KeeperTestSuite) TestConnOpenAck() { suite.Require().NoError(err) version = &types.Version{} - }, false}, + }, errorsmod.Wrap(types.ErrInvalidConnectionState, "the counterparty selected version is not supported by versions selected on INIT")}, {"feature set verification failed - unsupported feature", func() { err := path.EndpointA.ConnOpenInit() suite.Require().NoError(err) @@ -262,12 +268,12 @@ func (suite *KeeperTestSuite) TestConnOpenAck() { suite.Require().NoError(err) version = types.NewVersion(types.DefaultIBCVersionIdentifier, []string{"ORDER_ORDERED", "ORDER_UNORDERED", "ORDER_DAG"}) - }, false}, + }, errorsmod.Wrap(types.ErrInvalidConnectionState, "the counterparty selected version identifier:\"1\" features:\"ORDER_ORDERED\" features:\"ORDER_UNORDERED\" features:\"ORDER_DAG\" is not supported by versions selected on INIT")}, {"connection state verification failed", func() { // chainB connection is not in INIT err := path.EndpointA.ConnOpenInit() suite.Require().NoError(err) - }, false}, + }, errorsmod.Wrap(commitmenttypes.ErrInvalidProof, "failed connection state verification for client (07-tendermint-0): commitment proof must be existence proof. got: int at index &{1374414228888}")}, } for _, tc := range testCases { @@ -292,10 +298,11 @@ func (suite *KeeperTestSuite) TestConnOpenAck() { path.EndpointB.ConnectionID, tryProof, proofHeight, ) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) } else { suite.Require().Error(err) + suite.Require().ErrorIs(err, tc.expErr) } }) } @@ -308,7 +315,7 @@ func (suite *KeeperTestSuite) TestConnOpenConfirm() { testCases := []struct { msg string malleate func() - expPass bool + expErr error }{ {"success", func() { err := path.EndpointA.ConnOpenInit() @@ -319,14 +326,14 @@ func (suite *KeeperTestSuite) TestConnOpenConfirm() { err = path.EndpointA.ConnOpenAck() suite.Require().NoError(err) - }, true}, + }, nil}, {"connection not found", func() { // connections are never created - }, false}, + }, errorsmod.Wrap(types.ErrConnectionNotFound, "")}, {"chain B's connection state is not TRYOPEN", func() { // connections are OPEN path.CreateConnections() - }, false}, + }, errorsmod.Wrap(types.ErrInvalidConnectionState, "connection state is not TRYOPEN (got STATE_OPEN)")}, {"connection state verification failed", func() { // chainA is in INIT err := path.EndpointA.ConnOpenInit() @@ -334,7 +341,7 @@ func (suite *KeeperTestSuite) TestConnOpenConfirm() { err = path.EndpointB.ConnOpenTry() suite.Require().NoError(err) - }, false}, + }, errorsmod.Wrap(commitmenttypes.ErrInvalidProof, "failed connection state verification for client (07-tendermint-0): failed to verify membership proof at index 0: provided value doesn't match proof")}, } for _, tc := range testCases { @@ -358,10 +365,11 @@ func (suite *KeeperTestSuite) TestConnOpenConfirm() { suite.chainB.GetContext(), path.EndpointB.ConnectionID, ackProof, proofHeight, ) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) } else { suite.Require().Error(err) + suite.Require().ErrorIs(err, tc.expErr) } }) } diff --git a/modules/core/03-connection/keeper/keeper_test.go b/modules/core/03-connection/keeper/keeper_test.go index ecdcdd3ab59..60b184ad704 100644 --- a/modules/core/03-connection/keeper/keeper_test.go +++ b/modules/core/03-connection/keeper/keeper_test.go @@ -1,6 +1,7 @@ package keeper_test import ( + "errors" "testing" testifysuite "github.com/stretchr/testify/suite" @@ -139,13 +140,13 @@ func (suite *KeeperTestSuite) TestDefaultSetParams() { // TestSetAndGetParams tests that param setting and retrieval works properly func (suite *KeeperTestSuite) TestSetAndGetParams() { testCases := []struct { - name string - input types.Params - expPass bool + name string + input types.Params + expErr error }{ - {"success: set default params", types.DefaultParams(), true}, - {"success: valid value for MaxExpectedTimePerBlock", types.NewParams(10), true}, - {"failure: invalid value for MaxExpectedTimePerBlock", types.NewParams(0), false}, + {"success: set default params", types.DefaultParams(), nil}, + {"success: valid value for MaxExpectedTimePerBlock", types.NewParams(10), nil}, + {"failure: invalid value for MaxExpectedTimePerBlock", types.NewParams(0), errors.New("MaxExpectedTimePerBlock cannot be zero")}, } for _, tc := range testCases { @@ -156,13 +157,14 @@ func (suite *KeeperTestSuite) TestSetAndGetParams() { ctx := suite.chainA.GetContext() err := tc.input.Validate() suite.chainA.GetSimApp().IBCKeeper.ConnectionKeeper.SetParams(ctx, tc.input) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) expected := tc.input p := suite.chainA.GetSimApp().IBCKeeper.ConnectionKeeper.GetParams(ctx) suite.Require().Equal(expected, p) } else { suite.Require().Error(err) + suite.Require().Equal(err.Error(), tc.expErr.Error()) } }) } diff --git a/modules/core/03-connection/keeper/verify_test.go b/modules/core/03-connection/keeper/verify_test.go index 4177aebbbce..83b844a2fec 100644 --- a/modules/core/03-connection/keeper/verify_test.go +++ b/modules/core/03-connection/keeper/verify_test.go @@ -4,10 +4,14 @@ import ( "fmt" "time" + errorsmod "cosmossdk.io/errors" + clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types" "github.com/cosmos/ibc-go/v9/modules/core/03-connection/types" channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types" + commitmenttypes "github.com/cosmos/ibc-go/v9/modules/core/23-commitment/types" host "github.com/cosmos/ibc-go/v9/modules/core/24-host" + ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors" "github.com/cosmos/ibc-go/v9/modules/core/exported" ibctm "github.com/cosmos/ibc-go/v9/modules/light-clients/07-tendermint" ibctesting "github.com/cosmos/ibc-go/v9/testing" @@ -26,24 +30,24 @@ func (suite *KeeperTestSuite) TestVerifyConnectionState() { cases := []struct { name string malleate func() - expPass bool + expErr error }{ - {"verification success", func() {}, true}, + {"verification success", func() {}, nil}, {"client state not found - changed client ID", func() { path.EndpointA.UpdateConnection(func(c *types.ConnectionEnd) { c.ClientId = ibctesting.InvalidID }) - }, false}, + }, errorsmod.Wrap(clienttypes.ErrClientNotActive, "client (IDisInvalid) status is Unauthorized")}, {"consensus state not found - increased proof height", func() { heightDiff = 5 - }, false}, + }, errorsmod.Wrap(ibcerrors.ErrInvalidHeight, "failed connection state verification for client (07-tendermint-0): client state height < proof height ({1 9} < {1 14}), please ensure the client has been updated")}, {"verification failed - connection state is different than proof", func() { path.EndpointA.UpdateConnection(func(c *types.ConnectionEnd) { c.State = types.TRYOPEN }) - }, false}, + }, errorsmod.Wrap(ibcerrors.ErrInvalidHeight, "failed connection state verification for client (07-tendermint-0): client state height < proof height ({1 9} < {1 14}), please ensure the client has been updated")}, {"client status is not active - client is expired", func() { clientState, ok := path.EndpointA.GetClientState().(*ibctm.ClientState) suite.Require().True(ok) clientState.FrozenHeight = clienttypes.NewHeight(0, 1) path.EndpointA.SetClientState(clientState) - }, false}, + }, errorsmod.Wrap(clienttypes.ErrClientNotActive, "client (07-tendermint-0) status is Frozen")}, } for _, tc := range cases { @@ -69,10 +73,11 @@ func (suite *KeeperTestSuite) TestVerifyConnectionState() { malleateHeight(proofHeight, heightDiff), proof, path.EndpointB.ConnectionID, expectedConnection, ) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) } else { suite.Require().Error(err) + suite.Require().ErrorIs(err, tc.expErr) } }) } @@ -88,24 +93,24 @@ func (suite *KeeperTestSuite) TestVerifyChannelState() { cases := []struct { name string malleate func() - expPass bool + expErr error }{ - {"verification success", func() {}, true}, + {"verification success", func() {}, nil}, {"client state not found- changed client ID", func() { path.EndpointA.UpdateConnection(func(c *types.ConnectionEnd) { c.ClientId = ibctesting.InvalidID }) - }, false}, + }, errorsmod.Wrap(clienttypes.ErrClientNotActive, "client (IDisInvalid) status is Unauthorized")}, {"consensus state not found - increased proof height", func() { heightDiff = 5 - }, false}, + }, errorsmod.Wrap(ibcerrors.ErrInvalidHeight, "failed channel state verification for client (07-tendermint-0): client state height < proof height ({1 15} < {1 20}), please ensure the client has been updated")}, {"verification failed - changed channel state", func() { path.EndpointA.UpdateChannel(func(channel *channeltypes.Channel) { channel.State = channeltypes.TRYOPEN }) - }, false}, + }, errorsmod.Wrap(ibcerrors.ErrInvalidHeight, "failed channel state verification for client (07-tendermint-0): client state height < proof height ({1 15} < {1 20}), please ensure the client has been updated")}, {"client status is not active - client is expired", func() { clientState, ok := path.EndpointA.GetClientState().(*ibctm.ClientState) suite.Require().True(ok) clientState.FrozenHeight = clienttypes.NewHeight(0, 1) path.EndpointA.SetClientState(clientState) - }, false}, + }, errorsmod.Wrap(clienttypes.ErrClientNotActive, "client (07-tendermint-0) status is Frozen")}, } for _, tc := range cases { @@ -130,10 +135,11 @@ func (suite *KeeperTestSuite) TestVerifyChannelState() { path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, channel, ) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) } else { suite.Require().Error(err) + suite.Require().ErrorIs(err, tc.expErr) } }) } @@ -153,36 +159,36 @@ func (suite *KeeperTestSuite) TestVerifyPacketCommitment() { cases := []struct { name string malleate func() - expPass bool + expErr error }{ - {"verification success", func() {}, true}, + {"verification success", func() {}, nil}, {"verification success: delay period passed", func() { delayTimePeriod = uint64(1 * time.Second.Nanoseconds()) - }, true}, + }, nil}, {"delay time period has not passed", func() { delayTimePeriod = uint64(1 * time.Hour.Nanoseconds()) - }, false}, + }, errorsmod.Wrap(ibctm.ErrDelayPeriodNotPassed, "failed packet commitment verification for client (07-tendermint-0): cannot verify packet until time: 1577926940000000000, current time: 1577923345000000000")}, {"delay block period has not passed", func() { // make timePerBlock 1 nanosecond so that block delay is not passed. // must also set a non-zero time delay to ensure block delay is enforced. delayTimePeriod = uint64(1 * time.Second.Nanoseconds()) timePerBlock = 1 - }, false}, + }, errorsmod.Wrap(ibctm.ErrDelayPeriodNotPassed, "failed packet commitment verification for client (07-tendermint-0): cannot verify packet until height: 1-1000000016, current height: 1-17")}, {"client state not found- changed client ID", func() { path.EndpointB.UpdateConnection(func(c *types.ConnectionEnd) { c.ClientId = ibctesting.InvalidID }) - }, false}, + }, errorsmod.Wrap(clienttypes.ErrClientNotActive, "client (07-tendermint-0) status is Frozen")}, {"consensus state not found - increased proof height", func() { heightDiff = 5 - }, false}, + }, errorsmod.Wrap(ibcerrors.ErrInvalidHeight, "failed packet commitment verification for client (07-tendermint-0): client state height < proof height ({1 17} < {1 22}), please ensure the client has been updated")}, {"verification failed - changed packet commitment state", func() { packet.Data = []byte(ibctesting.InvalidID) - }, false}, + }, errorsmod.Wrap(commitmenttypes.ErrInvalidProof, "failed packet commitment verification for client (07-tendermint-0): failed to verify membership proof at index 0: provided value doesn't match proof")}, {"client status is not active - client is expired", func() { clientState, ok := path.EndpointB.GetClientState().(*ibctm.ClientState) suite.Require().True(ok) clientState.FrozenHeight = clienttypes.NewHeight(0, 1) path.EndpointB.SetClientState(clientState) - }, false}, + }, errorsmod.Wrap(clienttypes.ErrClientNotActive, "client (07-tendermint-0) status is Frozen")}, } for _, tc := range cases { @@ -220,10 +226,11 @@ func (suite *KeeperTestSuite) TestVerifyPacketCommitment() { packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence(), commitment, ) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) } else { suite.Require().Error(err) + suite.Require().ErrorIs(err, tc.expErr) } }) } @@ -244,36 +251,36 @@ func (suite *KeeperTestSuite) TestVerifyPacketAcknowledgement() { cases := []struct { name string malleate func() - expPass bool + expErr error }{ - {"verification success", func() {}, true}, + {"verification success", func() {}, nil}, {"verification success: delay period passed", func() { delayTimePeriod = uint64(1 * time.Second.Nanoseconds()) - }, true}, + }, nil}, {"delay time period has not passed", func() { delayTimePeriod = uint64(1 * time.Hour.Nanoseconds()) - }, false}, + }, errorsmod.Wrap(ibctm.ErrDelayPeriodNotPassed, "failed packet acknowledgement verification for client (07-tendermint-0): cannot verify packet until time: 1577934160000000000, current time: 1577930565000000000")}, {"delay block period has not passed", func() { // make timePerBlock 1 nanosecond so that block delay is not passed. // must also set a non-zero time delay to ensure block delay is enforced. delayTimePeriod = uint64(1 * time.Second.Nanoseconds()) timePerBlock = 1 - }, false}, + }, errorsmod.Wrap(ibctm.ErrDelayPeriodNotPassed, "failed packet acknowledgement verification for client (07-tendermint-0): cannot verify packet until height: 1-1000000018, current height: 1-19")}, {"client state not found- changed client ID", func() { path.EndpointA.UpdateConnection(func(c *types.ConnectionEnd) { c.ClientId = ibctesting.InvalidID }) - }, false}, + }, errorsmod.Wrap(clienttypes.ErrClientNotActive, "client (07-tendermint-0) status is Frozen")}, {"consensus state not found - increased proof height", func() { heightDiff = 5 - }, false}, + }, errorsmod.Wrap(ibcerrors.ErrInvalidHeight, "failed packet acknowledgement verification for client (07-tendermint-0): client state height < proof height ({1 19} < {1 24}), please ensure the client has been updated")}, {"verification failed - changed acknowledgement", func() { ack = ibcmock.MockFailAcknowledgement - }, false}, + }, errorsmod.Wrap(commitmenttypes.ErrInvalidProof, "failed packet acknowledgement verification for client (07-tendermint-0): failed to verify membership proof at index 0: provided value doesn't match proof")}, {"client status is not active - client is expired", func() { clientState, ok := path.EndpointA.GetClientState().(*ibctm.ClientState) suite.Require().True(ok) clientState.FrozenHeight = clienttypes.NewHeight(0, 1) path.EndpointA.SetClientState(clientState) - }, false}, + }, errorsmod.Wrap(clienttypes.ErrClientNotActive, "client (07-tendermint-0) status is Frozen")}, } for _, tc := range cases { @@ -320,10 +327,11 @@ func (suite *KeeperTestSuite) TestVerifyPacketAcknowledgement() { packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(), ack.Acknowledgement(), ) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) } else { suite.Require().Error(err) + suite.ErrorIs(err, tc.expErr) } }) } @@ -344,27 +352,27 @@ func (suite *KeeperTestSuite) TestVerifyPacketReceiptAbsence() { cases := []struct { name string malleate func() - expPass bool + expErr error }{ - {"verification success", func() {}, true}, + {"verification success", func() {}, nil}, {"verification success: delay period passed", func() { delayTimePeriod = uint64(1 * time.Second.Nanoseconds()) - }, true}, + }, nil}, {"delay time period has not passed", func() { delayTimePeriod = uint64(1 * time.Hour.Nanoseconds()) - }, false}, + }, errorsmod.Wrap(ibctm.ErrDelayPeriodNotPassed, "failed packet commitment verification for client (07-tendermint-0): cannot verify packet until time: 1577926940000000000, current time: 1577923345000000000")}, {"delay block period has not passed", func() { // make timePerBlock 1 nanosecond so that block delay is not passed. // must also set a non-zero time delay to ensure block delay is enforced. delayTimePeriod = uint64(1 * time.Second.Nanoseconds()) timePerBlock = 1 - }, false}, + }, errorsmod.Wrap(ibctm.ErrDelayPeriodNotPassed, "failed packet commitment verification for client (07-tendermint-0): cannot verify packet until height: 1-1000000016, current height: 1-17")}, {"client state not found - changed client ID", func() { path.EndpointA.UpdateConnection(func(c *types.ConnectionEnd) { c.ClientId = ibctesting.InvalidID }) - }, false}, + }, errorsmod.Wrap(clienttypes.ErrClientNotActive, "client (07-tendermint-0) status is Frozen")}, {"consensus state not found - increased proof height", func() { heightDiff = 5 - }, false}, + }, errorsmod.Wrap(ibcerrors.ErrInvalidHeight, "failed packet commitment verification for client (07-tendermint-0): client state height < proof height ({1 17} < {1 22}), please ensure the client has been updated")}, {"verification failed - acknowledgement was received", func() { // increment receiving chain's (chainB) time by 2 hour to always pass receive suite.coordinator.IncrementTimeBy(time.Hour * 2) @@ -372,13 +380,13 @@ func (suite *KeeperTestSuite) TestVerifyPacketReceiptAbsence() { err := path.EndpointB.RecvPacket(packet) suite.Require().NoError(err) - }, false}, + }, errorsmod.Wrap(commitmenttypes.ErrInvalidProof, "failed packet commitment verification for client (07-tendermint-0): failed to verify membership proof at index 0: provided value doesn't match proof")}, {"client status is not active - client is expired", func() { clientState, ok := path.EndpointA.GetClientState().(*ibctm.ClientState) suite.Require().True(ok) clientState.FrozenHeight = clienttypes.NewHeight(0, 1) path.EndpointA.SetClientState(clientState) - }, false}, + }, errorsmod.Wrap(clienttypes.ErrClientNotActive, "client (07-tendermint-0) status is Frozen")}, } for _, tc := range cases { @@ -426,10 +434,11 @@ func (suite *KeeperTestSuite) TestVerifyPacketReceiptAbsence() { packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(), ) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) } else { suite.Require().Error(err) + suite.Require().ErrorIs(err, tc.expErr) } }) } @@ -450,36 +459,36 @@ func (suite *KeeperTestSuite) TestVerifyNextSequenceRecv() { cases := []struct { name string malleate func() - expPass bool + expErr error }{ - {"verification success", func() {}, true}, + {"verification success", func() {}, nil}, {"verification success: delay period passed", func() { delayTimePeriod = uint64(1 * time.Second.Nanoseconds()) - }, true}, + }, nil}, {"delay time period has not passed", func() { delayTimePeriod = uint64(1 * time.Hour.Nanoseconds()) - }, false}, + }, errorsmod.Wrap(ibctm.ErrDelayPeriodNotPassed, "failed packet commitment verification for client (07-tendermint-0): cannot verify packet until time: 1577926940000000000, current time: 1577923345000000000")}, {"delay block period has not passed", func() { // make timePerBlock 1 nanosecond so that block delay is not passed. // must also set a non-zero time delay to ensure block delay is enforced. delayTimePeriod = uint64(1 * time.Second.Nanoseconds()) timePerBlock = 1 - }, false}, + }, errorsmod.Wrap(ibctm.ErrDelayPeriodNotPassed, "failed packet commitment verification for client (07-tendermint-0): cannot verify packet until height: 1-1000000016, current height: 1-17")}, {"client state not found- changed client ID", func() { path.EndpointA.UpdateConnection(func(c *types.ConnectionEnd) { c.ClientId = ibctesting.InvalidID }) - }, false}, + }, errorsmod.Wrap(clienttypes.ErrClientNotActive, "client (07-tendermint-0) status is Frozen")}, {"consensus state not found - increased proof height", func() { heightDiff = 5 - }, false}, + }, errorsmod.Wrap(ibcerrors.ErrInvalidHeight, "failed packet commitment verification for client (07-tendermint-0): client state height < proof height ({1 17} < {1 22}), please ensure the client has been updated")}, {"verification failed - wrong expected next seq recv", func() { offsetSeq = 1 - }, false}, + }, errorsmod.Wrap(commitmenttypes.ErrInvalidProof, "failed packet commitment verification for client (07-tendermint-0): failed to verify membership proof at index 0: provided value doesn't match proof")}, {"client status is not active - client is expired", func() { clientState, ok := path.EndpointA.GetClientState().(*ibctm.ClientState) suite.Require().True(ok) clientState.FrozenHeight = clienttypes.NewHeight(0, 1) path.EndpointA.SetClientState(clientState) - }, false}, + }, errorsmod.Wrap(clienttypes.ErrClientNotActive, "client (07-tendermint-0) status is Frozen")}, } for _, tc := range cases { @@ -524,10 +533,11 @@ func (suite *KeeperTestSuite) TestVerifyNextSequenceRecv() { packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()+offsetSeq, ) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) } else { suite.Require().Error(err) + suite.Require().ErrorIs(err, tc.expErr) } }) } @@ -542,12 +552,12 @@ func (suite *KeeperTestSuite) TestVerifyUpgradeErrorReceipt() { cases := []struct { name string malleate func() - expPass bool + expErr error }{ { name: "success", malleate: func() {}, - expPass: true, + expErr: nil, }, { name: "fails when client state is frozen", @@ -557,14 +567,14 @@ func (suite *KeeperTestSuite) TestVerifyUpgradeErrorReceipt() { clientState.FrozenHeight = clienttypes.NewHeight(0, 1) path.EndpointB.SetClientState(clientState) }, - expPass: false, + expErr: errorsmod.Wrap(clienttypes.ErrClientNotActive, "client (07-tendermint-0) status is Frozen"), }, { name: "fails with bad client id", malleate: func() { path.EndpointB.UpdateConnection(func(c *types.ConnectionEnd) { c.ClientId = ibctesting.InvalidID }) }, - expPass: false, + expErr: errorsmod.Wrap(clienttypes.ErrClientNotActive, "client (IDisInvalid) status is Unauthorized"), }, { name: "verification fails when the key does not exist", @@ -572,7 +582,7 @@ func (suite *KeeperTestSuite) TestVerifyUpgradeErrorReceipt() { suite.chainA.DeleteKey(host.ChannelUpgradeErrorKey(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)) suite.coordinator.CommitBlock(suite.chainA) }, - expPass: false, + expErr: errorsmod.Wrap(ibcerrors.ErrInvalidHeight, "failed upgrade error receipt verification for client (07-tendermint-0): client state height < proof height ({1 17} < {1 18}), please ensure the client has been updated"), }, { name: "verification fails when message differs", @@ -580,7 +590,7 @@ func (suite *KeeperTestSuite) TestVerifyUpgradeErrorReceipt() { originalSequence := upgradeError.GetErrorReceipt().Sequence upgradeError = channeltypes.NewUpgradeError(originalSequence, fmt.Errorf("new error")) }, - expPass: false, + expErr: errorsmod.Wrap(commitmenttypes.ErrInvalidProof, "failed upgrade error receipt verification for client (07-tendermint-0): failed to verify membership proof at index 0: provided value doesn't match proof"), }, } @@ -606,10 +616,11 @@ func (suite *KeeperTestSuite) TestVerifyUpgradeErrorReceipt() { err := suite.chainB.GetSimApp().IBCKeeper.ConnectionKeeper.VerifyChannelUpgradeError(suite.chainB.GetContext(), path.EndpointB.GetConnection(), proofHeight, proof, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, upgradeError.GetErrorReceipt()) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) } else { suite.Require().Error(err) + suite.Require().ErrorIs(err, tc.expErr) } }) } @@ -624,12 +635,12 @@ func (suite *KeeperTestSuite) TestVerifyUpgrade() { cases := []struct { name string malleate func() - expPass bool + expErr error }{ { name: "success", malleate: func() {}, - expPass: true, + expErr: nil, }, { name: "fails when client state is frozen", @@ -639,21 +650,21 @@ func (suite *KeeperTestSuite) TestVerifyUpgrade() { clientState.FrozenHeight = clienttypes.NewHeight(0, 1) path.EndpointB.SetClientState(clientState) }, - expPass: false, + expErr: errorsmod.Wrap(clienttypes.ErrClientNotActive, "client (07-tendermint-0) status is Frozen"), }, { name: "fails with bad client id", malleate: func() { path.EndpointB.UpdateConnection(func(c *types.ConnectionEnd) { c.ClientId = ibctesting.InvalidID }) }, - expPass: false, + expErr: errorsmod.Wrap(clienttypes.ErrClientNotActive, "client (IDisInvalid) status is Unauthorized"), }, { name: "fails when the upgrade field is different", malleate: func() { upgrade.Fields.Ordering = channeltypes.ORDERED }, - expPass: false, + expErr: errorsmod.Wrap(commitmenttypes.ErrInvalidProof, "failed upgrade verification for client (07-tendermint-0) on channel (channel-7): failed to verify membership proof at index 0: provided value doesn't match proof"), }, } @@ -684,10 +695,11 @@ func (suite *KeeperTestSuite) TestVerifyUpgrade() { err := suite.chainB.GetSimApp().IBCKeeper.ConnectionKeeper.VerifyChannelUpgrade(suite.chainB.GetContext(), path.EndpointB.GetConnection(), proofHeight, proof, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, upgrade) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) } else { suite.Require().Error(err) + suite.Require().ErrorIs(err, tc.expErr) } }) } diff --git a/modules/core/genesis_test.go b/modules/core/genesis_test.go index c35321b6dce..ea378605fd1 100644 --- a/modules/core/genesis_test.go +++ b/modules/core/genesis_test.go @@ -1,6 +1,7 @@ package ibc_test import ( + "errors" "fmt" "testing" @@ -62,12 +63,12 @@ func (suite *IBCTestSuite) TestValidateGenesis() { testCases := []struct { name string genState *types.GenesisState - expPass bool + expError error }{ { name: "default", genState: types.DefaultGenesisState(), - expPass: true, + expError: nil, }, { name: "valid genesis", @@ -145,7 +146,7 @@ func (suite *IBCTestSuite) TestValidateGenesis() { channeltypes.Params{UpgradeTimeout: channeltypes.DefaultTimeout}, ), }, - expPass: true, + expError: nil, }, { name: "invalid client genesis", @@ -172,7 +173,7 @@ func (suite *IBCTestSuite) TestValidateGenesis() { ), ConnectionGenesis: connectiontypes.DefaultGenesisState(), }, - expPass: false, + expError: errors.New("genesis metadata key cannot be empty"), }, { name: "invalid connection genesis", @@ -189,7 +190,7 @@ func (suite *IBCTestSuite) TestValidateGenesis() { connectiontypes.Params{}, ), }, - expPass: false, + expError: errors.New("invalid connection"), }, { name: "invalid channel genesis", @@ -202,17 +203,18 @@ func (suite *IBCTestSuite) TestValidateGenesis() { }, }, }, - expPass: false, + expError: errors.New("invalid acknowledgement"), }, } for _, tc := range testCases { tc := tc err := tc.genState.Validate() - if tc.expPass { + if tc.expError == nil { suite.Require().NoError(err, tc.name) } else { suite.Require().Error(err, tc.name) + suite.Require().Contains(err.Error(), tc.expError.Error()) } } } diff --git a/modules/core/keeper/keeper_test.go b/modules/core/keeper/keeper_test.go index f03bd02a0c9..77d11f82c70 100644 --- a/modules/core/keeper/keeper_test.go +++ b/modules/core/keeper/keeper_test.go @@ -51,29 +51,39 @@ func (suite *KeeperTestSuite) TestNewKeeper() { testCases := []struct { name string malleate func() - expPass bool + expPanic string }{ - {"failure: empty upgrade keeper value", func() { - emptyUpgradeKeeperValue := upgradekeeper.Keeper{} - - upgradeKeeper = emptyUpgradeKeeperValue - }, false}, - {"failure: empty upgrade keeper pointer", func() { - emptyUpgradeKeeperPointer := &upgradekeeper.Keeper{} - - upgradeKeeper = emptyUpgradeKeeperPointer - }, false}, - {"failure: empty authority", func() { - newIBCKeeperFn = func() { - ibckeeper.NewKeeper( - suite.chainA.GetSimApp().AppCodec(), - runtime.NewKVStoreService(suite.chainA.GetSimApp().GetKey(ibcexported.StoreKey)), - suite.chainA.GetSimApp().GetSubspace(ibcexported.ModuleName), - upgradeKeeper, - "", // authority - ) - } - }, false}, + { + name: "failure: empty upgrade keeper value", + malleate: func() { + emptyUpgradeKeeperValue := upgradekeeper.Keeper{} + upgradeKeeper = emptyUpgradeKeeperValue + }, + expPanic: "cannot initialize IBC keeper: empty upgrade keeper", + }, + { + name: "failure: empty upgrade keeper pointer", + malleate: func() { + emptyUpgradeKeeperPointer := &upgradekeeper.Keeper{} + upgradeKeeper = emptyUpgradeKeeperPointer + }, + expPanic: "cannot initialize IBC keeper: empty upgrade keeper", + }, + { + name: "failure: empty authority", + malleate: func() { + newIBCKeeperFn = func() { + ibckeeper.NewKeeper( + suite.chainA.GetSimApp().AppCodec(), + runtime.NewKVStoreService(suite.chainA.GetSimApp().GetKey(ibcexported.StoreKey)), + suite.chainA.GetSimApp().GetSubspace(ibcexported.ModuleName), + upgradeKeeper, + "", // authority + ) + } + }, + expPanic: "authority cannot be empty", + }, } for _, tc := range testCases { @@ -96,14 +106,19 @@ func (suite *KeeperTestSuite) TestNewKeeper() { tc.malleate() - if tc.expPass { - suite.Require().NotPanics( - newIBCKeeperFn, - ) + if tc.expPanic != "" { + suite.Require().Panics(func() { + newIBCKeeperFn() + }, "expected panic but no panic occurred") + + defer func() { + if r := recover(); r != nil { + suite.Require().Contains(r.(error).Error(), tc.expPanic, "unexpected panic message") + } + }() + } else { - suite.Require().Panics( - newIBCKeeperFn, - ) + suite.Require().NotPanics(newIBCKeeperFn, "unexpected panic occurred") } }) } diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go index b787a7f0bad..519c36d1438 100644 --- a/modules/core/keeper/msg_server_test.go +++ b/modules/core/keeper/msg_server_test.go @@ -45,7 +45,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { testCases := []struct { name string malleate func() - expPass bool + expError error expRevert bool async bool // indicate no ack written replay bool // indicate replay (no-op) @@ -58,7 +58,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { suite.Require().NoError(err) packet = channeltypes.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0) - }, true, false, false, false}, + }, nil, false, false, false}, {"success: UNORDERED", func() { path.Setup() @@ -66,7 +66,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { suite.Require().NoError(err) packet = channeltypes.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0) - }, true, false, false, false}, + }, nil, false, false, false}, {"success: UNORDERED out of order packet", func() { // setup uses an UNORDERED channel path.Setup() @@ -78,7 +78,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { packet = channeltypes.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0) } - }, true, false, false, false}, + }, nil, false, false, false}, {"success: OnRecvPacket callback returns revert=true", func() { path.Setup() @@ -86,7 +86,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { suite.Require().NoError(err) packet = channeltypes.NewPacket(ibctesting.MockFailPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0) - }, true, true, false, false}, + }, nil, true, false, false}, {"success: ORDERED - async acknowledgement", func() { path.SetChannelOrdered() path.Setup() @@ -95,7 +95,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { suite.Require().NoError(err) packet = channeltypes.NewPacket(ibcmock.MockAsyncPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0) - }, true, false, true, false}, + }, nil, false, true, false}, {"success: UNORDERED - async acknowledgement", func() { path.Setup() @@ -103,7 +103,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { suite.Require().NoError(err) packet = channeltypes.NewPacket(ibcmock.MockAsyncPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0) - }, true, false, true, false}, + }, nil, false, true, false}, {"failure: ORDERED out of order packet", func() { path.SetChannelOrdered() path.Setup() @@ -115,15 +115,15 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { packet = channeltypes.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0) } - }, false, false, false, false}, + }, errors.New("packet sequence is out of order"), false, false, false}, {"channel does not exist", func() { // any non-nil value of packet is valid suite.Require().NotNil(packet) - }, false, false, false, false}, + }, errors.New("channel not found"), false, false, false}, {"packet not sent", func() { path.Setup() packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0) - }, false, false, false, false}, + }, errors.New("receive packet verification failed: couldn't verify counterparty packet commitment"), false, false, false}, {"successful no-op: ORDERED - packet already received (replay)", func() { // mock will panic if application callback is called twice on the same packet path.SetChannelOrdered() @@ -135,7 +135,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { packet = channeltypes.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0) err = path.EndpointB.RecvPacket(packet) suite.Require().NoError(err) - }, true, false, false, true}, + }, nil, false, false, true}, {"successful no-op: UNORDERED - packet already received (replay)", func() { // mock will panic if application callback is called twice on the same packet path.Setup() @@ -146,7 +146,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { packet = channeltypes.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0) err = path.EndpointB.RecvPacket(packet) suite.Require().NoError(err) - }, true, false, false, true}, + }, nil, false, false, true}, } for _, tc := range testCases { @@ -176,7 +176,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { events := ctx.EventManager().Events() - if tc.expPass { + if tc.expError == nil { suite.Require().NoError(err) // replay should not fail since it will be treated as a no-op @@ -211,6 +211,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { } } else { suite.Require().Error(err) + suite.Require().Contains(err.Error(), tc.expError.Error()) } }) } @@ -307,7 +308,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { testCases := []struct { name string malleate func() - expPass bool + expError error replay bool // indicate replay (no-op) }{ {"success: ORDERED", func() { @@ -320,7 +321,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { packet = channeltypes.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0) err = path.EndpointB.RecvPacket(packet) suite.Require().NoError(err) - }, true, false}, + }, nil, false}, {"success: UNORDERED", func() { path.Setup() @@ -330,7 +331,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { packet = channeltypes.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0) err = path.EndpointB.RecvPacket(packet) suite.Require().NoError(err) - }, true, false}, + }, nil, false}, {"success: UNORDERED acknowledge out of order packet", func() { // setup uses an UNORDERED channel path.Setup() @@ -344,7 +345,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { err = path.EndpointB.RecvPacket(packet) suite.Require().NoError(err) } - }, true, false}, + }, nil, false}, {"failure: ORDERED acknowledge out of order packet", func() { path.SetChannelOrdered() path.Setup() @@ -358,11 +359,11 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { err = path.EndpointB.RecvPacket(packet) suite.Require().NoError(err) } - }, false, false}, + }, errors.New("packet sequence is out of order"), false}, {"channel does not exist", func() { // any non-nil value of packet is valid suite.Require().NotNil(packet) - }, false, false}, + }, errors.New("channel not found"), false}, {"packet not received", func() { path.Setup() @@ -370,7 +371,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { suite.Require().NoError(err) packet = channeltypes.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0) - }, false, false}, + }, errors.New("invalid proof"), false}, {"successful no-op: ORDERED - packet already acknowledged (replay)", func() { path.Setup() @@ -383,7 +384,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { err = path.EndpointA.AcknowledgePacket(packet, ibctesting.MockAcknowledgement) suite.Require().NoError(err) - }, true, true}, + }, nil, true}, {"successful no-op: UNORDERED - packet already acknowledged (replay)", func() { path.Setup() @@ -396,7 +397,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { err = path.EndpointA.AcknowledgePacket(packet, ibctesting.MockAcknowledgement) suite.Require().NoError(err) - }, true, true}, + }, nil, true}, } for _, tc := range testCases { @@ -424,7 +425,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { events := ctx.EventManager().Events() - if tc.expPass { + if tc.expError == nil { suite.Require().NoError(err) // verify packet commitment was deleted on source chain @@ -444,6 +445,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { } } else { suite.Require().Error(err) + suite.Require().Contains(err.Error(), tc.expError.Error()) } }) } @@ -464,7 +466,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutPacket() { testCases := []struct { name string malleate func() - expPass bool + expErr error noop bool // indicate no-op }{ {"success: ORDERED", func() { @@ -484,7 +486,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutPacket() { packet = channeltypes.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, timeoutTimestamp) packetKey = host.NextSequenceRecvKey(packet.GetDestPort(), packet.GetDestChannel()) - }, true, false}, + }, nil, false}, {"success: UNORDERED", func() { path.Setup() @@ -501,7 +503,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutPacket() { packet = channeltypes.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, timeoutTimestamp) packetKey = host.PacketReceiptKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) - }, true, false}, + }, nil, false}, {"success: UNORDERED timeout out of order packet", func() { // setup uses an UNORDERED channel path.Setup() @@ -522,7 +524,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutPacket() { suite.Require().NoError(err) packetKey = host.PacketReceiptKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) - }, true, false}, + }, nil, false}, {"success: ORDERED timeout out of order packet", func() { path.SetChannelOrdered() path.Setup() @@ -543,18 +545,18 @@ func (suite *KeeperTestSuite) TestHandleTimeoutPacket() { suite.Require().NoError(err) packetKey = host.NextSequenceRecvKey(packet.GetDestPort(), packet.GetDestChannel()) - }, true, false}, + }, nil, false}, {"channel does not exist", func() { // any non-nil value of packet is valid suite.Require().NotNil(packet) packetKey = host.NextSequenceRecvKey(packet.GetDestPort(), packet.GetDestChannel()) - }, false, false}, + }, errors.New("channel not found"), false}, {"successful no-op: UNORDERED - packet not sent", func() { path.Setup() packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, clienttypes.NewHeight(0, 1), 0) packetKey = host.PacketReceiptKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) - }, true, true}, + }, nil, true}, } for _, tc := range testCases { @@ -581,7 +583,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutPacket() { events := ctx.EventManager().Events() - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) // replay should not return an error as it is treated as a no-op @@ -602,6 +604,8 @@ func (suite *KeeperTestSuite) TestHandleTimeoutPacket() { } else { suite.Require().Error(err) + + suite.Require().Contains(err.Error(), tc.expErr.Error()) } }) } @@ -623,7 +627,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutOnClosePacket() { testCases := []struct { name string malleate func() - expPass bool + expError error }{ {"success: ORDERED", func() { path.SetChannelOrdered() @@ -642,7 +646,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutOnClosePacket() { // close counterparty channel path.EndpointB.UpdateChannel(func(channel *channeltypes.Channel) { channel.State = channeltypes.CLOSED }) - }, true}, + }, nil}, {"success: UNORDERED", func() { path.Setup() @@ -659,7 +663,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutOnClosePacket() { // close counterparty channel path.EndpointB.UpdateChannel(func(channel *channeltypes.Channel) { channel.State = channeltypes.CLOSED }) - }, true}, + }, nil}, {"success: UNORDERED timeout out of order packet", func() { // setup uses an UNORDERED channel path.Setup() @@ -681,7 +685,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutOnClosePacket() { // close counterparty channel path.EndpointB.UpdateChannel(func(channel *channeltypes.Channel) { channel.State = channeltypes.CLOSED }) - }, true}, + }, nil}, {"success: ORDERED timeout out of order packet", func() { path.SetChannelOrdered() path.Setup() @@ -703,13 +707,13 @@ func (suite *KeeperTestSuite) TestHandleTimeoutOnClosePacket() { // close counterparty channel path.EndpointB.UpdateChannel(func(channel *channeltypes.Channel) { channel.State = channeltypes.CLOSED }) - }, true}, + }, nil}, {"channel does not exist", func() { // any non-nil value of packet is valid suite.Require().NotNil(packet) packetKey = host.NextSequenceRecvKey(packet.GetDestPort(), packet.GetDestChannel()) - }, false}, + }, errors.New("channel not found")}, {"successful no-op: UNORDERED - packet not sent", func() { path.Setup() packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, clienttypes.NewHeight(0, 1), 0) @@ -717,7 +721,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutOnClosePacket() { // close counterparty channel path.EndpointB.UpdateChannel(func(channel *channeltypes.Channel) { channel.State = channeltypes.CLOSED }) - }, true}, + }, nil}, {"ORDERED: channel not closed", func() { path.SetChannelOrdered() path.Setup() @@ -732,7 +736,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutOnClosePacket() { packet = channeltypes.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0) packetKey = host.NextSequenceRecvKey(packet.GetDestPort(), packet.GetDestChannel()) - }, false}, + }, errors.New("invalid proof")}, } for _, tc := range testCases { @@ -753,7 +757,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutOnClosePacket() { _, err := suite.chainA.App.GetIBCKeeper().TimeoutOnClose(suite.chainA.GetContext(), msg) - if tc.expPass { + if tc.expError == nil { suite.Require().NoError(err) // replay should not return an error as it will be treated as a no-op @@ -766,6 +770,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutOnClosePacket() { } else { suite.Require().Error(err) + suite.Require().Contains(err.Error(), tc.expError.Error()) } }) } @@ -782,9 +787,9 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { msg *clienttypes.MsgUpgradeClient ) cases := []struct { - name string - setup func() - expPass bool + name string + setup func() + expErr error }{ { name: "successful upgrade", @@ -822,7 +827,7 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { upgradeClientProof, upgradedConsensusStateProof, suite.chainA.SenderAccount.GetAddress().String()) suite.Require().NoError(err) }, - expPass: true, + expErr: nil, }, { name: "VerifyUpgrade fails", @@ -855,7 +860,7 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { msg, err = clienttypes.NewMsgUpgradeClient(path.EndpointA.ClientID, upgradedClient, upgradedConsState, nil, nil, suite.chainA.SenderAccount.GetAddress().String()) suite.Require().NoError(err) }, - expPass: false, + expErr: errors.New("invalid merkle proof"), }, } @@ -879,7 +884,7 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { ctx := suite.chainA.GetContext() _, err = suite.chainA.GetSimApp().GetIBCKeeper().UpgradeClient(ctx, msg) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err, "upgrade handler failed on valid case: %s", tc.name) newClient, ok := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientState(suite.chainA.GetContext(), path.EndpointA.ClientID) suite.Require().True(ok) @@ -899,6 +904,7 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { ibctesting.AssertEvents(&suite.Suite, expectedEvents, ctx.EventManager().Events().ToABCIEvents()) } else { suite.Require().Error(err, "upgrade handler passed on invalid case: %s", tc.name) + suite.Require().Contains(err.Error(), tc.expErr.Error()) } } } @@ -2529,34 +2535,34 @@ func (suite *KeeperTestSuite) TestIBCSoftwareUpgrade() { func (suite *KeeperTestSuite) TestUpdateClientParams() { signer := suite.chainA.App.GetIBCKeeper().GetAuthority() testCases := []struct { - name string - msg *clienttypes.MsgUpdateParams - expPass bool + name string + msg *clienttypes.MsgUpdateParams + expError error }{ { "success: valid signer and default params", clienttypes.NewMsgUpdateParams(signer, clienttypes.DefaultParams()), - true, + nil, }, { "failure: malformed signer address", clienttypes.NewMsgUpdateParams(ibctesting.InvalidID, clienttypes.DefaultParams()), - false, + errors.New("unauthorized"), }, { "failure: empty signer address", clienttypes.NewMsgUpdateParams("", clienttypes.DefaultParams()), - false, + errors.New("unauthorized"), }, { "failure: whitespace signer address", clienttypes.NewMsgUpdateParams(" ", clienttypes.DefaultParams()), - false, + errors.New("unauthorized"), }, { "failure: unauthorized signer address", clienttypes.NewMsgUpdateParams(ibctesting.TestAccAddress, clienttypes.DefaultParams()), - false, + errors.New("unauthorized"), }, } @@ -2565,12 +2571,13 @@ func (suite *KeeperTestSuite) TestUpdateClientParams() { suite.Run(tc.name, func() { suite.SetupTest() _, err := suite.chainA.App.GetIBCKeeper().UpdateClientParams(suite.chainA.GetContext(), tc.msg) - if tc.expPass { + if tc.expError == nil { suite.Require().NoError(err) p := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetParams(suite.chainA.GetContext()) suite.Require().Equal(tc.msg.Params, p) } else { suite.Require().Error(err) + suite.Require().Contains(err.Error(), tc.expError.Error()) } }) } @@ -2580,34 +2587,34 @@ func (suite *KeeperTestSuite) TestUpdateClientParams() { func (suite *KeeperTestSuite) TestUpdateConnectionParams() { signer := suite.chainA.App.GetIBCKeeper().GetAuthority() testCases := []struct { - name string - msg *connectiontypes.MsgUpdateParams - expPass bool + name string + msg *connectiontypes.MsgUpdateParams + expErr error }{ { "success: valid signer and default params", connectiontypes.NewMsgUpdateParams(signer, connectiontypes.DefaultParams()), - true, + nil, }, { "failure: malformed signer address", connectiontypes.NewMsgUpdateParams(ibctesting.InvalidID, connectiontypes.DefaultParams()), - false, + errors.New("unauthorized"), }, { "failure: empty signer address", connectiontypes.NewMsgUpdateParams("", connectiontypes.DefaultParams()), - false, + errors.New("unauthorized"), }, { "failure: whitespace signer address", connectiontypes.NewMsgUpdateParams(" ", connectiontypes.DefaultParams()), - false, + errors.New("unauthorized"), }, { "failure: unauthorized signer address", connectiontypes.NewMsgUpdateParams(ibctesting.TestAccAddress, connectiontypes.DefaultParams()), - false, + errors.New("unauthorized"), }, } @@ -2616,12 +2623,13 @@ func (suite *KeeperTestSuite) TestUpdateConnectionParams() { suite.Run(tc.name, func() { suite.SetupTest() _, err := suite.chainA.App.GetIBCKeeper().UpdateConnectionParams(suite.chainA.GetContext(), tc.msg) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) p := suite.chainA.App.GetIBCKeeper().ConnectionKeeper.GetParams(suite.chainA.GetContext()) suite.Require().Equal(tc.msg.Params, p) } else { suite.Require().Error(err) + suite.Require().Contains(err.Error(), tc.expErr.Error()) } }) } diff --git a/modules/light-clients/08-wasm/keeper/grpc_query_test.go b/modules/light-clients/08-wasm/keeper/grpc_query_test.go index a54cc20f616..4ede97842fe 100644 --- a/modules/light-clients/08-wasm/keeper/grpc_query_test.go +++ b/modules/light-clients/08-wasm/keeper/grpc_query_test.go @@ -3,6 +3,11 @@ package keeper_test import ( "encoding/hex" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + errorsmod "cosmossdk.io/errors" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" @@ -16,7 +21,7 @@ func (suite *KeeperTestSuite) TestQueryCode() { testCases := []struct { name string malleate func() - expPass bool + expErr error }{ { "success", @@ -29,21 +34,27 @@ func (suite *KeeperTestSuite) TestQueryCode() { req = &types.QueryCodeRequest{Checksum: hex.EncodeToString(res.Checksum)} }, - true, + nil, }, { "fails with empty request", func() { req = &types.QueryCodeRequest{} }, - false, + status.Error( + codes.NotFound, + errorsmod.Wrap(types.ErrWasmChecksumNotFound, "").Error(), + ), }, { "fails with non-existent checksum", func() { req = &types.QueryCodeRequest{Checksum: "test"} }, - false, + status.Error( + codes.InvalidArgument, + types.ErrInvalidChecksum.Error(), + ), }, } @@ -55,12 +66,13 @@ func (suite *KeeperTestSuite) TestQueryCode() { res, err := GetSimApp(suite.chainA).WasmClientKeeper.Code(suite.chainA.GetContext(), req) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) suite.Require().NotNil(res) suite.Require().NotEmpty(res.Data) } else { suite.Require().Error(err) + suite.Require().ErrorIs(err, tc.expErr) } }) } @@ -72,14 +84,14 @@ func (suite *KeeperTestSuite) TestQueryChecksums() { testCases := []struct { name string malleate func() - expPass bool + expErr error }{ { "success with no checksums", func() { expChecksums = []string{} }, - true, + nil, }, { "success with one checksum", @@ -92,7 +104,7 @@ func (suite *KeeperTestSuite) TestQueryChecksums() { expChecksums = append(expChecksums, hex.EncodeToString(res.Checksum)) }, - true, + nil, }, } @@ -105,7 +117,7 @@ func (suite *KeeperTestSuite) TestQueryChecksums() { req := &types.QueryChecksumsRequest{} res, err := GetSimApp(suite.chainA).WasmClientKeeper.Checksums(suite.chainA.GetContext(), req) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) suite.Require().NotNil(res) suite.Require().Equal(len(expChecksums), len(res.Checksums)) diff --git a/modules/light-clients/08-wasm/keeper/keeper_test.go b/modules/light-clients/08-wasm/keeper/keeper_test.go index 1039494e0d1..dacea997a09 100644 --- a/modules/light-clients/08-wasm/keeper/keeper_test.go +++ b/modules/light-clients/08-wasm/keeper/keeper_test.go @@ -315,8 +315,7 @@ func (suite *KeeperTestSuite) TestInitializedPinnedCodes() { err := wasmClientKeeper.InitializePinnedCodes(ctx) - expPass := tc.expError == nil - if expPass { + if tc.expError == nil { suite.Require().NoError(err) suite.ElementsMatch(checksumIDs, capturedChecksums) } else { @@ -432,8 +431,7 @@ func (suite *KeeperTestSuite) TestMigrateContract() { clientState, ok = endpointA.GetClientState().(*types.ClientState) suite.Require().True(ok) - expPass := tc.expErr == nil - if expPass { + if tc.expErr == nil { suite.Require().NoError(err) suite.Require().Equal(expClientState, clientState) } else { diff --git a/modules/light-clients/08-wasm/types/client_message_test.go b/modules/light-clients/08-wasm/types/client_message_test.go index c405e3fe0f8..0cc2111fcf6 100644 --- a/modules/light-clients/08-wasm/types/client_message_test.go +++ b/modules/light-clients/08-wasm/types/client_message_test.go @@ -1,6 +1,8 @@ package types_test import ( + errorsmod "cosmossdk.io/errors" + "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types" ) @@ -8,28 +10,28 @@ func (suite *TypesTestSuite) TestClientMessageValidateBasic() { testCases := []struct { name string clientMessage *types.ClientMessage - expPass bool + expErr error }{ { "valid client message", &types.ClientMessage{ Data: []byte("data"), }, - true, + nil, }, { "data is nil", &types.ClientMessage{ Data: nil, }, - false, + errorsmod.Wrap(types.ErrInvalidData, "data cannot be empty"), }, { "data is empty", &types.ClientMessage{ Data: []byte{}, }, - false, + errorsmod.Wrap(types.ErrInvalidData, "data cannot be empty"), }, } @@ -40,10 +42,11 @@ func (suite *TypesTestSuite) TestClientMessageValidateBasic() { suite.Require().Equal(types.Wasm, clientMessage.ClientType()) err := clientMessage.ValidateBasic() - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) } else { suite.Require().Error(err) + suite.Require().ErrorIs(err, tc.expErr) } }) } diff --git a/modules/light-clients/08-wasm/types/client_state_test.go b/modules/light-clients/08-wasm/types/client_state_test.go index ecd6e70ca01..f042e0183b5 100644 --- a/modules/light-clients/08-wasm/types/client_state_test.go +++ b/modules/light-clients/08-wasm/types/client_state_test.go @@ -1,6 +1,8 @@ package types_test import ( + errorsmod "cosmossdk.io/errors" + wasmtesting "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/testing" "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types" clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types" @@ -10,32 +12,32 @@ func (suite *TypesTestSuite) TestValidate() { testCases := []struct { name string clientState *types.ClientState - expPass bool + expErr error }{ { name: "valid client", clientState: types.NewClientState([]byte{0}, wasmtesting.Code, clienttypes.ZeroHeight()), - expPass: true, + expErr: nil, }, { name: "nil data", clientState: types.NewClientState(nil, wasmtesting.Code, clienttypes.ZeroHeight()), - expPass: false, + expErr: errorsmod.Wrap(types.ErrInvalidData, "data cannot be empty"), }, { name: "empty data", clientState: types.NewClientState([]byte{}, wasmtesting.Code, clienttypes.ZeroHeight()), - expPass: false, + expErr: errorsmod.Wrap(types.ErrInvalidData, "data cannot be empty"), }, { name: "nil checksum", clientState: types.NewClientState([]byte{0}, nil, clienttypes.ZeroHeight()), - expPass: false, + expErr: errorsmod.Wrap(types.ErrInvalidChecksum, "checksum cannot be empty"), }, { name: "empty checksum", clientState: types.NewClientState([]byte{0}, []byte{}, clienttypes.ZeroHeight()), - expPass: false, + expErr: errorsmod.Wrap(types.ErrInvalidChecksum, "checksum cannot be empty"), }, { name: "longer than 32 bytes checksum", @@ -49,17 +51,18 @@ func (suite *TypesTestSuite) TestValidate() { }, clienttypes.ZeroHeight(), ), - expPass: false, + expErr: errorsmod.Wrap(types.ErrInvalidChecksum, "checksum cannot be empty"), }, } for _, tc := range testCases { suite.Run(tc.name, func() { err := tc.clientState.Validate() - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err, tc.name) } else { suite.Require().Error(err, tc.name) + suite.Require().ErrorIs(err, tc.expErr) } }) } diff --git a/modules/light-clients/08-wasm/types/codec_test.go b/modules/light-clients/08-wasm/types/codec_test.go index 91cf3846e16..a5ec44d2123 100644 --- a/modules/light-clients/08-wasm/types/codec_test.go +++ b/modules/light-clients/08-wasm/types/codec_test.go @@ -1,6 +1,7 @@ package types_test import ( + "fmt" "testing" "github.com/stretchr/testify/require" @@ -16,42 +17,42 @@ func TestCodecTypeRegistration(t *testing.T) { testCases := []struct { name string typeURL string - expPass bool + expErr error }{ { "success: ClientState", sdk.MsgTypeURL(&types.ClientState{}), - true, + nil, }, { "success: ConsensusState", sdk.MsgTypeURL(&types.ConsensusState{}), - true, + nil, }, { "success: ClientMessage", sdk.MsgTypeURL(&types.ClientMessage{}), - true, + nil, }, { "success: MsgStoreCode", sdk.MsgTypeURL(&types.MsgStoreCode{}), - true, + nil, }, { "success: MsgMigrateContract", sdk.MsgTypeURL(&types.MsgMigrateContract{}), - true, + nil, }, { "success: MsgRemoveChecksum", sdk.MsgTypeURL(&types.MsgRemoveChecksum{}), - true, + nil, }, { "type not registered on codec", "ibc.invalid.MsgTypeURL", - false, + fmt.Errorf("unable to resolve type URL ibc.invalid.MsgTypeURL"), }, } @@ -62,12 +63,13 @@ func TestCodecTypeRegistration(t *testing.T) { encodingCfg := moduletestutil.MakeTestEncodingConfig(wasm.AppModuleBasic{}) msg, err := encodingCfg.Codec.InterfaceRegistry().Resolve(tc.typeURL) - if tc.expPass { + if tc.expErr == nil { require.NotNil(t, msg) require.NoError(t, err) } else { require.Nil(t, msg) require.Error(t, err) + require.Equal(t, err.Error(), tc.expErr.Error()) } }) } diff --git a/modules/light-clients/08-wasm/types/genesis_test.go b/modules/light-clients/08-wasm/types/genesis_test.go index ab5c6d73af5..3c9d0df46c1 100644 --- a/modules/light-clients/08-wasm/types/genesis_test.go +++ b/modules/light-clients/08-wasm/types/genesis_test.go @@ -1,6 +1,8 @@ package types_test import ( + errorsmod "cosmossdk.io/errors" + "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types" ) @@ -8,31 +10,32 @@ func (suite *TypesTestSuite) TestValidateGenesis() { testCases := []struct { name string genState *types.GenesisState - expPass bool + expErr error }{ { "valid genesis", &types.GenesisState{ Contracts: []types.Contract{{CodeBytes: []byte{1}}}, }, - true, + nil, }, { "invalid genesis", &types.GenesisState{ Contracts: []types.Contract{{CodeBytes: []byte{}}}, }, - false, + errorsmod.Wrap(types.ErrWasmEmptyCode, "wasm bytecode validation failed"), }, } for _, tc := range testCases { tc := tc err := tc.genState.Validate() - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) } else { suite.Require().Error(err) + suite.Require().ErrorIs(err, tc.expErr) } } } diff --git a/modules/light-clients/08-wasm/types/msgs_test.go b/modules/light-clients/08-wasm/types/msgs_test.go index 4326f634782..9167386ec17 100644 --- a/modules/light-clients/08-wasm/types/msgs_test.go +++ b/modules/light-clients/08-wasm/types/msgs_test.go @@ -1,6 +1,7 @@ package types_test import ( + "fmt" "testing" "github.com/stretchr/testify/require" @@ -49,8 +50,7 @@ func TestMsgStoreCodeValidateBasic(t *testing.T) { tc := tc err := tc.msg.ValidateBasic() - expPass := tc.expErr == nil - if expPass { + if tc.expErr == nil { require.NoError(t, err) } else { require.ErrorIs(t, err, tc.expErr) @@ -62,10 +62,10 @@ func (suite *TypesTestSuite) TestMsgStoreCodeGetSigners() { testCases := []struct { name string address sdk.AccAddress - expPass bool + expErr error }{ - {"success: valid address", sdk.AccAddress(ibctesting.TestAccAddress), true}, - {"failure: nil address", nil, false}, + {"success: valid address", sdk.AccAddress(ibctesting.TestAccAddress), nil}, + {"failure: nil address", nil, fmt.Errorf("empty address string is not allowed")}, } for _, tc := range testCases { @@ -77,11 +77,12 @@ func (suite *TypesTestSuite) TestMsgStoreCodeGetSigners() { msg := types.NewMsgStoreCode(address.String(), wasmtesting.Code) signers, _, err := GetSimApp(suite.chainA).AppCodec().GetMsgV1Signers(msg) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) suite.Require().Equal(address.Bytes(), signers[0]) } else { suite.Require().Error(err) + suite.Require().Equal(err.Error(), tc.expErr.Error()) } }) } @@ -165,10 +166,10 @@ func (suite *TypesTestSuite) TestMsgMigrateContractGetSigners() { testCases := []struct { name string address sdk.AccAddress - expPass bool + expErr error }{ - {"success: valid address", sdk.AccAddress(ibctesting.TestAccAddress), true}, - {"failure: nil address", nil, false}, + {"success: valid address", sdk.AccAddress(ibctesting.TestAccAddress), nil}, + {"failure: nil address", nil, fmt.Errorf("empty address string is not allowed")}, } for _, tc := range testCases { @@ -180,11 +181,12 @@ func (suite *TypesTestSuite) TestMsgMigrateContractGetSigners() { msg := types.NewMsgMigrateContract(address.String(), defaultWasmClientID, checksum, []byte("{}")) signers, _, err := GetSimApp(suite.chainA).AppCodec().GetMsgV1Signers(msg) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) suite.Require().Equal(address.Bytes(), signers[0]) } else { suite.Require().Error(err) + suite.Require().Equal(err.Error(), tc.expErr.Error()) } }) } diff --git a/modules/light-clients/09-localhost/light_client_module_test.go b/modules/light-clients/09-localhost/light_client_module_test.go index 108b52c8f99..8c059d37f3c 100644 --- a/modules/light-clients/09-localhost/light_client_module_test.go +++ b/modules/light-clients/09-localhost/light_client_module_test.go @@ -1,6 +1,7 @@ package localhost_test import ( + "errors" "testing" testifysuite "github.com/stretchr/testify/suite" @@ -76,7 +77,7 @@ func (suite *LocalhostTestSuite) TestVerifyMembership() { testCases := []struct { name string malleate func() - expPass bool + expErr error }{ { "success: connection state verification", @@ -97,7 +98,7 @@ func (suite *LocalhostTestSuite) TestVerifyMembership() { path = merklePath value = suite.chain.Codec.MustMarshal(&connectionEnd) }, - true, + nil, }, { "success: channel state verification", @@ -119,7 +120,7 @@ func (suite *LocalhostTestSuite) TestVerifyMembership() { path = merklePath value = suite.chain.Codec.MustMarshal(&channel) }, - true, + nil, }, { "success: next sequence recv verification", @@ -134,7 +135,7 @@ func (suite *LocalhostTestSuite) TestVerifyMembership() { path = merklePath value = sdk.Uint64ToBigEndian(nextSeqRecv) }, - true, + nil, }, { "success: packet commitment verification", @@ -160,7 +161,7 @@ func (suite *LocalhostTestSuite) TestVerifyMembership() { path = merklePath value = commitmentBz }, - true, + nil, }, { "success: packet acknowledgement verification", @@ -174,21 +175,21 @@ func (suite *LocalhostTestSuite) TestVerifyMembership() { path = merklePath value = ibctesting.MockAcknowledgement }, - true, + nil, }, { "failure: invalid type for key path", func() { path = mock.KeyPath{} }, - false, + errors.New("expected v2.MerklePath, got mock.KeyPath: invalid type"), }, { "failure: key path has too many elements", func() { path = commitmenttypes.NewMerklePath([]byte("ibc"), []byte("test"), []byte("key")) }, - false, + errors.New("invalid path"), }, { "failure: no value found at provided key path", @@ -200,7 +201,7 @@ func (suite *LocalhostTestSuite) TestVerifyMembership() { path = merklePath value = ibctesting.MockAcknowledgement }, - false, + errors.New("value not found for path"), }, { "failure: invalid value, bytes are not equal", @@ -225,7 +226,7 @@ func (suite *LocalhostTestSuite) TestVerifyMembership() { channel.State = channeltypes.CLOSED value = suite.chain.Codec.MustMarshal(&channel) }, - false, + errors.New("value provided does not equal value stored at path"), }, } @@ -250,10 +251,11 @@ func (suite *LocalhostTestSuite) TestVerifyMembership() { value, ) - if tc.expPass { + if tc.expErr == nil { suite.Require().NoError(err) } else { suite.Require().Error(err) + suite.ErrorContains(err, tc.expErr.Error()) } }) } @@ -265,7 +267,7 @@ func (suite *LocalhostTestSuite) TestVerifyNonMembership() { testCases := []struct { name string malleate func() - expPass bool + expError error }{ { "success: packet receipt absence verification", @@ -276,7 +278,7 @@ func (suite *LocalhostTestSuite) TestVerifyNonMembership() { path = merklePath }, - true, + nil, }, { "packet receipt absence verification fails", @@ -289,21 +291,21 @@ func (suite *LocalhostTestSuite) TestVerifyNonMembership() { path = merklePath }, - false, + errors.New("non-membership verification failed"), }, { "invalid type for key path", func() { path = mock.KeyPath{} }, - false, + errors.New("expected v2.MerklePath, got mock.KeyPath: invalid type"), }, { "key path has too many elements", func() { path = commitmenttypes.NewMerklePath([]byte("ibc"), []byte("test"), []byte("key")) }, - false, + errors.New("invalid path"), }, } @@ -327,10 +329,11 @@ func (suite *LocalhostTestSuite) TestVerifyNonMembership() { path, ) - if tc.expPass { + if tc.expError == nil { suite.Require().NoError(err) } else { suite.Require().Error(err) + suite.ErrorContains(err, tc.expError.Error()) } }) }