From 23d77246db11a9434443012c7c1df14a9fa09c1f Mon Sep 17 00:00:00 2001 From: Lucas Bertrand Date: Tue, 10 Oct 2023 12:00:55 -0700 Subject: [PATCH] fix(`observer`): remove error return in `IsAuthorized` (#1250) * update function * regenerate interfaces * update for crosschain --- testutil/keeper/mocks/crosschain/account.go | 2 +- testutil/keeper/mocks/crosschain/bank.go | 2 +- testutil/keeper/mocks/crosschain/fungible.go | 2 +- testutil/keeper/mocks/crosschain/observer.go | 16 +++------------- testutil/keeper/mocks/crosschain/staking.go | 2 +- testutil/keeper/mocks/fungible/account.go | 2 +- testutil/keeper/mocks/fungible/bank.go | 2 +- testutil/keeper/mocks/fungible/evm.go | 2 +- testutil/keeper/mocks/fungible/observer.go | 2 +- x/crosschain/keeper/keeper_chain_nonces.go | 10 ++++------ .../keeper_cross_chain_tx_vote_inbound_tx.go | 5 ++--- .../keeper_cross_chain_tx_vote_outbound_tx.go | 5 ++--- x/crosschain/keeper/keeper_gas_price.go | 9 ++++----- x/crosschain/keeper/keeper_out_tx_tracker.go | 7 ++----- x/crosschain/types/expected_keepers.go | 2 +- x/observer/keeper/keeper_utils.go | 8 ++++---- x/observer/keeper/msg_server_add_blame_vote.go | 5 ++--- x/observer/keeper/msg_server_add_block_header.go | 5 ++--- 18 files changed, 34 insertions(+), 54 deletions(-) diff --git a/testutil/keeper/mocks/crosschain/account.go b/testutil/keeper/mocks/crosschain/account.go index bde82aa1df..69df546af1 100644 --- a/testutil/keeper/mocks/crosschain/account.go +++ b/testutil/keeper/mocks/crosschain/account.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.34.2. DO NOT EDIT. +// Code generated by mockery v2.35.2. DO NOT EDIT. package mocks diff --git a/testutil/keeper/mocks/crosschain/bank.go b/testutil/keeper/mocks/crosschain/bank.go index bf4e73987b..8b4e4c1b04 100644 --- a/testutil/keeper/mocks/crosschain/bank.go +++ b/testutil/keeper/mocks/crosschain/bank.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.34.2. DO NOT EDIT. +// Code generated by mockery v2.35.2. DO NOT EDIT. package mocks diff --git a/testutil/keeper/mocks/crosschain/fungible.go b/testutil/keeper/mocks/crosschain/fungible.go index ba803b9a52..1a1452f5c7 100644 --- a/testutil/keeper/mocks/crosschain/fungible.go +++ b/testutil/keeper/mocks/crosschain/fungible.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.34.2. DO NOT EDIT. +// Code generated by mockery v2.35.2. DO NOT EDIT. package mocks diff --git a/testutil/keeper/mocks/crosschain/observer.go b/testutil/keeper/mocks/crosschain/observer.go index 0f1d2b310c..8ddc1504cd 100644 --- a/testutil/keeper/mocks/crosschain/observer.go +++ b/testutil/keeper/mocks/crosschain/observer.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.34.2. DO NOT EDIT. +// Code generated by mockery v2.35.2. DO NOT EDIT. package mocks @@ -333,27 +333,17 @@ func (_m *CrosschainObserverKeeper) GetParams(ctx types.Context) observertypes.P } // IsAuthorized provides a mock function with given fields: ctx, address, chain -func (_m *CrosschainObserverKeeper) IsAuthorized(ctx types.Context, address string, chain *common.Chain) (bool, error) { +func (_m *CrosschainObserverKeeper) IsAuthorized(ctx types.Context, address string, chain *common.Chain) bool { ret := _m.Called(ctx, address, chain) var r0 bool - var r1 error - if rf, ok := ret.Get(0).(func(types.Context, string, *common.Chain) (bool, error)); ok { - return rf(ctx, address, chain) - } if rf, ok := ret.Get(0).(func(types.Context, string, *common.Chain) bool); ok { r0 = rf(ctx, address, chain) } else { r0 = ret.Get(0).(bool) } - if rf, ok := ret.Get(1).(func(types.Context, string, *common.Chain) error); ok { - r1 = rf(ctx, address, chain) - } else { - r1 = ret.Error(1) - } - - return r0, r1 + return r0 } // IsInboundEnabled provides a mock function with given fields: ctx diff --git a/testutil/keeper/mocks/crosschain/staking.go b/testutil/keeper/mocks/crosschain/staking.go index 0faf55a58a..7288ae4125 100644 --- a/testutil/keeper/mocks/crosschain/staking.go +++ b/testutil/keeper/mocks/crosschain/staking.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.34.2. DO NOT EDIT. +// Code generated by mockery v2.35.2. DO NOT EDIT. package mocks diff --git a/testutil/keeper/mocks/fungible/account.go b/testutil/keeper/mocks/fungible/account.go index 81f510c6d3..6a77932f69 100644 --- a/testutil/keeper/mocks/fungible/account.go +++ b/testutil/keeper/mocks/fungible/account.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.34.2. DO NOT EDIT. +// Code generated by mockery v2.35.2. DO NOT EDIT. package mocks diff --git a/testutil/keeper/mocks/fungible/bank.go b/testutil/keeper/mocks/fungible/bank.go index da56574169..d04228faf7 100644 --- a/testutil/keeper/mocks/fungible/bank.go +++ b/testutil/keeper/mocks/fungible/bank.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.34.2. DO NOT EDIT. +// Code generated by mockery v2.35.2. DO NOT EDIT. package mocks diff --git a/testutil/keeper/mocks/fungible/evm.go b/testutil/keeper/mocks/fungible/evm.go index e0ac00173c..fe04262d80 100644 --- a/testutil/keeper/mocks/fungible/evm.go +++ b/testutil/keeper/mocks/fungible/evm.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.34.2. DO NOT EDIT. +// Code generated by mockery v2.35.2. DO NOT EDIT. package mocks diff --git a/testutil/keeper/mocks/fungible/observer.go b/testutil/keeper/mocks/fungible/observer.go index fc7ec327b6..d627f7d980 100644 --- a/testutil/keeper/mocks/fungible/observer.go +++ b/testutil/keeper/mocks/fungible/observer.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.34.2. DO NOT EDIT. +// Code generated by mockery v2.35.2. DO NOT EDIT. package mocks diff --git a/x/crosschain/keeper/keeper_chain_nonces.go b/x/crosschain/keeper/keeper_chain_nonces.go index 90291845ed..7ecd816075 100644 --- a/x/crosschain/keeper/keeper_chain_nonces.go +++ b/x/crosschain/keeper/keeper_chain_nonces.go @@ -4,13 +4,12 @@ import ( "context" "fmt" - zetaObserverTypes "github.com/zeta-chain/zetacore/x/observer/types" - "github.com/cosmos/cosmos-sdk/store/prefix" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/types/query" "github.com/zeta-chain/zetacore/x/crosschain/types" + observertypes "github.com/zeta-chain/zetacore/x/observer/types" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) @@ -108,12 +107,11 @@ func (k msgServer) NonceVoter(goCtx context.Context, msg *types.MsgNonceVoter) ( ctx := sdk.UnwrapSDKContext(goCtx) chain := k.zetaObserverKeeper.GetParams(ctx).GetChainFromChainID(msg.ChainId) if chain == nil { - return nil, zetaObserverTypes.ErrSupportedChains + return nil, observertypes.ErrSupportedChains } - ok, err := k.zetaObserverKeeper.IsAuthorized(ctx, msg.Creator, chain) - if !ok { - return nil, err + if ok := k.zetaObserverKeeper.IsAuthorized(ctx, msg.Creator, chain); !ok { + return nil, observertypes.ErrNotAuthorizedPolicy } chainNonce, isFound := k.GetChainNonces(ctx, chain.ChainName.String()) diff --git a/x/crosschain/keeper/keeper_cross_chain_tx_vote_inbound_tx.go b/x/crosschain/keeper/keeper_cross_chain_tx_vote_inbound_tx.go index 62c7813976..a384c320ba 100644 --- a/x/crosschain/keeper/keeper_cross_chain_tx_vote_inbound_tx.go +++ b/x/crosschain/keeper/keeper_cross_chain_tx_vote_inbound_tx.go @@ -77,9 +77,8 @@ func (k msgServer) VoteOnObservedInboundTx(goCtx context.Context, msg *types.Msg tssPub = tss.TssPubkey } // IsAuthorized does various checks against the list of observer mappers - ok, err := k.zetaObserverKeeper.IsAuthorized(ctx, msg.Creator, observationChain) - if !ok { - return nil, err + if ok := k.zetaObserverKeeper.IsAuthorized(ctx, msg.Creator, observationChain); !ok { + return nil, observerTypes.ErrNotAuthorizedPolicy } index := msg.Digest() diff --git a/x/crosschain/keeper/keeper_cross_chain_tx_vote_outbound_tx.go b/x/crosschain/keeper/keeper_cross_chain_tx_vote_outbound_tx.go index f8b845a925..f7d781bfd2 100644 --- a/x/crosschain/keeper/keeper_cross_chain_tx_vote_outbound_tx.go +++ b/x/crosschain/keeper/keeper_cross_chain_tx_vote_outbound_tx.go @@ -73,9 +73,8 @@ func (k msgServer) VoteOnObservedOutboundTx(goCtx context.Context, msg *types.Ms return nil, err } //Check is msg.Creator is authorized to vote - ok, err := k.zetaObserverKeeper.IsAuthorized(ctx, msg.Creator, observationChain) - if !ok { - return nil, err + if ok := k.zetaObserverKeeper.IsAuthorized(ctx, msg.Creator, observationChain); !ok { + return nil, observerTypes.ErrNotAuthorizedPolicy } // Check if CCTX exists diff --git a/x/crosschain/keeper/keeper_gas_price.go b/x/crosschain/keeper/keeper_gas_price.go index 63ea8e527f..50a103bec9 100644 --- a/x/crosschain/keeper/keeper_gas_price.go +++ b/x/crosschain/keeper/keeper_gas_price.go @@ -13,7 +13,7 @@ import ( sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/types/query" "github.com/zeta-chain/zetacore/x/crosschain/types" - zetaObserverTypes "github.com/zeta-chain/zetacore/x/observer/types" + observertypes "github.com/zeta-chain/zetacore/x/observer/types" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) @@ -127,11 +127,10 @@ func (k msgServer) GasPriceVoter(goCtx context.Context, msg *types.MsgGasPriceVo chain := k.zetaObserverKeeper.GetParams(ctx).GetChainFromChainID(msg.ChainId) if chain == nil { - return nil, zetaObserverTypes.ErrSupportedChains + return nil, observertypes.ErrSupportedChains } - ok, err := k.zetaObserverKeeper.IsAuthorized(ctx, msg.Creator, chain) - if !ok { - return nil, err + if ok := k.zetaObserverKeeper.IsAuthorized(ctx, msg.Creator, chain); !ok { + return nil, observertypes.ErrNotAuthorizedPolicy } if chain == nil { return nil, sdkerrors.Wrap(types.ErrUnsupportedChain, fmt.Sprintf("ChainID : %d ", msg.ChainId)) diff --git a/x/crosschain/keeper/keeper_out_tx_tracker.go b/x/crosschain/keeper/keeper_out_tx_tracker.go index 5053687bd4..a11329aea2 100644 --- a/x/crosschain/keeper/keeper_out_tx_tracker.go +++ b/x/crosschain/keeper/keeper_out_tx_tracker.go @@ -173,11 +173,8 @@ func (k msgServer) AddToOutTxTracker(goCtx context.Context, msg *types.MsgAddToO adminPolicyAccount := k.zetaObserverKeeper.GetParams(ctx).GetAdminPolicyAccount(observertypes.Policy_Type_group1) isAdmin := msg.Creator == adminPolicyAccount - isObserver, err := k.zetaObserverKeeper.IsAuthorized(ctx, msg.Creator, chain) - if err != nil { - ctx.Logger().Error("Error while checking if the account is an observer", err) - return nil, cosmoserrors.Wrap(observertypes.ErrNotAuthorized, fmt.Sprintf("error IsAuthorized %s", msg.Creator)) - } + isObserver := k.zetaObserverKeeper.IsAuthorized(ctx, msg.Creator, chain) + // Sender needs to be either the admin policy account or an observer if !(isAdmin || isObserver) { return nil, cosmoserrors.Wrap(observertypes.ErrNotAuthorized, fmt.Sprintf("Creator %s", msg.Creator)) diff --git a/x/crosschain/types/expected_keepers.go b/x/crosschain/types/expected_keepers.go index fc5960b5e5..5b83869099 100644 --- a/x/crosschain/types/expected_keepers.go +++ b/x/crosschain/types/expected_keepers.go @@ -63,7 +63,7 @@ type ZetaObserverKeeper interface { SetLastObserverCount(ctx sdk.Context, lbc *zetaObserverTypes.LastObserverCount) AddVoteToBallot(ctx sdk.Context, ballot zetaObserverTypes.Ballot, address string, observationType zetaObserverTypes.VoteType) (zetaObserverTypes.Ballot, error) CheckIfFinalizingVote(ctx sdk.Context, ballot zetaObserverTypes.Ballot) (zetaObserverTypes.Ballot, bool) - IsAuthorized(ctx sdk.Context, address string, chain *common.Chain) (bool, error) + IsAuthorized(ctx sdk.Context, address string, chain *common.Chain) bool FindBallot(ctx sdk.Context, index string, chain *common.Chain, observationType zetaObserverTypes.ObservationType) (ballot zetaObserverTypes.Ballot, isNew bool, err error) AddBallotToList(ctx sdk.Context, ballot zetaObserverTypes.Ballot) GetBlockHeader(ctx sdk.Context, hash []byte) (val common.BlockHeader, found bool) diff --git a/x/observer/keeper/keeper_utils.go b/x/observer/keeper/keeper_utils.go index e36cd8e934..8424985d31 100644 --- a/x/observer/keeper/keeper_utils.go +++ b/x/observer/keeper/keeper_utils.go @@ -32,17 +32,17 @@ func (k Keeper) CheckIfFinalizingVote(ctx sdk.Context, ballot types.Ballot) (typ } // IsAuthorized checks whether a signer is authorized to sign , by checking their address against the observer mapper which contains the observer list for the chain and type -func (k Keeper) IsAuthorized(ctx sdk.Context, address string, chain *common.Chain) (bool, error) { +func (k Keeper) IsAuthorized(ctx sdk.Context, address string, chain *common.Chain) bool { observerMapper, found := k.GetObserverMapper(ctx, chain) if !found { - return false, errors.Wrap(types.ErrNotAuthorized, fmt.Sprintf("observer list not present for chain %s", chain.String())) + return false } for _, obs := range observerMapper.ObserverList { if obs == address { - return true, nil + return true } } - return false, errors.Wrap(types.ErrNotAuthorized, fmt.Sprintf("address: %s", address)) + return false } func (k Keeper) FindBallot(ctx sdk.Context, index string, chain *common.Chain, observationType types.ObservationType) (ballot types.Ballot, isNew bool, err error) { diff --git a/x/observer/keeper/msg_server_add_blame_vote.go b/x/observer/keeper/msg_server_add_blame_vote.go index b21bd30c69..1330cc4815 100644 --- a/x/observer/keeper/msg_server_add_blame_vote.go +++ b/x/observer/keeper/msg_server_add_blame_vote.go @@ -19,9 +19,8 @@ func (k msgServer) AddBlameVote(goCtx context.Context, vote *types.MsgAddBlameVo return nil, sdkerrors.Wrap(crosschainTypes.ErrUnsupportedChain, fmt.Sprintf("ChainID %d, Blame vote", vote.ChainId)) } // IsAuthorized does various checks against the list of observer mappers - ok, err := k.IsAuthorized(ctx, vote.Creator, observationChain) - if !ok { - return nil, err + if ok := k.IsAuthorized(ctx, vote.Creator, observationChain); !ok { + return nil, types.ErrNotAuthorizedPolicy } index := vote.Digest() diff --git a/x/observer/keeper/msg_server_add_block_header.go b/x/observer/keeper/msg_server_add_block_header.go index 8ee4ffa5cd..185fd9d061 100644 --- a/x/observer/keeper/msg_server_add_block_header.go +++ b/x/observer/keeper/msg_server_add_block_header.go @@ -17,9 +17,8 @@ func (k msgServer) AddBlockHeader(goCtx context.Context, msg *types.MsgAddBlockH // check authorization for this chain chain := common.GetChainFromChainID(msg.ChainId) - ok, err := k.IsAuthorized(ctx, msg.Creator, chain) - if !ok { - return nil, cosmoserrors.Wrap(types.ErrNotAuthorizedPolicy, err.Error()) + if ok := k.IsAuthorized(ctx, msg.Creator, chain); !ok { + return nil, types.ErrNotAuthorizedPolicy } // add vote to ballot