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.

Sign up for GitHub

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

Jump to bottom

chore: add support for new style Dafny test vectors #817

Merged
merged 25 commits into from
Mar 7, 2025
Merged

chore: add support for new style Dafny test vectors #817

merged 25 commits into from
Mar 7, 2025

Conversation

ajewellamz
Copy link
Contributor

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Check any applicable:

  • Were any files moved? Moving files changes their URL, which breaks all hyperlinks to the files.

@ajewellamz ajewellamz requested a review from a team as a code owner March 5, 2025 15:22
@ajewellamz ajewellamz temporarily deployed to continuous-integration March 5, 2025 15:22 — with GitHub Actions Inactive
@ajewellamz ajewellamz temporarily deployed to continuous-integration March 5, 2025 15:22 — with GitHub Actions Inactive
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 5, 2025 15:29 — with GitHub Actions Failure
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 5, 2025 15:29 — with GitHub Actions Error
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 5, 2025 15:29 — with GitHub Actions Failure
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 5, 2025 15:29 — with GitHub Actions Failure
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 5, 2025 15:58 — with GitHub Actions Failure
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 5, 2025 15:58 — with GitHub Actions Failure
@ajewellamz ajewellamz temporarily deployed to continuous-integration March 5, 2025 15:58 — with GitHub Actions Inactive
@ajewellamz ajewellamz temporarily deployed to continuous-integration March 5, 2025 15:58 — with GitHub Actions Inactive
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 5, 2025 16:36 — with GitHub Actions Failure
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 5, 2025 16:36 — with GitHub Actions Failure
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 5, 2025 16:36 — with GitHub Actions Failure
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 5, 2025 16:36 — with GitHub Actions Error
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 5, 2025 16:56 — with GitHub Actions Failure
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 5, 2025 16:56 — with GitHub Actions Error
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 5, 2025 16:56 — with GitHub Actions Failure
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 5, 2025 16:56 — with GitHub Actions Failure
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 5, 2025 17:34 — with GitHub Actions Failure
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 5, 2025 17:34 — with GitHub Actions Error
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 5, 2025 17:34 — with GitHub Actions Failure
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 5, 2025 17:34 — with GitHub Actions Failure
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 6, 2025 11:19 — with GitHub Actions Failure
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 6, 2025 11:19 — with GitHub Actions Error
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 6, 2025 11:19 — with GitHub Actions Failure
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 6, 2025 11:19 — with GitHub Actions Error
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 6, 2025 11:51 — with GitHub Actions Failure
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 6, 2025 11:51 — with GitHub Actions Error
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 6, 2025 11:51 — with GitHub Actions Failure
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 6, 2025 11:51 — with GitHub Actions Error
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 6, 2025 12:46 — with GitHub Actions Failure
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 6, 2025 12:46 — with GitHub Actions Failure
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 6, 2025 12:46 — with GitHub Actions Failure
@ajewellamz ajewellamz had a problem deploying to continuous-integration March 6, 2025 12:46 — with GitHub Actions Failure
@ajewellamz ajewellamz temporarily deployed to continuous-integration March 6, 2025 12:48 — with GitHub Actions Inactive
@ajewellamz ajewellamz temporarily deployed to continuous-integration March 6, 2025 12:48 — with GitHub Actions Inactive
@ajewellamz ajewellamz temporarily deployed to continuous-integration March 6, 2025 12:49 — with GitHub Actions Inactive
@ajewellamz ajewellamz temporarily deployed to continuous-integration March 6, 2025 12:49 — with GitHub Actions Inactive
mahnushm
mahnushm previously approved these changes Mar 6, 2025
View reviewed changes
tests/TestVectors/do_encrypt.cpp
return MakeDecryptJson(test, ciphertext_result, dir);
}

static json RunEncryptTestK(
Copy link

Choose a reason for hiding this comment

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

Very similar to RunEncryptTest(), can we just pass one extra parameter (keyring) and handle it inside the function to reduce code duplication?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't understand, that's exactly what I did.
There is no code duplication. If I have a keyring, I call RunEncryptTestK which creates a cmm, and calls RunEncryptTest.
Which of the two lines of RunEncryptTestK feel like duplication?

@ajewellamz ajewellamz temporarily deployed to continuous-integration March 6, 2025 19:54 — with GitHub Actions Inactive
@ajewellamz ajewellamz temporarily deployed to continuous-integration March 6, 2025 19:54 — with GitHub Actions Inactive
@ajewellamz ajewellamz temporarily deployed to continuous-integration March 6, 2025 19:54 — with GitHub Actions Inactive
@ajewellamz ajewellamz temporarily deployed to continuous-integration March 6, 2025 19:54 — with GitHub Actions Inactive
mahnushm
mahnushm approved these changes Mar 7, 2025
View reviewed changes
Copy link

@mahnushm mahnushm left a comment

Choose a reason for hiding this comment

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

All is good, Thanks!

@ajewellamz ajewellamz merged commit ca6ae59 into master Mar 7, 2025
11 checks passed
@ajewellamz ajewellamz deleted the ajewell/test_vectors branch March 7, 2025 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@texastony texastony texastony left review comments

@mahnushm mahnushm mahnushm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ajewellamz @texastony @mahnushm