Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Feature/val 1404 eip 7251 head watcher alerts for new el requests #58
base: develop
Are you sure you want to change the base?
Feature/val 1404 eip 7251 head watcher alerts for new el requests #58
Changes from 4 commits
35442e6
18bcefd
4bd5b5f
7339592
13deb61
be7d453
ba8e56d
3e7617c
fb1a38c
824d994
d652744
f8c7d92
73e1ac6
33475b5
c5d30f7
a17b7d5
0e52491
149d827
64ed4c2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably implement a new alert class for these types of events (donations). It might handle two cases: when someone changes 0x01 validator withdrawal credentials to Lido withdrawal credentials, and when someone changes 0x02 validator withdrawal credentials to Lido withdrawal credentials. And we may have two new types of alerts for these events. Let's discuss it in more detail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to make alert description more informative. So, if the
source_pubkey
is a pubkey of Lido validator, print also the operator name to which this validator belongs and wrap this info to beaconcha.in link (if it's not too complex to get the validator_index, or maybe validator pubkey is enough to build a meaningful beaconcha.in link?)The same logic for the
target_pubkey
.Maybe it's better to have separate description templates for each type of alert, but maybe not. I'm not sure about it. Both decisions are probably OK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks suboptimal to send separate alerts for each consolidation. If we have N consolidations of the same type in the head block, the N separate alerts of the same type will be sent. I think it would be better to group alerts of the same type, list all necessary info in the alert description, and send only one alert for each type. Something like it's done in the
exit.py
. Feel free to disagree if you see any disadvantages in this idea.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same thoughts as for the Consolidation alerts. Seems better to print the opeartor name and beaconcha.in links where possible and group alerts of the same type.