fix: return lock instead of locking inside lock function to prevent…

… deadlock.
ssvlabs · Aug 28, 2024 · 6d55275 · 6d55275
1 parent c968db4
commit 6d55275
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 23 deletions.
diff --git a/signer/sign_attestation.go b/signer/sign_attestation.go
@@ -19,9 +19,10 @@ func (signer *SimpleSigner) SignBeaconAttestation(attestation *phase0.Attestatio
 	}
 
 	// 2. lock for current account
-	signer.lock(account.ID(), "attestation")
+	val := signer.lock(account.ID(), "attestation")
+	val.Lock()
 	defer func() {
-		signer.unlock(account.ID(), "attestation")
+		val.Unlock()
 	}()
 
 	// 3. far future check

diff --git a/signer/sign_block.go b/signer/sign_block.go
@@ -23,8 +23,9 @@ func (signer *SimpleSigner) SignBlock(block ssz.HashRoot, slot phase0.Slot, doma
 	}
 
 	// 2. lock for current account
-	signer.lock(account.ID(), "proposal")
-	defer signer.unlock(account.ID(), "proposal")
+	val := signer.lock(account.ID(), "proposal")
+	val.Lock()
+	defer val.Unlock()
 
 	// 3. far future check
 	if !IsValidFarFutureSlot(signer.network, slot) {

diff --git a/signer/sign_sync_committee.go b/signer/sign_sync_committee.go
@@ -21,8 +21,9 @@ func (signer *SimpleSigner) SignSyncCommittee(msgBlockRoot []byte, domain phase0
 	}
 
 	// 2. lock for current account
-	signer.lock(account.ID(), "sync_committee")
-	defer signer.unlock(account.ID(), "sync_committee")
+	val := signer.lock(account.ID(), "sync_committee")
+	val.Lock()
+	defer val.Unlock()
 
 	// 3. sign
 	sszRoot := SSZBytes(msgBlockRoot)
@@ -51,8 +52,9 @@ func (signer *SimpleSigner) SignSyncCommitteeSelectionData(data *altair.SyncAggr
 	}
 
 	// 2. lock for current account
-	signer.lock(account.ID(), "sync_committee_selection_data")
-	defer signer.unlock(account.ID(), "sync_committee_selection_data")
+	val := signer.lock(account.ID(), "sync_committee_selection_data")
+	val.Lock()
+	defer val.Unlock()
 
 	// 3. sign
 	if data == nil {
@@ -83,8 +85,9 @@ func (signer *SimpleSigner) SignSyncCommitteeContributionAndProof(contribAndProo
 	}
 
 	// 2. lock for current account
-	signer.lock(account.ID(), "sync_committee_selection_and_proof")
-	defer signer.unlock(account.ID(), "sync_committee_selection_and_proof")
+	val := signer.lock(account.ID(), "sync_committee_selection_and_proof")
+	val.Lock()
+	defer val.Unlock()
 
 	// 3. sign
 	if contribAndProof == nil {

diff --git a/signer/validator_signer.go b/signer/validator_signer.go
@@ -51,26 +51,16 @@ func NewSimpleSigner(wallet core.Wallet, slashingProtector core.SlashingProtecto
 }
 
 // lock locks signer
-func (signer *SimpleSigner) lock(accountID uuid.UUID, operation string) {
+func (signer *SimpleSigner) lock(accountID uuid.UUID, operation string) *sync.RWMutex {
 	signer.mapLock.Lock()
 	defer signer.mapLock.Unlock()
 
 	k := accountID.String() + "_" + operation
 	if val, ok := signer.signLocks[k]; ok {
-		val.Lock()
+		return val
 	} else {
 		signer.signLocks[k] = &sync.RWMutex{}
-		signer.signLocks[k].Lock()
-	}
-}
-
-func (signer *SimpleSigner) unlock(accountID uuid.UUID, operation string) {
-	signer.mapLock.RLock()
-	defer signer.mapLock.RUnlock()
-
-	k := accountID.String() + "_" + operation
-	if val, ok := signer.signLocks[k]; ok {
-		val.Unlock()
+		return signer.signLocks[k]
 	}
 }