Implement VoteStateV4#206
Conversation
buffalojoec
force-pushed
the
wip-vote-state-v4-hk
branch
from
1292249 to
c0a662f
Compare
buffalojoec
force-pushed
the
wip-vote-state-v4-hk
branch
from
c0a662f to
c3226b4
Compare
buffalojoec
force-pushed
the
wip-vote-state-v4-hk
branch
from
9d4a1c2 to
f98a67a
Compare
buffalojoec
changed the title
VoteStateV4VoteStateV4
| pub authorized_withdrawer: Pubkey, | ||
|
|
||
| /// The collector account for inflation rewards. | ||
| pub inflation_rewards_collector: Pubkey, |
There was a problem hiding this comment.
so, the intention is that we will update vote rewards payment to this account for votestatev4?
will this be defaulted to vote-pubkey?
There was a problem hiding this comment.
| pub inflation_rewards_commission_bps: u16, | ||
| /// Basis points (0-10,000) that represent how much of the block revenue | ||
| /// should be given to this vote account. | ||
| pub block_revenue_commission_bps: u16, |
| pub block_revenue_commission_bps: u16, | ||
|
|
||
| /// Reward amount pending distribution to stake delegators. | ||
| pub pending_delegator_rewards: u64, |
|
Do you mind doing the deserialization code move (into new module) in a separate PR to reduce the diff size of this one and made it easier to review? |
Yeah sure, can do. I also am going to consider refactoring or replacing |
| /// Compressed BLS pubkey for Alpenglow. | ||
| #[cfg_attr( | ||
| feature = "serde", | ||
| serde_as(as = "Option<[_; BLS_PUBKEY_COMPRESSED_BYTES]>") | ||
| )] | ||
| pub bls_pubkey_compressed: Option<[u8; BLS_PUBKEY_COMPRESSED_BYTES]>, |
There was a problem hiding this comment.
For Alpenglow, we'll want to have a bls_pubkey_compressed associated with each authorized voter. This way, an authorized voter change results in an appropriate change to the corresponding BLS pubkey.
Accordingly, we'll probably either want to:
(1) rewrite bls_pubkey_compressed as bls_pubkeys: Option<BTreeMap<Epoch, [u8; BLS_PUBKEY_COMPRESSED_BYTES]>>
(2) modify AuthorizedVoters to have the value of the BTreeMap include Option<[u8; BLS_PUBKEY_COMPRESSED_BYTES]>.
Slightly partial to option (1), as it seems a bit cleaner / more space-efficient.
There was a problem hiding this comment.
Not suggesting that we change within this SIMD, though - why do we need to keep around a BTreeMap<Epoch, _> rather than "current epoch" and "next epoch?"
There was a problem hiding this comment.
Oh this is news to me. Sounds like we will want to amend SIMD-0185 with this change. And I'm not opposed to removing the btreemap. In SIMD-0185, I also added an entry for the "previous epoch" authorized voter as well so that when transitioning into a new epoch we can still process votes from the previous authorized voter because votes for slots near the end of the epoch signed by the previous authorized voter might land in the next epoch. But I'm thinking this isn't necessary for alpenglow?
There was a problem hiding this comment.
@samkim-crypto I think we also need a proof of ownership for the BLS pubkey (a BLS signature):
pub const BLS_SIGNATURE_COMPRESSED_BYTES: usize = 96;
Two options are:
- we add it to the vote account
- we verify it in the tx that adds/updates the bls pubkey
(1 doesn't exclude 2)
I think it's cleaner to do it in 2), but it won't be possible in BPF without a syscall, so the vote program needs to remain native (until at least we add the syscall).
There was a problem hiding this comment.
@0x0ece fyi we are discussing some of this in the #consensus discord channel
There was a problem hiding this comment.
Yes, we do need to verify BLS pubkeys. @0x0ece can you expand on adding the verification to the vote account? Option 2 is essentially required, no?
There was a problem hiding this comment.
Yes I agree option 2 is better. (I was thinking vote program in bpf, no syscall to do bls sigverify, then we could store the proof in the vote account and do the verification when we boot / load the keys. Unnecessary complication.)
|
I'll be breaking this PR up and handling the evolving state changes in other PRs. |
6 participants
Includes the new interface, full serialization support, and the flipping of "current" to be v4.