refactor: add VoteOnBallot function (#2464)

* refactor: add FindBallotAndFinalize function Signed-off-by: Francisco de Borja Aranda Castillejo <[email protected]> * refactor blame and block header Signed-off-by: Francisco de Borja Aranda Castillejo <[email protected]> * refactor: move VoteOnBallot to utils.go Signed-off-by: Francisco de Borja Aranda Castillejo <[email protected]> * refactor: run make generate Signed-off-by: Francisco de Borja Aranda Castillejo <[email protected]> * remove wontfix issues Signed-off-by: Francisco de Borja Aranda Castillejo <[email protected]> * refactor: add changelog Signed-off-by: Francisco de Borja Aranda Castillejo <[email protected]> * refactor: remove sprintf Signed-off-by: Francisco de Borja Aranda Castillejo <[email protected]> * refactor: add VoteOnBallot testing suite Signed-off-by: Francisco de Borja Aranda Castillejo <[email protected]> * Update x/observer/keeper/voting_test.go Co-authored-by: Tanmay <[email protected]> * refactor: replace index with sample.ZetaIndex(t) Signed-off-by: Francisco de Borja Aranda Castillejo <[email protected]> * refactored errors returning from votes Signed-off-by: Francisco de Borja Aranda Castillejo <[email protected]> --------- Signed-off-by: Francisco de Borja Aranda Castillejo <[email protected]> Co-authored-by: Tanmay <[email protected]>
zeta-chain · Jul 15, 2024 · 0d79cfb · 0d79cfb
1 parent a076710
commit 0d79cfb
Show file tree

Hide file tree

Showing 12 changed files with 444 additions and 166 deletions.
diff --git a/changelog.md b/changelog.md
@@ -30,7 +30,7 @@
 * [2339](https://github.com/zeta-chain/node/pull/2339) - add binaries related question to syncing issue form
 * [2366](https://github.com/zeta-chain/node/pull/2366) - add migration script for adding authorizations table
 * [2372](https://github.com/zeta-chain/node/pull/2372) - add queries for tss fund migration info
-* [2416g](https://github.com/zeta-chain/node/pull/2416) - add Solana chain information
+* [2416](https://github.com/zeta-chain/node/pull/2416) - add Solana chain information
 
 ### Refactor
 
@@ -59,6 +59,7 @@
 * [2380](https://github.com/zeta-chain/node/pull/2380) - use `ChainInfo` in `authority` to allow dynamically support new chains
 * [2395](https://github.com/zeta-chain/node/pull/2395) - converge AppContext with ZetaCoreContext in zetaclient
 * [2428](https://github.com/zeta-chain/node/pull/2428) - propagate context across codebase & refactor zetacore client
+* [2464](https://github.com/zeta-chain/node/pull/2464) - move common voting logic to voting.go and add new function VoteOnBallot
 
 ### Tests
 
@@ -95,6 +96,7 @@
 * [2434](https://github.com/zeta-chain/node/pull/2434) - the default database when running `zetacored init` is now pebbledb
 
 ### CI
+
 * [2388](https://github.com/zeta-chain/node/pull/2388) - added GitHub attestations of binaries produced in the release workflow. 
 * [2285](https://github.com/zeta-chain/node/pull/2285) - added nightly EVM performance testing pipeline, modified localnet testing docker image to utilitze debian:bookworm, removed build-jet runners where applicable, removed deprecated/removed upgrade path testing pipeline
 * [2268](https://github.com/zeta-chain/node/pull/2268) - updated the publish-release pipeline to utilize the Github Actions Ubuntu 20.04 Runners
@@ -214,7 +216,7 @@
 * [1861](https://github.com/zeta-chain/node/pull/1861) - fix `ObserverSlashAmount` invalid read
 * [1880](https://github.com/zeta-chain/node/issues/1880) - lower the gas price multiplier for EVM chains
 * [1883](https://github.com/zeta-chain/node/issues/1883) - zetaclient should check 'IsSupported' flag to pause/unpause a specific chain
-* * [2076](https://github.com/zeta-chain/node/pull/2076) - automatically deposit native zeta to an address if it doesn't exist on ZEVM
+* [2076](https://github.com/zeta-chain/node/pull/2076) - automatically deposit native zeta to an address if it doesn't exist on ZEVM
 * [1633](https://github.com/zeta-chain/node/issues/1633) - zetaclient should be able to pick up new connector and erc20Custody addresses
 * [1944](https://github.com/zeta-chain/node/pull/1944) - fix evm signer unit tests
 * [1888](https://github.com/zeta-chain/node/issues/1888) - zetaclient should stop inbound/outbound txs according to cross-chain flags
@@ -237,11 +239,12 @@
 ## Version: v15.0.0
 
 ### Features
+
 * [1912](https://github.com/zeta-chain/node/pull/1912) - add reset chain nonces msg
 
 ## Version: v14.0.1
 
-- [1817](https://github.com/zeta-chain/node/pull/1817) - Add migration script to fix pending and chain nonces on testnet
+* [1817](https://github.com/zeta-chain/node/pull/1817) - Add migration script to fix pending and chain nonces on testnet
 
 ## Version: v13.0.0
 
@@ -550,4 +553,4 @@ Getting the correct TSS address for Bitcoin now requires proviidng the Bitcoin c
 ### CI
 
 * [1218](https://github.com/zeta-chain/node/pull/1218) - cross-compile release binaries and simplify PR testings
-* [1302](https://github.com/zeta-chain/node/pull/1302) - add mainnet builds to goreleaser
+* [1302](https://github.com/zeta-chain/node/pull/1302) - add mainnet builds to goreleaser
diff --git 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
@@ -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,22 +74,21 @@ 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
 	// This may happen if the same inbound is observed twice where msg.Digest gives a different index
 	// 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
@@ -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,45 +63,48 @@ 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,
 		msg.OutboundChain,
 		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())
 	if err != nil {
 		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_update_observer.go b/x/observer/keeper/msg_server_update_observer.go
@@ -2,7 +2,6 @@ package keeper
 
 import (
 	"context"
-	"fmt"
 
 	errorsmod "cosmossdk.io/errors"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -25,18 +24,16 @@ func (k msgServer) UpdateObserver(
 		return nil, errorsmod.Wrap(types.ErrUpdateObserver, err.Error())
 	}
 	if !ok {
-		return nil, errorsmod.Wrap(
+		return nil, errorsmod.Wrapf(
 			types.ErrUpdateObserver,
-			fmt.Sprintf("Unable to update observer with update reason : %s", msg.UpdateReason),
-		)
+			"Unable to update observer with update reason : %s", msg.UpdateReason)
 	}
 
 	// We do not use IsNonTombstonedObserver here because we want to allow tombstoned observers to be updated
 	if !k.IsAddressPartOfObserverSet(ctx, msg.OldObserverAddress) {
-		return nil, errorsmod.Wrap(
+		return nil, errorsmod.Wrapf(
 			types.ErrNotObserver,
-			fmt.Sprintf("Observer address is not authorized : %s", msg.OldObserverAddress),
-		)
+			"Observer address is not authorized : %s", msg.OldObserverAddress)
 	}
 
 	err = k.IsValidator(ctx, msg.NewObserverAddress)
@@ -53,10 +50,9 @@ func (k msgServer) UpdateObserver(
 	// Update the node account with the new operator address
 	nodeAccount, found := k.GetNodeAccount(ctx, msg.OldObserverAddress)
 	if !found {
-		return nil, errorsmod.Wrap(
+		return nil, errorsmod.Wrapf(
 			types.ErrNodeAccountNotFound,
-			fmt.Sprintf("Observer node account not found : %s", msg.OldObserverAddress),
-		)
+			"Observer node account not found : %s", msg.OldObserverAddress)
 	}
 	newNodeAccount := nodeAccount
 	newNodeAccount.Operator = msg.NewObserverAddress
@@ -68,15 +64,15 @@ func (k msgServer) UpdateObserver(
 	// Check LastBlockObserver count just to be safe
 	observerSet, found := k.GetObserverSet(ctx)
 	if !found {
-		return nil, errorsmod.Wrap(types.ErrObserverSetNotFound, fmt.Sprintf("Observer set not found"))
+		return nil, errorsmod.Wrap(types.ErrObserverSetNotFound, "Observer set not found")
 	}
 	totalObserverCountCurrentBlock := observerSet.LenUint()
 	lastBlockCount, found := k.GetLastObserverCount(ctx)
 	if !found {
-		return nil, errorsmod.Wrap(types.ErrLastObserverCountNotFound, fmt.Sprintf("Observer count not found"))
+		return nil, errorsmod.Wrap(types.ErrLastObserverCountNotFound, "Observer count not found")
 	}
 	if lastBlockCount.Count != totalObserverCountCurrentBlock {
-		return nil, errorsmod.Wrap(types.ErrUpdateObserver, fmt.Sprintf("Observer count mismatch"))
+		return nil, errorsmod.Wrap(types.ErrUpdateObserver, "Observer count mismatch")
 	}
 	return &types.MsgUpdateObserverResponse{}, nil
 }
@@ -88,9 +84,7 @@ func (k Keeper) CheckUpdateReason(ctx sdk.Context, msg *types.MsgUpdateObserver)
 			if msg.Creator != msg.OldObserverAddress {
 				return false, errorsmod.Wrap(
 					types.ErrUpdateObserver,
-					fmt.Sprintf(
-						"Creator address and old observer address need to be same for updating tombstoned observer",
-					),
+					"Creator address and old observer address need to be same for updating tombstoned observer",
 				)
 			}
 			return k.IsOperatorTombstoned(ctx, msg.Creator)