InstructionStabilizer

sdk/program/src/program.rs

@@ -293,10 +293,13 @@ pub fn invoke_signed_unchecked(
 ) -> ProgramResult {
     #[cfg(target_os = "solana")]
     {
-        let instruction = StableInstruction::from(instruction.clone());
+        use crate::stable_layout::stable_instruction::InstructionStabilizer;
+        let stabilizer = InstructionStabilizer::stabilize(instruction);
+        let instruction_addr = stabilizer.instruction_addr();
+
         let result = unsafe {
             crate::syscalls::sol_invoke_signed_rust(
-                &instruction as *const _ as *const u8,
+                instruction_addr,
                 account_infos as *const _ as *const u8,
                 account_infos.len() as u64,
                 signers_seeds as *const _ as *const u8,

sdk/program/src/stable_layout/stable_instruction.rs

@@ -1,5 +1,9 @@
 //! `Instruction`, with a stable memory layout
 
+use std::marker::PhantomData;
+
+use super::stable_vec::stabilize_instruction;
+
 use {
     crate::{
         instruction::{AccountMeta, Instruction},
@@ -46,6 +50,82 @@ impl From<Instruction> for StableInstruction {
     }
 }
 
+/// This wrapper type with no constructor ensures that no user can
+/// manually drop the inner type.
+///
+/// We provide only an immutable borrow method, which ensures that
+/// the inner type is not modified in the absence of unsafe code.
+///
+/// StableInstruction uses NonNull<T> which is invariant over T.
+/// NonNull<T> is clonable. It's the same type used by Rc<T> and
+/// Arc<T>. It is safe to have an aliasing pointer to the same
+/// allocation as the underlying vectors so long as we perform
+/// no modificiations.
+///
+/// A constructor api is chosen internally over simply making the inner type
+/// pub(super) or pub(super) so that not even users within this crate (outside
+/// of this module) can modify the inner type.
+///
+/// Most importantly...
+/// No unsafe code was written in the creation or usage of this type :)
+pub struct InstructionStabilizer<'a> {
+    /// A stable instruction that will not be dropped. By circumventing the
+    /// `Drop` implementation, this becomes a view (similar to a slice)
+    /// into the original vector's buffer. Since we provide only a borrow
+    /// method on this wrapper, we can guarantee that the `StableInstruction`
+    /// is never modified.
+    stabilized_instruction: core::mem::ManuallyDrop<StableInstruction>,
+
+    /// A read-only view (into the buffers owned by the inner vectors) is
+    /// only safe for as long as the `&'a Instruction` lives.
+    ///
+    /// This could be a `&'a Instruction` but we don't actually need the
+    /// instruction. We can pretend to hold a `&'a Instruction`` instead.
+    ///
+    /// Using a `PhantomData<&'a Instruction>` forces this struct and the
+    /// compiler to act like it is holding the reference without increasing
+    /// the size of the type.
+    phantom_instruction: PhantomData<&'a Instruction>,
+}
+
+impl<'ix> InstructionStabilizer<'ix> {
+    #[inline(always)]
+    pub fn stabilize(instruction: &Instruction) -> InstructionStabilizer {
+        stabilize_instruction(instruction)
+    }
+
+    /// NOTE:
+    ///
+    /// A constructor api is chosen internally over simply making the inner type
+    /// pub(super) or pub(super) so that not even users within this crate (outside
+    /// of this module) can modify the inner type.
+    #[inline(always)]
+    pub(super) fn new(
+        stabilized_instruction: core::mem::ManuallyDrop<StableInstruction>,
+        // Note: This is where 'ix is inherited
+        _instruction: &'ix Instruction,
+    ) -> InstructionStabilizer<'ix> {
+        Self {
+            stabilized_instruction,
+            phantom_instruction: PhantomData::<&'ix Instruction>,
+        }
+    }
+
+    #[inline(always)]
+    pub fn stable_instruction_ref<'borrow>(&'borrow self) -> &'borrow StableInstruction
+    where
+        // 'ix must live at least as long as 'borrow
+        'ix: 'borrow,
+    {
+        &self.stabilized_instruction
+    }
+
+    #[inline(always)]
+    pub fn instruction_addr(&self) -> *const u8 {
+        self.stable_instruction_ref() as *const StableInstruction as *const u8
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use {

sdk/program/src/stable_layout/stable_vec.rs

@@ -2,6 +2,10 @@
 
 use std::{marker::PhantomData, mem::ManuallyDrop, ptr::NonNull};
 
+use crate::instruction::{AccountMeta, Instruction};
+
+use super::stable_instruction::{InstructionStabilizer, StableInstruction};
+
 /// `Vec`, with a stable memory layout
 ///
 /// This container is used within the runtime to ensure memory mapping and memory accesses are
@@ -30,49 +34,116 @@ pub struct StableVec<T> {
     _marker: PhantomData<T>,
 }
 
-impl<T> StableVec<T> {
-    #[inline]
-    pub fn as_ptr(&self) -> *const T {
-        // We shadow the slice method of the same name to avoid going through
-        // `deref`, which creates an intermediate reference.
-        self.ptr.as_ptr()
-    }
+// Only to be used by super::stable_instruction, but only ancestors are allowed for visibility
+#[inline(always)] // only one call site (wrapper fn) so inline there
+pub(super) fn stabilize_instruction<'ix_ref>(
+    ix: &'ix_ref Instruction,
+) -> InstructionStabilizer<'ix_ref> {
+    // Get StableVec out of instruction data Vec<u8>
+    let data: StableVec<u8> = {
+        // Get vector parts
+        let ptr = NonNull::new(ix.data.as_ptr() as *mut u8).expect("vector ptr should be valid");
+        let len = ix.data.len();
+        let cap = ix.data.capacity();
+
+        StableVec {
+            ptr,
+            cap,
+            len,
+            _marker: std::marker::PhantomData,
+        }
+    };
 
-    #[inline]
-    pub fn as_mut_ptr(&mut self) -> *mut T {
-        // We shadow the slice method of the same name to avoid going through
-        // `deref_mut`, which creates an intermediate reference.
-        self.ptr.as_ptr()
-    }
+    // Get StableVec out of instruction accounts Vec<Accountmeta>
+    let accounts: StableVec<AccountMeta> = {
+        // Get vector parts
+        let ptr = NonNull::new(ix.accounts.as_ptr() as *mut AccountMeta)
+            .expect("vector ptr should be valid");
+        let len = ix.accounts.len();
+        let cap = ix.accounts.capacity();
+
+        StableVec {
+            ptr,
+            cap,
+            len,
+            _marker: std::marker::PhantomData,
+        }
+    };
+
+    InstructionStabilizer::<'ix_ref>::new(
+        ManuallyDrop::new(StableInstruction {
+            accounts,
+            data,
+            program_id: ix.program_id,
+        }),
+        ix,
+    )
 }
 
-impl<T> AsRef<[T]> for StableVec<T> {
-    fn as_ref(&self) -> &[T] {
-        self
-    }
+#[test]
+fn instruction_stability() {
+    let instruction = Instruction {
+        program_id: Default::default(),
+        accounts: Default::default(),
+        data: Default::default(),
+    };
+
+    let stabilizer: InstructionStabilizer = stabilize_instruction(&instruction);
+    // This call to drop correctly doesn't compile, due to the
+    // `PhantomData<&'a Instruction>` held by the stabilizer.
+    //
+    // drop(instruction);
+    let stable: &StableInstruction = stabilizer.stable_instruction_ref();
+    assert_eq!(&instruction.accounts, &stable.accounts);
+    assert_eq!(&instruction.data, &stable.data);
+    assert_eq!(instruction.program_id, stable.program_id);
+
+    let _instruction_addr: *const u8 = stabilizer.instruction_addr();
 }
 
-impl<T> AsMut<[T]> for StableVec<T> {
-    fn as_mut(&mut self) -> &mut [T] {
-        self
-    }
+impl<T> StableVec<T> {
+    // #[inline]
+    // pub fn as_ptr(&self) -> *const T {
+    //     // We shadow the slice method of the same name to avoid going through
+    //     // `deref`, which creates an intermediate reference.
+    //     self.ptr.as_ptr()
+    // }
+
+    // #[inline]
+    // pub fn as_mut_ptr(&mut self) -> *mut T {
+    //     // We shadow the slice method of the same name to avoid going through
+    //     // `deref_mut`, which creates an intermediate reference.
+    //     self.ptr.as_ptr()
+    // }
 }
 
+// impl<T> AsRef<[T]> for StableVec<T> {
+//     fn as_ref(&self) -> &[T] {
+//         self
+//     }
+// }
+
+// impl<T> AsMut<[T]> for StableVec<T> {
+//     fn as_mut(&mut self) -> &mut [T] {
+//         self
+//     }
+// }
+
 impl<T> std::ops::Deref for StableVec<T> {
     type Target = [T];
 
     #[inline]
     fn deref(&self) -> &[T] {
-        unsafe { core::slice::from_raw_parts(self.as_ptr(), self.len) }
+        unsafe { core::slice::from_raw_parts(self.ptr.as_ptr(), self.len) }
     }
 }
 
-impl<T> std::ops::DerefMut for StableVec<T> {
-    #[inline]
-    fn deref_mut(&mut self) -> &mut [T] {
-        unsafe { core::slice::from_raw_parts_mut(self.as_mut_ptr(), self.len) }
-    }
-}
+// impl<T> std::ops::DerefMut for StableVec<T> {
+//     #[inline]
+//     fn deref_mut(&mut self) -> &mut [T] {
+//         unsafe { core::slice::from_raw_parts_mut(self.as_mut_ptr(), self.len) }
+//     }
+// }
 
 impl<T: std::fmt::Debug> std::fmt::Debug for StableVec<T> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {