Skip to content
Merged
125 changes: 91 additions & 34 deletions crates/vm/levm/src/precompiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -770,21 +770,23 @@ pub fn ecadd(calldata: &Bytes, gas_remaining: &mut u64, _fork: Fork) -> Result<B
pub fn ecmul(calldata: &Bytes, gas_remaining: &mut u64, _fork: Fork) -> Result<Bytes, VMError> {
// If calldata does not reach the required length, we should fill the rest with zeros
let calldata = fill_with_zeros(calldata, 96);

increase_precompile_consumed_gas(ECMUL_COST, gas_remaining)?;

let point_x = calldata.get(0..32).ok_or(InternalError::Slicing)?;
let point_y = calldata.get(32..64).ok_or(InternalError::Slicing)?;
let scalar = calldata.get(64..96).ok_or(InternalError::Slicing)?;

if u256_from_big_endian(point_x) >= ALT_BN128_PRIME
|| u256_from_big_endian(point_y) >= ALT_BN128_PRIME
{
return Err(PrecompileError::InvalidPoint.into());
}
let (Some(g1), Some(scalar)) = (
parse_bn254_g1(&calldata, 0),
parse_bn254_scalar(&calldata, 64),
) else {
return Err(InternalError::Slicing.into());
};
validate_bn254_g1_coords(&g1)?;
bn254_g1_mul(g1, scalar)
}

let x = ark_bn254::Fq::from_be_bytes_mod_order(point_x);
let y = ark_bn254::Fq::from_be_bytes_mod_order(point_y);
#[cfg(all(not(feature = "sp1"), not(feature = "risc0")))]
#[inline]
pub fn bn254_g1_mul(point: G1, scalar: U256) -> Result<Bytes, VMError> {
let x = ark_bn254::Fq::from_be_bytes_mod_order(&point.0.to_big_endian());
let y = ark_bn254::Fq::from_be_bytes_mod_order(&point.1.to_big_endian());

if x.is_zero() && y.is_zero() {
return Ok(Bytes::from([0u8; 64].to_vec()));
Expand All @@ -795,7 +797,7 @@ pub fn ecmul(calldata: &Bytes, gas_remaining: &mut u64, _fork: Fork) -> Result<B
return Err(PrecompileError::InvalidPoint.into());
}

let scalar = FrArk::from_be_bytes_mod_order(scalar);
let scalar = FrArk::from_be_bytes_mod_order(&scalar.to_big_endian());
if scalar.is_zero() {
return Ok(Bytes::from([0u8; 64].to_vec()));
}
Expand All @@ -811,6 +813,41 @@ pub fn ecmul(calldata: &Bytes, gas_remaining: &mut u64, _fork: Fork) -> Result<B
Ok(Bytes::from(out))
}

#[cfg(any(feature = "sp1", feature = "risc0"))]
#[inline]
pub fn bn254_g1_mul(g1: G1, scalar: U256) -> Result<Bytes, VMError> {
use substrate_bn::{AffineG1, Fq, Fr, G1, Group};

if g1.is_zero() || scalar.is_zero() {
return Ok(Bytes::from([0u8; 64].to_vec()));
}

let (g1_x, g1_y) = (
Fq::from_slice(&g1.0.to_big_endian()).map_err(|_| PrecompileError::ParsingInputError)?,
Fq::from_slice(&g1.1.to_big_endian()).map_err(|_| PrecompileError::ParsingInputError)?,
);

let g1 = AffineG1::new(g1_x, g1_y).map_err(|_| PrecompileError::InvalidPoint)?;

let scalar =
Fr::from_slice(&scalar.to_big_endian()).map_err(|_| PrecompileError::ParsingInputError)?;

#[cfg(feature = "risc0")]
// RISC0's substrate-bn patch does not implement Mul<Fr> for AffineG1, but
// it does implement Mul<Fr> for G1. So we convert AffineG1 to G1 first.
let result = G1::from(g1) * scalar;
#[cfg(feature = "sp1")]
let result = g1 * scalar;

let mut x_bytes = [0u8; 32];
let mut y_bytes = [0u8; 32];
result.x().to_big_endian(&mut x_bytes);
result.y().to_big_endian(&mut y_bytes);
let out = [x_bytes, y_bytes].concat();

Ok(Bytes::from(out))
}

const ALT_BN128_PRIME: U256 = U256([
0x3c208c16d87cfd47,
0x97816a916871ca8d,
Expand All @@ -833,38 +870,58 @@ impl G2 {
}
}

/// Parses 32 bytes as BN254 scalar
#[inline]
fn parse_bn254_coords(buf: &[u8; 192]) -> (G1, G2) {
let (g1_x, g1_y) = (
u256_from_big_endian(&buf[..32]),
u256_from_big_endian(&buf[32..64]),
);
let (g2_xy, g2_xx, g2_yy, g2_yx) = (
u256_from_big_endian(&buf[64..96]),
u256_from_big_endian(&buf[96..128]),
u256_from_big_endian(&buf[128..160]),
u256_from_big_endian(&buf[160..]),
);
fn parse_bn254_scalar(buf: &[u8], offset: usize) -> Option<U256> {
buf.get(offset..offset.checked_add(32)?)
.map(u256_from_big_endian)
}

/// Parses 64 bytes as a BN254 G1 point
#[inline]
fn parse_bn254_g1(buf: &[u8], offset: usize) -> Option<G1> {
let chunk = buf.get(offset..offset.checked_add(64)?)?;
let (x_bytes, y_bytes) = chunk.split_at_checked(32)?;
Some(G1(
u256_from_big_endian(x_bytes),
u256_from_big_endian(y_bytes),
))
}

(G1(g1_x, g1_y), G2(g2_xx, g2_xy, g2_yx, g2_yy))
/// Parses 128 bytes as a BN254 G2 point
fn parse_bn254_g2(buf: &[u8], offset: usize) -> Option<G2> {
let chunk = buf.get(offset..offset.checked_add(128)?)?;
let (g2_xy, rest) = chunk.split_at_checked(32)?;
let (g2_xx, rest) = rest.split_at_checked(32)?;
let (g2_yy, g2_yx) = rest.split_at_checked(32)?;
Some(G2(
u256_from_big_endian(g2_xx),
u256_from_big_endian(g2_xy),
u256_from_big_endian(g2_yx),
u256_from_big_endian(g2_yy),
))
}

#[inline]
fn validate_bn254_coords(g1: &G1, g2: &G2) -> Result<bool, VMError> {
fn validate_bn254_g1_coords(g1: &G1) -> Result<(), VMError> {
// check each element is in field
if g1.0 >= ALT_BN128_PRIME || g1.1 >= ALT_BN128_PRIME {
return Err(PrecompileError::CoordinateExceedsFieldModulus.into());
}
Ok(())
}

#[inline]
fn validate_bn254_g2_coords(g2: &G2) -> Result<(), VMError> {
// check each element is in field
if g2.0 >= ALT_BN128_PRIME
|| g2.1 >= ALT_BN128_PRIME
|| g2.2 >= ALT_BN128_PRIME
|| g2.3 >= ALT_BN128_PRIME
{
return Err(PrecompileError::CoordinateExceedsFieldModulus.into());
}

Ok(true)
Ok(())
}

/// Performs a bilinear pairing on points on the elliptic curve 'alt_bn128', returns 1 on success and 0 on failure
Expand All @@ -880,12 +937,12 @@ pub fn ecpairing(calldata: &Bytes, gas_remaining: &mut u64, _fork: Fork) -> Resu

let mut batch = Vec::new();
for input in calldata.chunks_exact(192) {
#[expect(unsafe_code, reason = "chunks_exact ensures the conversion is valid")]
let input: [u8; 192] = unsafe { input.try_into().unwrap_unchecked() };
let (g1, g2) = parse_bn254_coords(&input);
if validate_bn254_coords(&g1, &g2)? {
batch.push((g1, g2));
}
Comment on lines -883 to -888
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we remove this unsafe? We should double-check with @edg-l, @azteca1998, or @Oppen if this is not counterproductive.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we keep this behaviour when neither sp1 nor risc0 features are enabled?

Copy link
Contributor Author

@xqft xqft Oct 31, 2025

Choose a reason for hiding this comment

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

The function to parse bn254 points was changed to take a slice instead of a reference to a fixed size array (so it can be reused for precompiles whose input have different sizes), so this unsafe is not necessary anymore.

The branching on the error case (which should never happen) could signify a perf. hit, I could change this to an unreachable_unchecked!() and it should be equivalent to before the change, but I'm not sure how impactful that can be (consider that we have other, more evident optimization opportunities, like not cloning and slicing bytes many times while parsing and converting to the crypto primitive types)

let (Some(g1), Some(g2)) = (parse_bn254_g1(input, 0), parse_bn254_g2(input, 64)) else {
return Err(InternalError::Slicing.into());
};
validate_bn254_g1_coords(&g1)?;
validate_bn254_g2_coords(&g2)?;
batch.push((g1, g2));
}

let pairing_check = if batch.is_empty() {
Expand Down
Loading