Remove Precompile Alignment check/requirement (#5765)

precompiles/Cargo.toml

@@ -12,7 +12,6 @@ edition = { workspace = true }
 [dependencies]
 agave-feature-set = { workspace = true }
 bincode = { workspace = true }
-bytemuck = { workspace = true }
 digest = { workspace = true }
 ed25519-dalek = { workspace = true }
 lazy_static = { workspace = true }
@@ -28,6 +27,7 @@ solana-secp256k1-program = { workspace = true, features = ["serde"] }
 solana-secp256r1-program = { workspace = true, features = ["openssl-vendored"] }
 
 [dev-dependencies]
+bytemuck = { workspace = true }
 hex = { workspace = true }
 rand0-7 = { workspace = true }
 solana-instruction = { workspace = true }

precompiles/src/ed25519.rs

@@ -31,11 +31,13 @@ pub fn verify(
         let start = i
             .saturating_mul(SIGNATURE_OFFSETS_SERIALIZED_SIZE)
             .saturating_add(SIGNATURE_OFFSETS_START);
-        let end = start.saturating_add(SIGNATURE_OFFSETS_SERIALIZED_SIZE);
 
-        // bytemuck wants structures aligned
-        let offsets: &Ed25519SignatureOffsets = bytemuck::try_from_bytes(&data[start..end])
-            .map_err(|_| PrecompileError::InvalidDataOffsets)?;
+        // SAFETY:
+        // - data[start..] is guaranteed to be >= size of Ed25519SignatureOffsets
+        // - Ed25519SignatureOffsets is a POD type, so we can safely read it as an unaligned struct
+        let offsets = unsafe {
+            core::ptr::read_unaligned(data.as_ptr().add(start) as *const Ed25519SignatureOffsets)
+        };
 
         // Parse out signature
         let signature = get_data_slice(
@@ -113,6 +115,7 @@ fn get_data_slice<'a>(
 pub mod tests {
     use {
         super::*,
+        crate::test_verify_with_alignment,
         bytemuck::bytes_of,
         ed25519_dalek::Signer as EdSigner,
         hex,
@@ -121,6 +124,7 @@ pub mod tests {
             new_ed25519_instruction, offsets_to_ed25519_instruction, DATA_START,
         },
         solana_instruction::Instruction,
+        std::vec,
     };
 
     pub fn new_ed25519_instruction_raw(
@@ -190,7 +194,8 @@ pub mod tests {
         instruction_data[0..SIGNATURE_OFFSETS_START].copy_from_slice(bytes_of(&num_signatures));
         instruction_data[SIGNATURE_OFFSETS_START..DATA_START].copy_from_slice(bytes_of(offsets));
 
-        verify(
+        test_verify_with_alignment(
+            verify,
             &instruction_data,
             &[&[0u8; 100]],
             &FeatureSet::all_enabled(),
@@ -208,7 +213,8 @@ pub mod tests {
         instruction_data.truncate(instruction_data.len() - 1);
 
         assert_eq!(
-            verify(
+            test_verify_with_alignment(
+                verify,
                 &instruction_data,
                 &[&[0u8; 100]],
                 &FeatureSet::all_enabled(),
@@ -338,7 +344,13 @@ pub mod tests {
         let mut instruction = new_ed25519_instruction(&privkey, message_arr);
         let feature_set = FeatureSet::all_enabled();
 
-        assert!(verify(&instruction.data, &[&instruction.data], &feature_set).is_ok());
+        assert!(test_verify_with_alignment(
+            verify,
+            &instruction.data,
+            &[&instruction.data],
+            &feature_set
+        )
+        .is_ok());
 
         let index = loop {
             let index = thread_rng().gen_range(0, instruction.data.len());
@@ -349,7 +361,13 @@ pub mod tests {
         };
 
         instruction.data[index] = instruction.data[index].wrapping_add(12);
-        assert!(verify(&instruction.data, &[&instruction.data], &feature_set).is_err());
+        assert!(test_verify_with_alignment(
+            verify,
+            &instruction.data,
+            &[&instruction.data],
+            &feature_set
+        )
+        .is_err());
     }
 
     #[test]
@@ -395,7 +413,13 @@ pub mod tests {
 
         let feature_set = FeatureSet::all_enabled();
 
-        assert!(verify(&instruction.data, &[&instruction.data], &feature_set).is_ok());
+        assert!(test_verify_with_alignment(
+            verify,
+            &instruction.data,
+            &[&instruction.data],
+            &feature_set
+        )
+        .is_ok());
 
         let index = loop {
             let index = thread_rng().gen_range(0, instruction.data.len());
@@ -406,7 +430,13 @@ pub mod tests {
         };
 
         instruction.data[index] = instruction.data[index].wrapping_add(12);
-        assert!(verify(&instruction.data, &[&instruction.data], &feature_set).is_err());
+        assert!(test_verify_with_alignment(
+            verify,
+            &instruction.data,
+            &[&instruction.data],
+            &feature_set
+        )
+        .is_err());
     }
 
     #[test]
@@ -419,10 +449,22 @@ pub mod tests {
         let instruction = new_ed25519_instruction(&privkey, message_arr);
 
         let feature_set = FeatureSet::default();
-        assert!(verify(&instruction.data, &[&instruction.data], &feature_set).is_ok());
+        assert!(test_verify_with_alignment(
+            verify,
+            &instruction.data,
+            &[&instruction.data],
+            &feature_set
+        )
+        .is_ok());
 
         let feature_set = FeatureSet::all_enabled();
-        assert!(verify(&instruction.data, &[&instruction.data], &feature_set).is_ok());
+        assert!(test_verify_with_alignment(
+            verify,
+            &instruction.data,
+            &[&instruction.data],
+            &feature_set
+        )
+        .is_ok());
 
         // malleable sig: verify_strict does NOT pass
         // for example, test number 5:
@@ -436,10 +478,22 @@ pub mod tests {
         let instruction = new_ed25519_instruction_raw(pubkey, signature, message);
 
         let feature_set = FeatureSet::default();
-        assert!(verify(&instruction.data, &[&instruction.data], &feature_set).is_ok());
+        assert!(test_verify_with_alignment(
+            verify,
+            &instruction.data,
+            &[&instruction.data],
+            &feature_set
+        )
+        .is_ok());
 
         // verify_strict does NOT pass
         let feature_set = FeatureSet::all_enabled();
-        assert!(verify(&instruction.data, &[&instruction.data], &feature_set).is_err());
+        assert!(test_verify_with_alignment(
+            verify,
+            &instruction.data,
+            &[&instruction.data],
+            &feature_set
+        )
+        .is_err());
     }
 }

precompiles/src/lib.rs

@@ -119,3 +119,27 @@ pub fn verify_if_precompile(
     }
     Ok(())
 }
+
+#[cfg(test)]
+pub(crate) fn test_verify_with_alignment(
+    verify: Verify,
+    instruction_data: &[u8],
+    instruction_datas: &[&[u8]],
+    feature_set: &FeatureSet,
+) -> Result<(), PrecompileError> {
+    // Copy instruction data.
+    let mut instruction_data_copy = vec![0u8; instruction_data.len().checked_add(1).unwrap()];
+    instruction_data_copy[0..instruction_data.len()].copy_from_slice(instruction_data);
+    // Verify the instruction data.
+    let result = verify(
+        &instruction_data_copy[..instruction_data.len()],
+        instruction_datas,
+        feature_set,
+    );
+
+    // Shift alignment by 1 to test `verify` does not rely on alignment.
+    instruction_data_copy[1..].copy_from_slice(instruction_data);
+    let result_shifted = verify(&instruction_data_copy[1..], instruction_datas, feature_set);
+    assert_eq!(result, result_shifted);
+    result
+}

precompiles/src/secp256k1.rs

@@ -130,6 +130,7 @@ fn get_data_slice<'a>(
 pub mod tests {
     use {
         super::*,
+        crate::test_verify_with_alignment,
         rand0_7::{thread_rng, Rng},
         solana_keccak_hasher as keccak,
         solana_secp256k1_program::{new_secp256k1_instruction, DATA_START},
@@ -144,7 +145,7 @@ pub mod tests {
         let writer = std::io::Cursor::new(&mut instruction_data[1..]);
         bincode::serialize_into(writer, &offsets).unwrap();
         let feature_set = FeatureSet::all_enabled();
-        verify(&instruction_data, &[&[0u8; 100]], &feature_set)
+        test_verify_with_alignment(verify, &instruction_data, &[&[0u8; 100]], &feature_set)
     }
 
     #[test]
@@ -160,7 +161,7 @@ pub mod tests {
         let feature_set = FeatureSet::all_enabled();
 
         assert_eq!(
-            verify(&instruction_data, &[&[0u8; 100]], &feature_set),
+            test_verify_with_alignment(verify, &instruction_data, &[&[0u8; 100]], &feature_set),
             Err(PrecompileError::InvalidInstructionDataSize)
         );
 
@@ -289,7 +290,7 @@ pub mod tests {
         let feature_set = FeatureSet::all_enabled();
 
         assert_eq!(
-            verify(&instruction_data, &[&[0u8; 100]], &feature_set),
+            test_verify_with_alignment(verify, &instruction_data, &[&[0u8; 100]], &feature_set),
             Err(PrecompileError::InvalidInstructionDataSize)
         );
     }
@@ -307,11 +308,23 @@ pub mod tests {
         let message_arr = b"hello";
         let mut instruction = new_secp256k1_instruction(&secp_privkey, message_arr);
         let feature_set = FeatureSet::all_enabled();
-        assert!(verify(&instruction.data, &[&instruction.data], &feature_set).is_ok());
+        assert!(test_verify_with_alignment(
+            verify,
+            &instruction.data,
+            &[&instruction.data],
+            &feature_set
+        )
+        .is_ok());
 
         let index = thread_rng().gen_range(0, instruction.data.len());
         instruction.data[index] = instruction.data[index].wrapping_add(12);
-        assert!(verify(&instruction.data, &[&instruction.data], &feature_set).is_err());
+        assert!(test_verify_with_alignment(
+            verify,
+            &instruction.data,
+            &[&instruction.data],
+            &feature_set
+        )
+        .is_err());
     }
 
     // Signatures are malleable.
@@ -376,7 +389,8 @@ pub mod tests {
 
         instruction_data.extend(data);
 
-        verify(
+        test_verify_with_alignment(
+            verify,
             &instruction_data,
             &[&instruction_data],
             &FeatureSet::all_enabled(),

precompiles/src/secp256r1.rs

@@ -60,11 +60,13 @@ pub fn verify(
         let start = i
             .saturating_mul(SIGNATURE_OFFSETS_SERIALIZED_SIZE)
             .saturating_add(SIGNATURE_OFFSETS_START);
-        let end = start.saturating_add(SIGNATURE_OFFSETS_SERIALIZED_SIZE);
 
-        // bytemuck wants structures aligned
-        let offsets: &Secp256r1SignatureOffsets = bytemuck::try_from_bytes(&data[start..end])
-            .map_err(|_| PrecompileError::InvalidDataOffsets)?;
+        // SAFETY:
+        // - data[start..] is guaranteed to be >= size of Secp256r1SignatureOffsets
+        // - Secp256r1SignatureOffsets is a POD type, so we can safely read it as an unaligned struct
+        let offsets = unsafe {
+            core::ptr::read_unaligned(data.as_ptr().add(start) as *const Secp256r1SignatureOffsets)
+        };
 
         // Parse out signature
         let signature = get_data_slice(
@@ -167,6 +169,7 @@ fn get_data_slice<'a>(
 mod tests {
     use {
         super::*,
+        crate::test_verify_with_alignment,
         bytemuck::bytes_of,
         solana_secp256r1_program::{new_secp256r1_instruction, DATA_START, SECP256R1_ORDER},
     };
@@ -183,7 +186,8 @@ mod tests {
         let mut instruction_data = vec![0u8; DATA_START];
         instruction_data[0..SIGNATURE_OFFSETS_START].copy_from_slice(bytes_of(&num_signatures));
         instruction_data[SIGNATURE_OFFSETS_START..DATA_START].copy_from_slice(bytes_of(offsets));
-        verify(
+        test_verify_with_alignment(
+            verify,
             &instruction_data,
             &[&[0u8; 100]],
             &FeatureSet::all_enabled(),
@@ -201,7 +205,8 @@ mod tests {
         instruction_data.truncate(instruction_data.len() - 1);
 
         assert_eq!(
-            verify(
+            test_verify_with_alignment(
+                verify,
                 &instruction_data,
                 &[&[0u8; 100]],
                 &FeatureSet::all_enabled()
@@ -244,7 +249,7 @@ mod tests {
         // Test data.len() < SIGNATURE_OFFSETS_START
         let small_data = vec![0u8; SIGNATURE_OFFSETS_START - 1];
         assert_eq!(
-            verify(&small_data, &[&[]], &FeatureSet::all_enabled()),
+            test_verify_with_alignment(verify, &small_data, &[&[]], &FeatureSet::all_enabled()),
             Err(PrecompileError::InvalidInstructionDataSize)
         );
 
@@ -252,7 +257,7 @@ mod tests {
         let mut zero_sigs_data = vec![0u8; DATA_START];
         zero_sigs_data[0] = 0; // Set num_signatures to 0
         assert_eq!(
-            verify(&zero_sigs_data, &[&[]], &FeatureSet::all_enabled()),
+            test_verify_with_alignment(verify, &zero_sigs_data, &[&[]], &FeatureSet::all_enabled()),
             Err(PrecompileError::InvalidInstructionDataSize)
         );
 
@@ -260,7 +265,7 @@ mod tests {
         let mut too_many_sigs = vec![0u8; DATA_START];
         too_many_sigs[0] = 9; // Set num_signatures to 9
         assert_eq!(
-            verify(&too_many_sigs, &[&[]], &FeatureSet::all_enabled()),
+            test_verify_with_alignment(verify, &too_many_sigs, &[&[]], &FeatureSet::all_enabled()),
             Err(PrecompileError::InvalidInstructionDataSize)
         );
     }
@@ -358,7 +363,13 @@ mod tests {
         let mut instruction = new_secp256r1_instruction(message_arr, signing_key).unwrap();
         let feature_set = FeatureSet::all_enabled();
 
-        assert!(verify(&instruction.data, &[&instruction.data], &feature_set).is_ok());
+        assert!(test_verify_with_alignment(
+            verify,
+            &instruction.data,
+            &[&instruction.data],
+            &feature_set
+        )
+        .is_ok());
 
         // The message is the last field in the instruction data so
         // changing its last byte will also change the signature validity
@@ -366,7 +377,13 @@ mod tests {
         instruction.data[message_byte_index] =
             instruction.data[message_byte_index].wrapping_add(12);
 
-        assert!(verify(&instruction.data, &[&instruction.data], &feature_set).is_err());
+        assert!(test_verify_with_alignment(
+            verify,
+            &instruction.data,
+            &[&instruction.data],
+            &feature_set
+        )
+        .is_err());
     }
 
     #[test]
@@ -379,7 +396,8 @@ mod tests {
 
         // To double check that the untampered low-S value signature passes
         let feature_set = FeatureSet::all_enabled();
-        let tx_pass = verify(
+        let tx_pass = test_verify_with_alignment(
+            verify,
             instruction.data.as_slice(),
             &[instruction.data.as_slice()],
             &feature_set,
@@ -401,7 +419,12 @@ mod tests {
         // Replace the S value in the signature with our high S
         instruction.data[s_offset..s_offset + FIELD_SIZE].copy_from_slice(&high_s.to_vec());
 
-        let tx_fail = verify(&instruction.data, &[&instruction.data], &feature_set);
+        let tx_fail = test_verify_with_alignment(
+            verify,
+            &instruction.data,
+            &[&instruction.data],
+            &feature_set,
+        );
         assert!(tx_fail.unwrap_err() == PrecompileError::InvalidSignature);
     }
     #[test]
@@ -430,7 +453,13 @@ mod tests {
             if r_bytes.len() == 31 || s_bytes.len() == 31 {
                 // Once found, verify the signature and break out of the loop
                 let feature_set = FeatureSet::all_enabled();
-                assert!(verify(&instruction.data, &[&instruction.data], &feature_set).is_ok());
+                assert!(test_verify_with_alignment(
+                    verify,
+                    &instruction.data,
+                    &[&instruction.data],
+                    &feature_set
+                )
+                .is_ok());
                 break;
             }
         }

programs/sbf/Cargo.lock

@@ -100,7 +100,6 @@ version = "2.3.0"
 dependencies = [
  "agave-feature-set",
  "bincode",
- "bytemuck",
  "digest 0.10.7",
  "ed25519-dalek",
  "lazy_static",

svm/examples/Cargo.lock

@@ -100,7 +100,6 @@ version = "2.3.0"
 dependencies = [
  "agave-feature-set",
  "bincode",
- "bytemuck",
  "digest 0.10.7",
  "ed25519-dalek",
  "lazy_static",