header: verify NextValidatorsHash for light nodes by tuxcanfly · Pull Request #844 · evstack/ev-node

tuxcanfly · 2023-04-06T23:10:20Z

Fixes: #835

codecov-commenter · 2023-04-07T00:22:07Z

Codecov Report

Attention: 20 lines in your changes are missing coverage. Please review.

Comparison is base (497c015) 56.24% compared to head (eea19d1) 56.12%.

Additional details and impacted files

@@ Coverage Diff @@## main #844 +/- ##
==========================================
- Coverage 56.24% 56.12% -0.12% 
==========================================
Files 64 64 Lines 6815 6820 +5 ==========================================
- Hits 3833 3828 -5 - Misses 2593 2603 +10 
Partials 389 389

Files	Coverage Δ
conv/abci/block.go	`89.88% <100.00%> (ø)`
types/hashing.go	`72.97% <100.00%> (ø)`
types/serialization.go	`61.30% <100.00%> (+0.29%)`	⬆️
types/test_utils.go	`84.53% <100.00%> (+0.32%)`	⬆️
types/header.go	`50.00% <33.33%> (-30.96%)`	⬇️
block/manager.go	`8.18% <0.00%> (-0.17%)`	⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

tzdybal

LGTM

gupadhyaya · 2023-05-10T13:53:24Z

the PR #858 is currently blocked, but it has a commit that refactors the execute function, which you can copy-paste: d08e4d6

i will later pull your changes.

Manav-Aggarwal

LGTM

…re signing

sets header's next aggregator's hash although we need to call updateState before doing this, which will be fixed in an upcoming commit

tuxcanfly · 2023-09-29T02:31:41Z

Using newState after applyBlock refers to the correct NextValidators set and assigns the correct NextAggregatorsHash. Updated.

gupadhyaya

LGTM. Minor comment on removing the Validate from ApplyBlock. I am fine either removing or not removing, as it should not have any security implications.

Fixes: evstack#835 --------- Co-authored-by: Ganesha Upadhyaya <gupadhyaya@Ganeshas-MacBook-Pro-2.local>

Fixes: #835 --------- Co-authored-by: Ganesha Upadhyaya <gupadhyaya@Ganeshas-MacBook-Pro-2.local>

Fixes: evstack#835 --------- Co-authored-by: Ganesha Upadhyaya <gupadhyaya@Ganeshas-MacBook-Pro-2.local>

tuxcanfly marked this pull request as ready for review April 7, 2023 00:30

tuxcanfly requested review from gupadhyaya, nashqueue and tzdybal as code owners April 7, 2023 00:30

gupadhyaya reviewed Apr 7, 2023
View reviewed changes

Comment threadstate/executor.go Outdated

gupadhyaya reviewed Apr 7, 2023
View reviewed changes

Comment threadtypes/header.go Outdated

gupadhyaya reviewed Apr 7, 2023
View reviewed changes

Comment threadtypes/pb/tendermint/abci/types.pb.go Outdated

gupadhyaya reviewed Apr 7, 2023
View reviewed changes

Comment threadstate/executor.go Outdated

tzdybal reviewed Apr 12, 2023
View reviewed changes

Comment threadtypes/header.go Outdated
Comment threadproto/rollkit/rollkit.proto

tuxcanfly force-pushed the tux/verify-next-validatorset-hash branch from dd431ef to c7a433fCompare April 13, 2023 13:35

nashqueue added the C: sequencing label Apr 17, 2023

nashqueue assigned tuxcanfly Apr 17, 2023

tzdybal reviewed Apr 17, 2023
View reviewed changes

nashqueue force-pushed the tux/verify-next-validatorset-hash branch from ac79826 to a4b01d3Compare April 24, 2023 13:36

tzdybal previously approved these changes Apr 24, 2023
View reviewed changes

tuxcanfly dismissed tzdybal’s stale review via 01df546May 10, 2023 22:29

tuxcanfly force-pushed the tux/verify-next-validatorset-hash branch 3 times, most recently from 0ca992a to a980000Compare May 17, 2023 13:50

gupadhyaya mentioned this pull request May 19, 2023
fraud proof processing using go-fraud package #928
Merged

tuxcanfly force-pushed the tux/verify-next-validatorset-hash branch from a980000 to 156de39Compare May 23, 2023 13:48

tuxcanfly force-pushed the tux/verify-next-validatorset-hash branch from 156de39 to 59772abCompare June 21, 2023 16:14

tuxcanfly requested a review from gupadhyaya June 21, 2023 22:02

Manav-Aggarwal self-requested a review June 21, 2023 23:32

Manav-Aggarwal previously approved these changes Jun 21, 2023
View reviewed changes

gupadhyaya reviewed Jun 22, 2023
View reviewed changes

Comment threadstate/executor.go Outdated

tuxcanfly force-pushed the tux/verify-next-validatorset-hash branch from b7a9bdb to 1ae6386Compare June 26, 2023 18:39

nashqueue added the va label Sep 11, 2023

tuxcanflyand others added 13 commits September 28, 2023 11:58

state: improve next aggregator hash validation
5f51158

make proto-gen
4f46536

header: verify old header next aggregator hash with new
7ed3842

state: restore next aggregators hash
2909b23

types: update signed header test for next aggregators hash
227bc78

types: update next validators hash
7ff53ed

remove hash dependency, separate validate & apply, populate ISRs befo…
aade151
…re signing

header: set next aggregators hash
1ea1f1f
sets header's next aggregator's hash although we need to call updateState before doing this, which will be fixed in an upcoming commit

manager: restore next aggregators hash
ff3ca09

header: rm unnecessary check
ee1ca12

test: update checkValSet to assert assert NextValidatorsHash
bfdad58

go mod tidy
b190ecc

manager: use new state to assign next aggregators hash
aaf1921

tuxcanfly force-pushed the tux/verify-next-validatorset-hash branch from 6efd18e to aaf1921Compare September 29, 2023 02:27

go mod tidy
8eaff2c

gupadhyaya reviewed Sep 29, 2023
View reviewed changes

Comment threadstate/executor.go

gupadhyaya previously approved these changes Sep 29, 2023
View reviewed changes

state: restore ApplyBlock Validate
eea19d1

tuxcanfly dismissed gupadhyaya’s stale review via eea19d1September 29, 2023 15:42

gupadhyaya self-requested a review September 29, 2023 15:44

gupadhyaya approved these changes Sep 29, 2023
View reviewed changes

Manav-Aggarwal approved these changes Sep 29, 2023
View reviewed changes

Manav-Aggarwal added this pull request to the merge queue Sep 29, 2023

Merged via the queue into main with commit d9eb2daSep 29, 2023

Manav-Aggarwal deleted the tux/verify-next-validatorset-hash branch September 29, 2023 15:57

chandiniv1 pushed a commit to chandiniv1/rollkit that referenced this pull request Oct 19, 2023
header: verify NextValidatorsHash for light nodes (evstack#844)
e6ea3b4
Fixes: evstack#835 --------- Co-authored-by: Ganesha Upadhyaya <gupadhyaya@Ganeshas-MacBook-Pro-2.local>

gupadhyaya pushed a commit that referenced this pull request Oct 20, 2023
header: verify NextValidatorsHash for light nodes (#844)
76c41f7
Fixes: #835 --------- Co-authored-by: Ganesha Upadhyaya <gupadhyaya@Ganeshas-MacBook-Pro-2.local>

chandiniv1 pushed a commit to chandiniv1/rollkit that referenced this pull request Oct 31, 2023
header: verify NextValidatorsHash for light nodes (evstack#844)
b059d44
Fixes: evstack#835 --------- Co-authored-by: Ganesha Upadhyaya <gupadhyaya@Ganeshas-MacBook-Pro-2.local>

Navigation Menu

Uh oh!

header: verify NextValidatorsHash for light nodes - #844

header: verify NextValidatorsHash for light nodes#844
Manav-Aggarwal merged 17 commits into
mainfrom
tux/verify-next-validatorset-hash

tuxcanfly commented Apr 6, 2023•
edited by nashqueue
Loading

Uh oh!

codecov-commenter commented Apr 7, 2023•
edited by codecov Bot
Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tzdybal left a comment

Uh oh!

gupadhyaya commented May 10, 2023•
edited
Loading

Uh oh!

Manav-Aggarwal left a comment

Uh oh!

Uh oh!

tuxcanfly commented Sep 29, 2023

Uh oh!

Uh oh!

gupadhyaya left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Uh oh!

Conversation

tuxcanfly commented Apr 6, 2023• edited by nashqueue Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Apr 7, 2023• edited by codecov BotLoading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tzdybal left a comment

Choose a reason for hiding this comment

Uh oh!

gupadhyaya commented May 10, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Manav-Aggarwal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tuxcanfly commented Sep 29, 2023

Uh oh!

Uh oh!

gupadhyaya left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

tuxcanfly commented Apr 6, 2023•
edited by nashqueue
Loading

codecov-commenter commented Apr 7, 2023•
edited by codecov Bot
Loading

gupadhyaya commented May 10, 2023•
edited
Loading