Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 276
feat(syncing): detect sequencer double-signs and halt#3310
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
821a0eff4bd83defcd4d9e810cf7428a0746f3125aa9dcc90300adc2da54c68778e487dacba21e1f1e78e80455666e2f0b5a321b432428f3543efe6f4b1520e2e520b93ee385ab1587547c84a04999a4ef32a5ddFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is great! checks should always be performed, so ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alright detection always runs now, but I added a node.halt_on_double_sign flag when false it warns only instead of halting. Let me know if I should remove this too |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| package syncing | ||
| import ( | ||
| "fmt" | ||
| "sync" | ||
| "github.com/evstack/ev-node/types" | ||
| ) | ||
| // inBatchDoubleSignReporter reports two distinct, sequencer-signed headers observed at the same height | ||
| type inBatchDoubleSignReporter func(height uint64, canonical, alt *types.SignedHeader) | ||
| // doubleSignDedup collapses repeated (height, altHash) sightings so the same equivocation | ||
| // arriving from multiple batches or sources is only warned and counted once. | ||
| type doubleSignDedup struct { | ||
| mu sync.Mutex | ||
| seen map[string]struct{} | ||
| } | ||
| func newDoubleSignDedup() *doubleSignDedup { | ||
| return &doubleSignDedup{seen: make(map[string]struct{})} | ||
| } | ||
| // markSeen records (height, altHash) and returns true on first sight. | ||
| func (d *doubleSignDedup) markSeen(height uint64, altHash string) bool { | ||
| key := fmt.Sprintf("%d/%s", height, altHash) | ||
| d.mu.Lock() | ||
| defer d.mu.Unlock() | ||
| if _, ok := d.seen[key]; ok { | ||
| return false | ||
| } | ||
| d.seen[key] = struct{}{} | ||
| return true | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should handle this in the syncer.go directly, no need to duplicate it between the da retriever and p2p handler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review, I refactored out the logic into
Syncer.detectDoubleSignbut the retriever/handler calls must stay because of an in batch DA case if two conflicting headers from a malicious sequencer land at the same DA height, the retriever fetches both in one batch and drops the second one before it emitsDAHeightEvent. So the syncer would never see the alternate.