From 9ff66aec762b38e27e37d6759c78d23016583988 Mon Sep 17 00:00:00 2001 From: Francisco de Borja Aranda Castillejo Date: Mon, 15 Jul 2024 12:56:09 +0200 Subject: [PATCH] refactored errors returning from votes Signed-off-by: Francisco de Borja Aranda Castillejo --- .../keeper/msg_server_vote_gas_price.go | 3 +- .../keeper/msg_server_vote_inbound_tx.go | 22 +++--- .../keeper/msg_server_vote_outbound_tx.go | 77 +++++++++++-------- x/observer/keeper/msg_server_vote_blame.go | 11 ++- .../keeper/msg_server_vote_block_header.go | 14 +++- x/observer/keeper/msg_server_vote_tss.go | 43 ++++++----- x/observer/keeper/vote_inbound.go | 2 +- x/observer/keeper/vote_outbound.go | 2 +- x/observer/keeper/voting.go | 7 +- 9 files changed, 103 insertions(+), 78 deletions(-) diff --git a/x/crosschain/keeper/msg_server_vote_gas_price.go b/x/crosschain/keeper/msg_server_vote_gas_price.go index 040c85fd75..aa56a73e26 100644 --- a/x/crosschain/keeper/msg_server_vote_gas_price.go +++ b/x/crosschain/keeper/msg_server_vote_gas_price.go @@ -2,7 +2,6 @@ package keeper import ( "context" - "fmt" "math/big" "sort" "strconv" @@ -28,7 +27,7 @@ func (k msgServer) VoteGasPrice( chain, found := k.zetaObserverKeeper.GetSupportedChainFromChainID(ctx, msg.ChainId) if !found { - return nil, cosmoserrors.Wrap(types.ErrUnsupportedChain, fmt.Sprintf("ChainID : %d ", msg.ChainId)) + return nil, cosmoserrors.Wrapf(types.ErrUnsupportedChain, "ChainID: %d ", msg.ChainId) } if ok := k.zetaObserverKeeper.IsNonTombstonedObserver(ctx, msg.Creator); !ok { return nil, observertypes.ErrNotObserver diff --git a/x/crosschain/keeper/msg_server_vote_inbound_tx.go b/x/crosschain/keeper/msg_server_vote_inbound_tx.go index e597509ab1..5aa925b363 100644 --- a/x/crosschain/keeper/msg_server_vote_inbound_tx.go +++ b/x/crosschain/keeper/msg_server_vote_inbound_tx.go @@ -2,14 +2,15 @@ package keeper import ( "context" - "fmt" - cosmoserrors "cosmossdk.io/errors" + sdkerrors "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/zeta-chain/zetacore/x/crosschain/types" ) +const voteInboundID = "Vote Inbound" + // FIXME: use more specific error types & codes // VoteInbound casts a vote on an inbound transaction observed on a connected chain. If this @@ -73,7 +74,7 @@ func (k msgServer) VoteInbound( msg.InboundHash, ) if err != nil { - return nil, err + return nil, sdkerrors.Wrap(err, voteInboundID) } // If it is a new ballot, check if an inbound with the same hash, sender chain and event index has already been finalized @@ -81,14 +82,13 @@ func (k msgServer) VoteInbound( // This check prevents double spending if isNew { if k.IsFinalizedInbound(tmpCtx, msg.InboundHash, msg.SenderChainId, msg.EventIndex) { - return nil, cosmoserrors.Wrap( + return nil, sdkerrors.Wrapf( types.ErrObservedTxAlreadyFinalized, - fmt.Sprintf( - "inboundHash:%s, SenderChainID:%d, EventIndex:%d", - msg.InboundHash, - msg.SenderChainId, - msg.EventIndex, - ), + "%s, InboundHash:%s, SenderChainID:%d, EventIndex:%d", + voteInboundID, + msg.InboundHash, + msg.SenderChainId, + msg.EventIndex, ) } } @@ -100,7 +100,7 @@ func (k msgServer) VoteInbound( cctx, err := k.ValidateInbound(ctx, msg, true) if err != nil { - return nil, err + return nil, sdkerrors.Wrap(err, voteInboundID) } // Save the inbound CCTX to the store. This is called irrespective of the status of the CCTX or the outcome of the process function. diff --git a/x/crosschain/keeper/msg_server_vote_outbound_tx.go b/x/crosschain/keeper/msg_server_vote_outbound_tx.go index 8cb4a2ce6d..61e1d9389e 100644 --- a/x/crosschain/keeper/msg_server_vote_outbound_tx.go +++ b/x/crosschain/keeper/msg_server_vote_outbound_tx.go @@ -14,6 +14,8 @@ import ( observerkeeper "github.com/zeta-chain/zetacore/x/observer/keeper" ) +const voteOutboundID = "Vote Outbound" + // VoteOutbound casts a vote on an outbound transaction observed on a connected chain (after // it has been broadcasted to and finalized on a connected chain). If this is // the first vote, a new ballot is created. When a threshold of votes is @@ -61,14 +63,13 @@ func (k msgServer) VoteOutbound( ) (*types.MsgVoteOutboundResponse, error) { ctx := sdk.UnwrapSDKContext(goCtx) - // Validate the message params to verify it against an existing cctx + // Validate the message params to verify it against an existing CCTX. cctx, err := k.ValidateOutboundMessage(ctx, *msg) if err != nil { - return nil, err + return nil, cosmoserrors.Wrap(err, voteOutboundID) } - // get ballot index + ballotIndex := msg.Digest() - // vote on outbound ballot isFinalizingVote, isNew, ballot, observationChain, err := k.zetaObserverKeeper.VoteOnOutboundBallot( ctx, ballotIndex, @@ -76,23 +77,26 @@ func (k msgServer) VoteOutbound( msg.Status, msg.Creator) if err != nil { - return nil, err + return nil, cosmoserrors.Wrap(err, voteOutboundID) } - // if the ballot is new, set the index to the CCTX + + // If the ballot is new, set the index to the CCTX. if isNew { observerkeeper.EmitEventBallotCreated(ctx, ballot, msg.ObservedOutboundHash, observationChain) } - // if not finalized commit state here + + // If not finalized commit state here. if !isFinalizingVote { return &types.MsgVoteOutboundResponse{}, nil } - // if ballot successful, the value received should be the out tx amount + // If ballot is successful, the value received should be the out tx amount. err = cctx.AddOutbound(ctx, *msg, ballot.BallotStatus) if err != nil { - return nil, err + return nil, cosmoserrors.Wrap(err, voteOutboundID) } - // Fund the gas stability pool with the remaining funds + + // Fund the gas stability pool with the remaining funds. k.FundStabilityPool(ctx, &cctx) err = k.ValidateOutboundObservers(ctx, &cctx, ballot.BallotStatus, msg.ValueReceived.String()) @@ -100,6 +104,7 @@ func (k msgServer) VoteOutbound( k.SaveFailedOutbound(ctx, &cctx, err.Error(), ballotIndex) return &types.MsgVoteOutboundResponse{}, nil } + k.SaveSuccessfulOutbound(ctx, &cctx, ballotIndex) return &types.MsgVoteOutboundResponse{}, nil } @@ -112,7 +117,7 @@ func (k Keeper) FundStabilityPool(ctx sdk.Context, cctx *types.CrossChainTx) { // Fund the gas stability pool with the remaining funds if err := k.FundGasStabilityPoolFromRemainingFees(ctx, *cctx.GetCurrentOutboundParam(), cctx.GetCurrentOutboundParam().ReceiverChainId); err != nil { ctx.Logger(). - Error(fmt.Sprintf("VoteOutbound: CCTX: %s Can't fund the gas stability pool with remaining fees %s", cctx.Index, err.Error())) + Error("%s: CCTX: %s Can't fund the gas stability pool with remaining fees %s", voteOutboundID, cctx.Index, err.Error()) } } @@ -136,16 +141,18 @@ func (k Keeper) FundGasStabilityPoolFromRemainingFees( remainingGas := gasLimit - gasUsed remainingFees := math.NewUint(remainingGas).Mul(gasPrice).BigInt() - // We fund the stability pool with a portion of the remaining fees + // We fund the stability pool with a portion of the remaining fees. remainingFees = percentOf(remainingFees, RemainingFeesToStabilityPoolPercent) - // Fund the gas stability pool + + // Fund the gas stability pool. if err := k.fungibleKeeper.FundGasStabilityPool(ctx, chainID, remainingFees); err != nil { return err } } else { - return fmt.Errorf("VoteOutbound: The gas limit %d is less than the gas used %d", gasLimit, gasUsed) + return fmt.Errorf("%s: The gas limit %d is less than the gas used %d", voteOutboundID, gasLimit, gasUsed) } } + return nil } @@ -167,7 +174,6 @@ SaveFailedOutbound saves a failed outbound transaction.It does the following thi func (k Keeper) SaveFailedOutbound(ctx sdk.Context, cctx *types.CrossChainTx, errMessage string, ballotIndex string) { cctx.SetAbort(errMessage) ctx.Logger().Error(errMessage) - k.SaveOutbound(ctx, cctx, ballotIndex) } @@ -195,48 +201,51 @@ func (k Keeper) SaveOutbound(ctx sdk.Context, cctx *types.CrossChainTx, ballotIn outTxTssNonce := cctx.GetCurrentOutboundParam().TssNonce cctx.GetCurrentOutboundParam().BallotIndex = ballotIndex + // #nosec G115 always in range k.GetObserverKeeper().RemoveFromPendingNonces(ctx, tssPubkey, receiverChain, int64(outTxTssNonce)) k.RemoveOutboundTrackerFromStore(ctx, receiverChain, outTxTssNonce) ctx.Logger(). - Info(fmt.Sprintf("Remove tracker %s: , Block Height : %d ", getOutboundTrackerIndex(receiverChain, outTxTssNonce), ctx.BlockHeight())) + Info("%s: Remove tracker %s: , Block Height : %d ", voteOutboundID, getOutboundTrackerIndex(receiverChain, outTxTssNonce), ctx.BlockHeight()) + // This should set nonce to cctx only if a new revert is created. k.SetCctxAndNonceToCctxAndInboundHashToCctx(ctx, *cctx) } func (k Keeper) ValidateOutboundMessage(ctx sdk.Context, msg types.MsgVoteOutbound) (types.CrossChainTx, error) { - // check if CCTX exists and if the nonce matches + // Check if CCTX exists and if the nonce matches. cctx, found := k.GetCrossChainTx(ctx, msg.CctxHash) if !found { - return types.CrossChainTx{}, cosmoserrors.Wrap( + return types.CrossChainTx{}, cosmoserrors.Wrapf( sdkerrors.ErrInvalidRequest, - fmt.Sprintf("CCTX %s does not exist", msg.CctxHash), - ) + "%s, CCTX %s does not exist", voteOutboundID, msg.CctxHash) } + if cctx.GetCurrentOutboundParam().TssNonce != msg.OutboundTssNonce { - return types.CrossChainTx{}, cosmoserrors.Wrap( + return types.CrossChainTx{}, cosmoserrors.Wrapf( sdkerrors.ErrInvalidRequest, - fmt.Sprintf( - "OutboundTssNonce %d does not match CCTX OutboundTssNonce %d", - msg.OutboundTssNonce, - cctx.GetCurrentOutboundParam().TssNonce, - ), + "%s, OutboundTssNonce %d does not match CCTX OutboundTssNonce %d", + voteOutboundID, + msg.OutboundTssNonce, + cctx.GetCurrentOutboundParam().TssNonce, ) } - // do not process an outbound vote if TSS is not found + + // Do not process an outbound vote if TSS is not found. _, found = k.zetaObserverKeeper.GetTSS(ctx) if !found { - return types.CrossChainTx{}, types.ErrCannotFindTSSKeys + return types.CrossChainTx{}, cosmoserrors.Wrap(types.ErrCannotFindTSSKeys, voteOutboundID) } + if cctx.GetCurrentOutboundParam().ReceiverChainId != msg.OutboundChain { - return types.CrossChainTx{}, cosmoserrors.Wrap( + return types.CrossChainTx{}, cosmoserrors.Wrapf( sdkerrors.ErrInvalidRequest, - fmt.Sprintf( - "OutboundChain %d does not match CCTX OutboundChain %d", - msg.OutboundChain, - cctx.GetCurrentOutboundParam().ReceiverChainId, - ), + "%s, OutboundChain %d does not match CCTX OutboundChain %d", + voteOutboundID, + msg.OutboundChain, + cctx.GetCurrentOutboundParam().ReceiverChainId, ) } + return cctx, nil } diff --git a/x/observer/keeper/msg_server_vote_blame.go b/x/observer/keeper/msg_server_vote_blame.go index 8e8e9dbf74..61cbce01db 100644 --- a/x/observer/keeper/msg_server_vote_blame.go +++ b/x/observer/keeper/msg_server_vote_blame.go @@ -10,6 +10,8 @@ import ( "github.com/zeta-chain/zetacore/x/observer/types" ) +const voteBlameID = "Vote Blame" + func (k msgServer) VoteBlame( goCtx context.Context, msg *types.MsgVoteBlame, @@ -21,11 +23,12 @@ func (k msgServer) VoteBlame( if !found { return nil, sdkerrors.Wrapf( crosschainTypes.ErrUnsupportedChain, - "ChainID %d, Blame vote", msg.ChainId) + "%s, ChainID %d", voteBlameID, msg.ChainId) } if ok := k.IsNonTombstonedObserver(ctx, msg.Creator); !ok { - return nil, types.ErrNotObserver + return nil, sdkerrors.Wrap( + types.ErrNotObserver, voteBlameID) } ballot, isFinalized, isNew, err := k.VoteOnBallot( @@ -37,7 +40,9 @@ func (k msgServer) VoteBlame( types.VoteType_SuccessObservation, ) if err != nil { - return nil, sdkerrors.Wrap(err, errVoteOnBallot) + return nil, sdkerrors.Wrapf( + err, + "%s, BallotIdentifier %v", voteBlameID, ballot.BallotIdentifier) } if isNew { diff --git a/x/observer/keeper/msg_server_vote_block_header.go b/x/observer/keeper/msg_server_vote_block_header.go index 1193260baf..b4e0d57d0d 100644 --- a/x/observer/keeper/msg_server_vote_block_header.go +++ b/x/observer/keeper/msg_server_vote_block_header.go @@ -10,6 +10,8 @@ import ( "github.com/zeta-chain/zetacore/x/observer/types" ) +const voteBlockHeaderID = "Vote BlockHeader" + // VoteBlockHeader vote for a new block header to the storers func (k msgServer) VoteBlockHeader( goCtx context.Context, @@ -20,18 +22,22 @@ func (k msgServer) VoteBlockHeader( // check if the chain is enabled chain, found := k.GetSupportedChainFromChainID(ctx, msg.ChainId) if !found { - return nil, sdkerrors.Wrapf(types.ErrSupportedChains, "chain id: %d", msg.ChainId) + return nil, sdkerrors.Wrapf( + types.ErrSupportedChains, + "%s, ChainID %d", voteBlockHeaderID, msg.ChainId) } // check if observer if ok := k.IsNonTombstonedObserver(ctx, msg.Creator); !ok { - return nil, types.ErrNotObserver + return nil, sdkerrors.Wrap(types.ErrNotObserver, voteBlockHeaderID) } // check the new block header is valid parentHash, err := k.lightclientKeeper.CheckNewBlockHeader(ctx, msg.ChainId, msg.BlockHash, msg.Height, msg.Header) if err != nil { - return nil, sdkerrors.Wrap(lightclienttypes.ErrInvalidBlockHeader, err.Error()) + return nil, sdkerrors.Wrapf( + lightclienttypes.ErrInvalidBlockHeader, + "%s, parent hash %s", voteBlockHeaderID, parentHash) } _, isFinalized, isNew, err := k.VoteOnBallot( @@ -43,7 +49,7 @@ func (k msgServer) VoteBlockHeader( types.VoteType_SuccessObservation, ) if err != nil { - return nil, sdkerrors.Wrap(err, errVoteOnBallot) + return nil, sdkerrors.Wrap(err, voteBlockHeaderID) } if !isFinalized { diff --git a/x/observer/keeper/msg_server_vote_tss.go b/x/observer/keeper/msg_server_vote_tss.go index 402fbd2a6a..617411d575 100644 --- a/x/observer/keeper/msg_server_vote_tss.go +++ b/x/observer/keeper/msg_server_vote_tss.go @@ -12,6 +12,8 @@ import ( "github.com/zeta-chain/zetacore/x/observer/types" ) +const voteTSSid = "Vote TSS" + // VoteTSS votes on creating a TSS key and recording the information about it (public // key, participant and operator addresses, finalized and keygen heights). // @@ -25,34 +27,38 @@ import ( func (k msgServer) VoteTSS(goCtx context.Context, msg *types.MsgVoteTSS) (*types.MsgVoteTSSResponse, error) { ctx := sdk.UnwrapSDKContext(goCtx) - // 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 + // 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. _, found := k.GetNodeAccount(ctx, msg.Creator) if !found { return nil, errorsmod.Wrapf( sdkerrors.ErrorInvalidSigner, - "signer %s does not have a node account set", msg.Creator) + "%s, signer %s does not have a node account set", voteTSSid, msg.Creator) } - // no need to create a ballot if keygen does not exist + + // No need to create a ballot if keygen does not exist. keygen, found := k.GetKeygen(ctx) if !found { - return &types.MsgVoteTSSResponse{}, types.ErrKeygenNotFound + return &types.MsgVoteTSSResponse{}, errorsmod.Wrap(types.ErrKeygenNotFound, voteTSSid) } - // use a separate transaction to update KEYGEN status to pending when trying to change the TSS address + + // Use a separate transaction to update keygen status to pending when trying to change the TSS address. if keygen.Status == types.KeygenStatus_KeyGenSuccess { - return &types.MsgVoteTSSResponse{}, types.ErrKeygenCompleted + return &types.MsgVoteTSSResponse{}, errorsmod.Wrap(types.ErrKeygenCompleted, voteTSSid) } - // GetBallot checks against the supported chains list before querying for Ballot + // GetBallot checks against the supported chains list before querying for Ballot. ballotCreated := false index := msg.Digest() ballot, found := k.GetBallot(ctx, index) if !found { - // if ballot does not exist, create a new ballot + // If ballot does not exist, create a new ballot. var voterList []string for _, nodeAccount := range k.GetAllNodeAccount(ctx) { voterList = append(voterList, nodeAccount.Operator) } + ballot = types.Ballot{ Index: "", BallotIdentifier: index, @@ -69,29 +75,27 @@ func (k msgServer) VoteTSS(goCtx context.Context, msg *types.MsgVoteTSS) (*types ballotCreated = true } - // vote the ballot - var err error vote := types.VoteType_SuccessObservation if msg.Status == chains.ReceiveStatus_failed { vote = types.VoteType_FailureObservation } - ballot, err = k.AddVoteToBallot(ctx, ballot, msg.Creator, vote) + + ballot, err := k.AddVoteToBallot(ctx, ballot, msg.Creator, vote) if err != nil { - return &types.MsgVoteTSSResponse{}, err + return &types.MsgVoteTSSResponse{}, errorsmod.Wrap(err, voteTSSid) } - // returns here if the ballot is not finalized ballot, isFinalized := k.CheckIfFinalizingVote(ctx, ballot) if !isFinalized { return &types.MsgVoteTSSResponse{ - VoteFinalized: false, + VoteFinalized: isFinalized, BallotCreated: ballotCreated, KeygenSuccess: false, }, nil } - // set TSS only on success, set Keygen either way. - // keygen block can be updated using a policy transaction if keygen fails + // Set TSS only on success, set keygen either way. + // Keygen block can be updated using a policy transaction if keygen fails. keygenSuccess := false if ballot.BallotStatus == types.BallotStatus_BallotFinalized_FailureObservation { keygen.Status = types.KeygenStatus_KeyGenFailed @@ -104,8 +108,9 @@ func (k msgServer) VoteTSS(goCtx context.Context, msg *types.MsgVoteTSS) (*types FinalizedZetaHeight: ctx.BlockHeight(), KeyGenZetaHeight: msg.KeygenZetaHeight, } - // set TSS history only, current TSS is updated via admin transaction - // in Case this is the first TSS address update both current and history + + // Set TSS history only, current TSS is updated via admin transaction. + // In the case this is the first TSS address update both current and history. tssList := k.GetAllTSS(ctx) if len(tssList) == 0 { k.SetTssAndUpdateNonce(ctx, tss) @@ -119,7 +124,7 @@ func (k msgServer) VoteTSS(goCtx context.Context, msg *types.MsgVoteTSS) (*types k.SetKeygen(ctx, keygen) return &types.MsgVoteTSSResponse{ - VoteFinalized: true, + VoteFinalized: isFinalized, BallotCreated: ballotCreated, KeygenSuccess: keygenSuccess, }, nil diff --git a/x/observer/keeper/vote_inbound.go b/x/observer/keeper/vote_inbound.go index 20b38b7df7..2a2bc8a66b 100644 --- a/x/observer/keeper/vote_inbound.go +++ b/x/observer/keeper/vote_inbound.go @@ -69,7 +69,7 @@ func (k Keeper) VoteOnInboundBallot( types.VoteType_SuccessObservation, ) if err != nil { - return false, false, sdkerrors.Wrap(err, errVoteOnBallot) + return false, false, sdkerrors.Wrap(err, msgVoteOnBallot) } if isNew { diff --git a/x/observer/keeper/vote_outbound.go b/x/observer/keeper/vote_outbound.go index fc9094fc4c..5713ee2bf1 100644 --- a/x/observer/keeper/vote_outbound.go +++ b/x/observer/keeper/vote_outbound.go @@ -46,7 +46,7 @@ func (k Keeper) VoteOnOutboundBallot( observertypes.ConvertReceiveStatusToVoteType(receiveStatus), ) if err != nil { - return false, false, ballot, "", sdkerrors.Wrap(err, errVoteOnBallot) + return false, false, ballot, "", sdkerrors.Wrap(err, msgVoteOnBallot) } return isFinalized, isNew, ballot, observationChain.String(), nil diff --git a/x/observer/keeper/voting.go b/x/observer/keeper/voting.go index 756f1fe9ed..d340f0bf32 100644 --- a/x/observer/keeper/voting.go +++ b/x/observer/keeper/voting.go @@ -3,6 +3,7 @@ package keeper import ( "fmt" + sdkerrors "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/zeta-chain/zetacore/pkg/chains" @@ -10,7 +11,7 @@ import ( ) const ( - errVoteOnBallot = "error while voting on ballot" + msgVoteOnBallot = "error while voting on ballot" ) func (k Keeper) AddVoteToBallot( @@ -169,12 +170,12 @@ func (k Keeper) VoteOnBallot( err error) { ballot, isNew, err = k.FindBallot(ctx, ballotIndex, chain, observationType) if err != nil { - return ballot, false, false, err + return ballot, false, false, sdkerrors.Wrap(err, msgVoteOnBallot) } ballot, err = k.AddVoteToBallot(ctx, ballot, voter, voteType) if err != nil { - return ballot, false, isNew, err + return ballot, false, isNew, sdkerrors.Wrap(err, msgVoteOnBallot) } ballot, isFinalized = k.CheckIfFinalizingVote(ctx, ballot)