From 26cf63ee28ae8e3628b0829ca83fa549b0b41493 Mon Sep 17 00:00:00 2001 From: realbigsean Date: Thu, 18 Mar 2021 17:06:08 -0400 Subject: [PATCH 1/3] 404's on skip slots --- beacon_node/http_api/src/block_id.rs | 20 ++++++++++++++++++++ beacon_node/http_api/tests/tests.rs | 8 ++++++++ 2 files changed, 28 insertions(+) diff --git a/beacon_node/http_api/src/block_id.rs b/beacon_node/http_api/src/block_id.rs index 5e358a2d683..be5b77f7b6a 100644 --- a/beacon_node/http_api/src/block_id.rs +++ b/beacon_node/http_api/src/block_id.rs @@ -60,6 +60,26 @@ 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(|root_opt| match root_opt { + Some(block) => { + if block.slot() != *slot { + return Err(warp_utils::reject::custom_not_found( + "skip slot.".to_string(), + )); + } + 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 6ae779f91e5..75c52bce61e 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -795,6 +795,14 @@ impl ApiTester { } fn get_block(&self, block_id: BlockId) -> Option> { + match block_id { + BlockId::Slot(slot) => { + if SKIPPED_SLOTS.contains(&slot.as_u64()) { + return None; + } + } + _ => {} + } let root = self.get_block_root(block_id); root.and_then(|root| self.chain.get_block(&root).unwrap()) } From 064e3746f5441efcff3b8fec577124ac7eedb1a8 Mon Sep 17 00:00:00 2001 From: realbigsean Date: Fri, 19 Mar 2021 10:35:19 -0400 Subject: [PATCH 2/3] 404's on skip slots for block roots and headers --- beacon_node/beacon_chain/src/beacon_chain.rs | 10 ++++++++-- beacon_node/http_api/src/block_id.rs | 9 +++++---- beacon_node/http_api/tests/tests.rs | 8 -------- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 144f08550d8..58fb5de1b0a 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -520,14 +520,20 @@ impl BeaconChain { } /// 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, 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 be5b77f7b6a..793a10a5e8c 100644 --- a/beacon_node/http_api/src/block_id.rs +++ b/beacon_node/http_api/src/block_id.rs @@ -65,12 +65,13 @@ impl BlockId { chain .get_block(&root) .map_err(warp_utils::reject::beacon_chain_error) - .and_then(|root_opt| match root_opt { + .and_then(|block_opt| match block_opt { Some(block) => { if block.slot() != *slot { - return Err(warp_utils::reject::custom_not_found( - "skip slot.".to_string(), - )); + return Err(warp_utils::reject::custom_not_found(format!( + "slot {} was skipped", + slot + ))); } Ok(block) } diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 75c52bce61e..6ae779f91e5 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -795,14 +795,6 @@ impl ApiTester { } fn get_block(&self, block_id: BlockId) -> Option> { - match block_id { - BlockId::Slot(slot) => { - if SKIPPED_SLOTS.contains(&slot.as_u64()) { - return None; - } - } - _ => {} - } let root = self.get_block_root(block_id); root.and_then(|root| self.chain.get_block(&root).unwrap()) } From 2e2bdddd6193ed64a0815ff844d55150c77f3f6c Mon Sep 17 00:00:00 2001 From: realbigsean Date: Fri, 23 Apr 2021 08:49:17 -0400 Subject: [PATCH 3/3] add regression tests --- beacon_node/http_api/tests/tests.rs | 32 ++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index d367c54c98a..6027d76a864 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); }