Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

use proposer index cache for block proposal #4338

Closed
wants to merge 3 commits into from
Closed

use proposer index cache for block proposal #4338

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
78b1be0
use proposer index cache for block proposal
int88 May 26, 2023
db30728
minor fix
int88 May 30, 2023
ce002f6
delete proposer cache check in some tests
int88 May 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 34 additions & 1 deletion beacon_node/beacon_chain/src/beacon_chain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4308,7 +4308,40 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
state.latest_block_header().canonical_root()
};

let proposer_index = state.get_beacon_proposer_index(state.slot(), &self.spec)? as u64;
let dependent_root = state.proposer_shuffling_decision_root(self.genesis_block_root)?;

let proposer_slot = state.slot();
let proposer_epoch = proposer_slot.epoch(T::EthSpec::slots_per_epoch());

let cached_proposer = self
.beacon_proposer_cache
.lock()
.get_slot::<T::EthSpec>(dependent_root, proposer_slot);

let proposer_index = if let Some(proposer) = cached_proposer {
proposer.index as u64
} else {
let (proposers, decision_root, _, fork) =
compute_proposer_duties_from_head(proposer_epoch, self)
.map_err(BlockProductionError::BeaconChain)?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could simplify this a little by calling state.get_beacon_proposer_indices on the state directly. It has already been advanced through to the slot of the proposal (above) so it should be able to serve the call.

The compute_proposer_duties_from_head basically does the same thing except it also clones the head again (which is slow). Using the head is also not necessarily correct if we are proposing on a non-head block (sometimes in tests we propose blocks using an old parent instead of the head).

The fork can be set to state.fork() directly, and we also have the dependent_root (same as the decision root) above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michaelsproul updated! But delete some proposer cache prime check in some tests to pass CI.


let proposer_offset = (proposer_slot % T::EthSpec::slots_per_epoch()).as_usize();
let proposer =
*proposers
.get(proposer_offset)
.ok_or(BlockProductionError::BeaconChain(
BeaconChainError::NoProposerForSlot(proposer_slot),
))?;

self.beacon_proposer_cache.lock().insert(
proposer_epoch,
decision_root,
proposers,
fork,
)?;

proposer as u64
};

let pubkey = state
.validators()
Expand Down