Bug: bls aggregation service doesn't support >1 quorum

[samlaf]

Describe the bug

bls aggregation needs to add signatures and g2 pubkeys for every quorum that an operator is part of, but it currently only adds them once: https://github.com/Layr-Labs/eigensdk-go/blob/4a204d0e0c9118babf1d74353f72c2b97009d7db/services/bls_aggregation/blsagg.go#L295C31-L295C46

We have unit tests for >1 quorums (eg here) but those were wrongly implemented, so will need to be fixed. An integration test with the contract would have picked this up.

To Reproduce

Expected behavior

Screenshots

OS details

Additional context

[renlulu]

Do you mean the operator might use different bls key for different quorum? so we need to track it separately

[samlaf]

No same key. Just that each quorum needs to be verified, and the way we do this is by combining each quorum's sig in a single pairing check. Basically this means the aggregator needs to add a signature to the global aggregate signature for every quorum an operator is part of.

[renlulu]

Will that need change following struct? Especially last 8 fields

// BlsAggregationServiceResponse is the response from the bls aggregation service
type BlsAggregationServiceResponse struct {
    Err                error                    // if Err is not nil, the other fields are not valid
    TaskIndex          types.TaskIndex          // unique identifier of the task
    TaskResponse       types.TaskResponse       // the task response that was signed
    TaskResponseDigest types.TaskResponseDigest // digest of the task response that was signed
    // The below 8 fields are the data needed to build the IBLSSignatureChecker.NonSignerStakesAndSignature struct
    // users of this service will need to build the struct themselves by converting the bls points
    // into the BN254.G1/G2Point structs that the IBLSSignatureChecker expects
    // given that those are different for each AVS service manager that individually inherits BLSSignatureChecker
    NonSignersPubkeysG1          []*bls.G1Point
    QuorumApksG1                 []*bls.G1Point
    SignersApkG2                 *bls.G2Point
    SignersAggSigG1              *bls.Signature
    NonSignerQuorumBitmapIndices []uint32
    QuorumApkIndices             []uint32
    TotalStakeIndices            []uint32
    NonSignerStakeIndices        [][]uint32
}

[renlulu]

Not sure any change needed from contract side too. https://github.com/Layr-Labs/eigenlayer-middleware/blob/dev/src/BLSSignatureChecker.sol#L92

[renlulu]

Not sure any change needed from contract side too. https://github.com/Layr-Labs/eigenlayer-middleware/blob/dev/src/BLSSignatureChecker.sol#L92

I see, seems we don't have to change contract, but only disperse side need some adjustment, if we change sdk here

[samlaf]

Correct. I will make the changes. Shouldn’t change any of the structs, just their contents. It’s a pretty minimal change I feel.

[samlaf]

Created integration test framework in #277 . Left TODO for this PR:

1. implement 2 quorum integration test: https://github.com/Layr-Labs/eigensdk-go/pull/277/files#diff-071edecb38c0d57bcdf4ed46768441f2ebff6138344bd39ec8999b5c06511eb2R892 (which will break b/c of this issue's bug)
2. fix this bug

[samlaf]

Reopening as this was automatically closed by a PR which didn't actually implement this (only implemented integration test tooling to be able to test this feature once it's implemented). Sorry this is taking longer than planned.. getting sidetracked left and right.