fix comments 1

zeta-chain · Jun 4, 2024 · bcff29f · bcff29f
1 parent 28ae79b
commit bcff29f
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 16 deletions.
diff --git 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
@@ -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,14 +83,14 @@ 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) {
 		k, ctx := keepertest.AuthorityKeeper(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
@@ -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 → x/authority/types/policy_type.go b/x/authority/types/policytype.go → x/authority/types/policy_type.go
diff --git a/x/authority/types/policytype_test.go → x/authority/types/policy_type_test.go b/x/authority/types/policytype_test.go → 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,