Skip to content

fix(e2e): temporary disable validator set oscillation in generated manifests#4478

Merged
andynog merged 9 commits intomainfrom
cason/e2e-flip-fix-2
Nov 15, 2024
Merged

fix(e2e): temporary disable validator set oscillation in generated manifests#4478
andynog merged 9 commits intomainfrom
cason/e2e-flip-fix-2

Conversation

@cason
Copy link

@cason cason commented Nov 13, 2024

Workaround for #4492.

This PR:

  1. Describes the issue with introduced by feat(e2e): introduce oscillations for validators and consensus params #2283 in comments in the source code (check FIXMEs and TODOs)
  2. Temporarily disables the validator set oscillations in the generated manifest, used in nightly runs

Item 2. has the side effect that not only the validator set oscillations but also the consensus params oscillations are disabled, while we are only aware of issues in the former.


PR checklist

  • Tests written/updated
  • Changelog entry added in .changelog (we use unclog to manage our changelog)
  • Updated relevant documentation (docs/ or spec/) and code comments

@cason cason changed the title e2e: fix oscilate validator set logic when the manifest contains validator updates fix(e2e): oscilate validator set logic when manifest contains validator updates Nov 13, 2024
@andynog andynog added the e2e Related to our end-to-end tests label Nov 14, 2024
@andynog andynog added this to the 2024-Q4 milestone Nov 14, 2024
@andynog
Copy link
Collaborator

andynog commented Nov 14, 2024

@cason please let me know if you need help with this. I was looking at the initial implementation and it seems you are going into the direction of adding validator updates to the manifest.toml file (?). This will probably need some changes to the generator too.

@cason
Copy link
Author

cason commented Nov 15, 2024

@cason please let me know if you need help with this. I was looking at the initial implementation and it seems you are going into the direction of adding validator updates to the manifest.toml file (?). This will probably need some changes to the generator too.

Yes, I need. The generator has nothing to do here. It indeed produce validator updates at random heights, but this logic is ok.

The problem is that in the new logic, the pkg/testnet.go logic for producing a manifest introduces oscillations, validator updates at every height. Those are written to the application's config file.

The problem is that the updates produced by the generator (which are more relevant changes) might conflict with the validator updates produced by the oscillation logic (which just turn on and off a validator, with voting power 1 and 0). The latter has to be fixed.

@cason
Copy link
Author

cason commented Nov 15, 2024

By the way, if no one has bandwidth to proceed on this work, we should open an issue.

Until then, as workaround, we should probably disable the ConstantFlip flag in the produced manifests for nightly executions. Otherwise, the nightly e2e tests will continue fail for this reason, which is a lot of clutter.

@cason cason changed the title fix(e2e): oscilate validator set logic when manifest contains validator updates fix(e2e): temporary disable validator set oscillation logic Nov 15, 2024
@cason cason marked this pull request as ready for review November 15, 2024 07:37
@cason cason requested a review from a team as a code owner November 15, 2024 07:37
@cason cason requested a review from a team November 15, 2024 07:37
@cason cason changed the title fix(e2e): temporary disable validator set oscillation logic fix(e2e): temporary disable validator set oscillation logic in generated manifests Nov 15, 2024
@cason cason changed the title fix(e2e): temporary disable validator set oscillation logic in generated manifests fix(e2e): temporary disable validator set oscillation in generated manifests Nov 15, 2024
@cason cason added the bug Something isn't working label Nov 15, 2024
@cason
Copy link
Author

cason commented Nov 15, 2024

@andynog, feel free to merge it if you find it legit. So we don't have that much failures from this night's tests.

Copy link
Collaborator

@andynog andynog left a comment

Choose a reason for hiding this comment

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

lgtm

@andynog andynog added this pull request to the merge queue Nov 15, 2024
Merged via the queue into main with commit c6fa225 Nov 15, 2024
@andynog andynog deleted the cason/e2e-flip-fix-2 branch November 15, 2024 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working e2e Related to our end-to-end tests

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants