From de0aca54bfaf3d1216942842cdc4df9ff2639452 Mon Sep 17 00:00:00 2001 From: afkbyte Date: Mon, 23 Sep 2024 14:10:20 -0700 Subject: [PATCH 1/8] fix + testcase --- contracts/lib/eigenlayer-middleware | 2 +- services/bls_aggregation/blsagg.go | 23 ++++-- services/bls_aggregation/blsagg_test.go | 95 +++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 8 deletions(-) diff --git a/contracts/lib/eigenlayer-middleware b/contracts/lib/eigenlayer-middleware index 32148dee..74efe8e4 160000 --- a/contracts/lib/eigenlayer-middleware +++ b/contracts/lib/eigenlayer-middleware @@ -1 +1 @@ -Subproject commit 32148dee470cf8addfff85cbb34d73693bf1c04d +Subproject commit 74efe8e4ad1b562e0de26d01c7b310ad7e6e2362 diff --git a/services/bls_aggregation/blsagg.go b/services/bls_aggregation/blsagg.go index d3c630d6..75eb8794 100644 --- a/services/bls_aggregation/blsagg.go +++ b/services/bls_aggregation/blsagg.go @@ -336,12 +336,6 @@ func (a *BlsAggregatorService) singleTaskAggregatorGoroutineFunc( signedTaskResponseDigest, ) - err := a.verifySignature(taskIndex, signedTaskResponseDigest, operatorsAvsStateDict) - signedTaskResponseDigest.SignatureVerificationErrorC <- err - if err != nil { - continue - } - // compute the taskResponseDigest using the hash function taskResponseDigest, err := a.hashFunction(signedTaskResponseDigest.TaskResponse) if err != nil { @@ -350,8 +344,23 @@ func (a *BlsAggregatorService) singleTaskAggregatorGoroutineFunc( // happens.. continue } - // after verifying signature we aggregate its sig and pubkey, and update the signed stake amount + + // check if the operator has already signed for this digest digestAggregatedOperators, ok := aggregatedOperatorsDict[taskResponseDigest] + if ok { + if digestAggregatedOperators.signersOperatorIdsSet[signedTaskResponseDigest.OperatorId] { + signedTaskResponseDigest.SignatureVerificationErrorC <- fmt.Errorf("duplicate signature from operator %x for task %d", signedTaskResponseDigest.OperatorId, taskIndex) + continue + } + } + + err = a.verifySignature(taskIndex, signedTaskResponseDigest, operatorsAvsStateDict) + signedTaskResponseDigest.SignatureVerificationErrorC <- err + if err != nil { + continue + } + + // after verifying signature we aggregate its sig and pubkey, and update the signed stake amount if !ok { // first operator to sign on this digest digestAggregatedOperators = aggregatedOperators{ diff --git a/services/bls_aggregation/blsagg_test.go b/services/bls_aggregation/blsagg_test.go index cb736ced..fd10338c 100644 --- a/services/bls_aggregation/blsagg_test.go +++ b/services/bls_aggregation/blsagg_test.go @@ -413,6 +413,101 @@ func TestBlsAgg(t *testing.T) { } }) + t.Run("1 quorum 2 operators operator 1 double sign", func(t *testing.T) { + testOperator1 := types.TestOperator{ + OperatorId: types.OperatorId{1}, + StakePerQuorum: map[types.QuorumNum]types.StakeAmount{0: big.NewInt(100)}, + BlsKeypair: newBlsKeyPairPanics("0x1"), + } + testOperator2 := types.TestOperator{ + OperatorId: types.OperatorId{2}, + StakePerQuorum: map[types.QuorumNum]types.StakeAmount{0: big.NewInt(100)}, + BlsKeypair: newBlsKeyPairPanics("0x2"), + } + + blockNum := uint32(1) + taskIndex := types.TaskIndex(0) + quorumNumbers := types.QuorumNums{0} + quorumThresholdPercentages := []types.QuorumThresholdPercentage{100} + taskResponse := mockTaskResponse{123} + + fakeAvsRegistryService := avsregistry.NewFakeAvsRegistryService( + blockNum, + []types.TestOperator{testOperator1, testOperator2}, + ) + logger := testutils.GetTestLogger() + blsAggServ := NewBlsAggregatorService(fakeAvsRegistryService, hashFunction, logger) + + logger.Info("Initializing new task", "taskIndex", taskIndex) + err := blsAggServ.InitializeNewTask( + taskIndex, + blockNum, + quorumNumbers, + quorumThresholdPercentages, + 10*time.Second, // Longer expiry time for testing + ) + require.NoError(t, err) + + taskResponseDigest, err := hashFunction(taskResponse) + require.NoError(t, err) + + logger.Info("Processing first signature", "operatorId", testOperator1.OperatorId) + blsSigOp1 := testOperator1.BlsKeypair.SignMessage(taskResponseDigest) + err = blsAggServ.ProcessNewSignature( + context.Background(), + taskIndex, + taskResponse, + blsSigOp1, + testOperator1.OperatorId, + ) + require.NoError(t, err) + + logger.Info("Processing second signature (Operator 1 double sign)", "operatorId", testOperator1.OperatorId) + blsSigOp1Dup := testOperator1.BlsKeypair.SignMessage(taskResponseDigest) + err = blsAggServ.ProcessNewSignature( + context.Background(), + taskIndex, + taskResponse, + blsSigOp1Dup, + testOperator1.OperatorId, + ) + + if err != nil { + logger.Info("Received error from second signature", "error", err) + require.Contains(t, err.Error(), "duplicate signature") + } else { + t.Fatal("Expected an error for duplicate signature, but got nil") + } + + logger.Info("Processing second signature", "operatorId", testOperator2.OperatorId) + blsSigOp2 := testOperator2.BlsKeypair.SignMessage(taskResponseDigest) + err = blsAggServ.ProcessNewSignature( + context.Background(), + taskIndex, + taskResponse, + blsSigOp2, + testOperator2.OperatorId, + ) + require.NoError(t, err) + + wantAggregationServiceResponse := BlsAggregationServiceResponse{ + Err: nil, + TaskIndex: taskIndex, + TaskResponse: taskResponse, + TaskResponseDigest: taskResponseDigest, + NonSignersPubkeysG1: []*bls.G1Point{}, + QuorumApksG1: []*bls.G1Point{testOperator1.BlsKeypair.GetPubKeyG1(). + Add(testOperator2.BlsKeypair.GetPubKeyG1()), + }, + SignersApkG2: testOperator1.BlsKeypair.GetPubKeyG2(). + Add(testOperator2.BlsKeypair.GetPubKeyG2()), + SignersAggSigG1: testOperator1.BlsKeypair.SignMessage(taskResponseDigest). + Add(testOperator2.BlsKeypair.SignMessage(taskResponseDigest)), + } + gotAggregationServiceResponse := <-blsAggServ.aggregatedResponsesC + require.EqualValues(t, wantAggregationServiceResponse, gotAggregationServiceResponse) + + }) t.Run("1 quorum 1 operator 0 signatures - task expired", func(t *testing.T) { testOperator1 := types.TestOperator{ OperatorId: types.OperatorId{1}, From 64142d6aaaa641cebf781bb84df0fac8aca11121 Mon Sep 17 00:00:00 2001 From: afkbyte Date: Mon, 23 Sep 2024 14:16:48 -0700 Subject: [PATCH 2/8] add warn log --- services/bls_aggregation/blsagg.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/services/bls_aggregation/blsagg.go b/services/bls_aggregation/blsagg.go index 75eb8794..2b7b4afc 100644 --- a/services/bls_aggregation/blsagg.go +++ b/services/bls_aggregation/blsagg.go @@ -349,6 +349,11 @@ func (a *BlsAggregatorService) singleTaskAggregatorGoroutineFunc( digestAggregatedOperators, ok := aggregatedOperatorsDict[taskResponseDigest] if ok { if digestAggregatedOperators.signersOperatorIdsSet[signedTaskResponseDigest.OperatorId] { + a.logger.Warn( + "Duplicate signature received", + "operatorId", fmt.Sprintf("%x", signedTaskResponseDigest.OperatorId), + "taskIndex", taskIndex, + ) signedTaskResponseDigest.SignatureVerificationErrorC <- fmt.Errorf("duplicate signature from operator %x for task %d", signedTaskResponseDigest.OperatorId, taskIndex) continue } From 7d9e253f93088f98e79bed3cc633f5db49db5ad0 Mon Sep 17 00:00:00 2001 From: afkbyte Date: Mon, 23 Sep 2024 14:22:50 -0700 Subject: [PATCH 3/8] undo eigenlayer-middleware submodule change --- contracts/lib/eigenlayer-middleware | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/lib/eigenlayer-middleware b/contracts/lib/eigenlayer-middleware index 74efe8e4..32148dee 160000 --- a/contracts/lib/eigenlayer-middleware +++ b/contracts/lib/eigenlayer-middleware @@ -1 +1 @@ -Subproject commit 74efe8e4ad1b562e0de26d01c7b310ad7e6e2362 +Subproject commit 32148dee470cf8addfff85cbb34d73693bf1c04d From 51ce3e6c18e0eca3500122319aea373b803477fa Mon Sep 17 00:00:00 2001 From: afkbyte Date: Tue, 24 Sep 2024 08:16:30 -0700 Subject: [PATCH 4/8] change warn log to info --- services/bls_aggregation/blsagg.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/bls_aggregation/blsagg.go b/services/bls_aggregation/blsagg.go index 2b7b4afc..995419d3 100644 --- a/services/bls_aggregation/blsagg.go +++ b/services/bls_aggregation/blsagg.go @@ -349,7 +349,7 @@ func (a *BlsAggregatorService) singleTaskAggregatorGoroutineFunc( digestAggregatedOperators, ok := aggregatedOperatorsDict[taskResponseDigest] if ok { if digestAggregatedOperators.signersOperatorIdsSet[signedTaskResponseDigest.OperatorId] { - a.logger.Warn( + a.logger.Info( "Duplicate signature received", "operatorId", fmt.Sprintf("%x", signedTaskResponseDigest.OperatorId), "taskIndex", taskIndex, From 21635ca297d1b4b3e7e38f6926d045ce172e26f9 Mon Sep 17 00:00:00 2001 From: afk <84330705+afkbyte@users.noreply.github.com> Date: Tue, 24 Sep 2024 11:18:24 -0400 Subject: [PATCH 5/8] add sam's comment for clarity Co-authored-by: Samuel Laferriere --- services/bls_aggregation/blsagg.go | 1 + 1 file changed, 1 insertion(+) diff --git a/services/bls_aggregation/blsagg.go b/services/bls_aggregation/blsagg.go index 995419d3..3eee2c8d 100644 --- a/services/bls_aggregation/blsagg.go +++ b/services/bls_aggregation/blsagg.go @@ -360,6 +360,7 @@ func (a *BlsAggregatorService) singleTaskAggregatorGoroutineFunc( } err = a.verifySignature(taskIndex, signedTaskResponseDigest, operatorsAvsStateDict) + // return the err (or nil) to the operator, and then proceed to do aggregation logic asynchronously (when no error) signedTaskResponseDigest.SignatureVerificationErrorC <- err if err != nil { continue From 6668c09fd7349bd3daf6881e8400ebebb233fc89 Mon Sep 17 00:00:00 2001 From: afkbyte Date: Tue, 24 Sep 2024 09:05:09 -0700 Subject: [PATCH 6/8] run make fmt --- services/bls_aggregation/blsagg.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/bls_aggregation/blsagg.go b/services/bls_aggregation/blsagg.go index 3eee2c8d..4bf9f18a 100644 --- a/services/bls_aggregation/blsagg.go +++ b/services/bls_aggregation/blsagg.go @@ -360,7 +360,8 @@ func (a *BlsAggregatorService) singleTaskAggregatorGoroutineFunc( } err = a.verifySignature(taskIndex, signedTaskResponseDigest, operatorsAvsStateDict) - // return the err (or nil) to the operator, and then proceed to do aggregation logic asynchronously (when no error) + // return the err (or nil) to the operator, and then proceed to do aggregation logic asynchronously (when no + // error) signedTaskResponseDigest.SignatureVerificationErrorC <- err if err != nil { continue From 7627a5eec95d20ce0c9538fedfde374ab990696d Mon Sep 17 00:00:00 2001 From: afkbyte Date: Tue, 24 Sep 2024 09:27:02 -0700 Subject: [PATCH 7/8] reorder check --- services/bls_aggregation/blsagg.go | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/services/bls_aggregation/blsagg.go b/services/bls_aggregation/blsagg.go index 4bf9f18a..8dc6bc57 100644 --- a/services/bls_aggregation/blsagg.go +++ b/services/bls_aggregation/blsagg.go @@ -336,12 +336,29 @@ func (a *BlsAggregatorService) singleTaskAggregatorGoroutineFunc( signedTaskResponseDigest, ) + err := a.verifySignature(taskIndex, signedTaskResponseDigest, operatorsAvsStateDict) + if err != nil { + a.logger.Info( + "Signature verification failed", + "taskIndex", taskIndex, + "operatorId", fmt.Sprintf("%x", signedTaskResponseDigest.OperatorId), + "error", err, + ) + signedTaskResponseDigest.SignatureVerificationErrorC <- err + continue + } + // compute the taskResponseDigest using the hash function taskResponseDigest, err := a.hashFunction(signedTaskResponseDigest.TaskResponse) if err != nil { // this error should never happen, because we've already hashed the taskResponse in verifySignature, // but keeping here in case the verifySignature implementation ever changes or some catastrophic bug // happens.. + a.logger.Warn( + "Failed to hash task response", + "taskIndex", taskIndex, + "error", err, + ) continue } @@ -359,14 +376,6 @@ func (a *BlsAggregatorService) singleTaskAggregatorGoroutineFunc( } } - err = a.verifySignature(taskIndex, signedTaskResponseDigest, operatorsAvsStateDict) - // return the err (or nil) to the operator, and then proceed to do aggregation logic asynchronously (when no - // error) - signedTaskResponseDigest.SignatureVerificationErrorC <- err - if err != nil { - continue - } - // after verifying signature we aggregate its sig and pubkey, and update the signed stake amount if !ok { // first operator to sign on this digest From c996b9dd4ea5f986fe486107b5b02b285b98df9a Mon Sep 17 00:00:00 2001 From: afkbyte Date: Tue, 24 Sep 2024 09:40:13 -0700 Subject: [PATCH 8/8] revert prior commit --- services/bls_aggregation/blsagg.go | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/services/bls_aggregation/blsagg.go b/services/bls_aggregation/blsagg.go index 8dc6bc57..4bf9f18a 100644 --- a/services/bls_aggregation/blsagg.go +++ b/services/bls_aggregation/blsagg.go @@ -336,29 +336,12 @@ func (a *BlsAggregatorService) singleTaskAggregatorGoroutineFunc( signedTaskResponseDigest, ) - err := a.verifySignature(taskIndex, signedTaskResponseDigest, operatorsAvsStateDict) - if err != nil { - a.logger.Info( - "Signature verification failed", - "taskIndex", taskIndex, - "operatorId", fmt.Sprintf("%x", signedTaskResponseDigest.OperatorId), - "error", err, - ) - signedTaskResponseDigest.SignatureVerificationErrorC <- err - continue - } - // compute the taskResponseDigest using the hash function taskResponseDigest, err := a.hashFunction(signedTaskResponseDigest.TaskResponse) if err != nil { // this error should never happen, because we've already hashed the taskResponse in verifySignature, // but keeping here in case the verifySignature implementation ever changes or some catastrophic bug // happens.. - a.logger.Warn( - "Failed to hash task response", - "taskIndex", taskIndex, - "error", err, - ) continue } @@ -376,6 +359,14 @@ func (a *BlsAggregatorService) singleTaskAggregatorGoroutineFunc( } } + err = a.verifySignature(taskIndex, signedTaskResponseDigest, operatorsAvsStateDict) + // return the err (or nil) to the operator, and then proceed to do aggregation logic asynchronously (when no + // error) + signedTaskResponseDigest.SignatureVerificationErrorC <- err + if err != nil { + continue + } + // after verifying signature we aggregate its sig and pubkey, and update the signed stake amount if !ok { // first operator to sign on this digest