fix: addresses logic audit findings

[suyash67]

L1: ACIR num_bits Validation

Status: won't fix

It is true that create_logic_constraint uses BB_ASSERT (which aborts) rather than throwing an exception when num_bits is invalid. BB_ASSERT is the standard pattern throughout barretenberg for structural invariant checks. Additionally, the num_bits value originates from Noir's IntegerBitSize enum, which restricts values to {1, 8, 16, 32, 64, 128} at the compiler level, so invalid values cannot reach this code through normal operation.

L2: OriginTag Provenance

Status: fixed 0f3ca1c

create_logic_constraint was not propagating OriginTag when converting constant inputs to witnesses, or when returning the final result. Tags are now copied onto the witness before recursing (for both the left-constant and right-constant paths), and the final accumulated result is tagged with the merged tag of both operands. Note we do not propagate tags to the chunks because these are intermediate circuit variables that are not used outside of logic.

L3: Builder Context Validation

Status: fixed 249bfdc

When both inputs are witnesses, the code extracted the builder context from only one operand without verifying both refer to the same builder. Added validate_context<Builder>(a.get_context(), b.get_context()) to assert consistency, matching the pattern used in other stdlib primitives.

L4: Write-VK Mode Guard for Constant Inputs

Status: fixed 5b3d4b8

In write-VK mode, when both inputs are constants, create_logic_constraint returns a constant field_t. The ACIR bridge then attempts assert_equal against a result witness that holds a dummy zero, causing a spurious failure. The fix uses builder.set_variable() to patch the result witness with the correct computed value before the equality check.

I1: Witness Bounds Checks Are Debug-Only

Status: acknowledged, will be fixed later

The witness index bounds checks in get_variable() use BB_ASSERT_DEBUG, which is compiled out in release builds. This is known and it affects every circuit component that calls get_variable(), not just the logic gadget. We are discussing this the noir team if it can be exploited in any meaningful way. Regardless, we will enable these asserts in release mode soon after ensuring no significant hit in performance because of the asserts.

I2: Oversized Plookup Table

Status: won't fix

The code always uses UINT32_XOR/UINT32_AND plookup tables even when the last chunk is smaller than 32 bits. Smaller tables (UINT8, UINT16) exist and would use fewer sub-lookups. However, this is a minor optimization that would change the circuit structure, which we want to avoid at this point. The existing explicit range constraint on the last chunk ensures correctness regardless of table size.

I3: Operator Precedence Bug in Test

Status: fixed — 2a597fb

The test computed 1 << num_bits - 1 intending "all bits set" ((1 << num_bits) - 1), but C++ precedence evaluates it as 1 << (num_bits - 1) (a single bit). For num_bits = 8, this gives 128 instead of 255. Corrected the test.

I4: Redundant Range Constraints

Status: won't fix

Yes there could be cases of redundant range constraints (on the noir side as well as barretenberg) but we prefer to have redundancy over missing a range constraint. This was a good find but we decided to not use the optimisation.