Skip to content
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.

Sign up for GitHub

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

feat(sync): bifurcation for syncTarget #219

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
+322 −6
Open

feat(sync): bifurcation for syncTarget #219

Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
c120282
feat(sync): bifurcation for syncTarget
cristaloleg Sep 24, 2024
dfd308d
fix
cristaloleg Oct 4, 2024
9b8a974
new algo
cristaloleg Oct 9, 2024
6ff7cb7
add tests
cristaloleg Oct 14, 2024
46eabdc
fix returned error and split tests
cristaloleg Oct 14, 2024
f9d2daf
remove old comment
cristaloleg Oct 14, 2024
6d035aa
review suggestions
cristaloleg Nov 5, 2024
821c9b4
review suggestions
cristaloleg Nov 11, 2024
f9bbccb
fix panic msg
cristaloleg Nov 13, 2024
9734b93
remove a test
cristaloleg Nov 22, 2024
b215a04
add metric attributes
cristaloleg Dec 4, 2024
0e7942f
review suggestions
cristaloleg Dec 4, 2024
146b81e
review suggestion
cristaloleg Dec 5, 2024
0377c99
more review suggestions
cristaloleg Dec 5, 2024
ef9365b
upd log
cristaloleg Dec 5, 2024
7eec051
upd error and logging after a review
cristaloleg Jan 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions sync/sync_head.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func (s *Syncer[H]) incomingNetworkHead(ctx context.Context, head H) error {
}

// verify verifies given network head candidate.
func (s *Syncer[H]) verify(ctx context.Context, newHead H) (bool, error) {
func (s *Syncer[H]) verify(ctx context.Context, newHead H) (isSoft bool, _ error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the point of naming the return vals here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To say exactly what this bool means. It's not clear (at all) from doc-comment. I'm fine to add a line to doc-comment and leave result param unnamed.

sbjHead, err := s.subjectiveHead(ctx)
if err != nil {
log.Errorw("getting subjective head during validation", "err", err)
Expand Down Expand Up @@ -203,6 +203,8 @@ func (s *Syncer[H]) verify(ctx context.Context, newHead H) (bool, error) {
// subjectiveHead.
// A non-nil error is returned when networkHead can't be verified.
func (s *Syncer[H]) verifyBifurcating(ctx context.Context, subjHead, networkHead H) error {
log.Warnw("sync: header bifurcation started", "height", networkHead.Height(), "hash", networkHead.Hash().String())
cristaloleg marked this conversation as resolved.
Show resolved Hide resolved

subjHeight := subjHead.Height()

cristaloleg marked this conversation as resolved.
Show resolved Hide resolved
diff := networkHead.Height() - subjHeight
Expand Down Expand Up @@ -238,7 +240,10 @@ func (s *Syncer[H]) verifyBifurcating(ctx context.Context, subjHead, networkHead
s.metrics.failedBifurcation(ctx, int64(networkHead.Height()), networkHead.Hash().String())
log.Warnw("sync: header validation against subjHead", "height", networkHead.Height(), "hash", networkHead.Hash().String())
cristaloleg marked this conversation as resolved.
Show resolved Hide resolved

return fmt.Errorf("sync: header validation against subjHead height:%d hash:%x", networkHead.Height(), networkHead.Hash().String())
return &header.VerifyError{
Reason: fmt.Errorf("sync: header validation against subjHead height:%d hash:%x", networkHead.Height(), networkHead.Hash().String()),
cristaloleg marked this conversation as resolved.
Show resolved Hide resolved
SoftFailure: false,
}
}

// isExpired checks if header is expired against trusting period.
Expand Down
Loading