Dedup post merge readiness notifiers#6797
Conversation
beacon_node/client/src/notifier.rs
Outdated
| eth1_logging(&beacon_chain, &log); | ||
| bellatrix_readiness_logging(Slot::new(0), &beacon_chain, &log).await; | ||
| capella_readiness_logging(Slot::new(0), &beacon_chain, &log).await; | ||
| post_capella_readiness_logging(Slot::new(0), &beacon_chain, &log).await; |
There was a problem hiding this comment.
All forks after capella forgot to add the notifier here, resulting is the lack of readiness checks pre-genesis. Since it only affects devnets It was not noticed probably
| ); | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
No need to do this check here, we have the EL status upcheck service
macladson
left a comment
macladson
left a comment
There was a problem hiding this comment.
Looks great! I'm kind of jealous I didn't think to do this first 🥲
Just a small nit
michaelsproul
added
waiting-on-author
|
@dapplion or @macladson if one of you fixes the merge conflicts we can merge |
|
This pull request has merge conflicts. Could you please resolve them @dapplion? 🙏 |
dapplion
added
ready-for-review
|
Fixed conflicts and ready for review |
michaelsproul
added
waiting-on-author
|
sorry @dapplion bitrot got it :( if you can fix up the Fulu conflicts lets merge |
|
This pull request has merge conflicts. Could you please resolve them @dapplion? 🙏 |
|
Hi @dapplion, this pull request has been closed automatically due to 30 days of inactivity. If you’d like to continue working on it, feel free to reopen at any time. |
mergify
bot
added
the
stale
|
I would like to keep this, if you have some time to polish it up Lion |
michaelsproul
removed
the
stale
|
Hi @dapplion, this pull request has been closed automatically due to 30 days of inactivity. If you’d like to continue working on it, feel free to reopen at any time. |
mergify
bot
added
the
stale
3 participants
Issue Addressed
Lots of repeated code that just checks if the engine API has the required methods for the next fork. All readiness checks after the merge just check if a specific methods are in the list of capabilities.
Proposed Changes
Delete boilerplate so future forks don't need to copypasta again.
The logic now will:
If a future fork requires more complex readiness checks, it can add a dedicated _readiness file like for the merge.