diff --git a/crates/precompile/src/bls12_381/blst.rs b/crates/precompile/src/bls12_381/blst.rs index 3900b26626..4b02f92b27 100644 --- a/crates/precompile/src/bls12_381/blst.rs +++ b/crates/precompile/src/bls12_381/blst.rs @@ -140,14 +140,16 @@ pub(super) fn p1_msm( scalars_bytes.len() / SCALAR_LENGTH, "number of scalars should equal the number of g1 points" ); - // When no inputs are given, we trigger an assert. - // While it is mathematically sound to have no inputs (can return point at infinity) - // EIP2537 forbids this and since this is the only function that - // currently calls this method, we have this assert. - assert!( - !g1_points.is_empty(), - "number of inputs to pairing should be non-zero" - ); + + // When no inputs are given, we return the point at infinity. + // This case can only trigger, if the initial MSM pairs + // all had, either a zero scalar or the point at infinity. + // + // The precompile will return an error, if the initial input + // was empty, in accordance with EIP-2537. + if g1_points.is_empty() { + return blst_p1_affine::default(); + } // When there is only a single point, we use a simpler scalar multiplication // procedure @@ -183,14 +185,16 @@ pub(super) fn p2_msm( scalars_bytes.len() / SCALAR_LENGTH, "number of scalars should equal the number of g2 points" ); - // When no inputs are given, we trigger an assert. - // While it is mathematically sound to have no inputs (can return point at infinity) - // EIP2537 forbids this and since this is the only function that - // currently calls this method, we have this assert. - assert!( - !g2_points.is_empty(), - "number of inputs to pairing should be non-zero" - ); + + // When no inputs are given, we return the point at infinity. + // This case can only trigger, if the initial MSM pairs + // all had, either a zero scalar or the point at infinity. + // + // The precompile will return an error, if the initial input + // was empty, in accordance with EIP-2537. + if g2_points.is_empty() { + return blst_p2_affine::default(); + } // When there is only a single point, we use a simpler scalar multiplication // procedure @@ -284,15 +288,16 @@ fn is_fp12_one(f: &blst_fp12) -> bool { /// returns true if the result is equal to the identity element. #[inline] pub(super) fn pairing_check(pairs: &[(blst_p1_affine, blst_p2_affine)]) -> bool { - // When no inputs are given, we trigger an assert. - // While it is mathematically sound to have no inputs (can return true) - // EIP2537 forbids this and since this is the only function that - // currently calls this method, we have this assert. - assert!( - !pairs.is_empty(), - "number of inputs to pairing should be non-zero" - ); - + // When no inputs are given, we return true + // This case can only trigger, if the initial pairing components + // all had, either the G1 element as the point at infinity + // or the G2 element as the point at infinity. + // + // The precompile will return an error, if the initial input + // was empty, in accordance with EIP-2537. + if pairs.is_empty() { + return true; + } // Compute the miller loop for the first pair let (first_g1, first_g2) = &pairs[0]; let mut acc = compute_miller_loop(first_g1, first_g2); diff --git a/crates/precompile/src/bls12_381/g1_msm.rs b/crates/precompile/src/bls12_381/g1_msm.rs index 11614ee6b3..fdda2613fd 100644 --- a/crates/precompile/src/bls12_381/g1_msm.rs +++ b/crates/precompile/src/bls12_381/g1_msm.rs @@ -44,25 +44,31 @@ pub(super) fn g1_msm(input: &Bytes, gas_limit: u64) -> PrecompileResult { for i in 0..k { let encoded_g1_element = &input[i * G1_MSM_INPUT_LENGTH..i * G1_MSM_INPUT_LENGTH + PADDED_G1_LENGTH]; + let encoded_scalar = &input[i * G1_MSM_INPUT_LENGTH + PADDED_G1_LENGTH + ..i * G1_MSM_INPUT_LENGTH + PADDED_G1_LENGTH + SCALAR_LENGTH]; // Filter out points infinity as an optimization, since it is a no-op. - // Note: Previously, points were being batch converted from Jacobian to Affine. In `blst`, this would essentially, - // zero out all of the points. Since all points are in affine, this bug is avoided. + // Note: Previously, points were being batch converted from Jacobian to Affine. + // In `blst`, this would essentially, zero out all of the points. + // Since all points are now in affine, this bug is avoided. if encoded_g1_element.iter().all(|i| *i == 0) { continue; } - // NB: Scalar multiplications, MSMs and pairings MUST perform a subgroup check. let p0_aff = extract_g1_input(encoded_g1_element)?; + + // If the scalar is zero, then this is a no-op. + // + // Note: This check is made after checking that g1 is valid. + // this is because we want the precompile to error when + // G1 is invalid, even if the scalar is zero. + if encoded_scalar.iter().all(|i| *i == 0) { + continue; + } + g1_points.push(p0_aff); - scalars_bytes.extend_from_slice( - &extract_scalar_input( - &input[i * G1_MSM_INPUT_LENGTH + PADDED_G1_LENGTH - ..i * G1_MSM_INPUT_LENGTH + PADDED_G1_LENGTH + SCALAR_LENGTH], - )? - .b, - ); + scalars_bytes.extend_from_slice(&extract_scalar_input(encoded_scalar)?.b); } // Return the encoding for the point at the infinity according to EIP-2537 diff --git a/crates/precompile/src/bls12_381/g2_msm.rs b/crates/precompile/src/bls12_381/g2_msm.rs index 1d4305d676..2ca118935b 100644 --- a/crates/precompile/src/bls12_381/g2_msm.rs +++ b/crates/precompile/src/bls12_381/g2_msm.rs @@ -44,6 +44,8 @@ pub(super) fn g2_msm(input: &Bytes, gas_limit: u64) -> PrecompileResult { for i in 0..k { let encoded_g2_elements = &input[i * G2_MSM_INPUT_LENGTH..i * G2_MSM_INPUT_LENGTH + PADDED_G2_LENGTH]; + let encoded_scalar = &input[i * G2_MSM_INPUT_LENGTH + PADDED_G2_LENGTH + ..i * G2_MSM_INPUT_LENGTH + PADDED_G2_LENGTH + SCALAR_LENGTH]; // Filter out points infinity as an optimization, since it is a no-op. // Note: Previously, points were being batch converted from Jacobian to Affine. In `blst`, this would essentially, @@ -51,22 +53,24 @@ pub(super) fn g2_msm(input: &Bytes, gas_limit: u64) -> PrecompileResult { if encoded_g2_elements.iter().all(|i| *i == 0) { continue; } - // NB: Scalar multiplications, MSMs and pairings MUST perform a subgroup check. // // So we set the subgroup_check flag to `true` let p0_aff = extract_g2_input(encoded_g2_elements)?; + // If the scalar is zero, then this is a no-op. + // + // Note: This check is made after checking that g2 is valid. + // this is because we want the precompile to error when + // G2 is invalid, even if the scalar is zero. + if encoded_scalar.iter().all(|i| *i == 0) { + continue; + } + // Convert affine point to Jacobian coordinates using our helper function g2_points.push(p0_aff); - scalars_bytes.extend_from_slice( - &extract_scalar_input( - &input[i * G2_MSM_INPUT_LENGTH + PADDED_G2_LENGTH - ..i * G2_MSM_INPUT_LENGTH + PADDED_G2_LENGTH + SCALAR_LENGTH], - )? - .b, - ); + scalars_bytes.extend_from_slice(&extract_scalar_input(encoded_scalar)?.b); } // Return infinity point if all points are infinity diff --git a/crates/precompile/src/bls12_381/pairing.rs b/crates/precompile/src/bls12_381/pairing.rs index 6b0d1fd5c5..765a0946d1 100644 --- a/crates/precompile/src/bls12_381/pairing.rs +++ b/crates/precompile/src/bls12_381/pairing.rs @@ -38,21 +38,32 @@ pub(super) fn pairing(input: &Bytes, gas_limit: u64) -> PrecompileResult { // Collect pairs of points for the pairing check let mut pairs = Vec::with_capacity(k); for i in 0..k { - // NB: Scalar multiplications, MSMs and pairings MUST perform a subgroup check. - // extract_g1_input and extract_g2_input perform the necessary checks - let p1_aff = extract_g1_input( - &input[i * PAIRING_INPUT_LENGTH..i * PAIRING_INPUT_LENGTH + PADDED_G1_LENGTH], - )?; + let encoded_g1_element = + &input[i * PAIRING_INPUT_LENGTH..i * PAIRING_INPUT_LENGTH + PADDED_G1_LENGTH]; + let encoded_g2_element = &input[i * PAIRING_INPUT_LENGTH + PADDED_G1_LENGTH + ..i * PAIRING_INPUT_LENGTH + PADDED_G1_LENGTH + PADDED_G2_LENGTH]; + + // If either the G1 or G2 element is the encoded representation + // of the point at infinity, then these two points are no-ops + // in the pairing computation. + // + // Note: we do not skip the validation of these two elements even if + // one of them is the point at infinity because we could have G1 be + // the point at infinity and G2 be an invalid element or vice versa. + // In that case, the precompile should error because one of the elements + // was invalid. + let g1_is_zero = encoded_g1_element.iter().all(|i| *i == 0); + let g2_is_zero = encoded_g2_element.iter().all(|i| *i == 0); // NB: Scalar multiplications, MSMs and pairings MUST perform a subgroup check. - let p2_aff = extract_g2_input( - &input[i * PAIRING_INPUT_LENGTH + PADDED_G1_LENGTH - ..i * PAIRING_INPUT_LENGTH + PADDED_G1_LENGTH + PADDED_G2_LENGTH], - )?; + // extract_g1_input and extract_g2_input perform the necessary checks + let p1_aff = extract_g1_input(encoded_g1_element)?; + let p2_aff = extract_g2_input(encoded_g2_element)?; - pairs.push((p1_aff, p2_aff)); + if !g1_is_zero & !g2_is_zero { + pairs.push((p1_aff, p2_aff)); + } } - let result = if pairing_check(&pairs) { 1 } else { 0 }; Ok(PrecompileOutput::new( diff --git a/crates/precompile/src/bn128.rs b/crates/precompile/src/bn128.rs index 72c2912d7f..31beca9e12 100644 --- a/crates/precompile/src/bn128.rs +++ b/crates/precompile/src/bn128.rs @@ -174,11 +174,28 @@ pub fn run_pair( // This is where G1 ends. let g2_start = start + G1_LEN; + let encoded_g1_element = &input[g1_start..g2_start]; + let encoded_g2_element = &input[g2_start..g2_start + G2_LEN]; + + // If either the G1 or G2 element is the encoded representation + // of the point at infinity, then these two points are no-ops + // in the pairing computation. + // + // Note: we do not skip the validation of these two elements even if + // one of them is the point at infinity because we could have G1 be + // the point at infinity and G2 be an invalid element or vice versa. + // In that case, the precompile should error because one of the elements + // was invalid. + let g1_is_zero = encoded_g1_element.iter().all(|i| *i == 0); + let g2_is_zero = encoded_g2_element.iter().all(|i| *i == 0); + // Get G1 and G2 points from the input - let a = read_g1_point(&input[g1_start..g2_start])?; - let b = read_g2_point(&input[g2_start..g2_start + G2_LEN])?; + let a = read_g1_point(encoded_g1_element)?; + let b = read_g2_point(encoded_g2_element)?; - points.push((a, b)); + if !g1_is_zero && !g2_is_zero { + points.push((a, b)); + } } let success = pairing_check(&points);