Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(builder): introduce updateConstraintsCacheLock #8

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

thedevbirb
Copy link
Contributor

Introduces a updateConstraintsCacheLock on the builder to prevent racing conditions when subscribing to multiple builders for constrains. Even if shardmap has inner locks, the following edge case might occur in this code:

slotConstraints, _ := b.constraintsCache.Get(constraint.Message.Slot)
if len(slotConstraints) == 0 {
b.constraintsCache.Put(constraint.Message.Slot, decodedConstraints)
continue
}
for hash := range decodedConstraints {
slotConstraints[hash] = decodedConstraints[hash]
log.Debug(fmt.Sprintf("Adding constraint with hash %s to cache for slot %d", hash, constraint.Message.Slot))
}
b.constraintsCache.Put(constraint.Message.Slot, slotConstraints)

  1. relay 1 submits constraints A,B,C
  2. builder gets constraints from cache, let's assume there is none of them
  3. relay 2 submits constraints A,B
  4. builder gets constraints from cache, they are still empty
  5. builder saves constraints A,B,C from relay 1 with Put
  6. builder saves constraints, A,B from relay 2 with Put, overriding the constraints for the slots

…condition when subscribing to multiple relays
@thedevbirb thedevbirb force-pushed the lore/fix/multiple-relays-lock branch from 91a920e to 6af9695 Compare November 12, 2024 11:55
@thedevbirb thedevbirb merged commit 28acd2f into main Nov 12, 2024
0 of 2 checks passed
@thedevbirb thedevbirb deleted the lore/fix/multiple-relays-lock branch November 12, 2024 16:26
@thedevbirb thedevbirb restored the lore/fix/multiple-relays-lock branch November 13, 2024 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants