Skip to content

Commit

Permalink
Pre-generate test blobs bundle to improve test time. (sigp#4829)
Browse files Browse the repository at this point in the history
## Issue Addressed

Addresses sigp#4778, and potentially fixes the flaky deneb builder test `builder_works_post_deneb`.

The [deneb builder test](https://github.com/sigp/lighthouse/blob/c5c84f12138c6fb6b2d0317c467f66cc0f4f8111/beacon_node/http_api/tests/tests.rs#L5371) has been quite flaky on our CI (`release-tests`) since it was introduced. I'm guessing that it might be timing out on the builder `get_header` call (1 second), and therefore the local payload is used, while the test expects builder payload to be used. 

On my machine the [`get_header` ](https://github.com/sigp/lighthouse/blob/c5c84f12138c6fb6b2d0317c467f66cc0f4f8111/beacon_node/execution_layer/src/test_utils/mock_builder.rs#L367) call takes about 550ms, which could easily go over 1s on slower environments (our windows CI runner is much slower than the ubuntu one).

I did a profile on the test and it showed that `blob_to_kzg_commiment` and `compute_kzg_proof` was taking a large chunk of time, so perhaps pre-generating the blobs could help stablise this test.

## Proposed Changes

Pre-generate blobs bundle for Mainnet and Minimal presets.

Before the change `get_header` took about **550ms**, and it's now reduced to **50-55ms** after the change. If timeout was indeed the cause of the flaky test, this fix should stablise it. This also brings the flaky `builder_works_post_deneb` test time from 50s to 10s. (8s if we only use a single blob)
  • Loading branch information
jimmygchen authored and Gua00va committed Oct 18, 2023
1 parent a59432b commit a0f62a2
Show file tree
Hide file tree
Showing 10 changed files with 86 additions and 40 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -267,9 +267,7 @@ pub mod tests {
use crate::test_utils::{generate_rand_block_and_blobs, NumBlobs};
use crate::AvailabilityPendingExecutedBlock;
use crate::PayloadVerificationOutcome;
use eth2_network_config::get_trusted_setup;
use fork_choice::PayloadVerificationStatus;
use kzg::{Kzg, TrustedSetup};
use rand::rngs::StdRng;
use rand::SeedableRng;
use state_processing::ConsensusContext;
Expand All @@ -285,13 +283,9 @@ pub mod tests {
);

pub fn pre_setup() -> Setup<E> {
let trusted_setup: TrustedSetup =
serde_json::from_reader(get_trusted_setup::<<E as EthSpec>::Kzg>()).unwrap();
let kzg = Kzg::new_from_trusted_setup(trusted_setup).unwrap();

let mut rng = StdRng::seed_from_u64(0xDEADBEEF0BAD5EEDu64);
let (block, blobs_vec) =
generate_rand_block_and_blobs::<E>(ForkName::Deneb, NumBlobs::Random, &kzg, &mut rng);
generate_rand_block_and_blobs::<E>(ForkName::Deneb, NumBlobs::Random, &mut rng);
let mut blobs: FixedVector<_, <E as EthSpec>::MaxBlobsPerBlock> = FixedVector::default();

for blob in blobs_vec {
Expand Down
4 changes: 1 addition & 3 deletions beacon_node/beacon_chain/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2526,7 +2526,6 @@ pub enum NumBlobs {
pub fn generate_rand_block_and_blobs<E: EthSpec>(
fork_name: ForkName,
num_blobs: NumBlobs,
kzg: &Kzg<E::Kzg>,
rng: &mut impl Rng,
) -> (SignedBeaconBlock<E, FullPayload<E>>, Vec<BlobSidecar<E>>) {
let inner = map_fork_name!(fork_name, BeaconBlock, <_>::random_for_test(rng));
Expand All @@ -2540,8 +2539,7 @@ pub fn generate_rand_block_and_blobs<E: EthSpec>(
NumBlobs::None => 0,
};
let (bundle, transactions) =
execution_layer::test_utils::generate_random_blobs::<E, _>(num_blobs, kzg, rng)
.unwrap();
execution_layer::test_utils::generate_blobs::<E>(num_blobs).unwrap();

payload.execution_payload.transactions = <_>::default();
for tx in Vec::from(transactions) {
Expand Down
1 change: 1 addition & 0 deletions beacon_node/execution_layer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,4 @@ triehash = "0.8.4"
hash-db = "0.15.2"
pretty_reqwest_error = { workspace = true }
arc-swap = "1.6.0"
eth2_network_config = { workspace = true }
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::{
ExecutionBlockWithTransactions,
};
use eth2::types::BlobsBundle;
use kzg::Kzg;
use kzg::{Kzg, KzgCommitment, KzgProof};
use parking_lot::Mutex;
use rand::{rngs::StdRng, Rng, SeedableRng};
use serde::{Deserialize, Serialize};
Expand All @@ -20,12 +20,16 @@ use std::sync::Arc;
use tree_hash::TreeHash;
use tree_hash_derive::TreeHash;
use types::{
BlobSidecar, ChainSpec, EthSpec, ExecutionBlockHash, ExecutionPayload, ExecutionPayloadCapella,
ExecutionPayloadDeneb, ExecutionPayloadHeader, ExecutionPayloadMerge, ForkName, Hash256,
Transaction, Transactions, Uint256,
Blob, ChainSpec, EthSpec, EthSpecId, ExecutionBlockHash, ExecutionPayload,
ExecutionPayloadCapella, ExecutionPayloadDeneb, ExecutionPayloadHeader, ExecutionPayloadMerge,
ForkName, Hash256, Transaction, Transactions, Uint256,
};

use super::DEFAULT_TERMINAL_BLOCK;
use ssz::Decode;

const TEST_BLOB_BUNDLE_MAINNET: &[u8] = include_bytes!("fixtures/mainnet/test_blobs_bundle.ssz");
const TEST_BLOB_BUNDLE_MINIMAL: &[u8] = include_bytes!("fixtures/minimal/test_blobs_bundle.ssz");

const GAS_LIMIT: u64 = 16384;
const GAS_USED: u64 = GAS_LIMIT - 1;
Expand Down Expand Up @@ -627,8 +631,7 @@ impl<T: EthSpec> ExecutionBlockGenerator<T> {
// get random number between 0 and Max Blobs
let mut rng = self.rng.lock();
let num_blobs = rng.gen::<usize>() % (T::max_blobs_per_block() + 1);
let kzg = self.kzg.as_ref().ok_or("kzg not initialized")?;
let (bundle, transactions) = generate_random_blobs(num_blobs, kzg, &mut *rng)?;
let (bundle, transactions) = generate_blobs(num_blobs)?;
for tx in Vec::from(transactions) {
execution_payload
.transactions_mut()
Expand All @@ -645,30 +648,51 @@ impl<T: EthSpec> ExecutionBlockGenerator<T> {
}
}

pub fn generate_random_blobs<T: EthSpec, R: Rng>(
pub fn load_test_blobs_bundle<E: EthSpec>() -> Result<(KzgCommitment, KzgProof, Blob<E>), String> {
let blob_bundle_bytes = match E::spec_name() {
EthSpecId::Mainnet => TEST_BLOB_BUNDLE_MAINNET,
EthSpecId::Minimal => TEST_BLOB_BUNDLE_MINIMAL,
EthSpecId::Gnosis => {
return Err("Test blobs bundle not available for Gnosis preset".to_string())
}
};

let BlobsBundle {
commitments,
proofs,
blobs,
} = BlobsBundle::<E>::from_ssz_bytes(blob_bundle_bytes)
.map_err(|e| format!("Unable to decode SSZ: {:?}", e))?;

Ok((
commitments
.get(0)
.cloned()
.ok_or("commitment missing in test bundle")?,
proofs
.get(0)
.cloned()
.ok_or("proof missing in test bundle")?,
blobs.get(0).cloned().ok_or("blob missing in test bundle")?,
))
}

pub fn generate_blobs<E: EthSpec>(
n_blobs: usize,
kzg: &Kzg<T::Kzg>,
rng: &mut R,
) -> Result<(BlobsBundle<T>, Transactions<T>), String> {
let mut bundle = BlobsBundle::<T>::default();
let mut transactions = vec![];
for blob_index in 0..n_blobs {
let random_valid_sidecar = BlobSidecar::<T>::random_valid(rng, kzg)?;
) -> Result<(BlobsBundle<E>, Transactions<E>), String> {
let (kzg_commitment, kzg_proof, blob) = load_test_blobs_bundle::<E>()?;

let BlobSidecar {
blob,
kzg_commitment,
kzg_proof,
..
} = random_valid_sidecar;
let mut bundle = BlobsBundle::<E>::default();
let mut transactions = vec![];

let tx = static_valid_tx::<T>()
for blob_index in 0..n_blobs {
let tx = static_valid_tx::<E>()
.map_err(|e| format!("error creating valid tx SSZ bytes: {:?}", e))?;

transactions.push(tx);
bundle
.blobs
.push(blob)
.push(blob.clone())
.map_err(|_| format!("blobs are full, blob index: {:?}", blob_index))?;
bundle
.commitments
Expand Down Expand Up @@ -797,7 +821,8 @@ pub fn generate_pow_block(
#[cfg(test)]
mod test {
use super::*;
use types::MainnetEthSpec;
use kzg::TrustedSetup;
use types::{MainnetEthSpec, MinimalEthSpec};

#[test]
fn pow_chain_only() {
Expand Down Expand Up @@ -859,4 +884,33 @@ mod test {
assert!(generator.block_by_number(next_i).is_none());
}
}

#[test]
fn valid_test_blobs() {
assert!(
validate_blob::<MainnetEthSpec>().unwrap(),
"Mainnet preset test blobs bundle should contain valid proofs"
);
assert!(
validate_blob::<MinimalEthSpec>().unwrap(),
"Minimal preset test blobs bundle should contain valid proofs"
);
}

fn validate_blob<E: EthSpec>() -> Result<bool, String> {
let kzg = load_kzg::<E>()?;
let (kzg_commitment, kzg_proof, blob) = load_test_blobs_bundle::<E>()?;
let kzg_blob = E::blob_from_bytes(blob.as_ref())
.map_err(|e| format!("Error converting blob to kzg blob: {e:?}"))?;
kzg.verify_blob_kzg_proof(&kzg_blob, kzg_commitment, kzg_proof)
.map_err(|e| format!("Invalid blobs bundle: {e:?}"))
}

fn load_kzg<E: EthSpec>() -> Result<Kzg<E::Kzg>, String> {
let trusted_setup: TrustedSetup =
serde_json::from_reader(eth2_network_config::get_trusted_setup::<E::Kzg>())
.map_err(|e| format!("Unable to read trusted setup file: {e:?}"))?;
Kzg::new_from_trusted_setup(trusted_setup)
.map_err(|e| format!("Failed to load trusted setup: {e:?}"))
}
}
Binary file not shown.
Binary file not shown.
4 changes: 2 additions & 2 deletions beacon_node/execution_layer/src/test_utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ use warp::{http::StatusCode, Filter, Rejection};

use crate::EngineCapabilities;
pub use execution_block_generator::{
generate_genesis_block, generate_genesis_header, generate_pow_block, generate_random_blobs,
Block, ExecutionBlockGenerator,
generate_blobs, generate_genesis_block, generate_genesis_header, generate_pow_block, Block,
ExecutionBlockGenerator,
};
pub use hook::Hook;
pub use mock_builder::{MockBuilder, Operation};
Expand Down
4 changes: 1 addition & 3 deletions beacon_node/network/src/sync/block_lookups/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,8 @@ impl TestRig {
fork_name: ForkName,
num_blobs: NumBlobs,
) -> (SignedBeaconBlock<E>, Vec<BlobSidecar<E>>) {
let kzg = self.harness.chain.kzg.as_ref().unwrap();
let rng = &mut self.rng;

generate_rand_block_and_blobs::<E>(fork_name, num_blobs, kzg.as_ref(), rng)
generate_rand_block_and_blobs::<E>(fork_name, num_blobs, rng)
}

#[track_caller]
Expand Down
4 changes: 2 additions & 2 deletions common/eth2/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use mediatype::{names, MediaType, MediaTypeList};
use serde::{Deserialize, Deserializer, Serialize};
use serde_json::Value;
use ssz::{Decode, DecodeError};
use ssz_derive::Encode;
use ssz_derive::{Decode, Encode};
use std::convert::TryFrom;
use std::fmt::{self, Display};
use std::str::{from_utf8, FromStr};
Expand Down Expand Up @@ -2012,7 +2012,7 @@ pub struct ExecutionPayloadAndBlobs<E: EthSpec> {
pub blobs_bundle: BlobsBundle<E>,
}

#[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize, Encode)]
#[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize, Encode, Decode)]
#[serde(bound = "E: EthSpec")]
pub struct BlobsBundle<E: EthSpec> {
pub commitments: KzgCommitments<E>,
Expand Down

0 comments on commit a0f62a2

Please sign in to comment.