Skip to content

chore(l2): implement ecadd with substrate for ZisK and SP1 #5535

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
xqft wants to merge 1 commit into
base: main
Choose a base branch
from
Open

chore(l2): implement ecadd with substrate for ZisK and SP1 #5535

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 96 additions & 16 deletions crates/vm/levm/src/precompiles.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -707,23 +707,23 @@ pub fn ecadd(calldata: &Bytes, gas_remaining: &mut u64, _fork: Fork) -> Result<B

increase_precompile_consumed_gas(ECADD_COST, gas_remaining)?;

let first_point_x = calldata.get(0..32).ok_or(InternalError::Slicing)?;
let first_point_y = calldata.get(32..64).ok_or(InternalError::Slicing)?;
let second_point_x = calldata.get(64..96).ok_or(InternalError::Slicing)?;
let second_point_y = calldata.get(96..128).ok_or(InternalError::Slicing)?;

if u256_from_big_endian(first_point_x) >= ALT_BN128_PRIME
|| u256_from_big_endian(first_point_y) >= ALT_BN128_PRIME
|| u256_from_big_endian(second_point_x) >= ALT_BN128_PRIME
|| u256_from_big_endian(second_point_y) >= ALT_BN128_PRIME
{
return Err(PrecompileError::InvalidPoint.into());
}
let (Some(first_point), Some(second_point)) =
(parse_bn254_g1(&calldata, 0), parse_bn254_g1(&calldata, 64))
else {
return Err(InternalError::Slicing.into());
};
validate_bn254_g1_coords(&first_point)?;
validate_bn254_g1_coords(&second_point)?;
bn254_g1_add(first_point, second_point)
}

let first_point_x = ark_bn254::Fq::from_be_bytes_mod_order(first_point_x);
let first_point_y = ark_bn254::Fq::from_be_bytes_mod_order(first_point_y);
let second_point_x = ark_bn254::Fq::from_be_bytes_mod_order(second_point_x);
let second_point_y = ark_bn254::Fq::from_be_bytes_mod_order(second_point_y);
#[cfg(not(any(feature = "sp1", feature = "zisk")))]
#[inline]
pub fn bn254_g1_add(first_point: G1, second_point: G1) -> Result<Bytes, VMError> {
let first_point_x = ark_bn254::Fq::from_be_bytes_mod_order(&first_point.0.to_big_endian());
let first_point_y = ark_bn254::Fq::from_be_bytes_mod_order(&first_point.1.to_big_endian());
let second_point_x = ark_bn254::Fq::from_be_bytes_mod_order(&second_point.0.to_big_endian());
let second_point_y = ark_bn254::Fq::from_be_bytes_mod_order(&second_point.1.to_big_endian());

let first_point_is_zero = first_point_x.is_zero() && first_point_y.is_zero();
let second_point_is_zero = second_point_x.is_zero() && second_point_y.is_zero();
Expand Down Expand Up @@ -773,6 +773,86 @@ pub fn ecadd(calldata: &Bytes, gas_remaining: &mut u64, _fork: Fork) -> Result<B
Ok(Bytes::from(out))
}

#[cfg(any(feature = "sp1", feature = "zisk"))]
#[inline]
pub fn bn254_g1_add(first_point: G1, second_point: G1) -> Result<Bytes, VMError> {
use substrate_bn::{AffineG1, Fq, G1, Group};
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

[nitpick] The import of substrate_bn::G1 shadows the custom G1 struct defined at line 946. While this works because the import is scoped to this function, it could be confusing for readers. Consider using a qualified import or type alias for clarity:

use substrate_bn::{AffineG1, Fq, Group};
use substrate_bn::G1 as SubstrateG1;

Then use SubstrateG1 instead of G1 in the function body (lines 833, 837).

Copilot uses AI. Check for mistakes.

if first_point.is_zero() && second_point.is_zero() {
return Ok(Bytes::from([0u8; 64].to_vec()));
}

if first_point.is_zero() {
let (g1_x, g1_y) = (
Fq::from_slice(&second_point.0.to_big_endian())
.map_err(|_| PrecompileError::ParsingInputError)?,
Fq::from_slice(&second_point.1.to_big_endian())
.map_err(|_| PrecompileError::ParsingInputError)?,
);
// Validate that the point is on the curve
AffineG1::new(g1_x, g1_y).map_err(|_| PrecompileError::InvalidPoint)?;

let mut x_bytes = [0u8; 32];
let mut y_bytes = [0u8; 32];
g1_x.to_big_endian(&mut x_bytes);
g1_y.to_big_endian(&mut y_bytes);
return Ok(Bytes::from([x_bytes, y_bytes].concat()));
}

if second_point.is_zero() {
let (g1_x, g1_y) = (
Fq::from_slice(&first_point.0.to_big_endian())
.map_err(|_| PrecompileError::ParsingInputError)?,
Fq::from_slice(&first_point.1.to_big_endian())
.map_err(|_| PrecompileError::ParsingInputError)?,
);
// Validate that the point is on the curve
AffineG1::new(g1_x, g1_y).map_err(|_| PrecompileError::InvalidPoint)?;

let mut x_bytes = [0u8; 32];
let mut y_bytes = [0u8; 32];
g1_x.to_big_endian(&mut x_bytes);
g1_y.to_big_endian(&mut y_bytes);
return Ok(Bytes::from([x_bytes, y_bytes].concat()));
}
Comment on lines +785 to +817
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

The logic for handling first_point.is_zero() (lines 785-800) and second_point.is_zero() (lines 802-817) is duplicated. This could be extracted into a helper function that converts a G1 point to bytes while validating it's on the curve. This would reduce code duplication and improve maintainability.

Example:

fn g1_point_to_bytes(point: &G1) -> Result<Bytes, VMError> {
    let (g1_x, g1_y) = (
        Fq::from_slice(&point.0.to_big_endian())
            .map_err(|_| PrecompileError::ParsingInputError)?,
        Fq::from_slice(&point.1.to_big_endian())
            .map_err(|_| PrecompileError::ParsingInputError)?,
    );
    // Validate that the point is on the curve
    AffineG1::new(g1_x, g1_y).map_err(|_| PrecompileError::InvalidPoint)?;
    
    let mut x_bytes = [0u8; 32];
    let mut y_bytes = [0u8; 32];
    g1_x.to_big_endian(&mut x_bytes);
    g1_y.to_big_endian(&mut y_bytes);
    Ok(Bytes::from([x_bytes, y_bytes].concat()))
}

Then use:

if first_point.is_zero() {
    return g1_point_to_bytes(&second_point);
}
if second_point.is_zero() {
    return g1_point_to_bytes(&first_point);
}

Copilot uses AI. Check for mistakes.

let (first_x, first_y) = (
Fq::from_slice(&first_point.0.to_big_endian())
.map_err(|_| PrecompileError::ParsingInputError)?,
Fq::from_slice(&first_point.1.to_big_endian())
.map_err(|_| PrecompileError::ParsingInputError)?,
);

let (second_x, second_y) = (
Fq::from_slice(&second_point.0.to_big_endian())
.map_err(|_| PrecompileError::ParsingInputError)?,
Fq::from_slice(&second_point.1.to_big_endian())
.map_err(|_| PrecompileError::ParsingInputError)?,
);

let first: G1 = AffineG1::new(first_x, first_y)
.map_err(|_| PrecompileError::InvalidPoint)?
.into();

let second: G1 = AffineG1::new(second_x, second_y)
.map_err(|_| PrecompileError::InvalidPoint)?
.into();

#[allow(
clippy::arithmetic_side_effects,
reason = "G1 addition doesn't overflow, intermediate operations that could overflow should be handled correctly by the library"
)]
let result = first + second;

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))
}

/// Makes a scalar multiplication on the elliptic curve 'alt_bn128'
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
Expand Down