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

Migrate to most recent Winterfell release #1332

Merged
merged 14 commits into from
May 22, 2024
Merged

Migrate to most recent Winterfell release #1332

merged 14 commits into from
May 22, 2024

Conversation

Al-Kindi-0
Copy link
Contributor

Describe your changes

As the title says. Once the corresponding PR in the crypto crate crate is merged I will update the dependency in this PR.

Checklist before requesting a review

  • Repo forked and branch created from next according to naming convention.
  • Commit messages and codestyle follow conventions.
  • Relevant issues are linked in the PR description.
  • Tests added for new functionality.
  • Documentation/comments updated according to changes.

plafer
plafer approved these changes May 10, 2024
View reviewed changes
Copy link
Contributor

@plafer plafer left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for doing this!

(to merge after miden-crypto is merged)

bobbinth
bobbinth approved these changes May 21, 2024
View reviewed changes
Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you! I left a few small comments below. Once these are addressed, we can merge.

assembly/src/ast/instruction/mod.rs Outdated Show resolved Hide resolved
assembly/src/ast/instruction/opcode.rs Outdated Show resolved Hide resolved
core/Cargo.toml Outdated Show resolved Hide resolved
core/src/operations/mod.rs Outdated Show resolved Hide resolved
prover/Cargo.toml Outdated Show resolved Hide resolved
processor/src/trace/mod.rs Outdated Show resolved Hide resolved
@bobbinth
Copy link
Contributor

Oh - and we should also fix the CI (seems like no_std compilation is failing?).

bitwalker
bitwalker approved these changes May 22, 2024
View reviewed changes
Copy link
Contributor

@bitwalker bitwalker left a comment

Choose a reason for hiding this comment

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

Nothing catches my eye, so LGTM!

@Al-Kindi-0
Copy link
Contributor Author

Al-Kindi-0 commented May 22, 2024 •
edited

All comments were addressed, thanks all for the feedback

@bobbinth bobbinth merged commit 7e901ad into next May 22, 2024
15 checks passed
@bobbinth bobbinth deleted the al-migrate-winterfell branch May 22, 2024 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bitwalker bitwalker bitwalker approved these changes

@plafer plafer plafer approved these changes

@bobbinth bobbinth bobbinth 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.

None yet
4 participants
@Al-Kindi-0 @bobbinth @bitwalker @plafer