feat: add blockTimestamp to logs - #1644
Conversation
Closes NomicFoundation#1643. blockTimestamp was added to the Log schema by ethereum/execution-apis#639 (merged 2025-08-25) so that a consumer reading logs over a block range does not have to issue a second eth_getBlockByHash per block purely to timestamp them. reth, geth, besu, erigon, anvil and ethereumjs all serve it; EDR did not. The field is populated on two paths, and they are genuinely separate: - FullBlockLog gains block_timestamp, filled from header.timestamp in map_transaction_receipt_logs. This covers logs in transaction receipts. - LogOutput, the wire type behind eth_getLogs / eth_getFilterLogs / eth_getFilterChanges, gains the same field and carries it across in From<&FilterLog>. Without this second change receipts report the timestamp and eth_getLogs still does not, which is the case the issue is actually about. It is Option<u64> rather than u64 on purpose. The field is optional in the spec (the Log schema's only required field is transactionHash), and a node we fork from may predate the change, so deserializing a log without it must not fail and must not invent a value. A missing timestamp is recoverable by fetching the block; a fabricated one is not, and 0 sorts before every block. It is serialized as a hex QUANTITY and skipped entirely when absent. Locally mined blocks always have it. Tests are provider-level rather than serde-only: they run eth_getLogs and eth_getTransactionReceipt through a real provider and compare each log's blockTimestamp against the timestamp reported by eth_getBlockByNumber for that log's own block, using two blocks so that stamping every log with the latest block's time would fail. Round-tripping a log with the field absent is covered too.
Follow-up to the previous commit, from review.
The two provider integration tests hand-rolled provider construction,
eth_accounts, deploy and contractAddress extraction, twice, where
test_utils::create_test_config and test_utils::deploy_contract already do
exactly that and are what the neighbouring tests use. Reuse them, hoist the
shared setup into new_provider, and fold the repeated
u64::from_str_radix(trim_start_matches("0x")) into one quantity() helper that
carries the wire-form assertions. The block fetch now deserializes into
L1RpcBlock, as timestamp.rs does, instead of indexing raw JSON. The logs are
still read as raw JSON on purpose: a typed round-trip would accept a decimal
number, or an absent field, just as happily.
logs_carry_the_timestamp_of_their_own_block only asserted that the two blocks
differed by NUMBER, so the "every log stamped with the latest block's time"
bug it exists to catch was caught only incidentally, via EDR happening to
increment timestamps. Assert the timestamps differ too.
block_replay compared whole execution receipts, and PartialEq on FullBlockLog
now includes block_timestamp. The expected side comes from a remote node,
where the field is optional, so a node that does not serve it, or a response
cached before it existed, would read as a mismatch against a locally mined
receipt that always has it. Compare with the field stripped from both sides,
and assert it separately: exactly the replay header's timestamp locally, and
the same whenever the remote actually sent one. Needed a Clone and
MapReceiptLogs bound on ChainSpecT::ExecutionReceipt.
Doc comments on the two new fields repeated the same "avoids a second
eth_getBlockByHash" rationale that the changeset already gives, against
siblings that are one terse line each; trim, and link LogOutput's to
FullBlockLog's rather than restating it. Note in code why block_timestamp is
skipped when absent while its neighbours serialize as null.
A response is cached as the deserialized Rust value re-serialized, not as the bytes the node sent, and the cache key is the method and its parameters alone. So an entry only ever holds the fields its types knew about when it was written, and adding a field to a cached type does not make existing entries grow it: they keep answering without it, indefinitely and invisibly, even against a node that now serves it. The previous commit hit exactly that. eth_getLogs and eth_getTransactionReceipt are both cached, the latter keyed on transaction hash with no block-spec gate, so any project with a warm cache would keep getting logs without blockTimestamp for everything it had already fetched, while an identical query against the same node returned it on a cold cache. Put a version segment in the cache path so entries written by an older layout are simply never read. Bump it whenever the serialization of anything reachable from a cached response changes: the cost is one cold refetch, the cost of not bumping it is a cache that silently contradicts the node. The CI cache keys move in step, otherwise every run would carry the now-dead tree forever.
🦋 Changeset detectedLatest commit: 89c41a8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
…aller 083cb6c added `Clone` and `MapReceiptLogs` bounds to `run_full_block`, because it now compares receipts with block_timestamp stripped from the logs and so has to clone and re-map them. replay_chain_specific_block calls it generically and was not updated, so `cargo check --workspace --all-targets` failed on edr_tool_cli even though every crate the change was tested against built fine. Mirror the bounds on the caller. No behaviour change; the concrete chain specs this is instantiated with (L1, OP) already satisfy both.
|
Can we take a look at this? |
There was a problem hiding this comment.
Pull request overview
Adds blockTimestamp to locally mined and remote Ethereum logs while preserving compatibility with older nodes.
Changes:
- Propagates block timestamps through receipts, filters, subscriptions, and log serialization.
- Versions RPC caches to avoid stale log responses.
- Adds serialization and provider integration tests.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
.changeset/log-block-timestamp.md |
Documents the new field and cache migration. |
.github/workflows/edr-benchmark.yml |
Updates benchmark cache key. |
.github/workflows/edr-ci.yml |
Updates CI cache keys. |
.github/workflows/test-recent-mainnet-block.yml |
Updates mainnet replay cache key. |
.github/workflows/test-recent-optimism-block.yml |
Updates Optimism replay cache key. |
crates/block/local/src/lib.rs |
Assigns timestamps to locally produced logs. |
crates/edr_eth/src/filter.rs |
Exposes timestamps in filter log output. |
crates/edr_provider/tests/integration/block_timestamp_in_logs.rs |
Tests log and receipt timestamps. |
crates/edr_provider/tests/integration/eth_request_serialization.rs |
Tests optional timestamp serialization. |
crates/edr_provider/tests/integration/mod.rs |
Registers the new integration tests. |
crates/edr_receipt/src/log/block.rs |
Adds the optional serialized timestamp field. |
crates/edr_receipt/src/log/filter.rs |
Updates filter-log serialization coverage. |
crates/edr_rpc_client/src/client.rs |
Introduces versioned on-disk caches. |
crates/test/block_replay/src/lib.rs |
Normalizes timestamps during receipt comparisons. |
crates/test/receipt/src/lib.rs |
Updates receipt serialization fixtures. |
crates/tool/cli/src/remote_block.rs |
Adds bounds required by replay normalization. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Wodann
left a comment
There was a problem hiding this comment.
Thank you for adding this! Some small suggestions that I'll apply directly
Co-authored-by: Wodann <Wodann@users.noreply.github.com>
Moves the `MapReceiptLogs` bound on `ExecutionReceipt<FilterLog>` out of the `where` clauses and into the associated type bounds it belongs with, and moves `without_log_block_timestamps`' whole `ExecutionReceiptChainSpec` bound into a `where` clause so the folded bound stays readable.
The cache layout version is a separate item for the changelog. Also drops the paragraphs the review suggestion superseded but left in place.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1644 +/- ##
===========================================
- Coverage 79.90% 46.77% -33.14%
===========================================
Files 460 435 -25
Lines 80024 68807 -11217
Branches 80024 68807 -11217
===========================================
- Hits 63942 32182 -31760
- Misses 13877 35329 +21452
+ Partials 2205 1296 -909 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Tests are failing due to a missing |
e1974f4
Add blockTimestamp to logs fixes #1643