feat(sync): Basic sync logging improvement. #142
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
added 4 commits
February 13, 2024 19:23
WalkthroughThe changes focus on enhancing synchronization and context handling in the Beacon module. Updates include refining the Changes
Poem
TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #142 +/- ##
==========================================
+ Coverage 28.04% 28.05% +0.01%
==========================================
Files 102 102
Lines 3791 3793 +2
Branches 4 4
==========================================
+ Hits 1063 1064 +1
- Misses 2650 2651 +1
Partials 78 78
|
po-bera
reviewed
Feb 14, 2024
| @@ -168,10 +168,10 @@ func (s *Service) CheckSyncStatusAndForkchoice(ctx context.Context) error { | |||
|
|
|||
| // If the beacon chain is ahead of the execution chain, we need to trigger a forkchoice | |||
| // update to get the execution chain to start syncing, otherwise we can just return. | |||
| if bss.status != StatusBeaconAhead { | |||
| if bss.Status == StatusExecutionAhead { | |||
There was a problem hiding this comment.
The above comment is confusing, consider change it to "If the execution chain is ahead of the beacon chain, we can just return, otherwise ..."
po-bera
reviewed
Feb 14, 2024
| app.BeaconKitRunner.StartServices(ctx) | ||
|
|
||
| // Initial check for execution client sync. | ||
| if err := app.BeaconKitRunner.InitialSyncCheck( |
There was a problem hiding this comment.
DoingInitialSyncCheck here makes more sense to me!
po-bera
reviewed
Feb 14, 2024
| // ErrValidatorClientNotSynced is an error for when a | ||
| // validator tries to propose a block with an out of sync | ||
| // execution client. | ||
| ErrValidatorClientNotSynced = errors.New(`your validator tried to propose a |
There was a problem hiding this comment.
Suggested change
| ErrValidatorClientNotSynced = errors.New(`your validator tried to propose a | |
| ErrProposerClientNotSynced = errors.New(`your validator tried to propose a |
po-bera
approved these changes
Feb 14, 2024
abi87
pushed a commit
that referenced
this pull request
Jan 18, 2025
…lock deposits (#142) * fix(state-transition): Correctly enforce local validation of beacon block deposits * lint cleanup * Update state-transition/core/validation_deposits_test.go Signed-off-by: Cal Bera <[email protected]> * nit clean for loop * nosec --------- Signed-off-by: Cal Bera <[email protected]>
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.
We will need to refactor this later.
Summary by CodeRabbit
BeaconSyncProgressstruct to useStatusinstead ofstatus.RegisterAPIRoutesfunction.syncStatusinPreBlockermethod.syncServicefield to check synchronization status in proposal handling methods.