From bcff29fa9b12893e7c7e65ce855fc08d20e256de Mon Sep 17 00:00:00 2001 From: Tanmay Date: Tue, 4 Jun 2024 10:38:25 -0400 Subject: [PATCH] fix comments 1 --- .../keeper/msg_server_add_authorization.go | 5 +- .../msg_server_add_authorization_test.go | 48 +++++++++++++------ .../msg_server_remove_authorization_test.go | 12 +++++ .../types/{policytype.go => policy_type.go} | 0 ...policytype_test.go => policy_type_test.go} | 5 ++ 5 files changed, 54 insertions(+), 16 deletions(-) rename x/authority/types/{policytype.go => policy_type.go} (100%) rename x/authority/types/{policytype_test.go => policy_type_test.go} (87%) diff --git a/x/authority/keeper/msg_server_add_authorization.go b/x/authority/keeper/msg_server_add_authorization.go index 1e5a4ce171..252884cd3f 100644 --- a/x/authority/keeper/msg_server_add_authorization.go +++ b/x/authority/keeper/msg_server_add_authorization.go @@ -24,7 +24,10 @@ func (k msgServer) AddAuthorization( ) } - authorizationList, _ := k.GetAuthorizationList(ctx) + authorizationList, found := k.GetAuthorizationList(ctx) + if !found { + authorizationList = types.AuthorizationList{Authorizations: []types.Authorization{}} + } authorizationList.SetAuthorization(types.Authorization{MsgUrl: msg.MsgUrl, AuthorizedPolicy: msg.AuthorizedPolicy}) // validate the authorization list after adding the authorization as a precautionary measure. diff --git a/x/authority/keeper/msg_server_add_authorization_test.go b/x/authority/keeper/msg_server_add_authorization_test.go index dd830e7c0b..340c6f975e 100644 --- a/x/authority/keeper/msg_server_add_authorization_test.go +++ b/x/authority/keeper/msg_server_add_authorization_test.go @@ -13,12 +13,13 @@ import ( ) func TestMsgServer_AddAuthorization(t *testing.T) { + const url = "/zetachain.zetacore.sample.ABC" t.Run("successfully add authorization of type admin to existing authorization list", func(t *testing.T) { k, ctx := keepertest.AuthorityKeeper(t) admin := keepertest.SetAdminPolices(ctx, k) k.SetAuthorizationList(ctx, types.DefaultAuthorizationsList()) msgServer := keeper.NewMsgServerImpl(*k) - url := "/zetachain.zetacore.sample.ABC" + prevLen := len(types.DefaultAuthorizationsList().Authorizations) msg := &types.MsgAddAuthorization{ Creator: admin, @@ -34,14 +35,15 @@ func TestMsgServer_AddAuthorization(t *testing.T) { policy, err := authorizationList.GetAuthorizedPolicy(url) require.NoError(t, err) require.Equal(t, types.PolicyType_groupAdmin, policy) + require.Equal(t, prevLen+1, len(authorizationList.Authorizations)) }) t.Run("successfully add authorization of type operational to existing authorization list", func(t *testing.T) { k, ctx := keepertest.AuthorityKeeper(t) admin := keepertest.SetAdminPolices(ctx, k) k.SetAuthorizationList(ctx, types.DefaultAuthorizationsList()) + prevLen := len(types.DefaultAuthorizationsList().Authorizations) msgServer := keeper.NewMsgServerImpl(*k) - url := "/zetachain.zetacore.sample.ABC" msg := &types.MsgAddAuthorization{ Creator: admin, @@ -57,14 +59,15 @@ func TestMsgServer_AddAuthorization(t *testing.T) { policy, err := authorizationList.GetAuthorizedPolicy(url) require.NoError(t, err) require.Equal(t, types.PolicyType_groupOperational, policy) + require.Equal(t, prevLen+1, len(authorizationList.Authorizations)) }) t.Run("successfully add authorization of type emergency to existing authorization list", func(t *testing.T) { k, ctx := keepertest.AuthorityKeeper(t) admin := keepertest.SetAdminPolices(ctx, k) k.SetAuthorizationList(ctx, types.DefaultAuthorizationsList()) + prevLen := len(types.DefaultAuthorizationsList().Authorizations) msgServer := keeper.NewMsgServerImpl(*k) - url := "/zetachain.zetacore.sample.ABC" msg := &types.MsgAddAuthorization{ Creator: admin, @@ -80,6 +83,7 @@ func TestMsgServer_AddAuthorization(t *testing.T) { policy, err := authorizationList.GetAuthorizedPolicy(url) require.NoError(t, err) require.Equal(t, types.PolicyType_groupEmergency, policy) + require.Equal(t, prevLen+1, len(authorizationList.Authorizations)) }) t.Run("successfully add authorization to empty authorization list", func(t *testing.T) { @@ -87,7 +91,6 @@ func TestMsgServer_AddAuthorization(t *testing.T) { admin := keepertest.SetAdminPolices(ctx, k) k.SetAuthorizationList(ctx, types.AuthorizationList{}) msgServer := keeper.NewMsgServerImpl(*k) - url := "/zetachain.zetacore.sample.ABC" msg := &types.MsgAddAuthorization{ Creator: admin, @@ -103,13 +106,15 @@ func TestMsgServer_AddAuthorization(t *testing.T) { policy, err := authorizationList.GetAuthorizedPolicy(url) require.NoError(t, err) require.Equal(t, types.PolicyType_groupAdmin, policy) + require.Equal(t, 1, len(authorizationList.Authorizations)) }) t.Run("successfully set authorization when list is not found ", func(t *testing.T) { k, ctx := keepertest.AuthorityKeeper(t) admin := keepertest.SetAdminPolices(ctx, k) msgServer := keeper.NewMsgServerImpl(*k) - url := "/zetachain.zetacore.sample.ABC" + authorizationList, found := k.GetAuthorizationList(ctx) + require.False(t, found) msg := &types.MsgAddAuthorization{ Creator: admin, @@ -120,25 +125,28 @@ func TestMsgServer_AddAuthorization(t *testing.T) { _, err := msgServer.AddAuthorization(sdk.WrapSDKContext(ctx), msg) require.NoError(t, err) - authorizationList, found := k.GetAuthorizationList(ctx) + authorizationList, found = k.GetAuthorizationList(ctx) require.True(t, found) policy, err := authorizationList.GetAuthorizedPolicy(url) require.NoError(t, err) require.Equal(t, types.PolicyType_groupAdmin, policy) + require.Equal(t, 1, len(authorizationList.Authorizations)) }) t.Run("update existing authorization", func(t *testing.T) { k, ctx := keepertest.AuthorityKeeper(t) admin := keepertest.SetAdminPolices(ctx, k) - k.SetAuthorizationList(ctx, types.AuthorizationList{Authorizations: []types.Authorization{ + authorizationList := types.AuthorizationList{Authorizations: []types.Authorization{ { MsgUrl: "/zetachain.zetacore.sample.ABC", AuthorizedPolicy: types.PolicyType_groupOperational, }, }, - }) + } + k.SetAuthorizationList(ctx, authorizationList) + prevLen := len(authorizationList.Authorizations) + msgServer := keeper.NewMsgServerImpl(*k) - url := "/zetachain.zetacore.sample.ABC" msg := &types.MsgAddAuthorization{ Creator: admin, @@ -154,13 +162,14 @@ func TestMsgServer_AddAuthorization(t *testing.T) { policy, err := authorizationList.GetAuthorizedPolicy(url) require.NoError(t, err) require.Equal(t, types.PolicyType_groupAdmin, policy) + require.Equal(t, prevLen, len(authorizationList.Authorizations)) }) t.Run("fail to add authorization with invalid policy as creator", func(t *testing.T) { k, ctx := keepertest.AuthorityKeeper(t) k.SetAuthorizationList(ctx, types.DefaultAuthorizationsList()) + prevLen := len(types.DefaultAuthorizationsList().Authorizations) msgServer := keeper.NewMsgServerImpl(*k) - url := "/zetachain.zetacore.sample.ABC" msg := &types.MsgAddAuthorization{ Creator: sample.AccAddress(), @@ -170,24 +179,29 @@ func TestMsgServer_AddAuthorization(t *testing.T) { _, err := msgServer.AddAuthorization(sdk.WrapSDKContext(ctx), msg) require.ErrorIs(t, err, types.ErrUnauthorized) + + authorizationList, found := k.GetAuthorizationList(ctx) + require.True(t, found) + require.Equal(t, prevLen, len(authorizationList.Authorizations)) }) // This scenario is not possible as the authorization list is always valid.But it is good to have in case the validation logic is changed in the future t.Run("fail to set invalid authorization list", func(t *testing.T) { k, ctx := keepertest.AuthorityKeeper(t) admin := keepertest.SetAdminPolices(ctx, k) - k.SetAuthorizationList(ctx, types.AuthorizationList{Authorizations: []types.Authorization{ + authorizationList := types.AuthorizationList{Authorizations: []types.Authorization{ { - MsgUrl: "/zetachain.zetacore.sample.ABC", + MsgUrl: url, AuthorizedPolicy: types.PolicyType_groupOperational, }, { - MsgUrl: "/zetachain.zetacore.sample.ABC", + MsgUrl: url, AuthorizedPolicy: types.PolicyType_groupOperational, }, - }}) + }} + k.SetAuthorizationList(ctx, authorizationList) + prevLen := len(authorizationList.Authorizations) msgServer := keeper.NewMsgServerImpl(*k) - url := "/zetachain.zetacore.sample.ABC" msg := &types.MsgAddAuthorization{ Creator: admin, @@ -197,5 +211,9 @@ func TestMsgServer_AddAuthorization(t *testing.T) { _, err := msgServer.AddAuthorization(sdk.WrapSDKContext(ctx), msg) require.ErrorIs(t, err, types.ErrInvalidAuthorizationList) + + authorizationList, found := k.GetAuthorizationList(ctx) + require.True(t, found) + require.Equal(t, prevLen, len(authorizationList.Authorizations)) }) } diff --git a/x/authority/keeper/msg_server_remove_authorization_test.go b/x/authority/keeper/msg_server_remove_authorization_test.go index 90529fe58a..6fa326466e 100644 --- a/x/authority/keeper/msg_server_remove_authorization_test.go +++ b/x/authority/keeper/msg_server_remove_authorization_test.go @@ -17,6 +17,7 @@ func TestMsgServer_RemoveAuthorization(t *testing.T) { k, ctx := keepertest.AuthorityKeeper(t) admin := keepertest.SetAdminPolices(ctx, k) k.SetAuthorizationList(ctx, types.DefaultAuthorizationsList()) + prevLen := len(types.DefaultAuthorizationsList().Authorizations) msgServer := keeper.NewMsgServerImpl(*k) url := types.OperationPolicyMessages[0] @@ -37,12 +38,14 @@ func TestMsgServer_RemoveAuthorization(t *testing.T) { require.True(t, found) _, err = authorizationList.GetAuthorizedPolicy(url) require.ErrorIs(t, err, types.ErrAuthorizationNotFound) + require.Equal(t, prevLen-1, len(authorizationList.Authorizations)) }) t.Run("successfully remove admin policy authorization", func(t *testing.T) { k, ctx := keepertest.AuthorityKeeper(t) admin := keepertest.SetAdminPolices(ctx, k) k.SetAuthorizationList(ctx, types.DefaultAuthorizationsList()) + prevLen := len(types.DefaultAuthorizationsList().Authorizations) msgServer := keeper.NewMsgServerImpl(*k) url := types.AdminPolicyMessages[0] @@ -63,12 +66,14 @@ func TestMsgServer_RemoveAuthorization(t *testing.T) { require.True(t, found) _, err = authorizationList.GetAuthorizedPolicy(url) require.ErrorIs(t, err, types.ErrAuthorizationNotFound) + require.Equal(t, prevLen-1, len(authorizationList.Authorizations)) }) t.Run("successfully remove emergency policy authorization", func(t *testing.T) { k, ctx := keepertest.AuthorityKeeper(t) admin := keepertest.SetAdminPolices(ctx, k) k.SetAuthorizationList(ctx, types.DefaultAuthorizationsList()) + prevLen := len(types.DefaultAuthorizationsList().Authorizations) msgServer := keeper.NewMsgServerImpl(*k) url := types.EmergencyPolicyMessages[0] @@ -89,11 +94,13 @@ func TestMsgServer_RemoveAuthorization(t *testing.T) { require.True(t, found) _, err = authorizationList.GetAuthorizedPolicy(url) require.ErrorIs(t, err, types.ErrAuthorizationNotFound) + require.Equal(t, prevLen-1, len(authorizationList.Authorizations)) }) t.Run("unable to remove authorization if creator is not the correct policy", func(t *testing.T) { k, ctx := keepertest.AuthorityKeeper(t) k.SetAuthorizationList(ctx, types.DefaultAuthorizationsList()) + prevLen := len(types.DefaultAuthorizationsList().Authorizations) msgServer := keeper.NewMsgServerImpl(*k) url := types.OperationPolicyMessages[0] @@ -113,6 +120,7 @@ func TestMsgServer_RemoveAuthorization(t *testing.T) { authorizationList, found = k.GetAuthorizationList(ctx) require.True(t, found) require.Equal(t, types.DefaultAuthorizationsList(), authorizationList) + require.Equal(t, prevLen, len(authorizationList.Authorizations)) }) t.Run("unable to remove authorization if authorization list does not exist", func(t *testing.T) { @@ -188,5 +196,9 @@ func TestMsgServer_RemoveAuthorization(t *testing.T) { _, err := msgServer.RemoveAuthorization(sdk.WrapSDKContext(ctx), msg) require.ErrorIs(t, err, types.ErrInvalidAuthorizationList) + + authorizationListNew, found := k.GetAuthorizationList(ctx) + require.True(t, found) + require.Equal(t, authorizationList, authorizationListNew) }) } diff --git a/x/authority/types/policytype.go b/x/authority/types/policy_type.go similarity index 100% rename from x/authority/types/policytype.go rename to x/authority/types/policy_type.go diff --git a/x/authority/types/policytype_test.go b/x/authority/types/policy_type_test.go similarity index 87% rename from x/authority/types/policytype_test.go rename to x/authority/types/policy_type_test.go index f1957a36a2..c6778c9200 100644 --- a/x/authority/types/policytype_test.go +++ b/x/authority/types/policy_type_test.go @@ -34,6 +34,11 @@ func TestPolicyType_Validate(t *testing.T) { policyType: types.PolicyType(20), wantErr: true, }, + { + name: "invalid policy type more than max length", + policyType: types.PolicyType(len(types.PolicyType_name) + 1), + wantErr: true, + }, { name: "empty policy type", policyType: types.PolicyType_groupEmpty,