reduce before sub if needed

[han0110]

Currently the overflow of result in FiveColumnIntegerChip::sub is updated to be a.overflows + (b.overflows + 1) + 1, but this creates the possibility that overflow of result is equal to OVERFLOW_LIMIT, which causes panic in further operations.

halo2-snark-aggregator/halo2-ecc-circuit-lib/src/five/integer_chip.rs

Lines 652 to 675 in 35c4496

	fn sub(
	&self,
	ctx: &mut Context<N>,
	a: &AssignedInteger<W, N>,
	b: &AssignedInteger<W, N>,
	) -> Result<AssignedInteger<W, N>, Error> {
	let one = N::one();
	let upper_limbs = self.find_w_modulus_ceil(b);
	let mut limbs = vec![];
	for i in 0..LIMBS {
	let cell = self.base_gate().sum_with_constant(
	ctx,
	vec![(&a.limbs_le[i], one), (&b.limbs_le[i], -one)],
	bn_to_field(&upper_limbs[i]),
	)?;
	limbs.push(cell);
	}
	let overflow = a.overflows + (b.overflows + 1) + 1;
	let mut res = AssignedInteger::new(limbs.try_into().unwrap(), overflow);
	self.conditionally_reduce(ctx, &mut res)?;
	Ok(res)
	}

Not sure if it makes sense, but adding a check before sub should fix this:

 fn sub( 
     &self, 
     ctx: &mut Context<N>, 
     a: &AssignedInteger<W, N>, 
     b: &AssignedInteger<W, N>, 
 ) -> Result<AssignedInteger<W, N>, Error> { 
+    let a = if a.overflows + (b.overflows + 1) + 1 == OVERFLOW_LIMIT {
+        let mut a = a.clone();
+        self.reduce(ctx, &mut a)?;
+        a
+    } else {
+        a.clone()
+    };
     ...
}

[xgaozoyoe]

I think you are right. I think we do have lazy reduce policy some where, I will have a look.