Reduce size of futures in HTTP API to prevent stack overflows (#5104)

* Box::pin a few big futures

* Arc the blocks early in publication

* Fix more tests
This commit is contained in:
Michael Sproul
2024-01-23 15:32:07 +11:00
committed by GitHub
parent 02d1f36090
commit a403138ed0
17 changed files with 116 additions and 123 deletions
+4 -2
View File
@@ -1410,7 +1410,7 @@ pub fn serve<T: BeaconChainTypes>(
.and(network_tx_filter.clone())
.and(log_filter.clone())
.then(
move |block_contents: SignedBlindedBeaconBlock<T::EthSpec>,
move |block_contents: Arc<SignedBlindedBeaconBlock<T::EthSpec>>,
task_spawner: TaskSpawner<T::EthSpec>,
chain: Arc<BeaconChain<T>>,
network_tx: UnboundedSender<NetworkMessage<T::EthSpec>>,
@@ -1450,6 +1450,7 @@ pub fn serve<T: BeaconChainTypes>(
&block_bytes,
&chain.spec,
)
.map(Arc::new)
.map_err(|e| {
warp_utils::reject::custom_bad_request(format!("invalid SSZ: {e:?}"))
})?;
@@ -1478,7 +1479,7 @@ pub fn serve<T: BeaconChainTypes>(
.and(log_filter.clone())
.then(
move |validation_level: api_types::BroadcastValidationQuery,
blinded_block: SignedBlindedBeaconBlock<T::EthSpec>,
blinded_block: Arc<SignedBlindedBeaconBlock<T::EthSpec>>,
task_spawner: TaskSpawner<T::EthSpec>,
chain: Arc<BeaconChain<T>>,
network_tx: UnboundedSender<NetworkMessage<T::EthSpec>>,
@@ -1519,6 +1520,7 @@ pub fn serve<T: BeaconChainTypes>(
&block_bytes,
&chain.spec,
)
.map(Arc::new)
.map_err(|e| {
warp_utils::reject::custom_bad_request(format!("invalid SSZ: {e:?}"))
})?;
+14 -11
View File
@@ -194,7 +194,7 @@ pub async fn publish_block<T: BeaconChainTypes, B: IntoGossipVerifiedBlockConten
if let Some(gossip_verified_blobs) = gossip_verified_blobs {
for blob in gossip_verified_blobs {
if let Err(e) = chain.process_gossip_blob(blob).await {
if let Err(e) = Box::pin(chain.process_gossip_blob(blob)).await {
let msg = format!("Invalid blob: {e}");
return if let BroadcastValidation::Gossip = validation_level {
Err(warp_utils::reject::broadcast_without_import(msg))
@@ -210,14 +210,13 @@ pub async fn publish_block<T: BeaconChainTypes, B: IntoGossipVerifiedBlockConten
}
}
match chain
.process_block(
block_root,
gossip_verified_block,
NotifyExecutionLayer::Yes,
publish_fn,
)
.await
match Box::pin(chain.process_block(
block_root,
gossip_verified_block,
NotifyExecutionLayer::Yes,
publish_fn,
))
.await
{
Ok(AvailabilityProcessingStatus::Imported(root)) => {
info!(
@@ -291,7 +290,7 @@ pub async fn publish_block<T: BeaconChainTypes, B: IntoGossipVerifiedBlockConten
/// Handles a request from the HTTP API for blinded blocks. This converts blinded blocks into full
/// blocks before publishing.
pub async fn publish_blinded_block<T: BeaconChainTypes>(
blinded_block: SignedBlindedBeaconBlock<T::EthSpec>,
blinded_block: Arc<SignedBlindedBeaconBlock<T::EthSpec>>,
chain: Arc<BeaconChain<T>>,
network_tx: &UnboundedSender<NetworkMessage<T::EthSpec>>,
log: Logger,
@@ -319,7 +318,7 @@ pub async fn publish_blinded_block<T: BeaconChainTypes>(
pub async fn reconstruct_block<T: BeaconChainTypes>(
chain: Arc<BeaconChain<T>>,
block_root: Hash256,
block: SignedBlindedBeaconBlock<T::EthSpec>,
block: Arc<SignedBlindedBeaconBlock<T::EthSpec>>,
log: Logger,
) -> Result<ProvenancedBlock<T, PublishBlockRequest<T::EthSpec>>, Rejection> {
let full_payload_opt = if let Ok(payload_header) = block.message().body().execution_payload() {
@@ -380,6 +379,10 @@ pub async fn reconstruct_block<T: BeaconChainTypes>(
None
};
// Perf: cloning the block here to unblind it is a little sub-optimal. This is considered an
// acceptable tradeoff to avoid passing blocks around on the stack (unarced), which blows up
// the size of futures.
let block = (*block).clone();
match full_payload_opt {
// A block without a payload is pre-merge and we consider it locally
// built.
@@ -3,7 +3,7 @@ use beacon_chain::{
GossipVerifiedBlock, IntoGossipVerifiedBlockContents,
};
use eth2::reqwest::StatusCode;
use eth2::types::{BroadcastValidation, PublishBlockRequest, SignedBeaconBlock};
use eth2::types::{BroadcastValidation, PublishBlockRequest};
use http_api::test_utils::InteractiveTester;
use http_api::{publish_blinded_block, publish_block, reconstruct_block, ProvenancedBlock};
use std::sync::Arc;
@@ -63,7 +63,7 @@ pub async fn gossip_invalid() {
tester.harness.advance_slot();
let ((block, blobs), _): ((SignedBeaconBlock<E>, _), _) = tester
let ((block, blobs), _) = tester
.harness
.make_block_with_modifier(chain_state_before, slot, |b| {
*b.state_root_mut() = Hash256::zero();
@@ -115,7 +115,7 @@ pub async fn gossip_partial_pass() {
tester.harness.advance_slot();
let ((block, blobs), _): ((SignedBeaconBlock<E>, _), _) = tester
let ((block, blobs), _) = tester
.harness
.make_block_with_modifier(chain_state_before, slot, |b| {
*b.state_root_mut() = Hash256::random()
@@ -161,8 +161,7 @@ pub async fn gossip_full_pass() {
let slot_b = slot_a + 1;
let state_a = tester.harness.get_current_state();
let ((block, blobs), _): ((SignedBeaconBlock<E>, _), _) =
tester.harness.make_block(state_a, slot_b).await;
let ((block, blobs), _) = tester.harness.make_block(state_a, slot_b).await;
let response: Result<(), eth2::Error> = tester
.client
@@ -252,7 +251,7 @@ pub async fn consensus_invalid() {
tester.harness.advance_slot();
let ((block, blobs), _): ((SignedBeaconBlock<E>, _), _) = tester
let ((block, blobs), _) = tester
.harness
.make_block_with_modifier(chain_state_before, slot, |b| {
*b.state_root_mut() = Hash256::zero();
@@ -304,7 +303,7 @@ pub async fn consensus_gossip() {
let slot_b = slot_a + 1;
let state_a = tester.harness.get_current_state();
let ((block, blobs), _): ((SignedBeaconBlock<E>, _), _) = tester
let ((block, blobs), _) = tester
.harness
.make_block_with_modifier(state_a, slot_b, |b| *b.state_root_mut() = Hash256::zero())
.await;
@@ -418,8 +417,7 @@ pub async fn consensus_full_pass() {
let slot_b = slot_a + 1;
let state_a = tester.harness.get_current_state();
let ((block, blobs), _): ((SignedBeaconBlock<E>, _), _) =
tester.harness.make_block(state_a, slot_b).await;
let ((block, blobs), _) = tester.harness.make_block(state_a, slot_b).await;
let response: Result<(), eth2::Error> = tester
.client
@@ -465,7 +463,7 @@ pub async fn equivocation_invalid() {
tester.harness.advance_slot();
let ((block, blobs), _): ((SignedBeaconBlock<E>, _), _) = tester
let ((block, blobs), _) = tester
.harness
.make_block_with_modifier(chain_state_before, slot, |b| {
*b.state_root_mut() = Hash256::zero();
@@ -518,10 +516,9 @@ pub async fn equivocation_consensus_early_equivocation() {
let slot_b = slot_a + 1;
let state_a = tester.harness.get_current_state();
let ((block_a, blobs_a), state_after_a): ((SignedBeaconBlock<E>, _), _) =
let ((block_a, blobs_a), state_after_a) =
tester.harness.make_block(state_a.clone(), slot_b).await;
let ((block_b, blobs_b), state_after_b): ((SignedBeaconBlock<E>, _), _) =
tester.harness.make_block(state_a, slot_b).await;
let ((block_b, blobs_b), state_after_b) = tester.harness.make_block(state_a, slot_b).await;
/* check for `make_block` curios */
assert_eq!(block_a.state_root(), state_after_a.tree_hash_root());
@@ -590,7 +587,7 @@ pub async fn equivocation_gossip() {
let slot_b = slot_a + 1;
let state_a = tester.harness.get_current_state();
let ((block, blobs), _): ((SignedBeaconBlock<E>, _), _) = tester
let ((block, blobs), _) = tester
.harness
.make_block_with_modifier(state_a, slot_b, |b| *b.state_root_mut() = Hash256::zero())
.await;
@@ -645,10 +642,9 @@ pub async fn equivocation_consensus_late_equivocation() {
let slot_b = slot_a + 1;
let state_a = tester.harness.get_current_state();
let ((block_a, blobs_a), state_after_a): ((SignedBeaconBlock<E>, _), _) =
let ((block_a, blobs_a), state_after_a) =
tester.harness.make_block(state_a.clone(), slot_b).await;
let ((block_b, blobs_b), state_after_b): ((SignedBeaconBlock<E>, _), _) =
tester.harness.make_block(state_a, slot_b).await;
let ((block_b, blobs_b), state_after_b) = tester.harness.make_block(state_a, slot_b).await;
/* check for `make_block` curios */
assert_eq!(block_a.state_root(), state_after_a.tree_hash_root());
@@ -716,8 +712,7 @@ pub async fn equivocation_full_pass() {
let slot_b = slot_a + 1;
let state_a = tester.harness.get_current_state();
let ((block, blobs), _): ((SignedBeaconBlock<E>, _), _) =
tester.harness.make_block(state_a, slot_b).await;
let ((block, blobs), _) = tester.harness.make_block(state_a, slot_b).await;
let response: Result<(), eth2::Error> = tester
.client
@@ -1269,6 +1264,7 @@ pub async fn blinded_equivocation_consensus_late_equivocation() {
.make_blinded_block(state_a.clone(), slot_b)
.await;
let (block_b, state_after_b) = tester.harness.make_blinded_block(state_a, slot_b).await;
let block_b = Arc::new(block_b);
/* check for `make_blinded_block` curios */
assert_eq!(block_a.state_root(), state_after_a.tree_hash_root());
@@ -1278,7 +1274,7 @@ pub async fn blinded_equivocation_consensus_late_equivocation() {
let unblinded_block_a = reconstruct_block(
tester.harness.chain.clone(),
block_a.canonical_root(),
block_a,
Arc::new(block_a),
test_logger.clone(),
)
.await
@@ -1301,15 +1297,11 @@ pub async fn blinded_equivocation_consensus_late_equivocation() {
ProvenancedBlock::Builder(b, _) => b,
};
let gossip_block_b = GossipVerifiedBlock::new(
Arc::new(inner_block_b.clone().deconstruct().0),
&tester.harness.chain,
);
let gossip_block_b =
GossipVerifiedBlock::new(inner_block_b.clone().deconstruct().0, &tester.harness.chain);
assert!(gossip_block_b.is_ok());
let gossip_block_a = GossipVerifiedBlock::new(
Arc::new(inner_block_a.clone().deconstruct().0),
&tester.harness.chain,
);
let gossip_block_a =
GossipVerifiedBlock::new(inner_block_a.clone().deconstruct().0, &tester.harness.chain);
assert!(gossip_block_a.is_err());
let channel = tokio::sync::mpsc::unbounded_channel();
@@ -632,7 +632,7 @@ pub async fn proposer_boost_re_org_test(
panic!("Should not be a blinded block");
}
};
let block_c = harness.sign_beacon_block(unsigned_block_c, &state_b);
let block_c = Arc::new(harness.sign_beacon_block(unsigned_block_c, &state_b));
if should_re_org {
// Block C should build on A.
+6 -6
View File
@@ -2597,7 +2597,7 @@ impl ApiTester {
let signed_block = block.sign(&sk, &fork, genesis_validators_root, &self.chain.spec);
let signed_block_contents =
PublishBlockRequest::try_from(signed_block.clone()).unwrap();
PublishBlockRequest::try_from(Arc::new(signed_block.clone())).unwrap();
self.client
.post_beacon_blocks(&signed_block_contents)
@@ -2670,8 +2670,8 @@ impl ApiTester {
.unwrap();
assert_eq!(
self.chain.head_beacon_block().as_ref(),
signed_block_contents.signed_block()
self.chain.head_beacon_block(),
*signed_block_contents.signed_block()
);
self.chain.slot_clock.set_slot(slot.as_u64() + 1);
@@ -2763,8 +2763,8 @@ impl ApiTester {
.unwrap();
assert_eq!(
self.chain.head_beacon_block().as_ref(),
signed_block_contents.signed_block()
self.chain.head_beacon_block(),
*signed_block_contents.signed_block()
);
self.chain.slot_clock.set_slot(slot.as_u64() + 1);
@@ -2994,7 +2994,7 @@ impl ApiTester {
.data;
let signed_block = signed_block_contents.signed_block();
assert_eq!(&head_block, signed_block);
assert_eq!(head_block, **signed_block);
self.chain.slot_clock.set_slot(slot.as_u64() + 1);
}