feat(node): migrate vote tally #1139
Conversation
cryptoAtwill
changed the title
Co-authored-by: cryptoAtwill <[email protected]>
| /// A self-certified observation made by a validator. | ||
| #[derive(Serialize, Deserialize, Debug, Clone, Eq, PartialEq)] | ||
| pub struct CertifiedObservation { | ||
| observation: Observation, |
There was a problem hiding this comment.
Can you elaborate more on why both the inner and the envelop signatures are needed?
There was a problem hiding this comment.
It's required in Observation because we need it to publish a cert. But for certified_at field, we also need it to be nonforgeable but no need to be part of the certificate. In the future, there might be more fields to be added to counter different attacks.
There was a problem hiding this comment.
If the latter is just part of the messaging protocol for authentication, perhaps it's better to abstract it away from that struct which defines the message content.
There was a problem hiding this comment.
I tried something like:
struct RecoverablePayload<T> {
t: T,
sig: RecoverableSig
}But it becomes more nested than the above. If there is a use case for reuse, I think it's worth changing, but so far I feel not that much though
| }, | ||
| Ok(vote) = self.handler.gossip_rx.recv() => { | ||
| Ok(vote) = self.handler.gossip_rx.recv_vote() => { |
There was a problem hiding this comment.
Where does the Vote::v1_checked validation is taken place?
There was a problem hiding this comment.
When it's deserialized, it's checked.
Following previous PR, this one migrates the vote tally process over.
In this PR, the following key changes are made:
stmmodule, removingchainfield (not relevant to vote tally anymore) and removepause_votes(not needed). Alsodump_votesthat dump all the votes currently collected is added. This is mainly to aid debugging in case of stuck finality.VoteTallyClientthat utilises tokio channels to communicate with the underlying reactor.VoteStoretrait that stored votes collected. Currently it's in memory store by default. Will merge with Add persistent top-down finality cache #897 when recovery modes are introduced.GossipClienttrait that vote tally uses to communicate with the underlying libp2p gossip channel. Previously it's usingipld-resolverdirectly. But that crate is under utilised and quite heavy. Introducing this trait to decouple withipld-resolverfor easier testing as well.RecoverableECDSASignatureto validate signed votes from peers and sign votes for peers.