Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - 404's on API requests for slots that have been skipped or orphaned #2272

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions beacon_node/beacon_chain/src/beacon_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
}

Expand Down
21 changes: 21 additions & 0 deletions beacon_node/http_api/src/block_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 31 additions & 1 deletion beacon_node/http_api/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
}

Expand Down