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

Broadcast Slashing on equivocation #14693

Open
wants to merge 20 commits into
base: develop
Choose a base branch
from

Conversation

shyam-patel-kira
Copy link
Contributor

fixes #13088

The PR implements immediate broadcasting of slashing messages when detecting equivocating blocks, helping the network react more quickly to malicious behavior without relying on the full slasher service processing.

@shyam-patel-kira shyam-patel-kira requested a review from a team as a code owner December 6, 2024 17:34
// Check for equivocation before inserting into fork choice
slashing, slashingErr := s.detectEquivocatingBlock(cfg.ctx, cfg.roblock)
if slashingErr != nil {
return errors.Wrap(slashingErr, "could not detect equivocating block")

Choose a reason for hiding this comment

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

this should probably log instead of returning and preventing FCU? could not check for block equivocation seems more accurate

@0xalpharush
Copy link

I think this should probably be done in the gossip validation of beacon blocks for two reasons:

  1. I may not understand the spec properly but I believe signing equivocating blocks even if they're invalid should bet met with a slashing proposal
  2. There is probably no need for the slashing service to be sent a block which was already deemed equivocating and resulted in a slashing proposal being broadcast
    if features.Get().EnableSlasher {
    // Feed the block header to slasher if enabled. This action
    // is done in the background to avoid adding more load to this critical code path.
    go func() {
    blockHeader, err := interfaces.SignedBeaconBlockHeaderFromBlockInterface(blk)
    if err != nil {
    log.WithError(err).WithField("blockSlot", blk.Block().Slot()).Warn("Could not extract block header")
    return
    }
    s.cfg.slasherBlockHeadersFeed.Send(blockHeader)
    }()
    }

Fwiw I am just an interested bystander using this as a learning opportunity. Feel free to ignore or let me know if this is unhelpful

beacon-chain/blockchain/process_block.go Outdated Show resolved Hide resolved
beacon-chain/blockchain/process_block.go Outdated Show resolved Hide resolved
beacon-chain/blockchain/service.go Outdated Show resolved Hide resolved
beacon-chain/blockchain/service.go Outdated Show resolved Hide resolved
Copy link
Contributor

@potuz potuz left a comment

Choose a reason for hiding this comment

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

When moving this check to the right spot instead after the call to ReceiveBlock we don't have the root yet (since we only hash after we know we haven't seen the block), we should check if the signatures are different instead, as a different root will result in a different signature.

@shyam-patel-kira shyam-patel-kira changed the title [WIP] Broadcast Slashing on equivocation Broadcast Slashing on equivocation Jan 26, 2025
@shyam-patel-kira
Copy link
Contributor Author

Hey @potuz, Thanks for the review, addressed the changes that were requested

}

// Type assert to get the existing block
existingSignedBlock, ok := existingBlock.(interfaces.ReadOnlySignedBeaconBlock)
Copy link
Contributor

Choose a reason for hiding this comment

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

s.seenBlockCache in the Sync package never returns a block , we set it for example in setSeenBlockIndexSlot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay, current approach wouldn't work then. As suggested by you, made changes to use headBlock instead of blockCache for equivocation Detection.

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.

Broadcast slashing on equivocating blocks
3 participants