Fix authorized voter ahead of time #8303

carllin · 2020-02-15T06:03:23Z

Problem

Authorized voter can change at any point during an epoch, making it hard to detect without monitoring vote account.

Working towards: #8232

Summary of Changes

Fix the authorized voter ahead of time, similar to epoch stakes based on the leader_schedule_epoch
Fixes #

sagar-solana · 2020-02-15T06:06:45Z

programs/vote/src/vote_instruction.rs

@@ -39,6 +39,9 @@ pub enum VoteError {

    #[error("authorized voter has already been changed this epoch")]
    TooSoonToReauthorize,
+
+    #[error("cannot set same authorized voter as latest authorized voter")]
+    ReauthorizedSameVoter,


Should this be an error or a noop?

Neither, I got rid of the error and now the behavior locks in the authorized voter again for that future epoch, preventing a switch

sagar-solana · 2020-02-15T06:10:54Z

programs/vote/src/vote_state.rs

@@ -151,6 +157,14 @@ pub struct VoteState {
    pub votes: VecDeque<Lockout>,
    pub root_slot: Option<u64>,

+    /// the signer for vote transactions
+    authorized_voter: BTreeMap<u64, Pubkey>,


This is sorted so doesn't break consistency but make sure it doesn't grow unbounded.

It can only grow as large as the number of epochs that we're using for the leader_schedule_offset + 1, so by default should be at most 2

mvines · 2020-02-15T06:12:13Z

This is an ABI change that'll break compatibility with v0.23.x, so we'll need to figure out a migration plan. Let's chat more on Monday

cc: @ryoqun

programs/vote/src/vote_state.rs

codecov · 2020-02-16T00:46:24Z

Codecov Report

Merging #8303 into master will increase coverage by <.1%.
The diff coverage is 97%.

@@           Coverage Diff            @@
##           master   #8303     +/-   ##
========================================
+ Coverage    80.2%   80.3%   +<.1%     
========================================
  Files         253     254      +1     
  Lines       55975   56218    +243     
========================================
+ Hits        44929   45164    +235     
- Misses      11046   11054      +8

mvines · 2020-02-16T04:38:23Z

programs/vote/src/vote_state.rs

+        F: Fn(Pubkey) -> Result<(), InstructionError>,
+    {
+        let epoch_authorized_voter = self.get_and_update_authorized_voter(epoch).expect(
+            "the clock epoch is monotonically increasinig, so authorized voter must be known",


Suggested change

"the clock epoch is monotonically increasinig, so authorized voter must be known",

"the clock epoch is monotonically increasing, so authorized voter must be known",

programs/vote/src/vote_state.rs

mvines · 2020-02-16T04:45:20Z

Problem

Authorized voter can change at any point during an epoch, making it hard to detect without monitoring vote account.

Carl, what problem is this actually solving? It doesn't seem to to fix anything. Just trying to assess whether we should even make this change at all given it breaks ABI

mvines

.

carllin · 2020-02-16T05:32:21Z

@mvines, it allows for the ability to check for the authorized voter at the beginning of an epoch, and then have the confidence that that authorized voter will not change for the duration of the epoch without having to constantly checking the status of vote accounts

I need this to build an observer on gossip to count how much stake has been voting on a fork, which requires making sure the vote is coming from the authorized voter

Pull request has been modified.

mvines · 2020-02-16T16:32:54Z

@mvines, it allows for the ability to check for the authorized voter at the beginning of an epoch, and then have the confidence that that authorized voter will not change for the duration of the epoch without having to constantly checking the status of vote accounts

I need this to build an observer on gossip to count how much stake has been voting on a fork, which requires making sure the vote is coming from the authorized voter

Thanks, that helps. What Github issue are you working towards on this? I'd generally expect new features like this to reference an issue and not seemingly come out of the air :-/

mvines · 2020-02-16T16:39:29Z

programs/vote/src/vote_state.rs

@@ -133,15 +148,6 @@ pub struct VoteState {
    /// the node that votes in this account
    pub node_pubkey: Pubkey,

-    /// the signer for vote transactions


To preserve compatibility with 0.23.x, one option will be to create a VoteState023 struct that matches what's on 0.23.x. The master Vote program will then need to support deserializing VoteState023 into VoteState in its process instruction and elsewhere.

carllin · 2020-02-16T21:59:28Z

@mvines Updated with the relevant issue!

rob-solana

lgtm

Pull request has been modified.

automerge (cherry picked from commit 39282be)

automerge

carllin requested review from rob-solana and sagar-solana February 15, 2020 06:03

carllin force-pushed the FixVoteState branch from 78ceea6 to f400b2f Compare February 15, 2020 06:05

sagar-solana reviewed Feb 15, 2020

View reviewed changes

programs/vote/src/vote_state.rs Show resolved Hide resolved

sagar-solana reviewed Feb 15, 2020

View reviewed changes

programs/vote/src/vote_state.rs Outdated Show resolved Hide resolved

carllin force-pushed the FixVoteState branch 3 times, most recently from 9211718 to 546a37f Compare February 16, 2020 00:00

mvines reviewed Feb 16, 2020

View reviewed changes

programs/vote/src/vote_state.rs Outdated Show resolved Hide resolved

mvines previously requested changes Feb 16, 2020

View reviewed changes

carllin force-pushed the FixVoteState branch from e0781fa to 9c13604 Compare February 16, 2020 07:07

mvines reviewed Feb 16, 2020

View reviewed changes

rob-solana previously approved these changes Feb 16, 2020

View reviewed changes

carllin force-pushed the FixVoteState branch 5 times, most recently from cf015b7 to 3fb9002 Compare February 18, 2020 06:27

carllin mentioned this pull request Feb 19, 2020

Add VoteTracker for tracking cluster's votes in gossip #8327

Merged

carllin added the v0.23 label Feb 19, 2020

carllin force-pushed the FixVoteState branch 4 times, most recently from cfca8ed to c75fb53 Compare February 20, 2020 05:03

carllin mentioned this pull request Feb 20, 2020

Upgrade VoteState #8348

Merged

carllin force-pushed the FixVoteState branch 2 times, most recently from 6db62a9 to 9f820a7 Compare February 21, 2020 22:03

mvines added the v1.0 label Feb 22, 2020

carllin force-pushed the FixVoteState branch 2 times, most recently from ea0ca05 to 2781420 Compare February 24, 2020 20:19

carllin removed v0.23 labels Feb 25, 2020

carllin force-pushed the FixVoteState branch from 2781420 to d6e389e Compare February 25, 2020 02:24

Determine vote_state ahead of time

ca8d041

carllin force-pushed the FixVoteState branch from d6e389e to ca8d041 Compare February 25, 2020 02:24

carllin added the automerge Merge this Pull Request automatically once CI passes label Feb 25, 2020

solana-grimes merged commit 39282be into solana-labs:master Feb 25, 2020

carllin added the v1.0 label Feb 28, 2020

mergify bot pushed a commit that referenced this pull request Feb 28, 2020

Determine vote_state ahead of time (#8303)

8d6b75f

automerge (cherry picked from commit 39282be)

mergify bot mentioned this pull request Feb 28, 2020

Fix authorized voter ahead of time (bp #8303) #8521

Merged

solana-grimes pushed a commit that referenced this pull request Feb 28, 2020

Determine vote_state ahead of time (#8303) (#8521)

2c3632a

automerge

carllin pushed a commit to carllin/solana that referenced this pull request Mar 3, 2020

Determine vote_state ahead of time (solana-labs#8303) (solana-labs#8521)

dfb4065

automerge

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix authorized voter ahead of time #8303

Fix authorized voter ahead of time #8303

carllin commented Feb 15, 2020 •

edited

Loading

sagar-solana Feb 15, 2020

carllin Feb 15, 2020

sagar-solana Feb 15, 2020

carllin Feb 15, 2020

mvines commented Feb 15, 2020

codecov bot commented Feb 16, 2020 •

edited

Loading

mvines Feb 16, 2020

mvines commented Feb 16, 2020

Problem

mvines left a comment

carllin commented Feb 16, 2020 •

edited

Loading

mvines commented Feb 16, 2020 •

edited

Loading

mvines Feb 16, 2020

carllin commented Feb 16, 2020

rob-solana left a comment

	"the clock epoch is monotonically increasinig, so authorized voter must be known",
	"the clock epoch is monotonically increasing, so authorized voter must be known",

Fix authorized voter ahead of time #8303

Fix authorized voter ahead of time #8303

Conversation

carllin commented Feb 15, 2020 • edited Loading

Problem

Summary of Changes

sagar-solana Feb 15, 2020

Choose a reason for hiding this comment

carllin Feb 15, 2020

Choose a reason for hiding this comment

sagar-solana Feb 15, 2020

Choose a reason for hiding this comment

carllin Feb 15, 2020

Choose a reason for hiding this comment

mvines commented Feb 15, 2020

codecov bot commented Feb 16, 2020 • edited Loading

Codecov Report

mvines Feb 16, 2020

Choose a reason for hiding this comment

mvines commented Feb 16, 2020

Problem

mvines left a comment

Choose a reason for hiding this comment

carllin commented Feb 16, 2020 • edited Loading

mvines commented Feb 16, 2020 • edited Loading

mvines Feb 16, 2020

Choose a reason for hiding this comment

carllin commented Feb 16, 2020

rob-solana left a comment

Choose a reason for hiding this comment

carllin commented Feb 15, 2020 •

edited

Loading

codecov bot commented Feb 16, 2020 •

edited

Loading

carllin commented Feb 16, 2020 •

edited

Loading

mvines commented Feb 16, 2020 •

edited

Loading