Merge branch 'unstable' of https://github.com/sigp/lighthouse into single-attestation-full-implementation

Author: eserilev

.github/workflows/local-testnet.yml

@@ -67,6 +67,7 @@ jobs:
         working-directory: scripts/local_testnet
 
       - name: Upload logs artifact
+        if: always()
         uses: actions/upload-artifact@v4
         with:
           name: logs-local-testnet
@@ -125,6 +126,7 @@ jobs:
         working-directory: scripts/tests
 
       - name: Upload logs artifact
+        if: always()
         uses: actions/upload-artifact@v4
         with:
           name: logs-doppelganger-protection-success
@@ -160,13 +162,56 @@ jobs:
         working-directory: scripts/tests
 
       - name: Upload logs artifact
+        if: always()
         uses: actions/upload-artifact@v4
         with:
           name: logs-doppelganger-protection-failure
           path: |
             scripts/local_testnet/logs
           retention-days: 3
 
+  # Tests checkpoint syncing to a live network (current fork) and a running devnet (usually next scheduled fork)
+  checkpoint-sync-test:
+    name: checkpoint-sync-test-${{ matrix.network }}
+    runs-on: ubuntu-latest
+    needs: dockerfile-ubuntu
+    if: contains(github.event.pull_request.labels.*.name, 'syncing')
+    continue-on-error: true
+    strategy:
+      matrix:
+        network: [sepolia, devnet]
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Install Kurtosis
+        run: |
+          echo "deb [trusted=yes] https://apt.fury.io/kurtosis-tech/ /" | sudo tee /etc/apt/sources.list.d/kurtosis.list
+          sudo apt update
+          sudo apt install -y kurtosis-cli
+          kurtosis analytics disable
+
+      - name: Download Docker image artifact
+        uses: actions/download-artifact@v4
+        with:
+          name: lighthouse-docker
+          path: .
+
+      - name: Load Docker image
+        run: docker load -i lighthouse-docker.tar
+
+      - name: Run the checkpoint sync test script
+        run: |
+          ./checkpoint-sync.sh "sync-${{ matrix.network }}" "checkpoint-sync-config-${{ matrix.network }}.yaml"
+        working-directory: scripts/tests
+
+      - name: Upload logs artifact
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: logs-checkpoint-sync-${{ matrix.network }}
+          path: |
+            scripts/local_testnet/logs
+          retention-days: 3
 
   # This job succeeds ONLY IF all others succeed. It is used by the merge queue to determine whether
   # a PR is safe to merge. New jobs should be added here.
@@ -182,4 +227,6 @@ jobs:
     steps:
       - uses: actions/checkout@v4
       - name: Check that success job is dependent on all others
-        run: ./scripts/ci/check-success-job.sh ./.github/workflows/local-testnet.yml local-testnet-success
+        run: |
+          exclude_jobs='checkpoint-sync-test'
+          ./scripts/ci/check-success-job.sh ./.github/workflows/local-testnet.yml local-testnet-success "$exclude_jobs" 

Cargo.lock

@@ -218,6 +218,9 @@ run-state-transition-tests:
 # Downloads and runs the EF test vectors.
 test-ef: make-ef-tests run-ef-tests
 
+# Downloads and runs the nightly EF test vectors.
+test-ef-nightly: make-ef-tests-nightly run-ef-tests
+
 # Downloads and runs the EF test vectors with nextest.
 nextest-ef: make-ef-tests nextest-run-ef-tests
 
@@ -278,6 +281,10 @@ lint-full:
 make-ef-tests:
 	make -C $(EF_TESTS)
 
+# Download/extract the nightly EF test vectors.
+make-ef-tests-nightly:
+	CONSENSUS_SPECS_TEST_VERSION=nightly make -C $(EF_TESTS)
+
 # Verifies that crates compile with fuzzing features enabled
 arbitrary-fuzz:
 	cargo check -p state_processing --features arbitrary-fuzz,$(TEST_FEATURES)

Makefile

@@ -32,7 +32,7 @@ pub fn cli_app() -> Command {
         .about(
             "Imports one or more EIP-2335 passwords into a Lighthouse VC directory, \
             requesting passwords interactively. The directory flag provides a convenient \
-            method for importing a directory of keys generated by the eth2-deposit-cli \
+            method for importing a directory of keys generated by the ethstaker-deposit-cli \
             Python utility.",
         )
         .arg(

account_manager/src/validator/import.rs

@@ -486,24 +486,36 @@ where
 
         // Verify that blobs (if provided) match the block.
         if let Some(blobs) = &weak_subj_blobs {
-            let commitments = weak_subj_block
-                .message()
-                .body()
-                .blob_kzg_commitments()
-                .map_err(|e| format!("Blobs provided but block does not reference them: {e:?}"))?;
-            if blobs.len() != commitments.len() {
-                return Err(format!(
-                    "Wrong number of blobs, expected: {}, got: {}",
-                    commitments.len(),
-                    blobs.len()
-                ));
-            }
-            if commitments
-                .iter()
-                .zip(blobs.iter())
-                .any(|(commitment, blob)| *commitment != blob.kzg_commitment)
-            {
-                return Err("Checkpoint blob does not match block commitment".into());
+            let fulu_enabled = weak_subj_block.fork_name_unchecked().fulu_enabled();
+            if fulu_enabled && blobs.is_empty() {
+                // Blobs expected for this block, but the checkpoint server is not able to serve them.
+                // This is expected from Fulu, as only supernodes are able to serve blobs.
+                // We can consider using backfill to retrieve the data columns from the p2p network,
+                // but we can ignore this fow now until we have validator custody backfill
+                // implemented as we'll likely be able to reuse the logic.
+                // https://github.com/sigp/lighthouse/issues/6837
+            } else {
+                let commitments = weak_subj_block
+                    .message()
+                    .body()
+                    .blob_kzg_commitments()
+                    .map_err(|e| {
+                        format!("Blobs provided but block does not reference them: {e:?}")
+                    })?;
+                if blobs.len() != commitments.len() {
+                    return Err(format!(
+                        "Wrong number of blobs, expected: {}, got: {}",
+                        commitments.len(),
+                        blobs.len()
+                    ));
+                }
+                if commitments
+                    .iter()
+                    .zip(blobs.iter())
+                    .any(|(commitment, blob)| *commitment != blob.kzg_commitment)
+                {
+                    return Err("Checkpoint blob does not match block commitment".into());
+                }
             }
         }
 

beacon_node/beacon_chain/src/builder.rs

@@ -106,7 +106,7 @@ pub type SingleLookupId = u32;
 enum Action {
     Retry,
     ParentUnknown { parent_root: Hash256 },
-    Drop,
+    Drop(/* reason: */ String),
     Continue,
 }
 
@@ -194,19 +194,22 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
 
     /// Creates a parent lookup for the block with the given `block_root` and immediately triggers it.
     /// If a parent lookup exists or is triggered, a current lookup will be created.
+    ///
+    /// Returns true if the lookup is created or already exists
     #[instrument(parent = None,
         level = "info",
         fields(service = "lookup_sync"),
         name = "lookup_sync",
         skip_all
     )]
+    #[must_use = "only reference the new lookup if returns true"]
     pub fn search_child_and_parent(
         &mut self,
         block_root: Hash256,
         block_component: BlockComponent<T::EthSpec>,
         peer_id: PeerId,
         cx: &mut SyncNetworkContext<T>,
-    ) {
+    ) -> bool {
         let parent_root = block_component.parent_root();
 
         let parent_lookup_exists =
@@ -223,25 +226,29 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
                 // the lookup with zero peers to house the block components.
                 &[],
                 cx,
-            );
+            )
+        } else {
+            false
         }
     }
 
     /// Seach a block whose parent root is unknown.
+    ///
     /// Returns true if the lookup is created or already exists
     #[instrument(parent = None,
         level = "info",
         fields(service = "lookup_sync"),
         name = "lookup_sync",
         skip_all
     )]
+    #[must_use = "only reference the new lookup if returns true"]
     pub fn search_unknown_block(
         &mut self,
         block_root: Hash256,
         peer_source: &[PeerId],
         cx: &mut SyncNetworkContext<T>,
-    ) {
-        self.new_current_lookup(block_root, None, None, peer_source, cx);
+    ) -> bool {
+        self.new_current_lookup(block_root, None, None, peer_source, cx)
     }
 
     /// A block or blob triggers the search of a parent.
@@ -256,6 +263,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
         name = "lookup_sync",
         skip_all
     )]
+    #[must_use = "only reference the new lookup if returns true"]
     pub fn search_parent_of_child(
         &mut self,
         block_root_to_search: Hash256,
@@ -363,6 +371,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
         name = "lookup_sync",
         skip_all
     )]
+    #[must_use = "only reference the new lookup if returns true"]
     fn new_current_lookup(
         &mut self,
         block_root: Hash256,
@@ -656,7 +665,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
                 // This is unreachable because RPC blocks do not undergo gossip verification, and
                 // this error can *only* come from gossip verification.
                 error!(?block_root, "Single block lookup hit unreachable condition");
-                Action::Drop
+                Action::Drop("DuplicateImportStatusUnknown".to_owned())
             }
             BlockProcessingResult::Ignored => {
                 // Beacon processor signalled to ignore the block processing result.
@@ -665,14 +674,14 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
                     component = ?R::response_type(),
                     "Lookup component processing ignored, cpu might be overloaded"
                 );
-                Action::Drop
+                Action::Drop("Block processing ignored".to_owned())
             }
             BlockProcessingResult::Err(e) => {
                 match e {
                     BlockError::BeaconChainError(e) => {
                         // Internal error
                         error!(%block_root, error = ?e, "Beacon chain error processing lookup component");
-                        Action::Drop
+                        Action::Drop(format!("{e:?}"))
                     }
                     BlockError::ParentUnknown { parent_root, .. } => {
                         // Reverts the status of this request to `AwaitingProcessing` holding the
@@ -691,7 +700,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
                             error = ?e,
                             "Single block lookup failed. Execution layer is offline / unsynced / misconfigured"
                         );
-                        Action::Drop
+                        Action::Drop(format!("{e:?}"))
                     }
                     BlockError::AvailabilityCheck(e)
                         if e.category() == AvailabilityCheckErrorCategory::Internal =>
@@ -703,7 +712,7 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
                         // lookup state transition. This error invalidates both blob and block requests, and we don't know the
                         // state of both requests. Blobs may have already successfullly processed for example.
                         // We opt to drop the lookup instead.
-                        Action::Drop
+                        Action::Drop(format!("{e:?}"))
                     }
                     other => {
                         debug!(
@@ -757,19 +766,32 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
             }
             Action::ParentUnknown { parent_root } => {
                 let peers = lookup.all_peers();
+                // Mark lookup as awaiting **before** creating the parent lookup. At this point the
+                // lookup maybe inconsistent.
                 lookup.set_awaiting_parent(parent_root);
-                debug!(
-                    id = lookup.id,
-                    ?block_root,
-                    ?parent_root,
-                    "Marking lookup as awaiting parent"
-                );
-                self.search_parent_of_child(parent_root, block_root, &peers, cx);
-                Ok(LookupResult::Pending)
+                let parent_lookup_exists =
+                    self.search_parent_of_child(parent_root, block_root, &peers, cx);
+                if parent_lookup_exists {
+                    // The parent lookup exist or has been created. It's safe for `lookup` to
+                    // reference the parent as awaiting.
+                    debug!(
+                        id = lookup_id,
+                        ?block_root,
+                        ?parent_root,
+                        "Marking lookup as awaiting parent"
+                    );
+                    Ok(LookupResult::Pending)
+                } else {
+                    // The parent lookup is faulty and was not created, we must drop the `lookup` as
+                    // it's in an inconsistent state. We must drop all of its children too.
+                    Err(LookupRequestError::Failed(format!(
+                        "Parent lookup is faulty {parent_root:?}"
+                    )))
+                }
             }
-            Action::Drop => {
+            Action::Drop(reason) => {
                 // Drop with noop
-                Err(LookupRequestError::Failed)
+                Err(LookupRequestError::Failed(reason))
             }
             Action::Continue => {
                 // Drop this completed lookup only

beacon_node/network/src/sync/block_lookups/mod.rs

@@ -40,7 +40,7 @@ pub enum LookupRequestError {
     /// Inconsistent lookup request state
     BadState(String),
     /// Lookup failed for some other reason and should be dropped
-    Failed,
+    Failed(/* reason: */ String),
     /// Received MissingComponents when all components have been processed. This should never
     /// happen, and indicates some internal bug
     MissingComponentsAfterAllProcessed,

beacon_node/network/src/sync/block_lookups/single_block_lookup.rs

@@ -929,12 +929,20 @@ impl<T: BeaconChainTypes> SyncManager<T> {
     ) {
         match self.should_search_for_block(Some(slot), &peer_id) {
             Ok(_) => {
-                self.block_lookups.search_child_and_parent(
+                if self.block_lookups.search_child_and_parent(
                     block_root,
                     block_component,
                     peer_id,
                     &mut self.network,
-                );
+                ) {
+                    // Lookup created. No need to log here it's logged in `new_current_lookup`
+                } else {
+                    debug!(
+                        ?block_root,
+                        ?parent_root,
+                        "No lookup created for child and parent"
+                    );
+                }
             }
             Err(reason) => {
                 debug!(%block_root, %parent_root, reason, "Ignoring unknown parent request");
@@ -945,8 +953,15 @@ impl<T: BeaconChainTypes> SyncManager<T> {
     fn handle_unknown_block_root(&mut self, peer_id: PeerId, block_root: Hash256) {
         match self.should_search_for_block(None, &peer_id) {
             Ok(_) => {
-                self.block_lookups
-                    .search_unknown_block(block_root, &[peer_id], &mut self.network);
+                if self.block_lookups.search_unknown_block(
+                    block_root,
+                    &[peer_id],
+                    &mut self.network,
+                ) {
+                    // Lookup created. No need to log here it's logged in `new_current_lookup`
+                } else {
+                    debug!(?block_root, "No lookup created for unknown block");
+                }
             }
             Err(reason) => {
                 debug!(%block_root, reason, "Ignoring unknown block request");