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 BLS aggregation service to expose interface via handle #257

Open
MegaRedHand opened this issue Jan 30, 2025 · 1 comment · Fixed by #363
Open

Refactor BLS aggregation service to expose interface via handle #257

MegaRedHand opened this issue Jan 30, 2025 · 1 comment · Fixed by #363
Assignees

Comments

@MegaRedHand
Copy link
Contributor

MegaRedHand commented Jan 30, 2025

Currently, we have a lot of mutexes and shared state in the BLS aggregation service. We could simplify this by splitting the struct into service and interface. To do this, we:

  1. add a start function or similar, which consumes the service and starts it as a background task, returning a "handle" to it
  2. implement a "handle" struct, which stores all channels used to communicate with the service, but no service state
  3. move the current interface to the handle

An example interface would be:

#[derive(Debug, Clone)]
pub struct ServiceHandle {
    task_sender: UnboundedSender<TaskMetadata>,
    response_sender: UnboundedSender<TaskResponseInfo>,
}

#[derive(Debug)]
pub struct AggregateReceiver {
    aggregate_recver: UnboundedReceiver<AggregateSignature>,
}

#[derive(Debug)]
pub struct BlsAggregationService { ... }

impl BlsAggregationService {
    pub fn new() -> Self { ... }

    pub fn start(self) -> (ServiceHandle, AggregateReceiver) { ... }
}
@damiramirez
Copy link

I will take this

@MegaRedHand MegaRedHand linked a pull request Feb 19, 2025 that will close this issue
4 tasks
MegaRedHand added a commit that referenced this issue Feb 19, 2025
### What Changed?
This PR refactors the BLS Aggregator by separating its interface from
the internal service logic.
- Add new struct `ServiceHandle` to serve as the interface for
communicating with the BLS Aggregator service. This interface provides
two new methods
- `initialize_task`: Sends a message to the BLS Aggregator Service to
initialize a new task.
- `process_signature`: Sends a message to the BLS Aggregator Service to
process a signature.
- Add new struct `AggregateReceiver` to receive the aggregated responses
from the BLS Aggregator Service.
- Remove channels from `BlsAggregatorService`
- Add new methods to `BlsAggregatorService`:
- `start`: Starts the BLS Aggregator Service running the main loop in
background. This method return a tuple with `ServiceHandle` and
`AggregateReceiver`. User can use these methods to interact with the
service.
  - `run`: Runs the main loop of the BLS Aggregator Service.
- Remove `initialize_new_task` and `process_new_signature` functions
since their logic is now integrated in `run()`.

Close #257 

### Reviewer Checklist

- [ ] New features are tested and documented
- [ ] PR updates the changelog with a description of changes
- [ ] PR has one of the `changelog-X` labels (if applies)
- [ ] Code deprecates any old functionality before removing it

---------

Co-authored-by: Tomás Grüner <[email protected]>
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 a pull request may close this issue.

2 participants