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

refactor: simplify signature timeout logic #1764

Closed

Conversation

Oppen
Copy link
Collaborator

@Oppen Oppen commented Jan 20, 2025

ProcessNewSignature already takes a context to handle cancellation and
timeout. We need just to create the context and pass it off, and it will
return an appropriate error if the timeout expires.

This saves us from creating a goroutine and handling messages just for
that.

Also improve a little bit of the error reporting.

Type of change

  • Refactor

Checklist

  • “Hotfix” to testnet, everything else to staging
  • Linked to Github Issue
  • This change depends on code or research by an external entity
    • Acknowledgements were updated to give credit
  • Unit tests added
  • This change requires new documentation.
    • Documentation has been added/updated.
  • This change is an Optimization
    • Benchmarks added/run
  • Has a known issue
  • If your PR changes the Operator compatibility (Ex: Upgrade prover versions)
    • This PR adds compatibility for operator for both versions and do not change batcher/docs/examples
    • This PR updates batcher and docs/examples to the newer version. This requires the operator are already updated to be compatible

Testing

Basic testing: start a devnet, show it's able to verify proofs.
Alightly more involved testing: register three operators and leave only one up,
so it doesn't reach quorum, the timeout should kick.

Oppen added 2 commits January 20, 2025 16:55
`ProcessNewSignature` already takes a context to handle cancellation and
timeout. We need just to create the context and pass it off, and it will
return an appropriate error if the timeout expires.

This saves us from creating a goroutine and handling messages just for
that.

Also improve a little bit of the error reporting.
Copy link
Collaborator

@JuArce JuArce left a comment

Choose a reason for hiding this comment

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

This PR is wrong. Eigen's BLSAgg service only honors the context if it can start servicing the request

@JuArce JuArce closed this Jan 22, 2025
@JuArce JuArce deleted the refactor/simplify_signature_aggregation_timeout branch January 22, 2025 23:06
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