Snapshot GlobalParams, ValidatorSet, GlobalActiveStake, and LeaderSchedule #542
Conversation
mattfoley8
changed the title
| ValidatorPKID PKID | ||
| } | ||
|
|
||
| func (bav *UtxoView) SnapshotCurrentValidators(snapshotAtEpochNumber uint64) error { |
There was a problem hiding this comment.
I don't love this but I thought it may be good enough for now. I'm not sure how many validators we are expecting. I load all validators into memory to merge between the database and the UtxoView before snapshotting them. Alternatively, I could copy over all validators in the database one-by-one (without even byte-decoding them), and then just insert or delete any validators in the UtxoView. This would increase the number of db inserts/deletes, but reduce the memory footprint. Happy to make that change if others think it's way better. Also happy to punt on that change until our validator set gets unwieldy to load into memory.
There was a problem hiding this comment.
I think this is fine for now and can be optimized later.
lib/pos_snapshot_entries.go
Outdated
| return validatorEntry, nil | ||
| } | ||
|
|
||
| func (bav *UtxoView) GetSnapshotTopActiveValidatorsByStake(limit uint64, snapshotAtEpochNumber uint64) ([]*ValidatorEntry, error) { |
There was a problem hiding this comment.
There's some code duplication between e.g. GetTopActiveValidatorsByStake and GetSnapshotTopActiveValidatorsByStake. Not enough to IMO refactor into one function, but just calling it out. We could have the active (non-snapshot set) default to SnapshotAtEpochNumber = 0 and then reuse the indices + functions. So basically, if we are updating the current active validator set in consensus, we assume the SnapshotAtEpochNumber = 0, that's actually where we would store the current validator set. And any rows in that index where SnapshotAtEpochNumber > 0 are snapshot values. I think there's something nice to having them split apart (more future-proof in case their logic diverges), but I could do a pretty big refactor to combine the current + snapshot ValidatorEntries into a single index, one for by PKID and one for by stake.
There was a problem hiding this comment.
The same could be done for GlobalActiveStakeAmountNanos.
There was a problem hiding this comment.
I like keeping the snapshotting logic separate
| bav._setSnapshotLeaderScheduleValidator(validatorPKID, uint8(index), currentEpochEntry.EpochNumber) | ||
| } | ||
|
|
||
| // TODO: Jail inactive validators. |
There was a problem hiding this comment.
I'll address these in a future PR.
| ValidatorPKID PKID | ||
| } | ||
|
|
||
| func (bav *UtxoView) SnapshotCurrentValidators(snapshotAtEpochNumber uint64) error { |
There was a problem hiding this comment.
I think this is fine for now and can be optimized later.
lib/pos_snapshot_entries.go
Outdated
| return validatorEntry, nil | ||
| } | ||
|
|
||
| func (bav *UtxoView) GetSnapshotTopActiveValidatorsByStake(limit uint64, snapshotAtEpochNumber uint64) ([]*ValidatorEntry, error) { |
There was a problem hiding this comment.
I like keeping the snapshotting logic separate
mattfoley8
changed the base branch from
feature/proof-of-stake
to
mf/consolidate-pos-block-heights
lib/block_view_validator.go
Outdated
| // Validate sufficient epochs have elapsed for validator to be unjailed. | ||
| // TODO: Retrieve snapshot ValidatorJailEpochDuration, not current value. | ||
| if validatorEntry.JailedAtEpochNumber+bav.GetValidatorJailEpochDuration(0) > currentEpochNumber { | ||
| if validatorEntry.JailedAtEpochNumber+validatorJailEpochDuration > currentEpochNumber { |
There was a problem hiding this comment.
feels a little overkill, but should we have an overflow check? probably should have it for stakeLockupEpochDuration as well.
No description provided.