Skip to content

Fix signer stuck if signature already aggregated #1977

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Oct 3, 2024
Merged

Fix signer stuck if signature already aggregated #1977

merged 3 commits into from
Oct 3, 2024

Conversation

@Alenar
Copy link
Collaborator

@Alenar Alenar commented Oct 3, 2024 •
edited
Loading

Content

This PR fix #1976 by making changing to a "ok" the result of an error received when sending a single signature to an aggregator if that aggregator already certified the signature message.

Pre-submit checklist

  • Branch
    • Tests are provided (if possible)
    • Crates versions are updated (if relevant)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
  • PR
    • No clippy warnings in the CI
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested

Issue(s)

Closes #1976

@Alenar
fix: signer register-signature won't fail if already certified
42b7b31 
Else this prevent the signer state machine to mark the beacon as signed,
making it stay at that beacon until an epoch change.
jpraynaud
jpraynaud approved these changes Oct 3, 2024
View reviewed changes
Copy link
Member

@jpraynaud jpraynaud left a comment

Choose a reason for hiding this comment

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

LGTM

@Alenar Alenar force-pushed the djo/1976/fix_signer_stuck_if_signature_already_aggreagated branch from 5cda252 to 9959149 Compare October 3, 2024 15:52
@github-actions
Copy link

github-actions bot commented Oct 3, 2024

Test Results

    4 files  ±0     54 suites  ±0   9m 36s ⏱️ +13s
1 330 tests +1  1 330 ✅ +1  0 💤 ±0  0 ❌ ±0 
1 541 runs  +1  1 541 ✅ +1  0 💤 ±0  0 ❌ ±0 

Results for commit 9959149. ± Comparison against base commit 98febb5.

@Alenar Alenar temporarily deployed to testing-preview October 3, 2024 16:00 — with GitHub Actions Inactive
@Alenar Alenar temporarily deployed to testing-sanchonet October 3, 2024 16:00 — with GitHub Actions Inactive
@Alenar Alenar merged commit d8e4eff into main Oct 3, 2024
43 of 44 checks passed
@Alenar Alenar deleted the djo/1976/fix_signer_stuck_if_signature_already_aggreagated branch October 3, 2024 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jpraynaud jpraynaud jpraynaud approved these changes

+2 more reviewers

@sfauvel sfauvel sfauvel approved these changes

@dlachaume dlachaume dlachaume approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Handle signatures sent too late in the signer

5 participants

@Alenar @sfauvel @jpraynaud @dlachaume