Skip to content

eth: fix multiple checkpoint fetch logic#465

Merged
JekaMas merged 7 commits into
developfrom
manav/rfc35-bug-fix
Aug 11, 2022
Merged

eth: fix multiple checkpoint fetch logic#465
JekaMas merged 7 commits into
developfrom
manav/rfc35-bug-fix

Conversation

@manav2401

Copy link
Copy Markdown
Member

This PR fixes a bug that was introduced in the RFC35 forkchoice implementaiton (#425). When the whitelisting service tries to fetch checkpoints, it fetches them in decreasing order starting from latest checkpoint to the oldest one. So, when a new checkpoint arrives, instead of eliminating the oldest checkpoint, it eliminated the latest one. We now fetch the checkpoints in an increasing order to prevent this from happening.

@manav2401 manav2401 requested review from a team and JekaMas July 22, 2022 11:36
@JekaMas

JekaMas commented Jul 22, 2022

Copy link
Copy Markdown
Contributor

Could you add a test for the issue?

@manav2401

Copy link
Copy Markdown
Member Author

Could you add a test for the issue?

I actually tested this on a running node. But sure, I'll add some tests for this whole function to make it more deterministic.

@codecov-commenter

codecov-commenter commented Jul 22, 2022

Copy link
Copy Markdown

Codecov Report

Merging #465 (2a26da5) into develop (ac559bc) will increase coverage by 0.11%.
The diff coverage is 22.89%.

@@             Coverage Diff             @@
##           develop     #465      +/-   ##
===========================================
+ Coverage    56.79%   56.90%   +0.11%     
===========================================
  Files          605      609       +4     
  Lines        69942    69991      +49     
===========================================
+ Hits         39723    39828     +105     
+ Misses       26803    26763      -40     
+ Partials      3416     3400      -16     
Impacted Files Coverage Δ
accounts/abi/bind/auth.go 0.00% <0.00%> (ø)
consensus/bor/bor.go 8.20% <0.00%> (-0.02%) ⬇️
eth/backend.go 0.00% <0.00%> (ø)
eth/downloader/downloader.go 71.41% <ø> (ø)
eth/filters/test_backend.go 38.46% <0.00%> (-1.23%) ⬇️
log/handler_glog.go 0.00% <0.00%> (ø)
log/logger.go 1.12% <0.00%> (-0.03%) ⬇️
log/syslog.go 0.00% <ø> (ø)
eth/bor_checkpoint_verifier.go 8.00% <8.00%> (ø)
log/handler.go 3.63% <20.00%> (+0.88%) ⬆️
... and 37 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@manav2401 manav2401 requested review from a team and JekaMas and removed request for a team and JekaMas July 25, 2022 10:46
Comment thread eth/bor_checkpoint_verifier.go Outdated
Comment thread eth/bor_checkpoint_verifier.go Outdated
Comment thread eth/bor_checkpoint_verifier.go Outdated
@manav2401 manav2401 requested review from a team, JekaMas and temaniarpit27 August 3, 2022 12:10
@JekaMas

JekaMas commented Aug 11, 2022

Copy link
Copy Markdown
Contributor

@temaniarpit27 I believe, we need that one in v0.3.0

@JekaMas JekaMas merged commit 1d1f00c into develop Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants