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

BEEFY: slash validators voting on non-finalized forks #1120

Closed
Tracked by #2420
acatangiu opened this issue Jul 4, 2023 · 4 comments · May be fixed by paritytech/substrate#14744
Closed
Tracked by #2420

BEEFY: slash validators voting on non-finalized forks #1120

acatangiu opened this issue Jul 4, 2023 · 4 comments · May be fixed by paritytech/substrate#14744
Assignees

Comments

@acatangiu
Copy link
Contributor

acatangiu commented Jul 4, 2023

Description

BEEFY voters should only vote on GRANDPA finalized blocks - vote only on canonic chain/fork.

Votes on blocks from different forks should (automatically) lead to stake slashing (more aggressive slashing than GRANDPA equivocations, since this scenario is intentionally dishonest).

We should add client/voter detection of such misbehavior as well as runtime logic for reporting proofs of misbehavior and subsequent slashing consequences.

Proposed solution

Runtime side

  • add new pallet::call to pallet_beefy similar to report_equivocation that takes InvalidPayloadProof
  • hook pallet_beefy_mmr to pallet_beefy::EquivocationReportSystem to be able to verify InvalidPayloadProof:
    • verify inner GrandpaProof to make sure the current chain fork is finalized at least up to offending vote height,
    • verify offending_vote.mmr_root != on-chain mmr_root (combined with above, it proves what we need)
  • report offense to staking so offending vote author gets slashed

Client-side

  • add "fisherman" capabilities to voter gossip - on detecting votes for historical forks, it builds the required proof of misbehavior and submits report.

cc @Lederstrumpf, @andresilva

@acatangiu
Copy link
Contributor Author

Actually, on the runtime-side I don't think we get any benefit to use generate_proof() and verify_proof() to prove the bad vote. It is enough to show the bad vote mmr_root is different than the on-chain one.

Using verify_leaves_proof() even requires us to have the leaves, so it just adds complexity for no benefit.

@Lederstrumpf
Copy link
Contributor

Lederstrumpf commented Jul 5, 2023

Please correct me if I'm wrong, but I don't think checking misbehaviour at the voting level is sufficient (nor even necessary). What ultimately matters for attacking the bridge is the signatures on the payload (and here equivocating against what's finalised via GRANDPA).
Going through the voting process to aggregate this payload using substrate's p2p layer is convenient, but not necessary to construct this payload: an attacking majority can also coordinate this externally without relying on Polkadot's gossiping.

My intuition so far is that we should be checking the signatures in the payload updating the smart contract's MMRRoot (https://github.com/Snowfork/snowbridge/blob/main/core/packages/contracts/src/BeefyClient.sol).
Just my current thoughts - will update after thinking about this more deeply.

@acatangiu
Copy link
Contributor Author

That’s right, the important part here is offering the pallet::call so that relayers can submit proofs of misbehavior on the source chain when detecting invalid justifications posted to target chain.

The BEEFY voter gossip fisherman is just nice-to-have.

This issue tracks the machinery we need to have in place on Polkadot side to be able to accept a “invalid-fork-vote” proof of some sort, verify it on-chain and initiate slashing if valid.

Separate work (bridge repo) needs to go in the relayers (or maybe even dedicated fishermen) that follow both sides of the bridge and look for source_chain.header_at(target_chain.best_source_header_number()).hash() != target_chain.best_source_header_hash(). If that happens, they can get the offending justification that caused it from transactions, build “invalid-fork-vote” proof and post it to Polkadot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment