From b8aeb942f0001e80dfd45b0320ed0b9d4806f3e2 Mon Sep 17 00:00:00 2001 From: Mablr <59505383+mablr@users.noreply.github.com> Date: Tue, 26 Aug 2025 23:51:11 +0200 Subject: [PATCH] refactor(precompile): add new specific `PrecompileError` variants In cases where it is feasible, more specific variants should be used in place of the Other() error variant. --- crates/precompile/src/bls12_381/arkworks.rs | 26 ++----- crates/precompile/src/bls12_381/blst.rs | 20 ++---- crates/precompile/src/bls12_381/g1_add.rs | 5 +- crates/precompile/src/bls12_381/g1_msm.rs | 11 +-- crates/precompile/src/bls12_381/g2_add.rs | 5 +- crates/precompile/src/bls12_381/g2_msm.rs | 4 +- .../precompile/src/bls12_381/map_fp2_to_g2.rs | 5 +- .../precompile/src/bls12_381/map_fp_to_g1.rs | 10 +-- crates/precompile/src/bls12_381/pairing.rs | 4 +- crates/precompile/src/bls12_381/utils.rs | 19 ++---- crates/precompile/src/interface.rs | 68 ++++++++++++++++++- .../src/kzg_point_evaluation/arkworks.rs | 8 +-- .../src/kzg_point_evaluation/blst.rs | 15 ++-- 13 files changed, 99 insertions(+), 101 deletions(-) diff --git a/crates/precompile/src/bls12_381/arkworks.rs b/crates/precompile/src/bls12_381/arkworks.rs index 4ca5573cbd..dea815f156 100644 --- a/crates/precompile/src/bls12_381/arkworks.rs +++ b/crates/precompile/src/bls12_381/arkworks.rs @@ -13,7 +13,7 @@ use ark_ec::{ use ark_ff::{One, PrimeField, Zero}; use ark_serialize::{CanonicalDeserialize, CanonicalSerialize}; -use std::{string::ToString, vec::Vec}; +use std::vec::Vec; /// Reads a single `Fp` field element from the input slice. /// @@ -34,8 +34,7 @@ fn read_fp(input_be: &[u8]) -> Result { // Reverse in-place to convert from big-endian to little-endian. input_le.reverse(); - Fq::deserialize_uncompressed(&input_le[..]) - .map_err(|_| PrecompileError::Other("non-canonical fp value".to_string())) + Fq::deserialize_uncompressed(&input_le[..]).map_err(|_| PrecompileError::NonCanonicalFp) } /// Encodes an `Fp` field element into a big-endian byte array. @@ -80,9 +79,7 @@ fn new_g1_point_no_subgroup_check(px: Fq, py: Fq) -> Result Result Result Result { let point = read_g1_no_subgroup_check(x, y)?; if !point.is_in_correct_subgroup_assuming_on_curve() { - return Err(PrecompileError::Other( - "Element not in the correct subgroup".to_string(), - )); + return Err(PrecompileError::Bls12381G1NotInSubgroup); } Ok(point) } @@ -186,9 +179,7 @@ fn read_g2( ) -> Result { let point = read_g2_no_subgroup_check(a_x_0, a_x_1, a_y_0, a_y_1)?; if !point.is_in_correct_subgroup_assuming_on_curve() { - return Err(PrecompileError::Other( - "Element not in the correct subgroup".to_string(), - )); + return Err(PrecompileError::Bls12381G1NotInSubgroup); } Ok(point) } @@ -244,10 +235,7 @@ fn encode_g2_point(input: &G2Affine) -> [u8; G2_LENGTH] { #[inline] fn read_scalar(input: &[u8]) -> Result { if input.len() != SCALAR_LENGTH { - return Err(PrecompileError::Other(format!( - "Input should be {SCALAR_LENGTH} bytes, was {}", - input.len() - ))); + return Err(PrecompileError::Bls12381ScalarInputLength); } Ok(Fr::from_be_bytes_mod_order(input)) diff --git a/crates/precompile/src/bls12_381/blst.rs b/crates/precompile/src/bls12_381/blst.rs index 747c695f4a..d20c7af561 100644 --- a/crates/precompile/src/bls12_381/blst.rs +++ b/crates/precompile/src/bls12_381/blst.rs @@ -14,7 +14,6 @@ use blst::{ blst_p2_affine, blst_p2_affine_in_g2, blst_p2_affine_on_curve, blst_p2_from_affine, blst_p2_mult, blst_p2_to_affine, blst_scalar, blst_scalar_from_bendian, MultiPoint, }; -use std::string::ToString; use std::vec::Vec; // Big-endian non-Montgomery form. @@ -382,9 +381,7 @@ fn decode_g1_on_curve( // // SAFETY: Out is a blst value. if unsafe { !blst_p1_affine_on_curve(&out) } { - return Err(PrecompileError::Other( - "Element not on G1 curve".to_string(), - )); + return Err(PrecompileError::Bls12381G1NotOnCurve); } Ok(out) @@ -436,7 +433,7 @@ fn _extract_g1_input( // As endomorphism acceleration requires input on the correct subgroup, implementers MAY // use endomorphism acceleration. if unsafe { !blst_p1_affine_in_g1(&out) } { - return Err(PrecompileError::Other("Element not in G1".to_string())); + return Err(PrecompileError::Bls12381G1NotInSubgroup); } } Ok(out) @@ -481,9 +478,7 @@ fn decode_g2_on_curve( // // SAFETY: Out is a blst value. if unsafe { !blst_p2_affine_on_curve(&out) } { - return Err(PrecompileError::Other( - "Element not on G2 curve".to_string(), - )); + return Err(PrecompileError::Bls12381G2NotOnCurve); } Ok(out) @@ -559,7 +554,7 @@ fn _extract_g2_input( // As endomorphism acceleration requires input on the correct subgroup, implementers MAY // use endomorphism acceleration. if unsafe { !blst_p2_affine_in_g2(&out) } { - return Err(PrecompileError::Other("Element not in G2".to_string())); + return Err(PrecompileError::Bls12381G2NotInSubgroup); } } Ok(out) @@ -571,7 +566,7 @@ fn _extract_g2_input( /// Note: The field element is expected to be in big endian format. fn read_fp(input: &[u8; FP_LENGTH]) -> Result { if !is_valid_be(input) { - return Err(PrecompileError::Other("non-canonical fp value".to_string())); + return Err(PrecompileError::NonCanonicalFp); } let mut fp = blst_fp::default(); // SAFETY: `input` has fixed length, and `fp` is a blst value. @@ -595,10 +590,7 @@ fn read_fp(input: &[u8; FP_LENGTH]) -> Result { /// `q`. fn read_scalar(input: &[u8]) -> Result { if input.len() != SCALAR_LENGTH { - return Err(PrecompileError::Other(format!( - "Input should be {SCALAR_LENGTH} bytes, was {}", - input.len() - ))); + return Err(PrecompileError::Bls12381ScalarInputLength); } let mut out = blst_scalar::default(); diff --git a/crates/precompile/src/bls12_381/g1_add.rs b/crates/precompile/src/bls12_381/g1_add.rs index 3b2dd2e59d..07e0112321 100644 --- a/crates/precompile/src/bls12_381/g1_add.rs +++ b/crates/precompile/src/bls12_381/g1_add.rs @@ -22,10 +22,7 @@ pub fn g1_add(input: &[u8], gas_limit: u64) -> PrecompileResult { } if input.len() != G1_ADD_INPUT_LENGTH { - return Err(PrecompileError::Other(format!( - "G1ADD input should be {G1_ADD_INPUT_LENGTH} bytes, was {}", - input.len() - ))); + return Err(PrecompileError::Bls12381G1AddInputLength); } // Extract coordinates from padded input diff --git a/crates/precompile/src/bls12_381/g1_msm.rs b/crates/precompile/src/bls12_381/g1_msm.rs index c4840cfe1c..7a2d07505c 100644 --- a/crates/precompile/src/bls12_381/g1_msm.rs +++ b/crates/precompile/src/bls12_381/g1_msm.rs @@ -25,9 +25,7 @@ pub const PRECOMPILE: Precompile = pub fn g1_msm(input: &[u8], gas_limit: u64) -> PrecompileResult { let input_len = input.len(); if input_len == 0 || !input_len.is_multiple_of(G1_MSM_INPUT_LENGTH) { - return Err(PrecompileError::Other(format!( - "G1MSM input length should be multiple of {G1_MSM_INPUT_LENGTH}, was {input_len}", - ))); + return Err(PrecompileError::Bls12381G1MsmInputLength); } let k = input_len / G1_MSM_INPUT_LENGTH; @@ -66,11 +64,6 @@ mod test { fn bls_g1multiexp_g1_not_on_curve_but_in_subgroup() { let input = Bytes::from(hex!("000000000000000000000000000000000a2833e497b38ee3ca5c62828bf4887a9f940c9e426c7890a759c20f248c23a7210d2432f4c98a514e524b5184a0ddac00000000000000000000000000000000150772d56bf9509469f9ebcd6e47570429fd31b0e262b66d512e245c38ec37255529f2271fd70066473e393a8bead0c30000000000000000000000000000000000000000000000000000000000000000")); let fail = g1_msm(&input, G1_MSM_BASE_GAS_FEE); - assert_eq!( - fail, - Err(PrecompileError::Other( - "Element not on G1 curve".to_string() - )) - ); + assert_eq!(fail, Err(PrecompileError::Bls12381G1NotOnCurve)); } } diff --git a/crates/precompile/src/bls12_381/g2_add.rs b/crates/precompile/src/bls12_381/g2_add.rs index d3af54f4c3..63cb6b6a51 100644 --- a/crates/precompile/src/bls12_381/g2_add.rs +++ b/crates/precompile/src/bls12_381/g2_add.rs @@ -23,10 +23,7 @@ pub fn g2_add(input: &[u8], gas_limit: u64) -> PrecompileResult { } if input.len() != G2_ADD_INPUT_LENGTH { - return Err(PrecompileError::Other(format!( - "G2ADD input should be {G2_ADD_INPUT_LENGTH} bytes, was {}", - input.len() - ))); + return Err(PrecompileError::Bls12381G2AddInputLength); } // Extract coordinates from padded input diff --git a/crates/precompile/src/bls12_381/g2_msm.rs b/crates/precompile/src/bls12_381/g2_msm.rs index eaa2c4f4f7..a37a7df623 100644 --- a/crates/precompile/src/bls12_381/g2_msm.rs +++ b/crates/precompile/src/bls12_381/g2_msm.rs @@ -24,9 +24,7 @@ pub const PRECOMPILE: Precompile = pub fn g2_msm(input: &[u8], gas_limit: u64) -> PrecompileResult { let input_len = input.len(); if input_len == 0 || !input_len.is_multiple_of(G2_MSM_INPUT_LENGTH) { - return Err(PrecompileError::Other(format!( - "G2MSM input length should be multiple of {G2_MSM_INPUT_LENGTH}, was {input_len}", - ))); + return Err(PrecompileError::Bls12381G2MsmInputLength); } let k = input_len / G2_MSM_INPUT_LENGTH; diff --git a/crates/precompile/src/bls12_381/map_fp2_to_g2.rs b/crates/precompile/src/bls12_381/map_fp2_to_g2.rs index 847f9638d6..f16cb8cb99 100644 --- a/crates/precompile/src/bls12_381/map_fp2_to_g2.rs +++ b/crates/precompile/src/bls12_381/map_fp2_to_g2.rs @@ -24,10 +24,7 @@ pub fn map_fp2_to_g2(input: &[u8], gas_limit: u64) -> PrecompileResult { } if input.len() != PADDED_FP2_LENGTH { - return Err(PrecompileError::Other(format!( - "MAP_FP2_TO_G2 input should be {PADDED_FP2_LENGTH} bytes, was {}", - input.len() - ))); + return Err(PrecompileError::Bls12381MapFp2ToG2InputLength); } let input_p0_x = remove_fp_padding(&input[..PADDED_FP_LENGTH])?; diff --git a/crates/precompile/src/bls12_381/map_fp_to_g1.rs b/crates/precompile/src/bls12_381/map_fp_to_g1.rs index 64a8f11393..1babc3ae8a 100644 --- a/crates/precompile/src/bls12_381/map_fp_to_g1.rs +++ b/crates/precompile/src/bls12_381/map_fp_to_g1.rs @@ -21,10 +21,7 @@ pub fn map_fp_to_g1(input: &[u8], gas_limit: u64) -> PrecompileResult { } if input.len() != PADDED_FP_LENGTH { - return Err(PrecompileError::Other(format!( - "MAP_FP_TO_G1 input should be {PADDED_FP_LENGTH} bytes, was {}", - input.len() - ))); + return Err(PrecompileError::Bls12381MapFpToG1InputLength); } let input_p0 = remove_fp_padding(input)?; @@ -49,9 +46,6 @@ mod test { fn sanity_test() { let input = Bytes::from(hex!("000000000000000000000000000000006900000000000000636f6e7472616374595a603f343061cd305a03f40239f5ffff31818185c136bc2595f2aa18e08f17")); let fail = map_fp_to_g1(&input, MAP_FP_TO_G1_BASE_GAS_FEE); - assert_eq!( - fail, - Err(PrecompileError::Other("non-canonical fp value".to_string())) - ); + assert_eq!(fail, Err(PrecompileError::NonCanonicalFp)); } } diff --git a/crates/precompile/src/bls12_381/pairing.rs b/crates/precompile/src/bls12_381/pairing.rs index 4cf4c3f5b5..654d0865f9 100644 --- a/crates/precompile/src/bls12_381/pairing.rs +++ b/crates/precompile/src/bls12_381/pairing.rs @@ -30,9 +30,7 @@ pub const PRECOMPILE: Precompile = pub fn pairing(input: &[u8], gas_limit: u64) -> PrecompileResult { let input_len = input.len(); if input_len == 0 || !input_len.is_multiple_of(PAIRING_INPUT_LENGTH) { - return Err(PrecompileError::Other(format!( - "Pairing input length should be multiple of {PAIRING_INPUT_LENGTH}, was {input_len}" - ))); + return Err(PrecompileError::Bls12381PairingInputLength); } let k = input_len / PAIRING_INPUT_LENGTH; diff --git a/crates/precompile/src/bls12_381/utils.rs b/crates/precompile/src/bls12_381/utils.rs index c53f0cae5e..335ceda278 100644 --- a/crates/precompile/src/bls12_381/utils.rs +++ b/crates/precompile/src/bls12_381/utils.rs @@ -7,16 +7,11 @@ use crate::PrecompileError; /// Removes zeros with which the precompile inputs are left padded to 64 bytes. pub(super) fn remove_fp_padding(input: &[u8]) -> Result<&[u8; FP_LENGTH], PrecompileError> { if input.len() != PADDED_FP_LENGTH { - return Err(PrecompileError::Other(format!( - "Padded input should be {PADDED_FP_LENGTH} bytes, was {}", - input.len() - ))); + return Err(PrecompileError::Bls12381FpPaddingLength); } let (padding, unpadded) = input.split_at(FP_PAD_BY); if !padding.iter().all(|&x| x == 0) { - return Err(PrecompileError::Other(format!( - "{FP_PAD_BY} top bytes of input are not zero", - ))); + return Err(PrecompileError::Bls12381FpPaddingInvalid); } Ok(unpadded.try_into().unwrap()) } @@ -24,10 +19,7 @@ pub(super) fn remove_fp_padding(input: &[u8]) -> Result<&[u8; FP_LENGTH], Precom /// encoded G1 element. pub(super) fn remove_g1_padding(input: &[u8]) -> Result<[&[u8; FP_LENGTH]; 2], PrecompileError> { if input.len() != PADDED_G1_LENGTH { - return Err(PrecompileError::Other(format!( - "Input should be {PADDED_G1_LENGTH} bytes, was {}", - input.len() - ))); + return Err(PrecompileError::Bls12381G1PaddingLength); } let x = remove_fp_padding(&input[..PADDED_FP_LENGTH])?; @@ -39,10 +31,7 @@ pub(super) fn remove_g1_padding(input: &[u8]) -> Result<[&[u8; FP_LENGTH]; 2], P /// encoded G2 element. pub(super) fn remove_g2_padding(input: &[u8]) -> Result<[&[u8; FP_LENGTH]; 4], PrecompileError> { if input.len() != PADDED_G2_LENGTH { - return Err(PrecompileError::Other(format!( - "Input should be {PADDED_G2_LENGTH} bytes, was {}", - input.len() - ))); + return Err(PrecompileError::Bls12381G2PaddingLength); } let mut input_fps = [&[0; FP_LENGTH]; 4]; diff --git a/crates/precompile/src/interface.rs b/crates/precompile/src/interface.rs index cac543fffd..f4651c7a63 100644 --- a/crates/precompile/src/interface.rs +++ b/crates/precompile/src/interface.rs @@ -115,7 +115,7 @@ pub trait Crypto: Send + Sync + Debug { msg: &[u8; 32], ) -> Result<[u8; 32], PrecompileError> { crate::secp256k1::ecrecover_bytes(*sig, recid, *msg) - .ok_or_else(|| PrecompileError::other("ecrecover failed")) + .ok_or(PrecompileError::Secp256k1RecoverFailed) } /// Modular exponentiation. @@ -230,6 +230,50 @@ pub enum PrecompileError { BlobMismatchedVersion, /// The proof verification failed BlobVerifyKzgProofFailed, + /// Non-canonical field element + NonCanonicalFp, + /// BLS12-381 G1 point not on curve + Bls12381G1NotOnCurve, + /// BLS12-381 G1 point not in correct subgroup + Bls12381G1NotInSubgroup, + /// BLS12-381 G2 point not on curve + Bls12381G2NotOnCurve, + /// BLS12-381 G2 point not in correct subgroup + Bls12381G2NotInSubgroup, + /// BLS12-381 scalar input length error + Bls12381ScalarInputLength, + /// BLS12-381 G1 add input length error + Bls12381G1AddInputLength, + /// BLS12-381 G1 msm input length error + Bls12381G1MsmInputLength, + /// BLS12-381 G2 add input length error + Bls12381G2AddInputLength, + /// BLS12-381 G2 msm input length error + Bls12381G2MsmInputLength, + /// BLS12-381 pairing input length error + Bls12381PairingInputLength, + /// BLS12-381 map fp to g1 input length error + Bls12381MapFpToG1InputLength, + /// BLS12-381 map fp2 to g2 input length error + Bls12381MapFp2ToG2InputLength, + /// BLS12-381 padding error + Bls12381FpPaddingInvalid, + /// BLS12-381 fp padding length error + Bls12381FpPaddingLength, + /// BLS12-381 g1 padding length error + Bls12381G1PaddingLength, + /// BLS12-381 g2 padding length error + Bls12381G2PaddingLength, + /// KZG invalid G1 point + KzgInvalidG1Point, + /// KZG G1 point not on curve + KzgG1PointNotOnCurve, + /// KZG G1 point not in correct subgroup + KzgG1PointNotInSubgroup, + /// KZG input length error + KzgInvalidInputLength, + /// secp256k1 ecrecover failed + Secp256k1RecoverFailed, /// Fatal error with a custom error message Fatal(String), /// Catch-all variant for other errors @@ -266,6 +310,28 @@ impl fmt::Display for PrecompileError { Self::BlobInvalidInputLength => "invalid blob input length", Self::BlobMismatchedVersion => "mismatched blob version", Self::BlobVerifyKzgProofFailed => "verifying blob kzg proof failed", + Self::NonCanonicalFp => "non-canonical field element", + Self::Bls12381G1NotOnCurve => "bls12-381 g1 point not on curve", + Self::Bls12381G1NotInSubgroup => "bls12-381 g1 point not in correct subgroup", + Self::Bls12381G2NotOnCurve => "bls12-381 g2 point not on curve", + Self::Bls12381G2NotInSubgroup => "bls12-381 g2 point not in correct subgroup", + Self::Bls12381ScalarInputLength => "bls12-381 scalar input length error", + Self::Bls12381G1AddInputLength => "bls12-381 g1 add input length error", + Self::Bls12381G1MsmInputLength => "bls12-381 g1 msm input length error", + Self::Bls12381G2AddInputLength => "bls12-381 g2 add input length error", + Self::Bls12381G2MsmInputLength => "bls12-381 g2 msm input length error", + Self::Bls12381PairingInputLength => "bls12-381 pairing input length error", + Self::Bls12381MapFpToG1InputLength => "bls12-381 map fp to g1 input length error", + Self::Bls12381MapFp2ToG2InputLength => "bls12-381 map fp2 to g2 input length error", + Self::Bls12381FpPaddingInvalid => "bls12-381 fp 64 top bytes of input are not zero", + Self::Bls12381FpPaddingLength => "bls12-381 fp padding length error", + Self::Bls12381G1PaddingLength => "bls12-381 g1 padding length error", + Self::Bls12381G2PaddingLength => "bls12-381 g2 padding length error", + Self::KzgInvalidG1Point => "kzg invalid g1 point", + Self::KzgG1PointNotOnCurve => "kzg g1 point not on curve", + Self::KzgG1PointNotInSubgroup => "kzg g1 point not in correct subgroup", + Self::KzgInvalidInputLength => "kzg invalid input length", + Self::Secp256k1RecoverFailed => "secp256k1 signature recovery failed", Self::Fatal(s) => s, Self::Other(s) => s, }; diff --git a/crates/precompile/src/kzg_point_evaluation/arkworks.rs b/crates/precompile/src/kzg_point_evaluation/arkworks.rs index 2f23d2481e..fa30382100 100644 --- a/crates/precompile/src/kzg_point_evaluation/arkworks.rs +++ b/crates/precompile/src/kzg_point_evaluation/arkworks.rs @@ -7,7 +7,6 @@ use ark_ec::{AffineRepr, CurveGroup}; use ark_ff::{BigInteger, PrimeField}; use ark_serialize::CanonicalDeserialize; use core::ops::Neg; -use std::string::ToString; /// Verify KZG proof using BLS12-381 implementation. /// @@ -72,8 +71,7 @@ fn get_trusted_setup_g2() -> G2Affine { /// Parse a G1 point from compressed format (48 bytes) fn parse_g1_compressed(bytes: &[u8; 48]) -> Result { - G1Affine::deserialize_compressed(&bytes[..]) - .map_err(|_| PrecompileError::Other("Invalid compressed G1 point".to_string())) + G1Affine::deserialize_compressed(&bytes[..]).map_err(|_| PrecompileError::KzgInvalidG1Point) } /// Read a scalar field element from bytes and verify it's canonical @@ -84,9 +82,7 @@ fn read_scalar_canonical(bytes: &[u8; 32]) -> Result { let bytes_roundtrip = fr.into_bigint().to_bytes_be(); if bytes_roundtrip.as_slice() != bytes { - return Err(PrecompileError::Other( - "Non-canonical scalar field element".to_string(), - )); + return Err(PrecompileError::NonCanonicalFp); } Ok(fr) diff --git a/crates/precompile/src/kzg_point_evaluation/blst.rs b/crates/precompile/src/kzg_point_evaluation/blst.rs index fc13973e3d..381996d3cd 100644 --- a/crates/precompile/src/kzg_point_evaluation/blst.rs +++ b/crates/precompile/src/kzg_point_evaluation/blst.rs @@ -9,7 +9,6 @@ use ::blst::{ blst_p1_affine, blst_p1_affine_in_g1, blst_p1_affine_on_curve, blst_p2_affine, blst_scalar, blst_scalar_fr_check, blst_scalar_from_bendian, }; -use std::string::ToString; /// Verify KZG proof using BLST BLS12-381 implementation. /// @@ -94,21 +93,17 @@ fn parse_g1_compressed(bytes: &[u8; 48]) -> Result Result