Skip to content

Conversation

@ledwards2225
Copy link
Contributor

@ledwards2225 ledwards2225 commented Jan 21, 2026

Changes include:

  • Add explicit range constraints so that all inputs are constrained within bb (possibly redundant with noir-applied constraints)
  • Add explicit range constraints so that intermediate witnesses are constrained to be unique where possible
  • Get rid of apply_32_bit_range_constraint_via_lookup in favor of explicit range constraints since the former is only cheaper if the latter is not already in use (which in practice won't occur)

@ledwards2225 ledwards2225 marked this pull request as ready for review January 22, 2026 14:49
* @param input The field element to constrain to 32 bits.
*/
template <typename Builder>
void SHA256<Builder>::apply_32_bit_range_constraint_via_lookup(const field_t<Builder>& input)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As Kesha pointed out, this is only beneficial if we don't already use the traditional range constraint mechanism in the circuit. In practice, noir applies explicit range constraints to all inputs so this will effectively never be the case. Thus, ignoring the high overhead, the cost is 1.75 vs 3 gates for explicit vs lookup-imposed range constraints.

Copy link
Contributor

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

@ledwards2225 ledwards2225 merged commit 41c7e1f into merge-train/barretenberg Jan 23, 2026
8 checks passed
@ledwards2225 ledwards2225 deleted the lde/sha-audit-4 branch January 23, 2026 21:24
github-merge-queue bot pushed a commit that referenced this pull request Jan 27, 2026
BEGIN_COMMIT_OVERRIDE
feat: support JSON input files for bb verify command (#19800)
fix: update bootstrap.sh to use new JSON field names
chore: Update `index.js` so that `HAS_ZK` and `PUBLIC_INPUTS` variables
must always be set in tests (#19884)
chore: pippenger int audit (#19302)
chore: deduplicate batch affine addition trick (#19788)
chore: transcript+codec+poseidon2 fixes (#19419)
chore!: explicitly constrain inputs and intermediate witnesses (#19826)
fix: exclude nlohmann/json from WASM builds in json_output.hpp
chore: translator circuit builder and flavor audit (#19798)
Revert "fix: exclude nlohmann/json from WASM builds in json_output.hpp"
Revert "feat: support JSON input files for bb verify command (#19800)"
Revert "fix: update bootstrap.sh to use new JSON field names"
END_COMMIT_OVERRIDE
danielntmd pushed a commit that referenced this pull request Jan 27, 2026
Changes include:
- Add explicit range constraints so that all inputs are constrained
within bb (possibly redundant with noir-applied constraints)
- Add explicit range constraints so that intermediate witnesses are
constrained to be unique where possible
- Get rid of `apply_32_bit_range_constraint_via_lookup` in favor of
explicit range constraints since the former is only cheaper if the
latter is not already in use (which in practice won't occur)
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