Skip to content

fix: race condition in keymanager API#1150

Merged
usmansaleem merged 1 commit intoConsensys:masterfrom
aimxhaisse:fix/race-condition
Jan 15, 2026
Merged

fix: race condition in keymanager API#1150
usmansaleem merged 1 commit intoConsensys:masterfrom
aimxhaisse:fix/race-condition

Conversation

@aimxhaisse
Copy link
Copy Markdown
Contributor

@aimxhaisse aimxhaisse commented Jan 12, 2026

PR Description

The BiMap used to have a map of public key to db indexes is not properly synchronized: it can effectively be accessed from multiple threads in parallel:

  • in read-only from signing requests from validator clients (1),
  • in a writable-way from the key manager API (2).

When mixing the keymanager API with signatures at the same time, this sometimes causes corruption leading to stacktraces when trying to sign with a bogus subset of keys that are loaded, making them unavailable until restart.

This PR ensures the BiMap is synchronized, this implies some potential contention especially around (1) which is why it's using a readwrite lock approach. This was tested at scale both on Hoodi and Mainnet for multiple days without issues.

Fixed Issue(s)

  • Fix unregistered validator IllegalStateException due to race condition in keymanager API

Changelog

  • I thought about adding a changelog entry, and added one if I deemed necessary.

Testing

  • I thought about testing these changes in a realistic/non-local environment.

Note

Improves thread safety of the in-memory validator registry to prevent sporadic IllegalStateException when key manager operations run alongside signing.

  • Wraps RegisteredValidators BiMap access with a ReadWriteLock (ReentrantReadWriteLock) for reads (validatorIds, getPublicKeyForValidatorId, getValidatorIdForPublicKey) and writes (registerValidators)
  • Returns copies of sets via Set.copyOf(...) instead of exposing live views
  • Updates CHANGELOG.md under Upcoming Release: bug fix entry for unregistered validator race in keymanager API

Written by Cursor Bugbot for commit fab6259. This will update automatically on new commits. Configure here.

The BiMap used to have a map of public key to db indexes is not
properly synchronized: it can effectively be accessed from multiple
threads in parallel:

- in read-only from signing requests from validator clients (1),
- in a writable-way from the key manager API (2).

When mixing the keymanager API with signatures at the same time, this
sometimes causes memory corruption leading to IllegalStateException
stacktraces (Unregistered validator), making a subset of loaded keys
unavailable until restart.

This PR ensures the BiMap is synchronized, this implies some potential
contention especially around (1) which is why it's using a readwrite
lock approach. This was tested at scale both on Hoodi and Mainnet for
multiple days without issues.
Copy link
Copy Markdown
Collaborator

@usmansaleem usmansaleem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@usmansaleem usmansaleem merged commit 849e3c3 into Consensys:master Jan 15, 2026
8 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jan 15, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants