From 2c2c4437186793b5f052a9dbac1718b1d11a043b Mon Sep 17 00:00:00 2001
From: realbigsean <seananderson33@GMAIL.com>
Date: Sun, 25 Apr 2021 03:59:59 +0000
Subject: [PATCH] 404's on API requests for slots that have been skipped or
 orphaned (#2272)

## Issue Addressed

Resolves #2186

## Proposed Changes

404 for any block-related information on a slot that was skipped or orphaned

Affected endpoints:
- `/eth/v1/beacon/blocks/{block_id}`
- `/eth/v1/beacon/blocks/{block_id}/root`
- `/eth/v1/beacon/blocks/{block_id}/attestations`
- `/eth/v1/beacon/headers/{block_id}`

## Additional Info



Co-authored-by: realbigsean <seananderson33@gmail.com>
---
 beacon_node/beacon_chain/src/beacon_chain.rs | 10 ++++--
 beacon_node/http_api/src/block_id.rs         | 21 +++++++++++++
 beacon_node/http_api/tests/tests.rs          | 32 +++++++++++++++++++-
 3 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs
index e0e45e214..94079f028 100644
--- a/beacon_node/beacon_chain/src/beacon_chain.rs
+++ b/beacon_node/beacon_chain/src/beacon_chain.rs
@@ -520,14 +520,20 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
     }
 
     /// Returns the block root at the given slot, if any. Only returns roots in the canonical chain.
+    /// Returns `Ok(None)` if the given `Slot` was skipped.
     ///
     /// ## Errors
     ///
     /// May return a database error.
     pub fn block_root_at_slot(&self, slot: Slot) -> Result<Option<Hash256>, Error> {
         process_results(self.rev_iter_block_roots()?, |mut iter| {
-            iter.find(|(_, this_slot)| *this_slot == slot)
-                .map(|(root, _)| root)
+            let root_opt = iter
+                .find(|(_, this_slot)| *this_slot == slot)
+                .map(|(root, _)| root);
+            if let (Some(root), Some((prev_root, _))) = (root_opt, iter.next()) {
+                return (prev_root != root).then(|| root);
+            }
+            root_opt
         })
     }
 
diff --git a/beacon_node/http_api/src/block_id.rs b/beacon_node/http_api/src/block_id.rs
index 5e358a2d6..793a10a5e 100644
--- a/beacon_node/http_api/src/block_id.rs
+++ b/beacon_node/http_api/src/block_id.rs
@@ -60,6 +60,27 @@ impl BlockId {
             CoreBlockId::Head => chain
                 .head_beacon_block()
                 .map_err(warp_utils::reject::beacon_chain_error),
+            CoreBlockId::Slot(slot) => {
+                let root = self.root(chain)?;
+                chain
+                    .get_block(&root)
+                    .map_err(warp_utils::reject::beacon_chain_error)
+                    .and_then(|block_opt| match block_opt {
+                        Some(block) => {
+                            if block.slot() != *slot {
+                                return Err(warp_utils::reject::custom_not_found(format!(
+                                    "slot {} was skipped",
+                                    slot
+                                )));
+                            }
+                            Ok(block)
+                        }
+                        None => Err(warp_utils::reject::custom_not_found(format!(
+                            "beacon block with root {}",
+                            root
+                        ))),
+                    })
+            }
             _ => {
                 let root = self.root(chain)?;
                 chain
diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs
index d367c54c9..6027d76a8 100644
--- a/beacon_node/http_api/tests/tests.rs
+++ b/beacon_node/http_api/tests/tests.rs
@@ -880,6 +880,14 @@ impl ApiTester {
 
             let block_root_opt = self.get_block_root(block_id);
 
+            if let BlockId::Slot(slot) = block_id {
+                if block_root_opt.is_none() {
+                    assert!(SKIPPED_SLOTS.contains(&slot.as_u64()));
+                } else {
+                    assert!(!SKIPPED_SLOTS.contains(&slot.as_u64()));
+                }
+            }
+
             let block_opt = block_root_opt.and_then(|root| self.chain.get_block(&root).unwrap());
 
             if block_opt.is_none() && result.is_none() {
@@ -924,7 +932,13 @@ impl ApiTester {
                 .map(|res| res.data.root);
 
             let expected = self.get_block_root(block_id);
-
+            if let BlockId::Slot(slot) = block_id {
+                if expected.is_none() {
+                    assert!(SKIPPED_SLOTS.contains(&slot.as_u64()));
+                } else {
+                    assert!(!SKIPPED_SLOTS.contains(&slot.as_u64()));
+                }
+            }
             assert_eq!(result, expected, "{:?}", block_id);
         }
 
@@ -962,6 +976,14 @@ impl ApiTester {
         for block_id in self.interesting_block_ids() {
             let expected = self.get_block(block_id);
 
+            if let BlockId::Slot(slot) = block_id {
+                if expected.is_none() {
+                    assert!(SKIPPED_SLOTS.contains(&slot.as_u64()));
+                } else {
+                    assert!(!SKIPPED_SLOTS.contains(&slot.as_u64()));
+                }
+            }
+
             let json_result = self
                 .client
                 .get_beacon_blocks(block_id)
@@ -990,6 +1012,14 @@ impl ApiTester {
                 .get_block(block_id)
                 .map(|block| block.message.body.attestations.into());
 
+            if let BlockId::Slot(slot) = block_id {
+                if expected.is_none() {
+                    assert!(SKIPPED_SLOTS.contains(&slot.as_u64()));
+                } else {
+                    assert!(!SKIPPED_SLOTS.contains(&slot.as_u64()));
+                }
+            }
+
             assert_eq!(result, expected, "{:?}", block_id);
         }