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

merge queue: embarking unstable (641f6be) and [#5266 + #5368] together #5380

Closed
wants to merge 4 commits into from
Closed

merge queue: embarking unstable (641f6be) and [#5266 + #5368] together #5380

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a54372e
Fix `AddrInUse` error in cli tests.
jimmygchen Feb 20, 2024
88239d2
Optimise concurrent block production
michaelsproul Mar 7, 2024
15fe1ec
Merge of #5266
mergify[bot] Mar 8, 2024
8485095
Merge of #5368
mergify[bot] Mar 8, 2024
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
63 changes: 41 additions & 22 deletions 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 @@ -4166,21 +4166,14 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
(re_org_state.pre_state, re_org_state.state_root)
}
// Normal case: proposing a block atop the current head using the cache.
else if let Some((_, cached_state)) = self
.block_production_state
.lock()
.take()
.filter(|(cached_block_root, _)| *cached_block_root == head_block_root)
else if let Some((_, cached_state)) =
self.get_state_from_block_production_cache(head_block_root)
{
(cached_state.pre_state, cached_state.state_root)
}
// Fall back to a direct read of the snapshot cache.
else if let Some(pre_state) = self
.snapshot_cache
.try_read_for(BLOCK_PROCESSING_CACHE_LOCK_TIMEOUT)
.and_then(|snapshot_cache| {
snapshot_cache.get_state_for_block_production(head_block_root)
})
else if let Some(pre_state) =
self.get_state_from_snapshot_cache_for_block_production(head_block_root)
{
warn!(
self.log,
Expand Down Expand Up @@ -4221,6 +4214,40 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
Ok((state, state_root_opt))
}

/// Get the state cached for block production *if* it matches `head_block_root`.
///
/// This will clear the cache regardless of whether the block root matches, so only call this if
/// you think the `head_block_root` is likely to match!
fn get_state_from_block_production_cache(
&self,
head_block_root: Hash256,
) -> Option<(Hash256, BlockProductionPreState<T::EthSpec>)> {
// Take care to drop the lock as quickly as possible.
let mut lock = self.block_production_state.lock();
let result = lock
.take()
.filter(|(cached_block_root, _)| *cached_block_root == head_block_root);
drop(lock);
result
}

/// Get a state for block production from the snapshot cache.
fn get_state_from_snapshot_cache_for_block_production(
&self,
head_block_root: Hash256,
) -> Option<BlockProductionPreState<T::EthSpec>> {
if let Some(lock) = self
.snapshot_cache
.try_read_for(BLOCK_PROCESSING_CACHE_LOCK_TIMEOUT)
{
let result = lock.get_state_for_block_production(head_block_root);
drop(lock);
result
} else {
None
}
}

/// Fetch the beacon state to use for producing a block if a 1-slot proposer re-org is viable.
///
/// This function will return `None` if proposer re-orgs are disabled.
Expand Down Expand Up @@ -4313,12 +4340,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {

// Only attempt a re-org if we hit the block production cache or snapshot cache.
let pre_state = self
.block_production_state
.lock()
.take()
.and_then(|(cached_block_root, state)| {
(cached_block_root == re_org_parent_block).then_some(state)
})
.get_state_from_block_production_cache(re_org_parent_block)
.map(|(_, state)| state)
.or_else(|| {
warn!(
self.log,
Expand All @@ -4327,11 +4350,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
"slot" => slot,
"block_root" => ?re_org_parent_block
);
self.snapshot_cache
.try_read_for(BLOCK_PROCESSING_CACHE_LOCK_TIMEOUT)
.and_then(|snapshot_cache| {
snapshot_cache.get_state_for_block_production(re_org_parent_block)
})
self.get_state_from_snapshot_cache_for_block_production(re_org_parent_block)
})
.or_else(|| {
debug!(
Expand Down
31 changes: 15 additions & 16 deletions lighthouse/tests/beacon_node.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@ use types::{
Address, Checkpoint, Epoch, ExecutionBlockHash, ForkName, Hash256, MainnetEthSpec,
ProgressiveBalancesMode,
};
use unused_port::{unused_tcp4_port, unused_tcp6_port, unused_udp4_port, unused_udp6_port};

const DEFAULT_ETH1_ENDPOINT: &str = "http://localhost:8545/";

// These dummy ports should ONLY be used for `enr-xxx-port` flags that do not bind.
const DUMMY_ENR_TCP_PORT: u16 = 7777;
const DUMMY_ENR_UDP_PORT: u16 = 8888;
const DUMMY_ENR_QUIC_PORT: u16 = 9999;
Expand Down Expand Up @@ -891,7 +894,7 @@ fn network_port_flag_over_ipv4() {
);
});

let port = 9000;
let port = unused_tcp4_port().expect("Unable to find unused port.");
CommandLineTest::new()
.flag("port", Some(port.to_string().as_str()))
.flag("allow-insecure-genesis-sync", None)
Expand Down Expand Up @@ -928,7 +931,7 @@ fn network_port_flag_over_ipv6() {
);
});

let port = 9000;
let port = unused_tcp4_port().expect("Unable to find unused port.");
CommandLineTest::new()
.flag("listen-address", Some("::1"))
.flag("port", Some(port.to_string().as_str()))
Expand Down Expand Up @@ -978,8 +981,8 @@ fn network_port_flag_over_ipv4_and_ipv6() {
);
});

let port = 19000;
let port6 = 29000;
let port = unused_tcp4_port().expect("Unable to find unused port.");
let port6 = unused_tcp6_port().expect("Unable to find unused port.");
CommandLineTest::new()
.flag("listen-address", Some("127.0.0.1"))
.flag("listen-address", Some("::1"))
Expand Down Expand Up @@ -1320,9 +1323,8 @@ fn enr_tcp6_port_flag() {
fn enr_match_flag_over_ipv4() {
let addr = "127.0.0.2".parse::<Ipv4Addr>().unwrap();

// the reason we use the ENR dummy values is because, due to the nature of the `--enr-match` flag, these will eventually become ENR ports (as well as listening ports).
let udp4_port = DUMMY_ENR_UDP_PORT;
let tcp4_port = DUMMY_ENR_TCP_PORT;
let udp4_port = unused_udp4_port().expect("Unable to find unused port.");
let tcp4_port = unused_tcp4_port().expect("Unable to find unused port.");

CommandLineTest::new()
.flag("enr-match", None)
Expand Down Expand Up @@ -1352,9 +1354,8 @@ fn enr_match_flag_over_ipv6() {
const ADDR: &str = "::1";
let addr = ADDR.parse::<Ipv6Addr>().unwrap();

// the reason we use the ENR dummy values is because, due to the nature of the `--enr-match` flag, these will eventually become ENR ports (as well as listening ports).
let udp6_port = DUMMY_ENR_UDP_PORT;
let tcp6_port = DUMMY_ENR_TCP_PORT;
let udp6_port = unused_udp6_port().expect("Unable to find unused port.");
let tcp6_port = unused_tcp6_port().expect("Unable to find unused port.");

CommandLineTest::new()
.flag("enr-match", None)
Expand Down Expand Up @@ -1383,15 +1384,13 @@ fn enr_match_flag_over_ipv6() {
fn enr_match_flag_over_ipv4_and_ipv6() {
const IPV6_ADDR: &str = "::1";

// the reason we use the ENR dummy values is because, due to the nature of the `--enr-match` flag, these will eventually become ENR ports (as well as listening ports).
let udp6_port = DUMMY_ENR_UDP_PORT;
let tcp6_port = DUMMY_ENR_TCP_PORT;
let udp6_port = unused_udp6_port().expect("Unable to find unused port.");
let tcp6_port = unused_tcp6_port().expect("Unable to find unused port.");
let ipv6_addr = IPV6_ADDR.parse::<Ipv6Addr>().unwrap();

const IPV4_ADDR: &str = "127.0.0.1";
// the reason we use the ENR dummy values is because, due to the nature of the `--enr-match` flag, these will eventually become ENR ports (as well as listening ports).
let udp4_port = DUMMY_ENR_UDP_PORT;
let tcp4_port = DUMMY_ENR_TCP_PORT;
let udp4_port = unused_udp4_port().expect("Unable to find unused port.");
let tcp4_port = unused_tcp4_port().expect("Unable to find unused port.");
let ipv4_addr = IPV4_ADDR.parse::<Ipv4Addr>().unwrap();

CommandLineTest::new()
Expand Down
Loading