`zetacore` : allow operators to add votes to older keygen ballots without affecting the keygen status
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
- The Keygen status is set to success or failed only when the ballot is finalized, and the threshold for that is 1, meaning everyone on the ballot has already voted.
- We distribute observer rewards for voting on ballots, and we want signers to be able to add votes to get the rewards for their votes
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.
In addition , to above we should also consider the following scenario . Detailing the steps below for a network with 3 observers
- only 2 of the required 3 accounts vote (let’s call this ballot 1)
- the third account fails to vote for some reason, and we decide to retry the keygen ceremony with a new height
- 1 or 2 accounts vote in the new keygen
- At this point, we have two ballots, one for the older keygen and one for the newer.
- The 3rd account decides to vote on the original keygen, which changes the status to successful and updates the TSS
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
THe original audit finding which led to this change