feat: fork detection #6
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vgorkavenko
force-pushed
the
feat/fork-detection
branch
from
0665b22 to
afb168b
Compare
vgorkavenko
force-pushed
the
feat/fork-detection
branch
from
afb168b to
acd484d
Compare
madlabman
reviewed
Jun 12, 2023
src/providers/consensus/client.py
Outdated
| @@ -117,6 +125,16 @@ def get_validators_stream(self, slot_number: SlotNumber) -> Response: | |||
| ) | |||
| return stream | |||
|
|
|||
| def get_chain_reorg_stream(self) -> Response: | |||
| """Spec: https://consensys.github.io/teku/#tag/Events/operation/getEvents""" | |||
| special_client = copy(self) | |||
There was a problem hiding this comment.
Didn't get the reason for this approach.
There was a problem hiding this comment.
Change the default values. For the events handling, we should keep the connection alive. Maybe there is sense to placing the event request method to parent class
There was a problem hiding this comment.
looks too complex and not obvious imo
could you just add arguments to the provider functions and override default constants if they're present
I think it will be ok to use kwargs here and pass it deeply up to _get_stream_without_fallbacks
src/providers/consensus/client.py
Outdated
| """Spec: https://ethereum.github.io/beacon-APIs/#/Beacon/getBlockV2""" | ||
| # Set special timeout and retry params for this method. | ||
| # It is used for `head` request | ||
| special_client = copy(self) | ||
| special_client.HTTP_REQUEST_TIMEOUT = 2 | ||
| special_client.HTTP_REQUEST_TIMEOUT = 1.5 |
There was a problem hiding this comment.
We have to keep a balance between catching a new head and quickly changing host URL in case of an error. This value is just the result of N dev runs
skhomuti
requested changes
Jun 13, 2023
src/providers/consensus/client.py
Outdated
| @@ -117,6 +125,16 @@ def get_validators_stream(self, slot_number: SlotNumber) -> Response: | |||
| ) | |||
| return stream | |||
|
|
|||
| def get_chain_reorg_stream(self) -> Response: | |||
| """Spec: https://consensys.github.io/teku/#tag/Events/operation/getEvents""" | |||
| special_client = copy(self) | |||
There was a problem hiding this comment.
looks too complex and not obvious imo
could you just add arguments to the provider functions and override default constants if they're present
I think it will be ok to use kwargs here and pass it deeply up to _get_stream_without_fallbacks
skhomuti
previously approved these changes
Jun 14, 2023
madlabman
reviewed
Jun 15, 2023
madlabman
approved these changes
Jun 15, 2023
vgorkavenko
added a commit
that referenced
this pull request
Jun 23, 2023
* CODEOWNERS * feat: fork detection (#6) * feat: fork detection * feat: add unhandled head parent alert * fix: unhandled slot alert * fix: decrease timeouts and retries * fix: linter * fix: review * fix: type * fix: add `threading.Lock` * fix: update dependencies * chore(deps): bump docker/build-push-action from 3 to 4 (#5) Bumps [docker/build-push-action](https://github.com/docker/build-push-action) from 3 to 4. - [Release notes](https://github.com/docker/build-push-action/releases) - [Commits](docker/build-push-action@v3...v4) --- updated-dependencies: - dependency-name: docker/build-push-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Vladimir Gorkavenko <32727352+vgorkavenko@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Fix/errors handling (#8) * fix: unhandled errors * chore: labels for prometheus jobs * fix: remove useless `del` * fix: review * fix: alerts (#10) * fix: parse validators (#11) * feat: tests and polish (#12) * feat: tests and polish * fix: mypy * chore: fix workflow * refactor: add thread_as_daemon decorator * chore: add isort * fix: review --------- Co-authored-by: madlabman <10616301+madlabman@users.noreply.github.com> * fix: workflow (#13) --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: madlabman <10616301+madlabman@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.