Skip to content

add regression test for presignature timeout#500

Merged
jakmeier merged 3 commits intosig-net:developfrom
jakmeier:regression-485
Aug 29, 2025
Merged

add regression test for presignature timeout#500
jakmeier merged 3 commits intosig-net:developfrom
jakmeier:regression-485

Conversation

@jakmeier
Copy link
Copy Markdown
Contributor

Adds a test that would fail without the fix in #485

Adds a test that would fail without the fix in sig-net#485
@jakmeier
Copy link
Copy Markdown
Contributor Author

if things in CI work out the same as the do locally, tests should pass on this PR and fail over at #501

Copy link
Copy Markdown
Contributor

@volovyks volovyks left a comment

Choose a reason for hiding this comment

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

Nice!

.build()
.await;

tokio::time::timeout(Duration::from_secs(10), network.wait_for_presignatures(1))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

To be sure I understand this test correctly.

  • We start 3 nodes
  • They try to generate 5 per-signatures
  • Some or all (not clear) of this protocols fail, since we drop 20 per-signature messages on each node
  • Nodes start new protocols, at least one of them succeeds.

It means that old protocols are not waiting for new messages forever and the counter of ongoing generators represents protocols that are actually alive.

The requirement for such test is: The number of failed protocols must be >= number of concurrent pre-signature generations. This is not guaranteed by the test itself, but it is very nice to have anyways.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah this is very specific to the bug we had the other week

The number of failed protocols must be >= number of concurrent pre-signature generations.

yes

This is not guaranteed by the test itself, but it is very nice to have anyways.

True, the test relies on the default setup using a small enough max concurrent generators / max introduced setting. Would be nicer to fix it in the test... I might get back to it later if I want to clean things up a bit more but will merge it for now as-is.

@jakmeier jakmeier merged commit f1c21ae into sig-net:develop Aug 29, 2025
3 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.

2 participants