Vote Account v5: effective commission in vote state

[umar-bhatty]

This is the discussion thread for SIMD-0677 (#677). The PR went up without one, which was my mistake. It is back in draft until this settles.

Problem

SIMD-0249 delays commission updates: rewards earned in epoch E use the commission from vote state at the start of E - 1, and SIMD-0123 does the same for block revenue. So the rate that applies to an epoch is not in the vote account. The account shows the rate that applies two epoch boundaries from now, and the rate in force comes from epoch-boundary state kept outside the accounts database, as raised in #583. An on-chain program cannot read the rate in force at all. @joncinque noted in #583 that fixing this for commission needs new vote state.

Separately, SIMD-0674 (#674) needs a new vote state version for its location field, and @buffalojoec suggested one V5 SIMD covering both, plus cleanup of unused fields.

Proposal

Vote state v5:

1. Replace each commission field with a small fixed-size list of (effective epoch, bps) entries. The Vote program writes an entry on UpdateCommission / UpdateCommissionBps with effective epoch = current epoch + 2. The runtime only reads: the rate for epoch E is the entry with the greatest epoch not after E, with today's lookup as the fallback for accounts that are not converted yet. Reward amounts do not change.
2. Add the location field from SIMD-0674.
3. Remove votes, which SIMD-0357 says stays empty under Alpenglow.

Why not just current + next

An update in epoch X applies from X + 2. A validator can update in X - 1 and again in X, so during epoch X the account has to hold three rates: the one in force, one effective from X + 1 and one effective from X + 2. With two values, the second update has to replace the first pending one, and that changes what SIMD-0249 pays today.

Three entries is the minimum that keeps today's behaviour. The PR text has four, the extra one keeping the previous epoch's rate readable. That one is history, and I'll cut it.

Questions

1. Is it worth moving the commission lookup into vote state, or is the current epoch-stakes lookup acceptable long term?
2. If two values are preferred, is changing SIMD-0249's behaviour for updates in consecutive epochs acceptable?
3. Should one V5 SIMD carry the location field and the votes removal as well, or should those stay separate?
4. Removing votes ties V5 to the Alpenglow migration. Is that acceptable for SIMD-0674?
5. Nothing converts an account to V5 unless a Vote program instruction stores it. Is lazy conversion fine?

[umar-bhatty]

@buffalojoec @AshwinSekar i saw the V5 discussion in #development today. Two of the questions i had asked here are getting answered there itself, which fields to remove (votes, the optional BLS key, authorized_voters later on) and zero-copy layout. #677 is already doing both so no issue on that front.

One thing which is not coming up in that thread is the commission lookup. As of now the rate for epoch E is sitting in the epoch stakes snapshots, which runtime is keeping outside of the accounts db only. #677 brings it inside the account, vote program writes (effective epoch, bps) on every commission update and runtime simply reads the rate from the account. Two schedules, three fixed size entries each, so zero-copy is not a problem.

If you guys are doing one single V5 SIMD then this section should go in the same and i can contribute that part. Otherwise #677 remains as the commission SIMD on top of your V5 and i will rebase once your layout is finalised.

Only one design question i want to close first, effective rate should come from vote state or from the epoch stakes account in SIMD-0511?

[buffalojoec]

I think it should come from vote state. If we can eliminate or further reduce in-runtime caching as a result of the commission history approach, we should.

[topointon-jump]

Here's my proposed layout for Vote State V5. I believe it meets all the different use-cases. It's 4-byte aligned and can be zero-copy accessed, as everything is statically laid out.

| Offset | Size | Field | Type |
|-------:|-----:|-------|------|
| 0 | 4 | discriminant | `u32` = `4` |
| 4 | 32 | node_pubkey | `Pubkey` |
| 36 | 32 | authorized_withdrawer | `Pubkey` |
| 68 | 32 | inflation_rewards_collector | `Pubkey` |
| 100 | 32 | block_revenue_collector | `Pubkey` |
| 132 | 2 | inflation_rewards_commission_bps | `u16` |
| 134 | 2 | block_revenue_commission_bps | `u16` |
| 136 | 8 | pending_delegator_rewards | `u64` |
| 144 | 12 | location | `x: i32, y: i32, z: i32` |
| 156 | 48 | bls_pubkey_compressed | `[u8; 48]` |
| 204 | 8 | latest_vote | `u64` |
| 212 | 8 | root_slot | `u64`, `u64::MAX` = none |
| 220 | 8 | authorized_voters_len | `u64`, at most 4 |
| 228 | 160 | authorized_voters | `[(epoch: u64, pubkey: Pubkey); 4]` |
| 388 | 8 | epoch_credits_len | `u64`, at most 64 |
| 396 | 1536 | epoch_credits | `[(epoch: u64, credits: u64, prev_credits: u64); 64]` |
| 1932 | 16 | last_timestamp | `(slot: u64, timestamp: i64)` |

If this looks good to everyone I'll open a new SIMD.

[joncinque]

I had a look, unfortunately the stake program's DeactivateDelinquent instruction uses these entries to determine if the vote account has voted in the past 5 epochs.

We can potentially change / decrease this if it helps the vote account layout. Or if the stake program can figure out voting information elsewhere, it can certainly consume another account.

[buffalojoec]

Why not group len+elements sequentially? Seems weird to me that the commission lengths are separated from the commission entires by a few unrelated fields. Is this just to avoid messing with padding?

[topointon-jump]

Yea, that was just to avoid padding and to make it so every field starts at a multiple of 4. Rearranging the fields and preserving the alignment would make the struct larger by 4 bytes. We can do this by making the lengths u32:

| Offset | Size | Field | Type |
|-------:|-----:|-------|------|
| 0 | 4 | discriminant | `u32` = `4` |
| 4 | 32 | node_pubkey | `Pubkey` |
| 36 | 32 | authorized_withdrawer | `Pubkey` |
| 68 | 32 | inflation_rewards_collector | `Pubkey` |
| 100 | 32 | block_revenue_collector | `Pubkey` |
| 132 | 8 | pending_delegator_rewards | `u64` |
| 140 | 12 | location | `x: i32, y: i32, z: i32` | 
| 152 | 4 | inflation_rewards_commission_bps_len | `u32`, at most 4 |
| 156 | 32 | inflation_rewards_commission_bps_epochs | `[u64; 4]` |
| 188 | 8 | inflation_rewards_commission_bps | `[u16; 4]` |
| 196 | 4 | block_revenue_commission_bps_len | `u32`, at most 4 | 
| 200 | 32 | block_revenue_commission_bps_epochs | `[u64; 4]` |
| 232 | 8 | block_revenue_commission_bps | `[u16; 4]` |
| 240 | 48 | bls_pubkey_compressed | `[u8; 48]` |
| 288 | 8 | latest_vote | `u64`, `u64::MAX` = none |
| 296 | 8 | root_slot | `u64`, `u64::MAX` = none |
| 304 | 4 | authorized_voters_len | `u32`, at most 4 |
| 308 | 160 | authorized_voters | `[(epoch: u64, pubkey: Pubkey); 4]` |
| 468 | 4 | accumulated_lamports_len | `u32`, at most 64 |
| 472 | 1536 | accumulated_lamports | `[(epoch: u64, lamports: u64, prev_lamports: u64); 64]` |
| 2008 | 16 | last_timestamp | `(slot: u64, timestamp: i64)` |

I think we should go with that^

[buffalojoec]

That works for me! Seems like this accomplishes with @umar-bhatty was after in #677.

[umar-bhatty]

This looks fantastic! was traveling hence the latency, apologies.

[akhi3030]

We found some perf regressions in replay times because with Alpenglow we are updating vote accounts of all active validators each slot. On mainnet, that is around 700 and in the worst case, that can be 2k. We recently worked on various optimisations to alleviate the situation in a way that is backwards compatible.

Currently, the epoch_credits, votes, and root_slot fields in the vote state are updated each slot. epoch_credits tracks how much rewards the validator has accumulated which will be paid out at the start of the next epoch. votes and root_slot fields are accessed by the read layer to measure the health of the validator.

Moving forward, I want to propose that these fields for all the vote accounts be stored in a single PDA on chain. I haven't fully worked out the details but essentially, we would remove votes and root_slot fields and have to update the read layers to check the PDA instead. We would still keep the epoch_credits in the vote state but only update it at the end of the epoch from the PDA.

This way, we should be able to get away with updating just a single account each slot instead of vote accounts of every active validator.

[umar-bhatty]

Would still appreciate an update cadence of every slot regardless of where the values lie.

[umar-bhatty]

Good idea BTW.

A helper function in vote state that fetches the data pda internally, maps the validator identity to the appropriate data object and returning it would be beneficial to consumers. The pda address could be hard coded or derived. This would abstract this new pda for consumers, if which I am one off.

[topointon-jump]

Update - discussed offline, as vote state v5 is designed to facilitate zero-copy access, zero-copy access might be enough to mitigate the perf issues. We will move forward with vote state v5, and profile again once that is implemented. The single PDA approach might not be necessary.

[t-nelson]

i think the title of this discussion is no longer descriptive of the initiative?

---

i find redescribing the epoch assignments here unnecessary. what we want is for changes to commission, authorized voter (bls now?) key, (and collectors?) to be latched to the epoch for which they are first applied to the vat quorum and leader schedule. then there is no need to retain beyond rewards distribution for the corresponding epoch, though i don't believe there is any need to clean these up or perpetuate them into future epochs. just let them fall off as the operator makes changes

i'm opposed to any onchain data retention beyond what the protocol requires. read layer convenience does not belong under consensus

---

some other things that we've historically discussed adding to vote state

• private bit -- let operators explicitly declare their node private. require signature of an authority specified in the vote state (could reuse an existing one) to delegate stake
• effective_stake[] -- total effective stake delegated to the node for each extant leader schedule epoch. this gives us free recovery of EpochStakes sysvar, recent leader schedules and various caches that complicate the snapshot format

of lessor confidence that recording state would add a benefit over a pure logic implementation

• shutdown bit/epoch -- allow operator to explicitly declare intent to permanently shutdown their node. this could be done with a ...
• voter bond address -- address of a requisite stake account, authority controlled by the corresponding vote account and used for a variety of incentives games. this can be a pda off the vote account address

[sam0x17]

@t-nelson putting each epoch's stake weights and BLS identities under the bank hash is the part I want to preserve in #676. The eight-epoch window there was chosen for delegation scoring and delayed reads, so it does go beyond protocol needs.

If V5 can preserve the admitted stake set and the identity/key mappings for each epoch the protocol still needs, I'd rather use that than publish another copy. We'd need to account for validators leaving the set or closing their vote accounts while an older schedule is still in use. I'm happy to work through that with the V5 proposal and narrow #676 accordingly.