Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: add VoteOnBallot function #2464

Merged
merged 12 commits into from
Jul 15, 2024
11 changes: 7 additions & 4 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

fbac marked this conversation as resolved.
Show resolved Hide resolved
### Tests

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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
26 changes: 10 additions & 16 deletions x/observer/keeper/msg_server_update_observer.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import (
"context"
"fmt"

errorsmod "cosmossdk.io/errors"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand All @@ -25,18 +24,16 @@
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(

Check warning on line 34 in x/observer/keeper/msg_server_update_observer.go

View check run for this annotation

Codecov / codecov/patch

x/observer/keeper/msg_server_update_observer.go#L34

Added line #L34 was not covered by tests
types.ErrNotObserver,
fmt.Sprintf("Observer address is not authorized : %s", msg.OldObserverAddress),
)
"Observer address is not authorized : %s", msg.OldObserverAddress)

Check warning on line 36 in x/observer/keeper/msg_server_update_observer.go

View check run for this annotation

Codecov / codecov/patch

x/observer/keeper/msg_server_update_observer.go#L36

Added line #L36 was not covered by tests
}

err = k.IsValidator(ctx, msg.NewObserverAddress)
Expand All @@ -53,10 +50,9 @@
// 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
Expand All @@ -68,15 +64,15 @@
// 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")

Check warning on line 67 in x/observer/keeper/msg_server_update_observer.go

View check run for this annotation

Codecov / codecov/patch

x/observer/keeper/msg_server_update_observer.go#L67

Added line #L67 was not covered by tests
}
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")

Check warning on line 75 in x/observer/keeper/msg_server_update_observer.go

View check run for this annotation

Codecov / codecov/patch

x/observer/keeper/msg_server_update_observer.go#L75

Added line #L75 was not covered by tests
}
return &types.MsgUpdateObserverResponse{}, nil
}
Expand All @@ -88,9 +84,7 @@
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)
Expand Down
47 changes: 19 additions & 28 deletions x/observer/keeper/msg_server_vote_blame.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ package keeper

import (
"context"
"fmt"

cosmoserrors "cosmossdk.io/errors"
sdkerrors "cosmossdk.io/errors"
sdk "github.com/cosmos/cosmos-sdk/types"

crosschainTypes "github.com/zeta-chain/zetacore/x/crosschain/types"
Expand All @@ -13,52 +12,44 @@ import (

func (k msgServer) VoteBlame(
goCtx context.Context,
vote *types.MsgVoteBlame,
msg *types.MsgVoteBlame,
) (*types.MsgVoteBlameResponse, error) {
ctx := sdk.UnwrapSDKContext(goCtx)
observationType := types.ObservationType_TSSKeySign

// GetChainFromChainID makes sure we are getting only supported chains , if a chain support has been turned on using gov proposal, this function returns nil
observationChain, found := k.GetSupportedChainFromChainID(ctx, vote.ChainId)
observationChain, found := k.GetSupportedChainFromChainID(ctx, msg.ChainId)
if !found {
return nil, cosmoserrors.Wrap(
return nil, sdkerrors.Wrapf(
crosschainTypes.ErrUnsupportedChain,
fmt.Sprintf("ChainID %d, Blame vote", vote.ChainId),
)
"ChainID %d, Blame vote", msg.ChainId)
}

if ok := k.IsNonTombstonedObserver(ctx, vote.Creator); !ok {
if ok := k.IsNonTombstonedObserver(ctx, msg.Creator); !ok {
return nil, types.ErrNotObserver
}

index := vote.Digest()
// Add votes and Set Ballot
// GetBallot checks against the supported chains list before querying for Ballot
ballot, isNew, err := k.FindBallot(ctx, index, observationChain, observationType)
ballot, isFinalized, isNew, err := k.VoteOnBallot(
ctx,
observationChain,
msg.Digest(),
types.ObservationType_TSSKeySign,
msg.Creator,
types.VoteType_SuccessObservation,
)
if err != nil {
return nil, err
return nil, sdkerrors.Wrap(err, errVoteOnBallot)
fbac marked this conversation as resolved.
Show resolved Hide resolved
}

if isNew {
EmitEventBallotCreated(ctx, ballot, vote.BlameInfo.Index, observationChain.String())
EmitEventBallotCreated(ctx, ballot, msg.BlameInfo.Index, observationChain.String())
}

// AddVoteToBallot adds a vote and sets the ballot
ballot, err = k.AddVoteToBallot(ctx, ballot, vote.Creator, types.VoteType_SuccessObservation)
if err != nil {
return nil, err
}

_, isFinalized := k.CheckIfFinalizingVote(ctx, ballot)
if !isFinalized {
// Return nil here to add vote to ballot and commit state
// Return nil here to add vote to ballot and commit state.
return &types.MsgVoteBlameResponse{}, nil
}

// ******************************************************************************
// below only happens when ballot is finalized: exactly when threshold vote is in
// ******************************************************************************

k.SetBlame(ctx, vote.BlameInfo)
// Ballot is finalized: exactly when threshold vote is in.
k.SetBlame(ctx, msg.BlameInfo)
return &types.MsgVoteBlameResponse{}, nil
}
27 changes: 14 additions & 13 deletions x/observer/keeper/msg_server_vote_block_header.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package keeper
import (
"context"

cosmoserrors "cosmossdk.io/errors"
sdkerrors "cosmossdk.io/errors"
sdk "github.com/cosmos/cosmos-sdk/types"

lightclienttypes "github.com/zeta-chain/zetacore/x/lightclient/types"
Expand All @@ -20,7 +20,7 @@ func (k msgServer) VoteBlockHeader(
// check if the chain is enabled
chain, found := k.GetSupportedChainFromChainID(ctx, msg.ChainId)
if !found {
return nil, cosmoserrors.Wrapf(types.ErrSupportedChains, "chain id: %d", msg.ChainId)
return nil, sdkerrors.Wrapf(types.ErrSupportedChains, "chain id: %d", msg.ChainId)
}

// check if observer
Expand All @@ -31,29 +31,30 @@ func (k msgServer) VoteBlockHeader(
// 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, cosmoserrors.Wrap(lightclienttypes.ErrInvalidBlockHeader, err.Error())
return nil, sdkerrors.Wrap(lightclienttypes.ErrInvalidBlockHeader, err.Error())
}

// add vote to ballot
ballot, isNew, err := k.FindBallot(ctx, msg.Digest(), chain, types.ObservationType_InboundTx)
_, isFinalized, isNew, err := k.VoteOnBallot(
ctx,
chain,
msg.Digest(),
types.ObservationType_InboundTx,
msg.Creator,
types.VoteType_SuccessObservation,
)
if err != nil {
return nil, cosmoserrors.Wrap(err, "failed to find ballot")
return nil, sdkerrors.Wrap(err, errVoteOnBallot)
}
ballot, err = k.AddVoteToBallot(ctx, ballot, msg.Creator, types.VoteType_SuccessObservation)
if err != nil {
return nil, cosmoserrors.Wrap(err, "failed to add vote to ballot")
}
_, isFinalized := k.CheckIfFinalizingVote(ctx, ballot)

if !isFinalized {
return &types.MsgVoteBlockHeaderResponse{
BallotCreated: isNew,
VoteFinalized: false,
}, nil
}

// add the new block header to the store
// Add the new block header to the store.
k.lightclientKeeper.AddBlockHeader(ctx, msg.ChainId, msg.Height, msg.BlockHash, msg.Header, parentHash)

return &types.MsgVoteBlockHeaderResponse{
BallotCreated: isNew,
VoteFinalized: true,
Expand Down
8 changes: 2 additions & 6 deletions x/observer/keeper/msg_server_vote_tss.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package keeper

import (
"context"
"fmt"

errorsmod "cosmossdk.io/errors"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand All @@ -29,10 +28,9 @@ func (k msgServer) VoteTSS(goCtx context.Context, msg *types.MsgVoteTSS) (*types
// 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.Wrap(
return nil, errorsmod.Wrapf(
sdkerrors.ErrorInvalidSigner,
fmt.Sprintf("signer %s does not have a node account set", msg.Creator),
)
"signer %s does not have a node account set", msg.Creator)
}
// no need to create a ballot if keygen does not exist
keygen, found := k.GetKeygen(ctx)
Expand All @@ -44,9 +42,7 @@ func (k msgServer) VoteTSS(goCtx context.Context, msg *types.MsgVoteTSS) (*types
return &types.MsgVoteTSSResponse{}, types.ErrKeygenCompleted
}

// add votes and set Ballot
// GetBallot checks against the supported chains list before querying for Ballot
// TODO : https://github.com/zeta-chain/node/issues/896
lumtis marked this conversation as resolved.
Show resolved Hide resolved
ballotCreated := false
index := msg.Digest()
ballot, found := k.GetBallot(ctx, index)
Expand Down
37 changes: 16 additions & 21 deletions x/observer/keeper/vote_inbound.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
package keeper

import (
"fmt"

sdkerrors "cosmossdk.io/errors"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"

"github.com/zeta-chain/zetacore/pkg/coin"
"github.com/zeta-chain/zetacore/x/observer/types"
Expand All @@ -21,7 +19,7 @@ func (k Keeper) VoteOnInboundBallot(
voter string,
ballotIndex string,
inboundHash string,
) (bool, bool, error) {
) (isFinalized bool, isNew bool, err error) {
if !k.IsInboundEnabled(ctx) {
return false, false, types.ErrInboundDisabled
}
Expand All @@ -31,11 +29,10 @@ func (k Keeper) VoteOnInboundBallot(
// this function returns nil
senderChain, found := k.GetSupportedChainFromChainID(ctx, senderChainID)
if !found {
return false, false, sdkerrors.Wrap(types.ErrSupportedChains, fmt.Sprintf(
return false, false, sdkerrors.Wrapf(types.ErrSupportedChains,
"ChainID %d, Observation %s",
senderChainID,
types.ObservationType_InboundTx.String()),
)
types.ObservationType_InboundTx.String())
}

// checks the voter is authorized to vote on the observation chain
Expand All @@ -46,11 +43,10 @@ func (k Keeper) VoteOnInboundBallot(
// makes sure we are getting only supported chains
receiverChain, found := k.GetSupportedChainFromChainID(ctx, receiverChainID)
if !found {
return false, false, sdkerrors.Wrap(types.ErrSupportedChains, fmt.Sprintf(
return false, false, sdkerrors.Wrapf(types.ErrSupportedChains,
"ChainID %d, Observation %s",
receiverChainID,
types.ObservationType_InboundTx.String()),
)
types.ObservationType_InboundTx.String())
}

// check if we want to send ZETA to external chain, but there is no ZETA token.
Expand All @@ -64,22 +60,21 @@ func (k Keeper) VoteOnInboundBallot(
}
}

// checks against the supported chains list before querying for Ballot
ballot, isNew, err := k.FindBallot(ctx, ballotIndex, senderChain, types.ObservationType_InboundTx)
ballot, isFinalized, isNew, err := k.VoteOnBallot(
ctx,
senderChain,
ballotIndex,
types.ObservationType_InboundTx,
voter,
types.VoteType_SuccessObservation,
)
if err != nil {
return false, false, err
return false, false, sdkerrors.Wrap(err, errVoteOnBallot)
}

if isNew {
EmitEventBallotCreated(ctx, ballot, inboundHash, senderChain.String())
}

// adds a vote and sets the ballot
ballot, err = k.AddVoteToBallot(ctx, ballot, voter, types.VoteType_SuccessObservation)
if err != nil {
return false, isNew, err
}

// checks if the ballot is finalized
_, isFinalized := k.CheckIfFinalizingVote(ctx, ballot)
return isFinalized, isNew, nil
}
Loading
Loading