Skip to content

Conversation

@ndr-ds
Copy link
Contributor

@ndr-ds ndr-ds commented Sep 5, 2025

Motivation

We're setting the base timeout for the chain we're opening to u64::MAX. However, since this test is racey by design, if we happen to get a RoundTimeout in proposals in both of the chains, we'll effectively wait forever and the test will never return.

Proposal

Set the base timeout to something more reasonable. Setting to 10 seconds, as it's what the rest of the file is using.

Test Plan

Had a consistent repro of this happening and the test hanging forever. After this change, the test passes.

Release Plan

  • Nothing to do / These changes follow the usual release cycle.

Copy link
Contributor Author

ndr-ds commented Sep 5, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link
Contributor

@afck afck left a comment

Choose a reason for hiding this comment

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

In this scenario there should never be a round timeout, that's the point of this test: both chain owners are live, so one of them should always immediately propose a block.

This test used to work reliably. We need to find the commit that broke it.

(Edit: To clarify, clients will in some cases conclude that they have to wait for the timeout, but their wait will always very quickly be interrupted by a notification due to the other client's actions.)

Copy link
Contributor Author

ndr-ds commented Sep 8, 2025

Fix for the notifications is this: #4516
Closing this PR

@ndr-ds ndr-ds closed this Sep 8, 2025
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.

4 participants