search

NethermindEth / near-sffl

https://nffl.nethermind.io/
MIT License
6 stars 3 forks source link

Custom BLS aggregation service for Checkpoint Tasks #263

Open emlautarom1 opened 6 days ago

emlautarom1 commented 6 days ago

Current Behavior

See #214 for details

New Behavior

We introduce a TaskBlsAggregatorService that replaces EigenLayer's SDK BlsAggregationService specifically for checkpoint tasks.

Breaking Changes

There should be no breaking changes

Observations

There is a lot of what seems to be duplicated code between TaskBlsAggregatorService and MessageBlsAggregatorService. I've spent quite some time trying to reconcile these differences but I just cannot understand the modules well enough.

The main differences are that Messages work with MessageDigest, while Tasks use TaskIndex. Also, TaskBlsAggregation[ServiceResponse] and MessageBlsAggregation[ServiceResponse] look very similar, but not quite enough.

Hyodar commented 4 days ago

Looks good. Then, as mentioned in #214, we should integrate the same partial aggregation functionality present in MessageBlsAggregationService to this as well. IMO the rationale here is checking what specific parts should be changed and how much complexity it adds. It may just be better (esp as Go doesn't have many typing features) just making two separate modules with some duplicated code than trying to come up with a generalized interface and making everything more painful to deal with if specific changes are required.

emlautarom1 commented 3 days ago

Looks good. Then, as mentioned in https://github.com/NethermindEth/near-sffl/issues/214, we should integrate the same partial aggregation functionality present in MessageBlsAggregationService to this as well.

I would like to do that on a separate PR to keep the scope of this one small.

It may just be better (esp as Go doesn't have many typing features) just making two separate modules with some duplicated code than trying to come up with a generalized interface and making everything more painful to deal with if specific changes are required.

I actually spent quite some time trying to get the MessageBlsAggregationService to work with tasks but the signatures of the interfaces and the result values are just different though maybe there is a way to unify them based on some domain knowledge that I lack. For the time being I agree on keeping the current duplicated code rather than trying to force a common interface where there is none.