Skip to content

Resolve discrepancy between Aggregator and Full Nodes in Integration Test#644

Merged
Manav-Aggarwal merged 1 commit intomainfrom
manav/resolve_discrepancy_fn_agg
Dec 5, 2022
Merged

Resolve discrepancy between Aggregator and Full Nodes in Integration Test#644
Manav-Aggarwal merged 1 commit intomainfrom
manav/resolve_discrepancy_fn_agg

Conversation

@Manav-Aggarwal
Copy link
Copy Markdown
Member

@Manav-Aggarwal Manav-Aggarwal commented Dec 2, 2022 •
edited
Loading

Overview

The discrepancy stems from how an Aggregator and Full Node call SaveNode. Before, the Aggregator did not SaveNode after calling ApplyBlock. Since ApplyBlock modifies the ISR list in the block data, we need to call SaveNode again.

Also, resolves a issue with passing isMalicious boolean correctly in the integration test when creating and starting nodes.

Closes: #566

Checklist

  • New and updated code has appropriate documentation
  • New and updated code has new and/or updated testing
  • Required CI checks are passing
  • Visual proof for any user facing features like CLI or documentation updates
  • Linked issues closed with keywords

@Manav-Aggarwal Manav-Aggarwal changed the title Resolve discrepany Resolve discrepancy between Aggregator and Full Nodes in Integration Test Dec 2, 2022
@Manav-Aggarwal Manav-Aggarwal self-assigned this Dec 2, 2022
@Manav-Aggarwal Manav-Aggarwal assigned nashqueue and unassigned nashqueue Dec 2, 2022
@Manav-Aggarwal Manav-Aggarwal marked this pull request as ready for review December 2, 2022 21:21
@Manav-Aggarwal Manav-Aggarwal enabled auto-merge (squash) December 2, 2022 21:21
@Manav-Aggarwal Manav-Aggarwal requested a review from S1nus December 2, 2022 21:22
tzdybal
tzdybal approved these changes Dec 5, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@tzdybal tzdybal left a comment

Choose a reason for hiding this comment

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

Nice catch 👍

@Manav-Aggarwal Manav-Aggarwal merged commit 7ced28a into main Dec 5, 2022
@Manav-Aggarwal Manav-Aggarwal deleted the manav/resolve_discrepancy_fn_agg branch December 5, 2022 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nashqueue nashqueue Awaiting requested review from nashqueue

@S1nus S1nus Awaiting requested review from S1nus

2 more reviewers

@tzdybal tzdybal tzdybal approved these changes

@gupadhyaya gupadhyaya gupadhyaya approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@Manav-Aggarwal Manav-Aggarwal

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Investigate discrepancy between Aggregator and Full Nodes in Integration Test

4 participants

@Manav-Aggarwal @tzdybal @gupadhyaya @nashqueue