-
Notifications
You must be signed in to change notification settings - Fork 108
feat(testing): add flashblocks-e2e testing tool #348
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.
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
base: main
Are you sure you want to change the base?
Conversation
Port the flashblocks-e2e end-to-end testing tool with a reusable library architecture: - bin/flashblocks-e2e/: minimal CLI shim (~70 lines) - testing/e2e/: reusable library with TestClient, FlashblockHarness, test runner, and 22 tests across 7 categories Test categories: blocks, call, receipts, logs, sanity, metering, contracts
🟡 Heimdall Review Status
|
Establish naming pattern where testing crates include "testing" in the name for clarity.
* fix flashblocks timing and tests * formatting
* fix flashblocks timing and tests * formatting
Remove hardcoded recipient addresses from test files. The recipient must now be provided via --recipient CLI argument, and the tool aborts if recipient equals sender to prevent accidental self-transfers.
haardikk21
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments
Also a question more generally - what's the intention with this CLI? I may have missed the memo on this but is this something we're gonna run manually against dev nodes to try out things or something?
my prev understanding was its aiming to set the stage a bit for automated devnet-style testing of changes per-PR/pre-release, but then I don't really get the need for the CLI and instead have other things in mind that need to be addressed first.
| Test { | ||
| name: "get_latest_block".to_string(), | ||
| description: Some("Verify we can retrieve the latest block".to_string()), | ||
| run: Box::new(|client| Box::pin(test_get_latest_block(client))), | ||
| skip_if: None, | ||
| }, | ||
| Test { | ||
| name: "get_pending_block".to_string(), | ||
| description: Some("Verify we can retrieve the pending block".to_string()), | ||
| run: Box::new(|client| Box::pin(test_get_pending_block(client))), | ||
| skip_if: None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
non-blocking nit but this seems like the perfect use case for just definining a custom macro to get rid of these duplications throughout this crate
| fn skip_if_no_signer_or_recipient(client: &TestClient) -> Option<String> { | ||
| if !client.has_signer() { | ||
| Some("No PRIVATE_KEY configured".to_string()) | ||
| } else if client.recipient().is_none() { | ||
| Some("No --recipient configured".to_string()) | ||
| } else { | ||
| None | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this fn is present across all the different test files. can we just share this somewhere instead?
| let json_str = decode_ws_message(&msg)?; | ||
|
|
||
| let flashblock: Flashblock = | ||
| serde_json::from_str(&json_str).wrap_err("Failed to parse flashblock message")?; | ||
|
|
||
| return Ok(flashblock); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we just use
node-reth/crates/shared/flashtypes/src/block.rs
Lines 29 to 48 in a5de9fa
| impl Flashblock { | |
| /// Attempts to decode a flashblock from bytes that may be plain JSON or brotli-compressed JSON. | |
| pub fn try_decode_message(bytes: impl Into<Bytes>) -> Result<Self, FlashblockDecodeError> { | |
| let text = Self::try_parse_message(bytes.into())?; | |
| let payload: FlashblocksPayloadV1 = | |
| serde_json::from_str(&text).map_err(FlashblockDecodeError::PayloadParse)?; | |
| let metadata: Metadata = serde_json::from_value(payload.metadata.clone()) | |
| .map_err(FlashblockDecodeError::MetadataParse)?; | |
| Ok(Self { | |
| payload_id: payload.payload_id, | |
| index: payload.index, | |
| base: payload.base, | |
| diff: payload.diff, | |
| metadata, | |
| }) | |
| } | |
| pub struct FlashblockBase { | ||
| /// Base fee per gas. | ||
| pub base_fee_per_gas: String, | ||
| /// Block number. | ||
| pub block_number: String, | ||
| /// Block timestamp. | ||
| pub timestamp: String, | ||
| /// Fee recipient address. | ||
| pub fee_recipient: String, | ||
| /// Gas limit. | ||
| pub gas_limit: String, | ||
| /// Parent block hash. | ||
| pub parent_hash: String, | ||
| } | ||
|
|
||
| /// Diff/delta for this flashblock. | ||
| #[derive(Debug, Clone, Serialize, Deserialize)] | ||
| pub struct FlashblockDiff { | ||
| /// State root after this flashblock. | ||
| pub state_root: String, | ||
| /// Receipts root after this flashblock. | ||
| pub receipts_root: String, | ||
| /// Logs bloom filter. | ||
| pub logs_bloom: String, | ||
| /// Cumulative gas used. | ||
| pub gas_used: String, | ||
| /// Block hash (changes with each flashblock). | ||
| pub block_hash: String, | ||
| /// RLP-encoded transaction hex strings. | ||
| pub transactions: Vec<String>, | ||
| /// Withdrawals in this flashblock. | ||
| #[serde(default)] | ||
| pub withdrawals: Vec<serde_json::Value>, | ||
| /// Withdrawals root. | ||
| #[serde(default)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these should just be coming from the flashtypes crate
|
Review Error for haardikk21 @ 2026-01-12 06:12:24 UTC |
Summary
flashblocks-e2ebinary for end-to-end regression testing of flashblocks RPCtesting/e2elibrary crate with reusable test infrastructureArchitecture
Usage
Test plan
cargo check -p base-e2e -p flashblocks-e2ecargo clippy -p base-e2e -p flashblocks-e2e -- -D warnings