From c18bd7d3776754598a3ebc3a527995e71faf7f15 Mon Sep 17 00:00:00 2001 From: Tanmay Date: Thu, 30 May 2024 21:42:40 -0400 Subject: [PATCH] resolve comments --- .../zetacore/authority/authorization.proto | 2 + x/authority/keeper/authorization_list.go | 5 +- x/authority/types/authorizations.go | 176 ++++++------------ x/authority/types/authorizations_test.go | 133 +++++++++++++ x/authority/types/keys.go | 3 +- 5 files changed, 198 insertions(+), 121 deletions(-) diff --git a/proto/zetachain/zetacore/authority/authorization.proto b/proto/zetachain/zetacore/authority/authorization.proto index 4f5a4330ef..3a319c3837 100644 --- a/proto/zetachain/zetacore/authority/authorization.proto +++ b/proto/zetachain/zetacore/authority/authorization.proto @@ -6,12 +6,14 @@ import "zetachain/zetacore/authority/policies.proto"; option go_package = "github.com/zeta-chain/zetacore/x/authority/types"; +// Authorization defines the authorization required to access use a message which needs special permissions message Authorization { // The URL of the message that needs to be authorized string msg_url = 1; // The policy that is authorized to access the message PolicyType authorized_policy = 2; } + // AuthorizationList holds the list of authorizations on zetachain message AuthorizationList { repeated Authorization authorizations = 1 [ (gogoproto.nullable) = false ]; diff --git a/x/authority/keeper/authorization_list.go b/x/authority/keeper/authorization_list.go index 4d85f8dc21..0e7fba9163 100644 --- a/x/authority/keeper/authorization_list.go +++ b/x/authority/keeper/authorization_list.go @@ -1,6 +1,7 @@ package keeper import ( + "github.com/cosmos/cosmos-sdk/store/prefix" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/zeta-chain/zetacore/x/authority/types" @@ -8,14 +9,14 @@ import ( // 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) { - store := ctx.KVStore(k.storeKey) + store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(types.AuthorizationListKey)) b := k.cdc.MustMarshal(&list) store.Set([]byte{0}, b) } // GetAuthorizationList returns the authorization list from the store func (k Keeper) GetAuthorizationList(ctx sdk.Context) (val types.AuthorizationList, found bool) { - store := ctx.KVStore(k.storeKey) + store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(types.AuthorizationListKey)) b := store.Get([]byte{0}) if b == nil { return val, false diff --git a/x/authority/types/authorizations.go b/x/authority/types/authorizations.go index 4878992cfb..4d590d9d4d 100644 --- a/x/authority/types/authorizations.go +++ b/x/authority/types/authorizations.go @@ -6,130 +6,69 @@ import ( "cosmossdk.io/errors" ) +var ( + OperationPolicyMessages = []string{ + "/zetachain.zetacore.crosschain.MsgRefundAbortedCCTX", + "/zetachain.zetacore.crosschain.MsgAbortStuckCCTX", + "/zetachain.zetacore.crosschain.MsgUpdateRateLimiterFlags", + "/zetachain.zetacore.crosschain.MsgWhitelistERC20", + "/zetachain.zetacore.fungible.MsgDeployFungibleCoinZRC20", + "/zetachain.zetacore.fungible.MsgDeploySystemContracts", + "/zetachain.zetacore.fungible.MsgRemoveForeignCoin", + "/zetachain.zetacore.fungible.MsgUpdateZRC20LiquidityCap", + "/zetachain.zetacore.fungible.MsgUpdateZRC20WithdrawFee", + "/zetachain.zetacore.fungible.MsgUnpauseZRC20", + "/zetachain.zetacore.observer.MsgAddObserver", + "/zetachain.zetacore.observer.MsgRemoveChainParams", + "/zetachain.zetacore.observer.MsgResetChainNonces", + "/zetachain.zetacore.observer.MsgUpdateChainParams", + "/zetachain.zetacore.observer.MsgEnableCCTX", + "/zetachain.zetacore.observer.MsgUpdateGasPriceIncreaseFlags", + "/zetachain.zetacore.lightclient.MsgEnableHeaderVerification", + } + AdminPolicyMessages = []string{ + "/zetachain.zetacore.crosschain.MsgMigrateTssFunds", + "/zetachain.zetacore.crosschain.MsgUpdateTssAddress", + "/zetachain.zetacore.fungible.MsgUpdateContractBytecode", + "/zetachain.zetacore.fungible.MsgUpdateSystemContract", + "/zetachain.zetacore.observer.MsgUpdateObserver", + } + EmergencyPolicyMessages = []string{ + "/zetachain.zetacore.crosschain.MsgAddInboundTracker", + "/zetachain.zetacore.crosschain.MsgAddOutboundTracker", + "/zetachain.zetacore.crosschain.MsgRemoveOutboundTracker", + "/zetachain.zetacore.fungible.MsgPauseZRC20", + "/zetachain.zetacore.observer.MsgUpdateKeygen", + "/zetachain.zetacore.observer.MsgDisableCCTX", + "/zetachain.zetacore.lightclient.MsgDisableHeaderVerification", + } +) + // 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 - - authorizations = []Authorization{ - // OperationalPolicyMessageList - { - MsgUrl: "/zetachain.zetacore.crosschain.MsgRefundAbortedCCTX", - AuthorizedPolicy: PolicyType_groupOperational}, - { - MsgUrl: "/zetachain.zetacore.crosschain.MsgAbortStuckCCTX", - AuthorizedPolicy: PolicyType_groupOperational, - }, - { - MsgUrl: "/zetachain.zetacore.crosschain.MsgUpdateRateLimiterFlags", - AuthorizedPolicy: PolicyType_groupOperational, - }, - { - MsgUrl: "/zetachain.zetacore.crosschain.MsgWhitelistERC20", - AuthorizedPolicy: PolicyType_groupOperational}, - { - MsgUrl: "/zetachain.zetacore.fungible.MsgDeployFungibleCoinZRC20", - AuthorizedPolicy: PolicyType_groupOperational, - }, - { - MsgUrl: "/zetachain.zetacore.fungible.MsgDeploySystemContracts", - AuthorizedPolicy: PolicyType_groupOperational, - }, - { - MsgUrl: "/zetachain.zetacore.fungible.MsgRemoveForeignCoin", - AuthorizedPolicy: PolicyType_groupOperational, - }, - { - MsgUrl: "/zetachain.zetacore.fungible.MsgUpdateZRC20LiquidityCap", - AuthorizedPolicy: PolicyType_groupOperational, - }, - { - MsgUrl: "/zetachain.zetacore.fungible.MsgUpdateZRC20WithdrawFee", - AuthorizedPolicy: PolicyType_groupOperational, - }, - { - MsgUrl: "/zetachain.zetacore.fungible.MsgUnpauseZRC20", - AuthorizedPolicy: PolicyType_groupOperational, - }, - { - MsgUrl: "/zetachain.zetacore.observer.MsgAddObserver", - AuthorizedPolicy: PolicyType_groupOperational, - }, - { - MsgUrl: "/zetachain.zetacore.observer.MsgRemoveChainParams", - AuthorizedPolicy: PolicyType_groupOperational, - }, - { - MsgUrl: "/zetachain.zetacore.observer.MsgResetChainNonces", - AuthorizedPolicy: PolicyType_groupOperational, - }, - - { - MsgUrl: "/zetachain.zetacore.observer.MsgUpdateChainParams", + authorizations := make([]Authorization, len(OperationPolicyMessages)+len(AdminPolicyMessages)+len(EmergencyPolicyMessages)) + index := 0 + for _, msgURL := range OperationPolicyMessages { + authorizations[index] = Authorization{ + MsgUrl: msgURL, AuthorizedPolicy: PolicyType_groupOperational, - }, - { - MsgUrl: "/zetachain.zetacore.observer.MsgEnableCCTX", - AuthorizedPolicy: PolicyType_groupOperational, - }, - { - MsgUrl: "/zetachain.zetacore.observer.MsgUpdateGasPriceIncreaseFlags", - AuthorizedPolicy: PolicyType_groupOperational, - }, - { - MsgUrl: "/zetachain.zetacore.lightclient.MsgEnableHeaderVerification", - AuthorizedPolicy: PolicyType_groupOperational, - }, - // AdminPolicyMessageList - { - MsgUrl: "/zetachain.zetacore.crosschain.MsgMigrateTssFunds", - AuthorizedPolicy: PolicyType_groupAdmin, - }, - { - MsgUrl: "/zetachain.zetacore.crosschain.MsgUpdateTssAddress", - AuthorizedPolicy: PolicyType_groupAdmin, - }, - { - MsgUrl: "/zetachain.zetacore.fungible.MsgUpdateContractBytecode", - AuthorizedPolicy: PolicyType_groupAdmin, - }, - { - MsgUrl: "/zetachain.zetacore.fungible.MsgUpdateSystemContract", - AuthorizedPolicy: PolicyType_groupAdmin, - }, - { - MsgUrl: "/zetachain.zetacore.observer.MsgUpdateObserver", + } + index++ + } + for _, msgURL := range AdminPolicyMessages { + authorizations[index] = Authorization{ + MsgUrl: msgURL, AuthorizedPolicy: PolicyType_groupAdmin, - }, - // EmergencyPolicyMessageList - { - MsgUrl: "/zetachain.zetacore.crosschain.MsgAddInboundTracker", - AuthorizedPolicy: PolicyType_groupEmergency, - }, - { - MsgUrl: "/zetachain.zetacore.crosschain.MsgAddOutboundTracker", - AuthorizedPolicy: PolicyType_groupEmergency, - }, - { - MsgUrl: "/zetachain.zetacore.crosschain.MsgRemoveOutboundTracker", - AuthorizedPolicy: PolicyType_groupEmergency, - }, - { - MsgUrl: "/zetachain.zetacore.fungible.MsgPauseZRC20", - AuthorizedPolicy: PolicyType_groupEmergency, - }, - { - MsgUrl: "/zetachain.zetacore.observer.MsgUpdateKeygen", - AuthorizedPolicy: PolicyType_groupEmergency, - }, - { - MsgUrl: "/zetachain.zetacore.observer.MsgDisableCCTX", - AuthorizedPolicy: PolicyType_groupEmergency, - }, - { - MsgUrl: "/zetachain.zetacore.lightclient.MsgDisableHeaderVerification", + } + index++ + } + for _, msgURL := range EmergencyPolicyMessages { + authorizations[index] = Authorization{ + MsgUrl: msgURL, AuthorizedPolicy: PolicyType_groupEmergency, - }, + } + index++ } return AuthorizationList{ @@ -165,6 +104,7 @@ func (a *AuthorizationList) GetAuthorizedPolicy(msgURL string) (PolicyType, erro return auth.AuthorizedPolicy, nil } } + fmt.Println("Authorization not found", msgURL) // Returning first value of enum, can consider adding a default value of `EmptyPolicy` in the enum. return PolicyType(0), ErrAuthorizationNotFound } diff --git a/x/authority/types/authorizations_test.go b/x/authority/types/authorizations_test.go index ba2d0aacb4..2ea44c788c 100644 --- a/x/authority/types/authorizations_test.go +++ b/x/authority/types/authorizations_test.go @@ -45,6 +45,20 @@ func TestAuthorizationList_SetAuthorizations(t *testing.T) { }, }}, }, + { + name: "set new authorization successfully with empty list", + oldList: types.AuthorizationList{Authorizations: []types.Authorization{}}, + addAuthorization: types.Authorization{ + MsgUrl: "XYZ", + AuthorizedPolicy: types.PolicyType_groupOperational, + }, + expectedList: types.AuthorizationList{Authorizations: []types.Authorization{ + { + MsgUrl: "XYZ", + AuthorizedPolicy: types.PolicyType_groupOperational, + }, + }}, + }, { name: "update existing authorization successfully", oldList: types.AuthorizationList{Authorizations: []types.Authorization{ @@ -64,6 +78,41 @@ func TestAuthorizationList_SetAuthorizations(t *testing.T) { }, }}, }, + { + name: "update existing authorization successfully in the middle of the list", + oldList: types.AuthorizationList{Authorizations: []types.Authorization{ + { + MsgUrl: "ABC", + AuthorizedPolicy: types.PolicyType_groupOperational, + }, + { + MsgUrl: "XYZ", + AuthorizedPolicy: types.PolicyType_groupOperational, + }, + { + MsgUrl: "DEF", + AuthorizedPolicy: types.PolicyType_groupOperational, + }, + }}, + addAuthorization: types.Authorization{ + MsgUrl: "XYZ", + AuthorizedPolicy: types.PolicyType_groupEmergency, + }, + expectedList: types.AuthorizationList{Authorizations: []types.Authorization{ + { + MsgUrl: "ABC", + AuthorizedPolicy: types.PolicyType_groupOperational, + }, + { + MsgUrl: "XYZ", + AuthorizedPolicy: types.PolicyType_groupEmergency, + }, + { + MsgUrl: "DEF", + AuthorizedPolicy: types.PolicyType_groupOperational, + }, + }}, + }, } for _, tc := range tt { @@ -94,6 +143,42 @@ func TestAuthorizationList_GetAuthorizations(t *testing.T) { expectedPolicy: types.PolicyType_groupOperational, error: nil, }, + { + name: "get authorizations successfully for admin policy", + authorizations: types.AuthorizationList{Authorizations: []types.Authorization{ + { + MsgUrl: "ABC", + AuthorizedPolicy: types.PolicyType_groupAdmin, + }, + }}, + getPolicyMsgUrl: "ABC", + expectedPolicy: types.PolicyType_groupAdmin, + error: nil, + }, + { + name: "get authorizations successfully for emergency policy", + authorizations: types.AuthorizationList{Authorizations: []types.Authorization{ + { + MsgUrl: "ABC", + AuthorizedPolicy: types.PolicyType_groupEmergency, + }, + }}, + getPolicyMsgUrl: "ABC", + expectedPolicy: types.PolicyType_groupEmergency, + error: nil, + }, + { + name: "get authorizations fails when msg not found in list", + authorizations: types.AuthorizationList{Authorizations: []types.Authorization{ + { + MsgUrl: "ABC", + AuthorizedPolicy: types.PolicyType_groupOperational, + }, + }}, + getPolicyMsgUrl: "XYZ", + expectedPolicy: types.PolicyType(0), + error: types.ErrAuthorizationNotFound, + }, { name: "get authorizations fails when msg not found in list", authorizations: types.AuthorizationList{Authorizations: []types.Authorization{}}, @@ -138,6 +223,16 @@ func TestAuthorizationList_Validate(t *testing.T) { }}, expectedError: nil, }, + { + name: "validate successfully with empty list", + authorizations: types.AuthorizationList{Authorizations: []types.Authorization{}}, + expectedError: nil, + }, + { + name: "validate successfully for default list", + authorizations: types.DefaultAuthorizationsList(), + expectedError: nil, + }, { name: "validate failed with duplicate msg url with different policies", authorizations: types.AuthorizationList{Authorizations: []types.Authorization{ @@ -207,6 +302,44 @@ func TestAuthorizationList_RemoveAuthorizations(t *testing.T) { }, }}, }, + { + name: "remove authorization successfully in the middle of the list", + oldList: types.AuthorizationList{Authorizations: []types.Authorization{ + { + MsgUrl: "ABC", + AuthorizedPolicy: types.PolicyType_groupOperational, + }, + { + MsgUrl: "XYZ", + AuthorizedPolicy: types.PolicyType_groupOperational, + }, + { + MsgUrl: "DEF", + AuthorizedPolicy: types.PolicyType_groupOperational, + }, + }}, + removeAuthorization: types.Authorization{ + MsgUrl: "XYZ", + }, + expectedList: types.AuthorizationList{Authorizations: []types.Authorization{ + { + MsgUrl: "ABC", + AuthorizedPolicy: types.PolicyType_groupOperational, + }, + { + MsgUrl: "DEF", + AuthorizedPolicy: types.PolicyType_groupOperational, + }, + }}, + }, + { + name: "do not remove anything when trying to remove from an empty list", + oldList: types.AuthorizationList{Authorizations: []types.Authorization{}}, + removeAuthorization: types.Authorization{ + MsgUrl: "XYZ", + }, + expectedList: types.AuthorizationList{Authorizations: []types.Authorization{}}, + }, { name: "do not remove anything if authorization not found", oldList: types.AuthorizationList{Authorizations: []types.Authorization{ diff --git a/x/authority/types/keys.go b/x/authority/types/keys.go index 7593feed3a..04aad4730d 100644 --- a/x/authority/types/keys.go +++ b/x/authority/types/keys.go @@ -24,7 +24,8 @@ func KeyPrefix(p string) []byte { const ( // PoliciesKey is the key for the policies store - PoliciesKey = "Policies-value-" + PoliciesKey = "Policies-value-" + AuthorizationListKey = "AuthorizationList-value-" // ChainInfoKey is the key for the chain info store ChainInfoKey = "ChainInfo-value-"