-
Notifications
You must be signed in to change notification settings - Fork 686
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Advance state to next slot after importing block (#2174)
## Issue Addressed
NA
## Proposed Changes
Add an optimization to perform `per_slot_processing` from the *leading-edge* of block processing to the *trailing-edge*. Ultimately, this allows us to import the block at slot `n` faster because we used the tail-end of slot `n - 1` to perform `per_slot_processing`.
Additionally, add a "block proposer cache" which allows us to cache the block proposer for some epoch. Since we're now doing trailing-edge `per_slot_processing`, we can prime this cache with the values for the next epoch before those blocks arrive (assuming those blocks don't have some weird forking).
There were several ancillary changes required to achieve this:
- Remove the `state_root` field of `BeaconSnapshot`, since there's no need to know it on a `pre_state` and in all other cases we can just read it from `block.state_root()`.
- This caused some "dust" changes of `snapshot.beacon_state_root` to `snapshot.beacon_state_root()`, where the `BeaconSnapshot::beacon_state_root()` func just reads the state root from the block.
- Rename `types::ShuffingId` to `AttestationShufflingId`. I originally did this because I added a `ProposerShufflingId` struct which turned out to be not so useful. I thought this new name was more descriptive so I kept it.
- Address ethereum/consensus-specs#2196
- Add a debug log when we get a block with an unknown parent. There was previously no logging around this case.
- Add a function to `BeaconState` to compute all proposers for an epoch without re-computing the active indices for each slot.
## Additional Info
- ~~Blocked on #2173~~
- ~~Blocked on #2179~~ That PR was wrapped into this PR.
- There's potentially some places where we could avoid computing the proposer indices in `per_block_processing` but I haven't done this here. These would be an optimization beyond the issue at hand (improving block propagation times) and I think this PR is already doing enough. We can come back for that later.
## TODO
- [x] Tidy, improve comments.
- [x] ~~Try avoid computing proposer index in `per_block_processing`?~~- Loading branch information
1 parent
8e5c20b
commit 71864d2
Showing
25 changed files
with
1,067 additions
and
136 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| //! The `BeaconProposer` cache stores the proposer indices for some epoch. | ||
| //! | ||
| //! This cache is keyed by `(epoch, block_root)` where `block_root` is the block root at | ||
| //! `end_slot(epoch - 1)`. We make the assertion that the proposer shuffling is identical for all | ||
| //! blocks in `epoch` which share the common ancestor of `block_root`. | ||
| //! | ||
| //! The cache is a fairly unintelligent LRU cache that is not pruned after finality. This makes it | ||
| //! very simple to reason about, but it might store values that are useless due to finalization. The | ||
| //! values it stores are very small, so this should not be an issue. | ||
|
|
||
| use lru::LruCache; | ||
| use smallvec::SmallVec; | ||
| use types::{BeaconStateError, Epoch, EthSpec, Fork, Hash256, Slot, Unsigned}; | ||
|
|
||
| /// The number of sets of proposer indices that should be cached. | ||
| const CACHE_SIZE: usize = 16; | ||
|
|
||
| /// This value is fairly unimportant, it's used to avoid heap allocations. The result of it being | ||
| /// incorrect is non-substantial from a consensus perspective (and probably also from a | ||
| /// performance perspective). | ||
| const TYPICAL_SLOTS_PER_EPOCH: usize = 32; | ||
|
|
||
| /// For some given slot, this contains the proposer index (`index`) and the `fork` that should be | ||
| /// used to verify their signature. | ||
| pub struct Proposer { | ||
| pub index: usize, | ||
| pub fork: Fork, | ||
| } | ||
|
|
||
| /// The list of proposers for some given `epoch`, alongside the `fork` that should be used to verify | ||
| /// their signatures. | ||
| pub struct EpochBlockProposers { | ||
| /// The epoch to which the proposers pertain. | ||
| epoch: Epoch, | ||
| /// The fork that should be used to verify proposer signatures. | ||
| fork: Fork, | ||
| /// A list of length `T::EthSpec::slots_per_epoch()`, representing the proposers for each slot | ||
| /// in that epoch. | ||
| /// | ||
| /// E.g., if `self.epoch == 1`, then `self.proposers[0]` contains the proposer for slot `32`. | ||
| proposers: SmallVec<[usize; TYPICAL_SLOTS_PER_EPOCH]>, | ||
| } | ||
|
|
||
| /// A cache to store the proposers for some epoch. | ||
| /// | ||
| /// See the module-level documentation for more information. | ||
| pub struct BeaconProposerCache { | ||
| cache: LruCache<(Epoch, Hash256), EpochBlockProposers>, | ||
| } | ||
|
|
||
| impl Default for BeaconProposerCache { | ||
| fn default() -> Self { | ||
| Self { | ||
| cache: LruCache::new(CACHE_SIZE), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl BeaconProposerCache { | ||
| /// If it is cached, returns the proposer for the block at `slot` where the block has the | ||
| /// ancestor block root of `shuffling_decision_block` at `end_slot(slot.epoch() - 1)`. | ||
| pub fn get<T: EthSpec>( | ||
| &mut self, | ||
| shuffling_decision_block: Hash256, | ||
| slot: Slot, | ||
| ) -> Option<Proposer> { | ||
| let epoch = slot.epoch(T::slots_per_epoch()); | ||
| let key = (epoch, shuffling_decision_block); | ||
| if let Some(cache) = self.cache.get(&key) { | ||
| // This `if` statement is likely unnecessary, but it feels like good practice. | ||
| if epoch == cache.epoch { | ||
| cache | ||
| .proposers | ||
| .get(slot.as_usize() % T::SlotsPerEpoch::to_usize()) | ||
| .map(|&index| Proposer { | ||
| index, | ||
| fork: cache.fork, | ||
| }) | ||
| } else { | ||
| None | ||
| } | ||
| } else { | ||
| None | ||
| } | ||
| } | ||
|
|
||
| /// Insert the proposers into the cache. | ||
| /// | ||
| /// See `Self::get` for a description of `shuffling_decision_block`. | ||
| /// | ||
| /// The `fork` value must be valid to verify proposer signatures in `epoch`. | ||
| pub fn insert( | ||
| &mut self, | ||
| epoch: Epoch, | ||
| shuffling_decision_block: Hash256, | ||
| proposers: Vec<usize>, | ||
| fork: Fork, | ||
| ) -> Result<(), BeaconStateError> { | ||
| let key = (epoch, shuffling_decision_block); | ||
| if !self.cache.contains(&key) { | ||
| self.cache.put( | ||
| key, | ||
| EpochBlockProposers { | ||
| epoch, | ||
| fork, | ||
| proposers: proposers.into(), | ||
| }, | ||
| ); | ||
| } | ||
|
|
||
| Ok(()) | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.