fix: saving block height after committing validators to db#474
fix: saving block height after committing validators to db#474tzdybal merged 1 commit intoevstack:mainfrom
Conversation
distractedm1nd
requested review from
adlerjohn,
liamsi and
tzdybal
as code owners
|
IIRC test was flaky even before most recent changes related to height update (#442). But this change resolves the issue. |
|
The case is definitely that (when it flakes) we read the validators before we commit them to the db - you can confirm this by logging the reads and writes. Doesn't flake anymore with this fix, so the biggest question is just if changing the order of operations has adverse effects? |
|
@jbowen93 WDYT? Is there any particular reason why height was stored before commit? |
adlerjohn
left a comment
adlerjohn
left a comment
There was a problem hiding this comment.
Fundamentally, this is resolving a data race, no? This might resolve a single instance of the race, but there should be some sort of static check/guarantee that things that need to be committed to the db atomically are committed atomically.
|
I agree, but I don't think investigating other data races, or adding tooling to detect it, is really in the scope of this PR, right? |
Nope, can be left for future work. |
tzdybal
left a comment
tzdybal
left a comment
There was a problem hiding this comment.
Let's merge this for now. It fixes the bug, and shouldn't introduce completely new issues.
I also created https://github.com/celestiaorg/optimint/issues/477, which should resolve the core issue here.
3 participants
Would resolve #462 . Is there any concerns about this change? The handling may change when #457 is investigated anyways