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.

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

fix: block sync validation #3236

Merged
merged 3 commits into from
Sep 8, 2021

Conversation

SWvheerden
Copy link
Collaborator

Description

This fixes the block sync block body validation to validate the correct items
This consolidates all the syncing helper functions
This adds in unit tests for the validators

Motivation and Context

Block sync needs to validate all the required steps. This also consolidates all the possible validation functions to remove duplicated pieces of code between the validators so we use a single piece to test a specific consensus item.

How Has This Been Tested?

Synced a node up to the tip height 22014.
Added new unit tests.
Ran all integration and unit tests.

@stringhandler stringhandler added the P-do_not_merge Process - Not ready for merging label Aug 30, 2021
@stringhandler
Copy link
Collaborator

Adding a do not merge temporarily, just want to take a proper look through this

sdbondi
sdbondi previously approved these changes Sep 8, 2021
Copy link
Member

@sdbondi sdbondi left a comment

Choose a reason for hiding this comment

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

utAck - these tests are badly needed and I agree that we need the added checks in block sync.

stringhandler
stringhandler previously approved these changes Sep 8, 2021
Copy link
Collaborator

@stringhandler stringhandler left a comment

Choose a reason for hiding this comment

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

Nice. utACK

@stringhandler stringhandler added mq-approved and removed P-do_not_merge Process - Not ready for merging labels Sep 8, 2021
@aviator-app aviator-app bot added the mq-failed label Sep 8, 2021
Consolidate validation functions
Add validation tests
@aviator-app aviator-app bot merged commit fd081c8 into tari-project:development Sep 8, 2021
Cifko pushed a commit to Cifko/tari that referenced this pull request Sep 10, 2021
Description
---
This fixes the block sync block body validation to validate the correct items
This consolidates all the syncing helper functions
This adds in unit tests for the validators

Motivation and Context
---
Block sync needs to validate all the required steps. This also consolidates all the possible validation functions to remove duplicated pieces of code between the validators so we use a single piece to test a specific consensus item. 

How Has This Been Tested?
---
Synced a node up to the tip height 22014. 
Added new unit tests. 
Ran all integration and unit tests.
@SWvheerden SWvheerden deleted the sw_fix_validation branch September 17, 2021 06:28
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.

3 participants