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 equivocating blocks #13088

Open
potuz opened this issue Oct 22, 2023 · 8 comments · May be fixed by #14693
Open

Broadcast slashing on equivocating blocks #13088

potuz opened this issue Oct 22, 2023 · 8 comments · May be fixed by #14693
Assignees
Labels
Enhancement New feature or request Good First Issue Good for newcomers

Comments

@potuz
Copy link
Contributor

potuz commented Oct 22, 2023

Prysm has a slasher feature, which is demanding in resources. However, equivocating blocks are already handled in the usual path for every beacon. We should broadcast immediately a slashing object in the event of receiving an equivocating block.

@potuz potuz added Enhancement New feature or request Good First Issue Good for newcomers labels Oct 22, 2023
@v4lproik
Copy link
Contributor

v4lproik commented Dec 8, 2023

I would love to take this up if still needed :)

@RohanShrothrium
Copy link

Hi guys!
Can I take this up?

@v4lproik
Copy link
Contributor

v4lproik commented Jan 11, 2024

Hi @potuz!

I have a question related to this issue.

I understand that the slashing logic for an equivocating block is done in the slasher service which runs as a routine listening to the BeaconBlockHeadersFeed.

Breaking down what the slasher does when receiving a new block, we have:

  • detectProposerSlashings (read and write ops)
  • processProposerSlashings

At the moment, my PR simply implements the detectProposerSlashings read op bit in the beacon chain. I am not sure if you are expecting me to implement the write op too - which is merely saving the proposerSlashings to the database.

Also, I am not sure that implementing the broadcast in the beacon chain is what you are expecting as I would bypass the processProposerSlashings which first verifies the block headers signatures, log the slashing proposal and then insert it in the becon node's operations pool.

Could you give some pointers. Thanks!

@v4lproik
Copy link
Contributor

v4lproik commented Jan 11, 2024

Maybe two different possible roads would be:

  1. The BN also write the detectProposerSlashings write bit, which would result in sending a new type of event to the slasher in order to avoid stressing the DB and for further processing (verify the header signatures)
  2. The BN is the source of truth when it comes to flagging equivocating blocks and fills a cache (something along the lines of [epoch-1, epoch] as a range).

Let me know what you think.

@potuz
Copy link
Contributor Author

potuz commented Mar 25, 2024

Apologies for being late here. What I want in the issue is to act completely independent of the slasher: when the BN receives a block on gossip and it already has imported a different one for the same slot and proposer, it should immediately construct the slashing message and broadcast

@potuz
Copy link
Contributor Author

potuz commented Mar 25, 2024

And yes, I would expect you to bypass the db writing indeed, I'll review this tomorrow morning (without a computer today)

@v4lproik
Copy link
Contributor

Oh, I see, you want it completely independent, I used the slasherDB which is wrong, I am going to remove that. My bad!

I guess the best course of action here would be to use a cache as proposed here:

The BN is the source of truth when it comes to flagging equivocating blocks and fills a cache (something along the lines of [epoch-1, epoch] as a range).

I am thinking about retaining the block_roots >= finalized_slot and having an enum that would flag whether the block_root already exists or different and broadcast a potential slashing message looking into the cache after the validation of the block signature. Does that sound good to you?

@shyam-patel-kira shyam-patel-kira linked a pull request Dec 6, 2024 that will close this issue
@shyam-patel-kira
Copy link
Contributor

shyam-patel-kira commented Dec 6, 2024

I've made a PR for resolving this issue, it's work in progress but wanted to check if the approach looks correct. If yes, I can add unit tests later cc: @potuz

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request Good First Issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants