Skip to content

chore: uint audit part 1 (operator<<, operator>>, ror)#15823

Merged
suyash67 merged 6 commits into
sb/uint-combinedfrom
sb/audit-uint-1
Jul 26, 2025
Merged

chore: uint audit part 1 (operator<<, operator>>, ror)#15823
suyash67 merged 6 commits into
sb/uint-combinedfrom
sb/audit-uint-1

Conversation

@suyash67

@suyash67 suyash67 commented Jul 18, 2025

Copy link
Copy Markdown
Contributor

🧾 Audit Context

Audit of the uint module in stdlib, specifically the methods: operator>>, operator<< and ror (bitwise operations).

🛠️ Changes Made

  • All three methods were manually "slicing" a field element which was very error-prone. I replaced it with split_at method (wrote a generic method split_at1 that splits a field_t at a given bit index and returns the lower and higher slices).
  • Cleaned up and optimised uint constructor from byte_array and std::vector<bool_t> (resolving old TODO)
  • Added static assert on width so that we restrict uint to 8, 16, 32, and 64 bits only.

✅ Checklist

  • Audited all methods of the relevant module/class
  • Audited the interface of the module/class with other (relevant) components
  • Documented existing functionality and any changes made (as per Doxygen requirements)
  • Resolved and/or closed all issues/TODOs pertaining to the audited files
  • Confirmed and documented any security or other issues found (if applicable)
  • Verified that tests cover all critical paths (and added tests if necessary)
  • Updated audit tracking for the files audited (check the start of each file you audited)

📌 Notes for Reviewers

Please review the split_at function carefully, and check if we are testing it thoroughly. We're soon going to remove the function slice(msb, lsb) from field_t since we no longer need it (used only in logic module). Instead, the new function split_at will replace the use of slice in logic module:

field_pt b_slice = b.slice(static_cast<uint8_t>(num_bits - 1), 0)[1];

will be changed to:

field_pt a_slice = a.split_at(0, num_bits).second;

Note: we are not going to merge any PRs that change anything in field or bigfield as they're undergoing external audits at the moment. Thus, this PR will be merged after the external audit of field (and its fixes) are done.

Footnotes

  1. The existing slice(msb, lsb) method could be used but there were three issues in doing so:
    (a) found a bug in range-constraint on the hi limb in it,
    (b) slice(...) returns three limbs: hi, mid and lo so it naturally requires three range constraints. We only need two parts of a field element.
    (c) slice(...) assumes a 252-bit field element which results in (unnecessary) costly range-constraints. We'd have to change the function to accept num_bits as a parameter (or template parameter).

@suyash67 suyash67 requested a review from iakovenkos July 18, 2025 13:53
return result;
}

template <typename Builder>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nice! I can re-use it in byte_array byte decomposition method

Comment thread barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp Outdated
EXPECT_TRUE(builder.err() == "slice: hi value too large.");
}

static void test_split_at()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is it possible to produce a failure test with the current circuit design?

Comment thread barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/uint.hpp
Comment thread barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/uint.cpp Outdated
Comment thread barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/uint.cpp
Base automatically changed from merge-train/barretenberg to next July 21, 2025 14:15
@suyash67 suyash67 marked this pull request as ready for review July 22, 2025 10:07
@suyash67 suyash67 changed the base branch from next to merge-train/barretenberg July 22, 2025 10:07
Base automatically changed from merge-train/barretenberg to next July 22, 2025 10:46
@suyash67 suyash67 changed the base branch from next to merge-train/barretenberg July 22, 2025 12:03
Base automatically changed from merge-train/barretenberg to next July 22, 2025 23:47
suyash67 added a commit that referenced this pull request Jul 25, 2025
Reverts #15975

Need to get the logic ops cleanup
[PR](#15823) first.
@suyash67 suyash67 changed the base branch from next to merge-train/barretenberg July 25, 2025 14:29
@suyash67 suyash67 changed the base branch from merge-train/barretenberg to sb/uint-combined July 26, 2025 18:25
@suyash67 suyash67 merged commit b41bc5d into sb/uint-combined Jul 26, 2025
7 checks passed
@suyash67 suyash67 deleted the sb/audit-uint-1 branch July 26, 2025 19:04
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.

2 participants