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

simulators/portal: fix race condition in beacon-sync suite #1243

Merged

Conversation

acolytec3
Copy link
Contributor

The issue:
The portal/beacon-sync test suite currently pulls test data from a live beacon node in order to test syncing the Portal beacon light clients.

The bridge client that retrieves the finalized and optimistic updates continues to pull new updates from the beacon client as long as the test is running.

This can be an issue in the optimistic update test where a new optimistic update is produced every slot.

Only one optimistic update is pushed into the network and the simulator then retrieves a new update from the bridge to compare to when validating the test results.

The solution

Add a parameter to the start method in the BridgeService that tells the bridge whether to continue to retrive new updates from the Beacon Node provider or not. In the the optimistic update test, setting this parameter to false will ensure that we don't accidentally pull a new optimistic update when validating the test result since this is current test only tests for a single update.

@acolytec3
Copy link
Contributor Author

@njgheorghita see above

@njgheorghita
Copy link
Contributor

Thanks @acolytec3 ! I think this will work great for now. As the suite develops this will probably evolve to allow for more fine-grained control over what content is exactly being gossiped during the test run (aka, gossip multiple optimistic updates and make sure all clients stay up to date, etc.)

If you could just fix the few lint errors in the failing ci run, then everything looks good to me.

Ping @KolbyML for review / merge

@KolbyML
Copy link
Member

KolbyML commented Feb 11, 2025

It looks good, just need CI fixed

@acolytec3
Copy link
Contributor Author

Lint should be cleaned up. Thanks!

Copy link
Member

@KolbyML KolbyML left a comment

Choose a reason for hiding this comment

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

:shipit: looks good

@KolbyML KolbyML changed the title Fix race condition in beacon-sync suite simulators/portal: fix race condition in beacon-sync suite Feb 11, 2025
@KolbyML KolbyML merged commit a6ae163 into ethereum:master Feb 11, 2025
7 checks passed
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