Sign Gossip Vote Messages #3225
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3225 +/- ##
========================================
+ Coverage 79.1% 79.1% +<.1%
========================================
Files 153 153
Lines 22246 22258 +12
========================================
+ Hits 17598 17612 +14
+ Misses 4648 4646 -2 |
| @@ -282,7 +282,7 @@ impl ClusterInfo { | |||
|
|
|||
| pub fn push_vote(&mut self, vote: Transaction) { | |||
| let now = timestamp(); | |||
| let vote = Vote::new(vote, now); | |||
| let vote = Vote::new(&self.id(), vote, now); | |||
There was a problem hiding this comment.
Why not use the Voter pubkey?
There was a problem hiding this comment.
"Voter" pubkey here being the vote account_id? Because Fullnode may not own that keypair and it would be sort of wasteful to make another request to sign this CrdsValue::Vote with that keypair.
Instead this falls nicely in line with the way ContactInfo is gossiped. Essentially a node that puts data out into the gossip network must sign that data with a keypair that it owns. All gossiped values are signed by the peer that generated the message and it's easy to verify that.
The Vote itself will be verified by system program. This allows us to easily package wallclock into the crds vote.
There was a problem hiding this comment.
@sagar-solana Transaction.signatures[0] would be the signature. We just need to translate last_id into a wallclock.
There was a problem hiding this comment.
Yup but @rob-solana and I decided it was best to not try and tie in external state information (like blockhashes -> timestamp) into gossip. It was far more trivial to sign these messages with the node's keypair (used for signing blobs and other gossip messages anyway).
There was a problem hiding this comment.
@rob-solana any reason besides expendience?
There was a problem hiding this comment.
main reason was to try to keep gossip as dumb (i.e. simple) as possible (not needing to know how to unpack a TX, e.g.)
it'd be more expedient to just hack into a vote
|
@aeyakovenko @rob-solana, this good to go? |
|
good for me |
sagar-solana
force-pushed
the
enable_gossip_vote_sigs
branch
from
8962ece
to
ba24358
Compare
sagar-solana
added
the
automerge
Problem
After a recent change Gossip Vote messages were not being signed and signature verification was relying on the inner Vote Tx being signed correctly.
Summary of Changes
Enabled signing Gossip Vote messages. This time correctly track where the Vote Message was generated so that sig verification works correctly.
This is not mission critical to Beacons.
Fixes #3207