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

Fix slasher conditions #4976

Merged

Conversation

pawanjay176
Copy link
Member

@pawanjay176 pawanjay176 commented Dec 4, 2023

Issue Addressed

Addresses #4900 (comment)

Thanks @jimmygchen

Proposed Changes

Previously, we were adding a BlobSidecar's header to the slasher under the following conditions:

  1. Gossip verification fails and the signature is valid
  2. Just before importing a fully available block

These conditions do not cover all cases since a valid gossip blob with an equivocating header that gets into the observed_blob_cache will not fail gossip verification and will never be fully available since the proposer need not propagate the full signed equivocating block. Such a BlobSidecar with a slashable block header will just go sit in the DA cache and eventually get pruned.

Hence, this PR adds the signed header before adding the blobs to the DA cache.
I have also changed the BlockQueue data structure to be a HashSet instead of a Vec since we are adding potentially many duplicate headers per slot. Looking at the slasher code, I don't think changing the data structure is an issue, please correct me if that's wrong or unnecessary @michaelsproul


#[derive(Debug, Default)]
pub struct BlockQueue {
blocks: Mutex<Vec<SignedBeaconBlockHeader>>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call

@realbigsean
Copy link
Member

Nice! I think this just needs a cargo fmt

@realbigsean realbigsean merged commit 1fa71ec into sigp:sidecar-inclusion-proof Dec 4, 2023
24 checks passed
@michaelsproul
Copy link
Member

HashSet is fine for blocks I think, we just process them in a loop here:

for block in blocks {
if let ProposerSlashingStatus::DoubleVote(slashing) =
self.db.check_or_insert_block_proposal(txn, block)?
{
slashings.push(*slashing);
}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants