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 empty_block_validation_regression test #3513

Closed
piotr-dziubecki opened this issue Dec 20, 2022 · 3 comments · Fixed by #3565
Closed

Fix empty_block_validation_regression test #3513

piotr-dziubecki opened this issue Dec 20, 2022 · 3 comments · Fixed by #3565
Assignees

Comments

@piotr-dziubecki
Copy link

The changes in fast sync v2 have made this test no longer meaningful. It needs to be updated to ensure that a BlockPayload with accusations containing _everyone_else is created by the malicious runner.

@piotr-dziubecki
Copy link
Author

@Fraser999 to ping @afck about it

@piotr-dziubecki
Copy link
Author

@afck : Well, it's a regression test. So it's fine to delete it as long as nobody reintroduces the bug. 😄
In this case I actually think it's fine to remove it. In fact, I'd remove the optimization from the code that allows skipping validation for blocks that are both empty and contain accusations. If all blocks are always getting validated without exception there's no danger of bringing back this bug.
So maybe just remove ConsensusValueT::needs_validation, and assume it's always true.

@piotr-dziubecki piotr-dziubecki changed the title Fix empty_block_validation_regression test Delete empty_block_validation_regression test Jan 3, 2023
@piotr-dziubecki piotr-dziubecki assigned fizyk20 and unassigned Fraser999 Jan 11, 2023
@Fraser999
Copy link
Collaborator

As discussed in standup today, we'll instead implement a fix for the test and retain the original optimisation in the code.

@Fraser999 Fraser999 changed the title Delete empty_block_validation_regression test Fix empty_block_validation_regression test Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants