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

feat: reorg handling #44

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

feat: reorg handling #44

wants to merge 5 commits into from

Conversation

parketh
Copy link
Collaborator

@parketh parketh commented Nov 27, 2024

Summary

This PR adds reorg handling to the FG.

The current FG cannot handle L2 block reorgs because the new block hash breaks the finality votes query. This causes the finalized head to become stuck.

We implement a new whitelist system where:

  • a privileged admin can whitelisted reorged / forked blocks in the CW contract
  • the FG checks blocks against the CW whitelist and can skip them to unblock the derivation pipeline

Test plan

make test

@parketh parketh requested review from bap2pecs and lesterli November 27, 2024 20:16
Comment on lines +13 to +16
// QueryIsBlockBabylonFinalizedFromBabylon returns the finality status of a block by
// querying Babylon chain directly
rpc QueryIsBlockBabylonFinalizedFromBabylon(QueryIsBlockBabylonFinalizedRequest)
returns (QueryIsBlockFinalizedResponse);
Copy link
Collaborator

Choose a reason for hiding this comment

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

this seems irrelevant to the PR

do you know why we didn't include it? IIRC we intentionally removed this to prevent DoS attack. plus, there is no call site of this grpc method anyway

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we can remove it from this PR and add it to a new one. It is needed for debugging the isBlockFinalized call from Babylon. I was trying to call it during most recent debug of devnet-7 but realized we never exposed the grpc method.

Copy link
Collaborator

Choose a reason for hiding this comment

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

we can remove it from this PR and add it to a new one

it's fine to keep it here but the question is whether we should expose it. probably fine for now but we need to be aware of the potential DoS attacks

does calling this function making babylon rpc calls every time or it's reading from the db? if it's calling the Babylon rpc, then there is a DoS attack vector

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.

3 participants