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 failing legacy chain tests #2427

Merged
merged 2 commits into from
Jul 1, 2021
Merged

Fix failing legacy chain tests #2427

merged 2 commits into from
Jul 1, 2021

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Jul 1, 2021

Motivation

  1. In some rare cases, a legacy chain proptest will fail, because it expects blocks that are always invalid.
  2. The legacy chain tests check the blocks in the wrong order.

Solution

  • skip test cases that are unexpectedly valid, in tests that require invalid blocks
  • use the correct order for the block iterators in the tests
  • add proptest seeds
  • improve unclear documentation that might have caused these bugs

Review

@oxarbitrage can review this code, because he wrote the original legacy chain checks.

This fix is not urgent, unless CI starts failing with these bugs.

Reviewer Checklist

  • Code implements Specs and Designs
  • Tests for Expected Behaviour
  • Tests for Errors

Follow Up Work

We might want to add a proptest enum with AlwaysValid, MaybeInvalid, and AlwaysInvalid variants.

teor2345 added 2 commits July 1, 2021 14:28
Add proptest seeds for the failing test.
And improve some unclear documentation.
Also fix unclear documentation that might have led to this bug.
@teor2345 teor2345 added C-bug Category: This is a bug A-rust Area: Updates to Rust code P-Medium labels Jul 1, 2021
@teor2345 teor2345 added this to the 2021 Sprint 13 milestone Jul 1, 2021
@teor2345 teor2345 requested a review from oxarbitrage July 1, 2021 04:34
@teor2345 teor2345 self-assigned this Jul 1, 2021
@teor2345 teor2345 merged commit 936168b into main Jul 1, 2021
@teor2345 teor2345 deleted the legacy-chain-fixes branch July 1, 2021 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rust Area: Updates to Rust code C-bug Category: This is a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants