From 16d86344390f62521f8e97dac06f54aec9fadc51 Mon Sep 17 00:00:00 2001 From: Emanuele Cesena Date: Tue, 2 Apr 2024 19:09:54 -0500 Subject: [PATCH] Simd 129: alt_bn128 syscalls - simplified error code (#294) * alt_bn128: simplify errors in sycalls (alt_bn128, compress, poseidon) * add TODO for feature gate. remove validate from compress * add feature gate * fix one more error case * all changes under feature gate * revert removing from() * return unexpected errors in lib * add comment to remove error types, once the feature gate is activated * remove unnecessary/impossible error * fix mispelled comments (cherry picked from commit 64260fc831d7459fa1fd45c67b4a8f5ba40f4be9) --- programs/bpf_loader/src/syscalls/mod.rs | 53 ++++++++++++++++++++---- sdk/program/src/alt_bn128/compression.rs | 26 +++++++----- sdk/program/src/alt_bn128/mod.rs | 11 +++-- sdk/program/src/poseidon.rs | 4 +- 4 files changed, 72 insertions(+), 22 deletions(-) diff --git a/programs/bpf_loader/src/syscalls/mod.rs b/programs/bpf_loader/src/syscalls/mod.rs index 4a166fa1cf9996..6d500528d5e236 100644 --- a/programs/bpf_loader/src/syscalls/mod.rs +++ b/programs/bpf_loader/src/syscalls/mod.rs @@ -1573,14 +1573,24 @@ declare_builtin_function!( } }; + let simplify_alt_bn128_syscall_error_codes = invoke_context + .feature_set + .is_active(&feature_set::simplify_alt_bn128_syscall_error_codes::id()); + let result_point = match calculation(input) { Ok(result_point) => result_point, Err(e) => { - return Ok(e.into()); + return if simplify_alt_bn128_syscall_error_codes { + Ok(1) + } else { + Ok(e.into()) + }; } }; - if result_point.len() != output { + // This can never happen and should be removed when the + // simplify_alt_bn128_syscall_error_codes feature gets activated + if result_point.len() != output && !simplify_alt_bn128_syscall_error_codes { return Ok(AltBn128Error::SliceOutOfBounds.into()); } @@ -1720,10 +1730,19 @@ declare_builtin_function!( ) }) .collect::, Error>>()?; + + let simplify_alt_bn128_syscall_error_codes = invoke_context + .feature_set + .is_active(&feature_set::simplify_alt_bn128_syscall_error_codes::id()); + let hash = match poseidon::hashv(parameters, endianness, inputs.as_slice()) { Ok(hash) => hash, Err(e) => { - return Ok(e.into()); + return if simplify_alt_bn128_syscall_error_codes { + Ok(1) + } else { + Ok(e.into()) + }; } }; hash_result.copy_from_slice(&hash.to_bytes()); @@ -1807,12 +1826,20 @@ declare_builtin_function!( invoke_context.get_check_aligned(), )?; + let simplify_alt_bn128_syscall_error_codes = invoke_context + .feature_set + .is_active(&feature_set::simplify_alt_bn128_syscall_error_codes::id()); + match op { ALT_BN128_G1_COMPRESS => { let result_point = match alt_bn128_g1_compress(input) { Ok(result_point) => result_point, Err(e) => { - return Ok(e.into()); + return if simplify_alt_bn128_syscall_error_codes { + Ok(1) + } else { + Ok(e.into()) + }; } }; call_result.copy_from_slice(&result_point); @@ -1822,7 +1849,11 @@ declare_builtin_function!( let result_point = match alt_bn128_g1_decompress(input) { Ok(result_point) => result_point, Err(e) => { - return Ok(e.into()); + return if simplify_alt_bn128_syscall_error_codes { + Ok(1) + } else { + Ok(e.into()) + }; } }; call_result.copy_from_slice(&result_point); @@ -1832,7 +1863,11 @@ declare_builtin_function!( let result_point = match alt_bn128_g2_compress(input) { Ok(result_point) => result_point, Err(e) => { - return Ok(e.into()); + return if simplify_alt_bn128_syscall_error_codes { + Ok(1) + } else { + Ok(e.into()) + }; } }; call_result.copy_from_slice(&result_point); @@ -1842,7 +1877,11 @@ declare_builtin_function!( let result_point = match alt_bn128_g2_decompress(input) { Ok(result_point) => result_point, Err(e) => { - return Ok(e.into()); + return if simplify_alt_bn128_syscall_error_codes { + Ok(1) + } else { + Ok(e.into()) + }; } }; call_result.copy_from_slice(&result_point); diff --git a/sdk/program/src/alt_bn128/compression.rs b/sdk/program/src/alt_bn128/compression.rs index 2791b8fd35f8f5..0b63b202d4d854 100644 --- a/sdk/program/src/alt_bn128/compression.rs +++ b/sdk/program/src/alt_bn128/compression.rs @@ -20,6 +20,8 @@ mod alt_bn128_compression_size { pub const G2_COMPRESSED: usize = 64; } +// AltBn128CompressionError must be removed once the +// simplify_alt_bn128_syscall_error_codes feature gets activated #[derive(Debug, Error, Clone, PartialEq, Eq)] pub enum AltBn128CompressionError { #[error("Unexpected error")] @@ -51,13 +53,14 @@ impl From for AltBn128CompressionError { impl From for u64 { fn from(v: AltBn128CompressionError) -> u64 { + // note: should never return 0, as it risks to be confused with syscall success match v { AltBn128CompressionError::G1DecompressionFailed => 1, AltBn128CompressionError::G2DecompressionFailed => 2, AltBn128CompressionError::G1CompressionFailed => 3, AltBn128CompressionError::G2CompressionFailed => 4, AltBn128CompressionError::InvalidInputSize => 5, - AltBn128CompressionError::UnexpectedError => 0, + AltBn128CompressionError::UnexpectedError => 6, } } } @@ -118,7 +121,7 @@ mod target_arch { .map_err(|_| AltBn128CompressionError::G1CompressionFailed)?; let mut g1_bytes = [0u8; alt_bn128_compression_size::G1_COMPRESSED]; G1::serialize_compressed(&g1, g1_bytes.as_mut_slice()) - .map_err(|_| AltBn128CompressionError::G2CompressionFailed)?; + .map_err(|_| AltBn128CompressionError::G1CompressionFailed)?; Ok(convert_endianness::<32, 32>(&g1_bytes)) } @@ -131,9 +134,12 @@ mod target_arch { if g2_bytes == [0u8; alt_bn128_compression_size::G2_COMPRESSED] { return Ok([0u8; alt_bn128_compression_size::G2]); } - let decompressed_g2 = - G2::deserialize_compressed(convert_endianness::<64, 64>(&g2_bytes).as_slice()) - .map_err(|_| AltBn128CompressionError::G2DecompressionFailed)?; + let decompressed_g2 = G2::deserialize_with_mode( + convert_endianness::<64, 64>(&g2_bytes).as_slice(), + Compress::Yes, + Validate::No, + ) + .map_err(|_| AltBn128CompressionError::G2DecompressionFailed)?; let mut decompressed_g2_bytes = [0u8; alt_bn128_compression_size::G2]; decompressed_g2 .x @@ -160,7 +166,7 @@ mod target_arch { Compress::No, Validate::No, ) - .map_err(|_| AltBn128CompressionError::G2DecompressionFailed)?; + .map_err(|_| AltBn128CompressionError::G2CompressionFailed)?; let mut g2_bytes = [0u8; alt_bn128_compression_size::G2_COMPRESSED]; G2::serialize_compressed(&g2, g2_bytes.as_mut_slice()) .map_err(|_| AltBn128CompressionError::G2CompressionFailed)?; @@ -205,7 +211,7 @@ mod target_arch { match result { 0 => Ok(result_buffer), - error => Err(AltBn128CompressionError::from(error)), + _ => Err(AltBn128CompressionError::UnexpectedError), } } @@ -222,7 +228,7 @@ mod target_arch { match result { 0 => Ok(result_buffer), - error => Err(AltBn128CompressionError::from(error)), + _ => Err(AltBn128CompressionError::UnexpectedError), } } @@ -241,7 +247,7 @@ mod target_arch { match result { 0 => Ok(result_buffer), - error => Err(AltBn128CompressionError::from(error)), + _ => Err(AltBn128CompressionError::UnexpectedError), } } @@ -260,7 +266,7 @@ mod target_arch { match result { 0 => Ok(result_buffer), - error => Err(AltBn128CompressionError::from(error)), + _ => Err(AltBn128CompressionError::UnexpectedError), } } } diff --git a/sdk/program/src/alt_bn128/mod.rs b/sdk/program/src/alt_bn128/mod.rs index f214157152c114..815632712c3722 100644 --- a/sdk/program/src/alt_bn128/mod.rs +++ b/sdk/program/src/alt_bn128/mod.rs @@ -41,6 +41,8 @@ mod consts { pub const ALT_BN128_PAIRING: u64 = 3; } +// AltBn128Error must be removed once the +// simplify_alt_bn128_syscall_error_codes feature gets activated #[derive(Debug, Error, Clone, PartialEq, Eq)] pub enum AltBn128Error { #[error("The input data is invalid")] @@ -72,13 +74,14 @@ impl From for AltBn128Error { impl From for u64 { fn from(v: AltBn128Error) -> u64 { + // note: should never return 0, as it risks to be confused with syscall success match v { AltBn128Error::InvalidInputData => 1, AltBn128Error::GroupError => 2, AltBn128Error::SliceOutOfBounds => 3, AltBn128Error::TryIntoVecError(_) => 4, AltBn128Error::ProjectiveToG1Failed => 5, - AltBn128Error::UnexpectedError => 0, + AltBn128Error::UnexpectedError => 6, } } } @@ -319,7 +322,7 @@ mod target_arch { match result { 0 => Ok(result_buffer.to_vec()), - error => Err(AltBn128Error::from(error)), + _ => Err(AltBn128Error::UnexpectedError), } } @@ -339,7 +342,7 @@ mod target_arch { match result { 0 => Ok(result_buffer.to_vec()), - error => Err(AltBn128Error::from(error)), + _ => Err(AltBn128Error::UnexpectedError), } } @@ -363,7 +366,7 @@ mod target_arch { match result { 0 => Ok(result_buffer.to_vec()), - error => Err(AltBn128Error::from(error)), + _ => Err(AltBn128Error::UnexpectedError), } } } diff --git a/sdk/program/src/poseidon.rs b/sdk/program/src/poseidon.rs index 9c02fe90bc8b50..9e782fa5e85fe7 100644 --- a/sdk/program/src/poseidon.rs +++ b/sdk/program/src/poseidon.rs @@ -7,6 +7,8 @@ use thiserror::Error; /// Length of Poseidon hash result. pub const HASH_BYTES: usize = 32; +// PoseidonSyscallError must be removed once the +// simplify_alt_bn128_syscall_error_codes feature gets activated #[derive(Error, Debug)] pub enum PoseidonSyscallError { #[error("Invalid parameters.")] @@ -267,7 +269,7 @@ pub fn hashv( match result { 0 => Ok(PoseidonHash::new(hash_result)), - e => Err(PoseidonSyscallError::from(e)), + _ => Err(PoseidonSyscallError::Unexpected), } } }