-
Notifications
You must be signed in to change notification settings - Fork 28
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: UX improvement #258
base: dev
Are you sure you want to change the base?
feat: UX improvement #258
Conversation
.process_new_signature(task_index, task_response_digest, signature, operator_id) | ||
.await?; | ||
info!("processed signature for index {:?}", task_index); | ||
let quorum_reached = { |
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.
this assumed that 2 signatures are needed to process .
there's a better way in dev branch here
https://github.com/Layr-Labs/incredible-squaring-avs-rs/blob/b953a8127b604d91175d71e7c2b47ad4023f7e77/crates/aggregator/src/lib.rs#L320-L395
it handles all cases using total stake and quorum threshold percentage. Its hardcoded rn there with 4800 (40% of 12000(total stake))
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.
Yes. However, I was thinking of just waiting for a signature aggregate to be returned by the BLS aggregation service.
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.
^
#[derive(Debug, Default, Serialize, Deserialize, PartialEq, Eq, Clone)] | ||
pub struct OperatorRegistrationConfig { | ||
pub register_operator: bool, | ||
pub operator_to_avs_registration_sig_salt: String, |
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.
i think we can name this AvsOperatorRegistrationConfig and add another struct for EigenlayerOperatorRegistrationConfig so people are not confused.
we can make the params optional that are only available in pre slashing or post slashing. like we did in writer structs.
Ex : allocation_delay, salt etc
can make a PR if you agree with this
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.
commented on operator config
use serde::{Deserialize, Serialize}; | ||
|
||
#[derive(Debug, Default, Serialize, Deserialize, PartialEq, Eq, Clone)] | ||
pub struct ContractsConfig { |
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.
naming ? AvsContractsConfig?
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.
I have a doubt, what if some one wants to add 3 operators config using our config crate, can they do it right now? No right? since we are assuming only 1 key store pairs in EcdsaConfig and BlsConfig?
maybe one way can be |
Fixes #
What Changed?
This PR introduces two new crates to the SDK:
eigen-aggregator
andeigen-operator
, with a higher-level interface than the other crates.Reviewer Checklist
changelog-X
labels (if applies)