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

Add test helper function which stores a program and registers a user #1036

Merged
merged 5 commits into from
Sep 12, 2024

Conversation

ameba23
Copy link
Contributor

@ameba23 ameba23 commented Aug 30, 2024

Now that we no longer have pre-registered accounts, in order to test signing we need to first store a program and register a user. To reduce boilerplate in our siging tests, this adds a test helper function which stores a default program and registers a user using that program.

I've also made a change to the get_sign_tx_data test helper function. It now takes the verifying key as an argument, rather than defaulting to DAVE_VERIFYING_KEY, and sets the block number to the current block number rather than defaulting to zero.

One unrelated thing i noticed: We have a test called signature_request_with_derived_account_works which looks to me like it doesn't test anything extra than our other signing tests - it is left over from when we had two different signing flows. Do yous think we can get rid of it?

async fn signature_request_with_derived_account_works() {

@ameba23 ameba23 marked this pull request as draft August 30, 2024 13:55
* master:
  Add missing BoundedVec imports (#1039)
  Bump the patch-dependencies group with 3 updates (#1042)
  Bump the patch-dependencies group with 2 updates (#1040)
@ameba23 ameba23 marked this pull request as ready for review September 11, 2024 05:38
Copy link
Collaborator

@HCastano HCastano left a comment

Choose a reason for hiding this comment

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

LGTM.

For your question around signature_request_with_derived_account_works, the point of that one versus the other tests is that it's a happy path only test.

The ones ones afair test different failure conditions, so I didn't want to mix them up.

crates/threshold-signature-server/src/helpers/tests.rs Outdated Show resolved Hide resolved
crates/threshold-signature-server/src/helpers/tests.rs Outdated Show resolved Hide resolved
crates/threshold-signature-server/src/helpers/tests.rs Outdated Show resolved Hide resolved
@ameba23 ameba23 merged commit b900c6d into master Sep 12, 2024
8 checks passed
@ameba23 ameba23 deleted the peg/test-helper-register branch September 12, 2024 09:34
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