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

chore: unconstrained improvements #12

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

chore: unconstrained improvements #12

wants to merge 3 commits into from

Conversation

guipublic
Copy link

Description

Problem*

A few improvements over #9

Summary*

I did not have time to review #9 until it was merged, so I put my suggested changes as a PR.

Additional Context

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@@ -169,14 +169,12 @@ unconstrained fn build_msg_block<let N: u32>(
let mut msg_block: MSG_BLOCK = [0; INT_BLOCK_SIZE];

// We insert `BLOCK_SIZE` bytes (or up to the end of the message)
let block_input = if msg_start + BLOCK_SIZE > message_size {
Copy link
Author

Choose a reason for hiding this comment

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

this does not change the code size, but I believe it is a be nicer to read

src/sha256.nr Outdated
for j in 0..INT_SIZE {
let k = i * INT_SIZE + j;
let msg_byte = if k < block_input {
Copy link
Author

Choose a reason for hiding this comment

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

unless I am mistaken, k is always less than block_input?

Copy link
Author

@guipublic guipublic Feb 4, 2025

Choose a reason for hiding this comment

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

oh no it is not, I missed the int_input increment, so I restored the previous version.
We could try to peel the last iteration though, but it'll increase the bytecode.

@guipublic guipublic requested a review from TomAFrench February 4, 2025 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 📋 Backlog
Development

Successfully merging this pull request may close these issues.

1 participant