check the da cache and the attester cache in responding to RPC requests #5138

realbigsean · 2024-01-29T20:55:59Z

Issue Addressed

Resolves #5093

Proposed Changes

Check the DA cache then the attester cache in responding to block by range requests
~~This does not currently check if a block exists in the DA cache's overflow to disk, I'm sort of thinking that's overkill. But can add it in.~~ I decided to use the processing_cache because this includes payloads we haven't verified. This is technically correct according to the spec

dapplion · 2024-02-08T07:32:06Z

beacon_node/beacon_chain/src/data_availability_checker/processing_cache.rs

@@ -45,7 +46,7 @@ pub struct ProcessingComponents<E: EthSpec> {
    /// Blobs required for a block can only be known if we have seen the block. So `Some` here
    /// means we've seen it, a `None` means we haven't. The `kzg_commitments` value helps us figure
    /// out whether incoming blobs actually match the block.
-    pub block_commitments: Option<KzgCommitments<E>>,
+    pub block: Option<Arc<SignedBeaconBlock<E>>>,


If this is now retaining full blocks, might be good to add a metric to track processing_cache size in case it ever leaks

added a few more metrics related metrics as well 0cf408f

ethDreamer · 2024-02-08T07:21:39Z

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -1141,10 +1141,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
        >,


should probably rename this function get_blocks_checking_caches or something along these lines.. perhaps we should come up with a unified way of referring to these two caches.. idk if we'd just call them "early caches" or something..

…eck-da-cache-in-rpc-response

realbigsean · 2024-02-15T02:56:52Z

Ok feedback so far has been addressed

This reverts commit d097e9b.

jxs · 2024-02-15T21:33:49Z

beacon_node/beacon_chain/src/beacon_block_streamer.rs

@@ -19,7 +19,7 @@ use types::{
 };

 #[derive(PartialEq)]
-pub enum CheckEarlyAttesterCache {
+pub enum CheckCaches {


nit: why not use bool?

We usually try to use enums over bools when we need to pass something into a function. If you end up with two bools as params it can be ambiguous

…ts (#5138) Squashed commit of the following: commit a6144f6 Author: realbigsean <seananderson33@GMAIL.com> Date: Thu Feb 15 15:48:07 2024 -0500 Revert "make rustup update run on the runners" This reverts commit d097e9b. commit d097e9b Author: realbigsean <seananderson33@GMAIL.com> Date: Thu Feb 15 15:16:08 2024 -0500 make rustup update run on the runners commit 1be438e Merge: 597e05f 256d904 Author: realbigsean <seananderson33@GMAIL.com> Date: Wed Feb 14 21:55:45 2024 -0500 Merge branch 'unstable' of https://github.com/sigp/lighthouse into check-da-cache-in-rpc-response commit 597e05f Author: realbigsean <seananderson33@GMAIL.com> Date: Wed Feb 14 21:54:44 2024 -0500 rename early attester cache method commit 0cf408f Author: realbigsean <seananderson33@GMAIL.com> Date: Wed Feb 14 21:53:47 2024 -0500 add da cache metrics commit 2f6cf41 Author: realbigsean <seananderson33@GMAIL.com> Date: Mon Jan 29 18:31:32 2024 -0500 update comment commit 0512420 Author: realbigsean <seananderson33@GMAIL.com> Date: Mon Jan 29 18:25:15 2024 -0500 use the processing cache instead commit 66b911f Author: realbigsean <seananderson33@GMAIL.com> Date: Mon Jan 29 15:52:42 2024 -0500 check the da cache and the attester cache in responding to RPC requests

jimmygchen

LGTM!
So we're now serving blocks as soon as it passes gossip validation and cached in da cache, which I think is spec compliant 👍

paulhauner · 2024-02-19T01:46:06Z

@Mergifyio queue

mergify · 2024-02-19T01:46:11Z

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at f214729

check the da cache and the attester cache in responding to RPC requests

66b911f

realbigsean added ready-for-review The code is ready for review deneb labels Jan 29, 2024

realbigsean requested a review from ethDreamer January 29, 2024 20:56

realbigsean added 2 commits January 29, 2024 18:25

use the processing cache instead

0512420

update comment

2f6cf41

realbigsean added the v5.0.0 Q1 2024 label Jan 31, 2024

dapplion reviewed Feb 8, 2024

View reviewed changes

ethDreamer reviewed Feb 8, 2024

View reviewed changes

realbigsean added 3 commits February 14, 2024 21:53

add da cache metrics

0cf408f

rename early attester cache method

597e05f

Merge branch 'unstable' of https://github.com/sigp/lighthouse into ch…

1be438e

…eck-da-cache-in-rpc-response

realbigsean added 2 commits February 15, 2024 15:16

make rustup update run on the runners

d097e9b

Revert "make rustup update run on the runners"

a6144f6

This reverts commit d097e9b.

jxs reviewed Feb 15, 2024

View reviewed changes

paulhauner mentioned this pull request Feb 18, 2024

Release v5.0.0 #5254

Merged

jimmygchen approved these changes Feb 18, 2024

View reviewed changes

jimmygchen added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Feb 18, 2024

mergify bot added a commit that referenced this pull request Feb 19, 2024

Merge of #5138

9424e1f

This was referenced Feb 19, 2024

merge queue: embarking unstable (a264afd) and [#5138 + #5242 + #5252] together #5257

Closed

Add Deneb fork epoch for Gnosis #5242

Merged

Download checkpoint blobs during checkpoint sync #5252

Merged

mergify bot merged commit f214729 into sigp:unstable Feb 19, 2024
29 checks passed

realbigsean mentioned this pull request Mar 5, 2024

Check availability cache when serving block by root requests #5093

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

check the da cache and the attester cache in responding to RPC requests #5138

check the da cache and the attester cache in responding to RPC requests #5138

realbigsean commented Jan 29, 2024 •

edited

Loading

dapplion Feb 8, 2024

realbigsean Feb 15, 2024

realbigsean Feb 15, 2024

ethDreamer Feb 8, 2024

realbigsean Feb 15, 2024

realbigsean commented Feb 15, 2024

jxs Feb 15, 2024

realbigsean Feb 15, 2024

jimmygchen left a comment

paulhauner commented Feb 19, 2024

mergify bot commented Feb 19, 2024 •

edited

Loading

		@@ -1141,10 +1141,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
		>,

check the da cache and the attester cache in responding to RPC requests #5138

check the da cache and the attester cache in responding to RPC requests #5138

Conversation

realbigsean commented Jan 29, 2024 • edited Loading

Issue Addressed

Proposed Changes

dapplion Feb 8, 2024

Choose a reason for hiding this comment

realbigsean Feb 15, 2024

Choose a reason for hiding this comment

realbigsean Feb 15, 2024

Choose a reason for hiding this comment

ethDreamer Feb 8, 2024

Choose a reason for hiding this comment

realbigsean Feb 15, 2024

Choose a reason for hiding this comment

realbigsean commented Feb 15, 2024

jxs Feb 15, 2024

Choose a reason for hiding this comment

realbigsean Feb 15, 2024

Choose a reason for hiding this comment

jimmygchen left a comment

Choose a reason for hiding this comment

paulhauner commented Feb 19, 2024

mergify bot commented Feb 19, 2024 • edited Loading

✅ The pull request has been merged automatically

realbigsean commented Jan 29, 2024 •

edited

Loading

mergify bot commented Feb 19, 2024 •

edited

Loading