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

test: multi signature production #790

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

ChaoticTempest
Copy link
Member

Adds in test for doing signature production chaotically throughout a whole entire test

  • Also reduces the number of debug logs that were redundant where other logs already log that info

@ChaoticTempest ChaoticTempest requested a review from ppca July 31, 2024 22:51
.into_iter()
.unzip();

match wait_for::signatures_responded(&ctx, &tx_hashes).await {
Copy link
Collaborator

Choose a reason for hiding this comment

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

From what I see in signatures_responded() fn there is no guarantee that it will return all signatures. Some may be in progress. Should we check the length here? Or make sure we have N signatures in the function?

Ok(Outcome::Failed(err)) => {
return Err(WaitForError::Signature(SignatureError::Failed(err)))
}
Err(_) => continue,
Copy link
Member

Choose a reason for hiding this comment

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

This may result in one result pushed more than once to result, I suggest use a Map and skip checking already in Map hashs.


match wait_for::signatures_responded(&ctx, &tx_hashes).await {
Ok(_signatures) => tracing::info!(?tx_hashes, "got signatures"),
Err(err) => tracing::error!("unable to produce signatures in time: {err:?}"),
Copy link
Member

Choose a reason for hiding this comment

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

Would be helpful to collect some metrics: success percentage, average signature time. and we can compare daily result to know if our system is improved or hitting some regression

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