Fix bitwise rotate

[nyunyunyunyu]

The existing implementation of const_right_rotate_unsafe_internal is under-constraints. r_witness should have bit bits but it's also valid if it equals to a.

[jonathanpwang]

Good fix, I should have caught this the first time.

The main diff:

        let l_witness = Witness(F::from_u128(val_l));
        let r_witness = Witness(F::from_u128(val_r));
        let val_witness = self.mul_add(ctx, l_witness, Constant(self.pow_of_two()[bit]), r_witness);
        ctx.constrain_equal(&a, &val_witness);

        let val_l = val >> bit;
        let val_r = val - (val_l << bit);
        let l_witness = ctx.load_witness(F::from_u128(val_l));
        let r_witness = ctx.load_witness(F::from_u128(val_r));
        let val_witness = self.gate.mul_add(ctx, l_witness, Constant(self.gate.pow_of_two()[bit]), r_witness);
        self.range_check(ctx, l_witness, num_bits - bit);
        self.range_check(ctx, r_witness, bit);
        ctx.constrain_equal(&a, &val_witness);

Whenever a decomposition is done, there must be range checks to ensure overflow protection.

Please add a negative test (can use MockProver), where you prank l_witness = 0, r_witness = a. Unfortunately I think you either have to just prank by the exact offset in ctx or copy paste the function code over... Be aware of copy constraints when pranking, so it's actually checking the right thing. A good test is that the negative test will pass without the range checks.

[jonathanpwang]

A bit low level, but an optimization is to still use Witness(F::from_u128(val_l)) and then fetch l_witness = ctx.get(-3), r_witness = ctx.get(-4)

[jonathanpwang]

Going to merge this for expediency. A negative test would still be nice in the future.