chore(halo/voter): improve errors and logs #2437
Conversation
corverroos
requested review from
sideninja,
chmllr and
kc1116
as code owners
| return nil, errors.Wrap(err, "att root [BUG]") // Should error in Verify | ||
| } | ||
| if duplicate[attRoot] { | ||
| log.Warn(ctx, "Rejecting duplicate identical vote", nil) |
There was a problem hiding this comment.
when the same sig is included, it isn't a double sign
| } | ||
| if duplicate[attRoot] { | ||
| log.Warn(ctx, "Rejecting duplicate identical vote", nil) | ||
| return respReject, nil |
There was a problem hiding this comment.
Since we're returning with an error on duplicate votes and double signing, is it somehow possible to cause a dos by re-submitting valid votes of honest validators or by a perpetual double-signing by a malicious validator? Potentially the votes deduplicated somewhere in the stack before the voter can get them, but I wonder why just logging a warning and skipping is not enough?
There was a problem hiding this comment.
This is during VerifyVoteExtension when verifying other validator's votes. Not sure where else this could be done?
| doubleSignCounter.WithLabelValues(ethAddr.Hex()).Inc() | ||
| log.Warn(ctx, "Rejecting duplicate slashable vote", err) | ||
|
|
||
| return respReject, nil | ||
| } | ||
| duplicate[vote.AttestHeader.ToXChain()] = true | ||
| doubleSign[vote.AttestHeader.ToXChain()] = true |
There was a problem hiding this comment.
Nit: the name is potentially misleading as presence in the map does not imply a double-signing yet. It is used to detected the double signing... Same with duplicate actually.
There was a problem hiding this comment.
mmm, I don't find it so confusing, just read the code...?
Add temporary logs to voter to debug duplicate vote issue.
issue: #2286