diff --git a/soroban-env-host/benches/common/cost_types/bls12_381.rs b/soroban-env-host/benches/common/cost_types/bls12_381.rs index dbe888a2e..f4c07a20b 100644 --- a/soroban-env-host/benches/common/cost_types/bls12_381.rs +++ b/soroban-env-host/benches/common/cost_types/bls12_381.rs @@ -225,7 +225,7 @@ impl HostCostMeasurement for Bls12381MapFpToG1Measure { fn new_random_case(_host: &Host, rng: &mut StdRng, _input: u64) -> Bls12381MapFpToG1Sample { let fp = Fq::rand(rng); - Bls12381MapFpToG1Sample(fp) + Bls12381MapFpToG1Sample(fp, Bls12381MapFpToG1) } } @@ -242,7 +242,7 @@ impl HostCostMeasurement for Bls12381HashToG1Measure { .to_vec(); let mut msg = vec![0u8; len as usize]; rng.fill(msg.as_mut_slice()); - Bls12381HashToG1Sample(domain, msg) + Bls12381HashToG1Sample(domain, msg, Bls12381HashToG1) } } @@ -308,7 +308,7 @@ impl HostCostMeasurement for Bls12381MapFp2ToG2Measure { fn new_random_case(_host: &Host, rng: &mut StdRng, _input: u64) -> Bls12381MapFp2ToG2Sample { let fp2 = Fq2::rand(rng); - Bls12381MapFp2ToG2Sample(fp2) + Bls12381MapFp2ToG2Sample(fp2, Bls12381MapFp2ToG2) } } @@ -325,7 +325,7 @@ impl HostCostMeasurement for Bls12381HashToG2Measure { .to_vec(); let mut msg = vec![0u8; len as usize]; rng.fill(msg.as_mut_slice()); - Bls12381HashToG2Sample(domain, msg) + Bls12381HashToG2Sample(domain, msg, Bls12381HashToG2) } } diff --git a/soroban-env-host/src/cost_runner/cost_types/bls12_381.rs b/soroban-env-host/src/cost_runner/cost_types/bls12_381.rs index 06c1f3a5f..301791037 100644 --- a/soroban-env-host/src/cost_runner/cost_types/bls12_381.rs +++ b/soroban-env-host/src/cost_runner/cost_types/bls12_381.rs @@ -53,9 +53,9 @@ pub struct Bls12381G1MulSample(pub G1Affine, pub Fr); #[derive(Clone)] pub struct Bls12381G1MsmSample(pub Vec, pub Vec); #[derive(Clone)] -pub struct Bls12381MapFpToG1Sample(pub Fq); +pub struct Bls12381MapFpToG1Sample(pub Fq, pub ContractCostType); #[derive(Clone)] -pub struct Bls12381HashToG1Sample(pub Vec, pub Vec); +pub struct Bls12381HashToG1Sample(pub Vec, pub Vec, pub ContractCostType); #[derive(Clone)] pub struct Bls12381G2ProjectiveToAffineSample(pub G2Projective); #[derive(Clone)] @@ -65,9 +65,9 @@ pub struct Bls12381G2MulSample(pub G2Affine, pub Fr); #[derive(Clone)] pub struct Bls12381G2MsmSample(pub Vec, pub Vec); #[derive(Clone)] -pub struct Bls12381MapFp2ToG2Sample(pub Fq2); +pub struct Bls12381MapFp2ToG2Sample(pub Fq2, pub ContractCostType); #[derive(Clone)] -pub struct Bls12381HashToG2Sample(pub Vec, pub Vec); +pub struct Bls12381HashToG2Sample(pub Vec, pub Vec, pub ContractCostType); #[derive(Clone)] pub struct Bls12381PairingSample(pub Vec, pub Vec); #[derive(Clone)] @@ -122,10 +122,11 @@ impl_const_cost_runner_for_bls_consume_sample!( impl_const_cost_runner_for_bls_consume_sample!( Bls12381MapFpToG1Run, Bls12381MapFpToG1, - map_fp_to_g1_internal, + map_to_curve, Bls12381MapFpToG1Sample, G1Affine, - fq + fq, + ty ); impl_const_cost_runner_for_bls_consume_sample!( @@ -157,10 +158,11 @@ impl_const_cost_runner_for_bls_consume_sample!( impl_const_cost_runner_for_bls_consume_sample!( Bls12381MapFp2ToG2Run, Bls12381MapFp2ToG2, - map_fp2_to_g2_internal, + map_to_curve, Bls12381MapFp2ToG2Sample, G2Affine, - fq2 + fq2, + ty ); impl_const_cost_runner_for_bls_consume_sample!( Bls12381FrFromU256Run, @@ -182,20 +184,22 @@ impl_const_cost_runner_for_bls_consume_sample!( impl_lin_cost_runner_for_bls_deref_sample!( Bls12381HashToG1Run, Bls12381HashToG1, - hash_to_g1_internal, + hash_to_curve, Bls12381HashToG1Sample, G1Affine, domain, - msg + msg, + ty ); impl_lin_cost_runner_for_bls_deref_sample!( Bls12381HashToG2Run, Bls12381HashToG2, - hash_to_g2_internal, + hash_to_curve, Bls12381HashToG2Sample, G2Affine, domain, - msg + msg, + ty ); impl_lin_cost_runner_for_bls_deref_sample!( diff --git a/soroban-env-host/src/crypto/bls12_381.rs b/soroban-env-host/src/crypto/bls12_381.rs index c9aab90dc..c6554c698 100644 --- a/soroban-env-host/src/crypto/bls12_381.rs +++ b/soroban-env-host/src/crypto/bls12_381.rs @@ -6,18 +6,14 @@ use crate::{ U256Small, U256Val, Val, VecObject, U256, }; use ark_bls12_381::{ - g1, g2, Bls12_381, Fq, Fq12, Fq2, Fr, G1Affine, G1Projective, G2Affine, G2Projective, + Bls12_381, Fq, Fq12, Fq2, Fr, G1Affine, G1Projective, G2Affine, G2Projective, }; use ark_ec::{ hashing::{ - curve_maps::wb::WBMap, + curve_maps::wb::{WBConfig, WBMap}, map_to_curve_hasher::{MapToCurve, MapToCurveBasedHasher}, HashToCurve, - }, - pairing::{Pairing, PairingOutput}, - scalar_mul::variable_base::VariableBaseMSM, - short_weierstrass::{Affine, Projective, SWCurveConfig}, - CurveGroup, + }, pairing::{Pairing, PairingOutput}, scalar_mul::variable_base::VariableBaseMSM, short_weierstrass::{Affine, Projective, SWCurveConfig}, AffineRepr, CurveGroup }; use ark_ff::{field_hashers::DefaultFieldHasher, BigInteger, Field, PrimeField}; use ark_serialize::{CanonicalDeserialize, CanonicalSerialize, Compress, Validate}; @@ -475,55 +471,6 @@ impl Host { Ok(G1Projective::msm_unchecked(points, scalars)) } - pub(crate) fn map_fp_to_g1_internal(&self, fp: Fq) -> Result { - self.charge_budget(ContractCostType::Bls12381MapFpToG1, None)?; - let mapper = WBMap::::new().map_err(|e| { - self.err( - ScErrorType::Crypto, - ScErrorCode::InternalError, - format!("hash-to-curve error {e}").as_str(), - &[], - ) - })?; - mapper.map_to_curve(fp).map_err(|e| { - self.err( - ScErrorType::Crypto, - ScErrorCode::InternalError, - format!("hash-to-curve error {e}").as_str(), - &[], - ) - }) - } - - pub(crate) fn hash_to_g1_internal( - &self, - domain: &[u8], - msg: &[u8], - ) -> Result { - self.charge_budget(ContractCostType::Bls12381HashToG1, Some(msg.len() as u64))?; - let g1_mapper = MapToCurveBasedHasher::< - Projective, - DefaultFieldHasher, - WBMap, - >::new(domain) - .map_err(|e| { - self.err( - ScErrorType::Crypto, - ScErrorCode::InternalError, - format!("hash-to-curve error {e}").as_str(), - &[], - ) - })?; - g1_mapper.hash(msg.as_ref()).map_err(|e| { - self.err( - ScErrorType::Crypto, - ScErrorCode::InternalError, - format!("hash-to-curve error {e}").as_str(), - &[], - ) - }) - } - pub(crate) fn g2_vec_from_vecobj(&self, vp: VecObject) -> Result, HostError> { let len: u32 = self.vec_len(vp)?.into(); self.charge_budget( @@ -580,9 +527,26 @@ impl Host { Ok(G2Projective::msm_unchecked(points, scalars)) } - pub(crate) fn map_fp2_to_g2_internal(&self, fp: Fq2) -> Result { - self.charge_budget(ContractCostType::Bls12381MapFp2ToG2, None)?; - let mapper = WBMap::::new().map_err(|e| { + pub(crate) fn map_to_curve(&self, fp: as AffineRepr>::BaseField, ty: ContractCostType) -> Result, HostError> { + self.charge_budget(ty, None)?; + + // The `WBMap::new()` first calls + // `P::ISOGENY_MAP.apply(GENERATOR)` which returns error if the result + // point is not on curve. This should not happen if the map constants + // have been correctly defined. otherwise it would be an internal error + // since it's a bug in the library implementation. + // + // Then it returns `WBMap`, which wraps a `SWUMap

` where P is the + // `ark_bls12_381::curves::g2_swu_iso::SwuIsoConfig`. + // + // Potential panic condition: `SWUMap::new().unwrap()` + // + // The `SWUMap::new()` function performs some validation on the static + // parameters `ZETA`, `COEFF_A`, `COEFF_B`, all of which are statically + // defined in `ark_bls12_381::curves::g1_swu_iso` and `g2_swu_iso`. + // Realistically this panic cannot occur, otherwise it will panic every + // time including during tests + let mapper = WBMap::

::new().map_err(|e| { self.err( ScErrorType::Crypto, ScErrorCode::InternalError, @@ -590,6 +554,21 @@ impl Host { &[], ) })?; + + // The `SWUMap::map_to_curve` function contains several panic conditions + // 1. assert!(!div3.is_zero()) + // 2. gx1.sqrt().expect() + // 3. zeta_gx1.sqrt().expect() + // 4. assert!(point_on_curve.is_on_curve()) + // + // While all of these should theoretically just be debug assertions that + // can't happen if the map parameters are correctly defined (several of + // these have recently been downgraded to debug_assert, e.g see + // https://github.com/arkworks-rs/algebra/pull/659#discussion_r1450808159), + // we cannot guaruantee with 100% confidence these panics will never + // happen. + // + // Otherwise, this function should never Err. mapper.map_to_curve(fp).map_err(|e| { self.err( ScErrorType::Crypto, @@ -600,16 +579,24 @@ impl Host { }) } - pub(crate) fn hash_to_g2_internal( + pub(crate) fn hash_to_curve( &self, domain: &[u8], msg: &[u8], - ) -> Result { - self.charge_budget(ContractCostType::Bls12381HashToG2, Some(msg.len() as u64))?; + ty: &ContractCostType + ) -> Result, HostError> { + self.charge_budget(*ty, Some(msg.len() as u64))?; + + // The `new` function here constructs a DefaultFieldHasher and a WBMap. + // - The DefaultFieldHasher::new() function creates an ExpanderXmd with + // Sha256. This cannot fail or panic. + // - Construction of WBMap follows the exact same analysis as map_to_curve + // function earlier. + // This function cannot realistically produce an error or panic. let mapper = MapToCurveBasedHasher::< - Projective, + Projective

, DefaultFieldHasher, - WBMap, + WBMap

, >::new(domain) .map_err(|e| { self.err( @@ -619,6 +606,34 @@ impl Host { &[], ) })?; + + // `ark_ec::hashing::map_to_curve_hasher::MapToCurveBasedHasher::hash` + // contains the following calls + // - `DefaultFieldHasher::hash_to_field` + // - `SWUMap::map_to_curve` + // - `clear_cofactor`. This cannot fail or panic. + // + // `hash_to_field` calls the ExpanderXmd::expand function, there are two + // assertions on the length of bytes produced by the hash function. Both + // of these cannot happen because the output size can be computed + // analytically. Let's use G2: + // - `block_size = 384 (Fp bit size) + 128 (security padding) / 8 = 64` + // - `len_in_bytes = 2 (number of elements to produce) * 2 (extention + // degree of Fp2) * 64 (block_size) = 256` + // - `ell = 256 (len_in_bytes) / 32 (sha256 output size) = 8` + // + // # Assertion #1. ell <= 255, which is saying the expander cannot expand + // up to a certain length. in our case ell == 8. + // # Assertion #2. len_in_bytes < 2^16, which is clearly true as well. + // + // The rest is just hashing, dividing bytes into element size, and + // producing field elements from bytes. None of these can panic or + // error. + // + // The only panic conditions we cannot 100% exclude comes from + // `map_to_curve`, see previous analysis. + // + // This function should not Err. mapper.hash(msg.as_ref()).map_err(|e| { self.err( ScErrorType::Crypto, diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs index d519b82b1..a7be6f999 100644 --- a/soroban-env-host/src/host.rs +++ b/soroban-env-host/src/host.rs @@ -2978,7 +2978,7 @@ impl VmCallerEnv for Host { fp: BytesObject, ) -> Result { let fp = self.fp_deserialize_from_bytesobj(fp)?; - let g1 = self.map_fp_to_g1_internal(fp)?; + let g1 = self.map_to_curve(fp, ContractCostType::Bls12381MapFpToG1)?; self.g1_affine_serialize_uncompressed(g1) } @@ -3002,7 +3002,7 @@ impl VmCallerEnv for Host { &[], )); } - self.hash_to_g1_internal(dst.as_slice(), msg.as_slice()) + self.hash_to_curve(dst.as_slice(), msg.as_slice(), &ContractCostType::Bls12381HashToG1) }) })?; self.g1_affine_serialize_uncompressed(g1) @@ -3070,8 +3070,8 @@ impl VmCallerEnv for Host { fp2: BytesObject, ) -> Result { let fp2 = self.fp2_deserialize_from_bytesobj(fp2)?; - let g2 = self.map_fp2_to_g2_internal(fp2)?; - self.g2_affine_serialize_uncompressed(g2) + let g2 = self.map_to_curve(fp2, ContractCostType::Bls12381MapFp2ToG2)?; + self.g2_affine_serialize_uncompressed(&g2) } fn bls12_381_hash_to_g2( @@ -3094,7 +3094,7 @@ impl VmCallerEnv for Host { &[], )); } - self.hash_to_g2_internal(dst.as_slice(), msg.as_slice()) + self.hash_to_curve(dst.as_slice(), msg.as_slice(), &ContractCostType::Bls12381HashToG2) }) })?; self.g2_affine_serialize_uncompressed(g2)