zeta-chain / node

ZetaChain’s blockchain node and an observer validator client
https://zetachain.com
MIT License
163 stars 104 forks source link

`zetacore` : allow operators to add votes to older keygen ballots without affecting the keygen status #2675

Closed kingpinXD closed 2 weeks ago

kingpinXD commented 1 month ago

Current implementation of the message : https://github.com/zeta-chain/zeta-node/blob/f78ff55a55e9258fa5c8094c137fb896dd0a56d9/x/observer/keeper/msg_server_vote_tss.go#L27-L27

Points to note regarding the current logic

Considering refactoring the post-vote addition logic as below

ballot, isFinalized := k.CheckIfFinalizingVote(ctx, ballot)
if !isFinalized {
    return &types.MsgVoteTSSResponse{
       VoteFinalized: isFinalized,
       BallotCreated: ballotCreated,
       KeygenSuccess: false,
    }, nil
}

if keygen.Status != types.KeygenStatus_PendingKeygen {
    return &types.MsgVoteTSSResponse{}, nil
}

This would enable voters to add votes to ballots that were discarded for some reason.

kingpinXD commented 1 month ago

In addition , to above we should also consider the following scenario . Detailing the steps below for a network with 3 observers

This behaviour is not correct So overall, the modification can be

ballot, isFinalized := k.CheckIfFinalizingVote(ctx, ballot)
if !isFinalized {
    return &types.MsgVoteTSSResponse{
       VoteFinalized: isFinalized,
       BallotCreated: ballotCreated,
       KeygenSuccess: false,
    }, nil
}

if keygen.Status != types.KeygenStatus_PendingKeygen {
    return &types.MsgVoteTSSResponse{}, nil
}

if msg.KeygenZetaHeight != keygen.BlockNumber {
    return &types.MsgVoteTSSResponse{}, nil
}

This would allow voters to vote on pending ballot without affecting the keygen status

kingpinXD commented 1 month ago

THe original audit finding which led to this change

image