Skip to content

refactor(bin): CL events conditions#4643

Merged
mattsse merged 2 commits intomainfrom
alexey/cl-events-match-order
Sep 18, 2023
Merged

refactor(bin): CL events conditions#4643
mattsse merged 2 commits intomainfrom
alexey/cl-events-match-order

Conversation

@shekhirin
Copy link
Member

@shekhirin shekhirin commented Sep 18, 2023

Previous match logic was incorrect, because in case of an outdated FCU but no transition config exchange, we fell into NeverSeen branch:

// Short circuit if we recently had an FCU.
(_, Some(fork_choice))
if fork_choice.elapsed() <= NO_FORKCHOICE_UPDATE_RECEIVED_PERIOD =>
{
continue
}
// Otherwise, continue with health checks based on Transition Configuration exchange
// and Fork Choice update.
(None, _) => Poll::Ready(Some(ConsensusLayerHealthEvent::NeverSeen)),

@shekhirin shekhirin force-pushed the alexey/cl-events-match-order branch 2 times, most recently from 8879994 to a28c001 Compare September 18, 2023 18:23
@shekhirin shekhirin force-pushed the alexey/cl-events-match-order branch from a28c001 to a65d7cf Compare September 18, 2023 18:23
@codecov
Copy link

codecov bot commented Sep 18, 2023

Codecov Report

Merging #4643 (a65d7cf) into main (b5905c4) will decrease coverage by 0.09%.
Report is 2 commits behind head on main.
The diff coverage is 0.00%.

Impacted file tree graph

Files Changed Coverage Δ
bin/reth/src/node/cl_events.rs 0.00% <0.00%> (ø)

... and 17 files with indirect coverage changes

Flag Coverage Δ
integration-tests 16.93% <0.00%> (-0.03%) ⬇️
unit-tests 63.16% <0.00%> (-0.08%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
reth binary 31.95% <0.00%> (-0.01%) ⬇️
blockchain tree 83.59% <ø> (ø)
pipeline 88.54% <ø> (ø)
storage (db) 73.47% <ø> (ø)
trie 94.77% <ø> (+0.03%) ⬆️
txpool 49.44% <ø> (-0.49%) ⬇️
networking 77.17% <ø> (-0.05%) ⬇️
rpc 57.31% <ø> (-0.01%) ⬇️
consensus 62.58% <ø> (+0.16%) ⬆️
revm 19.66% <ø> (ø)
payload builder 9.06% <ø> (ø)
primitives 86.44% <ø> (-0.04%) ⬇️

@shekhirin shekhirin requested a review from mattsse September 18, 2023 18:50
@shekhirin shekhirin marked this pull request as ready for review September 18, 2023 18:50
@shekhirin shekhirin requested a review from onbjerg as a code owner September 18, 2023 18:50
@mattsse mattsse added C-bug An unexpected or incorrect behavior A-observability Related to tracing, metrics, logs and other observability tools A-cli Related to the reth CLI labels Sep 18, 2023
@mattsse mattsse added this pull request to the merge queue Sep 18, 2023
Merged via the queue into main with commit 733ee19 Sep 18, 2023
@mattsse mattsse deleted the alexey/cl-events-match-order branch September 18, 2023 20:35
@shekhirin shekhirin linked an issue Sep 26, 2023 that may be closed by this pull request
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cli Related to the reth CLI A-observability Related to tracing, metrics, logs and other observability tools C-bug An unexpected or incorrect behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CL health events shouldn't be reported during the pipeline sync

2 participants