From 12ff33d92e3da3e25a236e4081fd6142b48dc658 Mon Sep 17 00:00:00 2001 From: Tanmay Date: Wed, 29 May 2024 13:56:35 -0400 Subject: [PATCH] add comments --- x/authority/genesis_test.go | 114 ++++++++++++------ x/authority/keeper/authorization_list.go | 2 +- x/authority/keeper/authorization_list_test.go | 2 + x/authority/types/authorizations.go | 3 + x/authority/types/authorizations_test.go | 25 ++-- 5 files changed, 100 insertions(+), 46 deletions(-) diff --git a/x/authority/genesis_test.go b/x/authority/genesis_test.go index 4bdf82d82c..5a6ae2abf7 100644 --- a/x/authority/genesis_test.go +++ b/x/authority/genesis_test.go @@ -4,7 +4,6 @@ import ( "testing" "github.com/stretchr/testify/require" - keepertest "github.com/zeta-chain/zetacore/testutil/keeper" "github.com/zeta-chain/zetacore/testutil/nullify" "github.com/zeta-chain/zetacore/testutil/sample" @@ -13,37 +12,84 @@ import ( ) func TestGenesis(t *testing.T) { - genesisState := types.GenesisState{ - Policies: sample.Policies(), - AuthorizationList: sample.AuthorizationList("sample"), - ChainInfo: sample.ChainInfo(42), - } - - // Init - k, ctx := keepertest.AuthorityKeeper(t) - authority.InitGenesis(ctx, *k, genesisState) - - // Check policy is set - policies, found := k.GetPolicies(ctx) - require.True(t, found) - require.Equal(t, genesisState.Policies, policies) - - // Check authorization list is set - authorizationList, found := k.GetAuthorizationList(ctx) - require.True(t, found) - require.Equal(t, genesisState.AuthorizationList, authorizationList) - - // Check chain info is set - chainInfo, found := k.GetChainInfo(ctx) - require.True(t, found) - require.Equal(t, genesisState.ChainInfo, chainInfo) - - // Export - got := authority.ExportGenesis(ctx, *k) - require.NotNil(t, got) - - // Compare genesis after init and export - nullify.Fill(&genesisState) - nullify.Fill(got) - require.Equal(t, genesisState, *got) + t.Run("valid genesis", func(t *testing.T) { + genesisState := types.GenesisState{ + Policies: sample.Policies(), + AuthorizationList: sample.AuthorizationList("sample"), + ChainInfo: sample.ChainInfo(42), + } + + // Init + k, ctx := keepertest.AuthorityKeeper(t) + authority.InitGenesis(ctx, *k, genesisState) + + // Check policy is set + policies, found := k.GetPolicies(ctx) + require.True(t, found) + require.Equal(t, genesisState.Policies, policies) + + // Check authorization list is set + authorizationList, found := k.GetAuthorizationList(ctx) + require.True(t, found) + require.Equal(t, genesisState.AuthorizationList, authorizationList) + + // Check chain info is set + chainInfo, found := k.GetChainInfo(ctx) + require.True(t, found) + require.Equal(t, genesisState.ChainInfo, chainInfo) + + // Export + got := authority.ExportGenesis(ctx, *k) + require.NotNil(t, got) + + // Compare genesis after init and export + nullify.Fill(&genesisState) + nullify.Fill(got) + require.Equal(t, genesisState, *got) + }) + + t.Run("set genesis and export works but invalid authorization table is not set", func(t *testing.T) { + genesisState := types.GenesisState{ + Policies: sample.Policies(), + AuthorizationList: types.AuthorizationList{Authorizations: []types.Authorization{ + { + MsgUrl: "ABC", + AuthorizedPolicy: types.PolicyType_groupOperational, + }, + { + MsgUrl: "ABC", + AuthorizedPolicy: types.PolicyType_groupEmergency, + }, + }}, + ChainInfo: sample.ChainInfo(42), + } + + // Init + k, ctx := keepertest.AuthorityKeeper(t) + authority.InitGenesis(ctx, *k, genesisState) + + // Check policy is set + policies, found := k.GetPolicies(ctx) + require.True(t, found) + require.Equal(t, genesisState.Policies, policies) + + // Check chain info is set + chainInfo, found := k.GetChainInfo(ctx) + require.True(t, found) + require.Equal(t, genesisState.ChainInfo, chainInfo) + + // Check authorization list is not set + _, found = k.GetAuthorizationList(ctx) + require.False(t, found) + + // Export + got := authority.ExportGenesis(ctx, *k) + require.NotNil(t, got) + + // Compare genesis after init and export + nullify.Fill(&genesisState) + nullify.Fill(got) + require.Equal(t, genesisState, *got) + + }) } diff --git a/x/authority/keeper/authorization_list.go b/x/authority/keeper/authorization_list.go index b50401d6e9..3573549370 100644 --- a/x/authority/keeper/authorization_list.go +++ b/x/authority/keeper/authorization_list.go @@ -6,7 +6,7 @@ import ( "github.com/zeta-chain/zetacore/x/authority/types" ) -// SetAuthorizationList sets the authorization list to the store +// SetAuthorizationList sets the authorization list to the store.It returns an error if the list is invalid. func (k Keeper) SetAuthorizationList(ctx sdk.Context, list types.AuthorizationList) error { err := list.Validate() if err != nil { diff --git a/x/authority/keeper/authorization_list_test.go b/x/authority/keeper/authorization_list_test.go index 9fd4cea4cf..e445dfc1a8 100644 --- a/x/authority/keeper/authorization_list_test.go +++ b/x/authority/keeper/authorization_list_test.go @@ -32,6 +32,7 @@ func TestKeeper_SetAuthorizationList(t *testing.T) { k, ctx := keepertest.AuthorityKeeper(t) authorizationList := sample.AuthorizationList("sample") require.NoError(t, k.SetAuthorizationList(ctx, authorizationList)) + list, found := k.GetAuthorizationList(ctx) require.True(t, found) require.Equal(t, authorizationList, list) @@ -39,6 +40,7 @@ func TestKeeper_SetAuthorizationList(t *testing.T) { newAuthorizationList := sample.AuthorizationList("sample2") require.NotEqual(t, authorizationList, newAuthorizationList) require.NoError(t, k.SetAuthorizationList(ctx, newAuthorizationList)) + list, found = k.GetAuthorizationList(ctx) require.True(t, found) require.Equal(t, newAuthorizationList, list) diff --git a/x/authority/types/authorizations.go b/x/authority/types/authorizations.go index 3d0dfbf7ac..1fbb2533f5 100644 --- a/x/authority/types/authorizations.go +++ b/x/authority/types/authorizations.go @@ -6,6 +6,8 @@ import ( "cosmossdk.io/errors" ) +// DefaultAuthorizationsList list is the list of authorizations that presently exist in the system. +// This is the minimum set of authorizations that are required to be set when the authorization table is deployed func DefaultAuthorizationsList() AuthorizationList { var authorizations []Authorization @@ -178,6 +180,7 @@ func (a *AuthorizationList) GetAuthorizedPolicy(msgURL string) (PolicyType, erro } // Validate checks if the authorization list is valid. It returns an error if the message url is duplicated with different policies. +// It does not check if the list is empty or not, as an empty list is also considered valid. func (a *AuthorizationList) Validate() error { checkMsgUrls := make(map[string]bool) for _, authorization := range a.Authorizations { diff --git a/x/authority/types/authorizations_test.go b/x/authority/types/authorizations_test.go index 1e40501025..08e80a07c6 100644 --- a/x/authority/types/authorizations_test.go +++ b/x/authority/types/authorizations_test.go @@ -15,19 +15,21 @@ import ( ) func TestAuthorizationList_SetAuthorizations(t *testing.T) { - t.Run("Set new authorization successfully", func(t *testing.T) { + t.Run("set new authorization successfully", func(t *testing.T) { authorizationsList := types.DefaultAuthorizationsList() newAuthorization := sample.Authorization() require.False(t, authorizationsList.CheckAuthorizationExists(newAuthorization)) + authorizationsList.SetAuthorizations(newAuthorization) require.Len(t, authorizationsList.Authorizations, len(types.DefaultAuthorizationsList().Authorizations)+1) require.True(t, authorizationsList.CheckAuthorizationExists(newAuthorization)) }) - t.Run("Update existing authorization successfully", func(t *testing.T) { + t.Run("update existing authorization successfully", func(t *testing.T) { authorizationsList := types.DefaultAuthorizationsList() newAuthorization := sample.Authorization() require.False(t, authorizationsList.CheckAuthorizationExists(newAuthorization)) + authorizationsList.SetAuthorizations(newAuthorization) require.Len(t, authorizationsList.Authorizations, len(types.DefaultAuthorizationsList().Authorizations)+1) require.True(t, authorizationsList.CheckAuthorizationExists(newAuthorization)) @@ -42,16 +44,17 @@ func TestAuthorizationList_SetAuthorizations(t *testing.T) { } func TestAuthorizationList_GetAuthorizedPolicy(t *testing.T) { - t.Run("Get authorized policy successfully", func(t *testing.T) { + t.Run("get authorized policy successfully", func(t *testing.T) { authorizationsList := types.DefaultAuthorizationsList() newAuthorization := sample.Authorization() authorizationsList.SetAuthorizations(newAuthorization) + policy, err := authorizationsList.GetAuthorizedPolicy(newAuthorization.MsgUrl) require.NoError(t, err) require.Equal(t, newAuthorization.AuthorizedPolicy, policy) }) - t.Run("Get authorized policy failed with not found", func(t *testing.T) { + t.Run("get authorized policy fails when msg not found in list", func(t *testing.T) { authorizationsList := types.DefaultAuthorizationsList() policy, err := authorizationsList.GetAuthorizedPolicy("ABC") require.ErrorIs(t, err, types.ErrAuthorizationNotFound) @@ -60,21 +63,22 @@ func TestAuthorizationList_GetAuthorizedPolicy(t *testing.T) { } func TestAuthorizationList_CheckAuthorizationExists(t *testing.T) { - t.Run("Check authorization exists successfully", func(t *testing.T) { + t.Run("check authorization exists successfully", func(t *testing.T) { authorizationsList := types.DefaultAuthorizationsList() newAuthorization := sample.Authorization() require.False(t, authorizationsList.CheckAuthorizationExists(newAuthorization)) + authorizationsList.SetAuthorizations(newAuthorization) require.True(t, authorizationsList.CheckAuthorizationExists(newAuthorization)) }) } func TestAuthorizationList_Validate(t *testing.T) { - t.Run("Validate successfully", func(t *testing.T) { + t.Run("validate successfully", func(t *testing.T) { authorizationsList := types.DefaultAuthorizationsList() require.NoError(t, authorizationsList.Validate()) }) - t.Run("Validate failed with duplicate msg url with different policies", func(t *testing.T) { + t.Run("validate failed with duplicate msg url with different policies", func(t *testing.T) { authorizationsList := types.AuthorizationList{Authorizations: []types.Authorization{ { MsgUrl: "ABC", @@ -89,7 +93,7 @@ func TestAuthorizationList_Validate(t *testing.T) { require.ErrorIs(t, authorizationsList.Validate(), types.ErrInValidAuthorizationList) }) - t.Run("Validate failed with duplicate msg url with same policies", func(t *testing.T) { + t.Run("validate failed with duplicate msg url with same policies", func(t *testing.T) { authorizationsList := types.AuthorizationList{Authorizations: []types.Authorization{ { MsgUrl: "ABC", @@ -103,11 +107,10 @@ func TestAuthorizationList_Validate(t *testing.T) { require.ErrorIs(t, authorizationsList.Validate(), types.ErrInValidAuthorizationList) }) - } func TestAuthorizationList_RemoveAuthorizations(t *testing.T) { - t.Run("Remove authorization successfully", func(t *testing.T) { + t.Run("remove authorization successfully", func(t *testing.T) { authorizationsList := types.DefaultAuthorizationsList() newAuthorization := sample.Authorization() authorizationsList.SetAuthorizations(newAuthorization) @@ -125,7 +128,7 @@ func TestAuthorizationList_RemoveAuthorizations(t *testing.T) { } func TestDefaultAuthorizationsList(t *testing.T) { - t.Run("Default authorizations list", func(t *testing.T) { + t.Run("default authorizations list", func(t *testing.T) { var OperationalPolicyMessageList = []string{ sdk.MsgTypeURL(&crosschaintypes.MsgRefundAbortedCCTX{}), sdk.MsgTypeURL(&crosschaintypes.MsgAbortStuckCCTX{}),