Reduce unsafe methods in move-native

[brson]

There are a huge number of unsafe functions in move-native, and some safe functions that probably aren't really.

Reducing the number of unsafe methods would help future maintenance and potential auditors.

Most of the unsafety is due to functions interpreting MoveType plus AnyValue or MoveUntypedVector, the fields of which must be trusted to be correct. The basic approach to reducing unsafe functions is to make the fields of these types private, but add unsafe constructors. Then further functions/methods can be safe, assuming the unsafe types were constructed correctly.

Values of these types received from the compiler are assumed correct.

i.e.:

// These fields must maintain safety invariants.
// Because the fields are public, functions operating on MoveType must be unsafe.
// If an unsafe constructor is required to build this, then the constructor caller is responsible for the safety,
// and methods on MoveType can be declared safe.

#[repr(C)]
pub struct MoveType {
    pub name: StaticTypeName,
    pub type_desc: TypeDesc,
    pub type_info: *const TypeInfo,
}

Places where MoveType is combined with an AnyType or MoveUntypedVector can be changed to combine these two types into yet another container, again with an unsafe constructor and private fields, allowing further operations on the pair to be declared safe. Something like this is already done for and TypedMoveBorrowedRustVec and MoveBorrowedRustVecOfStruct, but the methods aren't quite factored correctly to declare methods safe.

e.g.:

// This should be a completely safe method on `TypedMoveBorrowedRustVecMut` -
// the unsafe constructor `borrow_typeed_move_vec_as_rust_vec_mut` should be lifted out
// (and turned into an unsafe constructor method on `TypedMoveBorrowedRustVecMut`).

pub unsafe fn swap(type_ve: &MoveType, v: &mut MoveUntypedVector, i: u64, j: u64) {
    let i = usize::try_from(i).expect("usize");
    let j = usize::try_from(j).expect("usize");

    let rust_vec: TypedMoveBorrowedRustVecMut = borrow_typed_move_vec_as_rust_vec_mut(type_ve, v);

    match rust_vec {
        TypedMoveBorrowedRustVecMut::Bool(mut v) => v.swap(i, j),
        TypedMoveBorrowedRustVecMut::U8(mut v) => v.swap(i, j),
        TypedMoveBorrowedRustVecMut::U16(mut v) => v.swap(i, j),
        TypedMoveBorrowedRustVecMut::U32(mut v) => v.swap(i, j),
        TypedMoveBorrowedRustVecMut::U64(mut v) => v.swap(i, j),
        TypedMoveBorrowedRustVecMut::U128(mut v) => v.swap(i, j),
        TypedMoveBorrowedRustVecMut::U256(mut v) => v.swap(i, j),
        TypedMoveBorrowedRustVecMut::Address(mut v) => v.swap(i, j),
        TypedMoveBorrowedRustVecMut::Signer(mut v) => v.swap(i, j),
        TypedMoveBorrowedRustVecMut::Vector(_t, mut v) => v.swap(i, j),
        TypedMoveBorrowedRustVecMut::Struct(mut v) => v.swap(i, j),
        TypedMoveBorrowedRustVecMut::Reference(_t, mut v) => v.swap(i, j),
    }
}

I'd also like to convert most free functions that operate on these types to methods.

[ksolana]

@brson Has this been fixed?

[brson]

The low-hanging fruit here is done, but there's so much unsafety in the runtime that there's plenty more that can potentially be done to introduce safe abstractions around the unsafe ones; though there's a question of whether it is worthwhile - the code contortions to make the internal abstractions truly safe will get uglier and may reduce performance.

Off-hand I recall that the vector abstractions still have many functions that are unsafe, and making them actually memory safe looked tricky.

It could be worth someone taking a fresh look at, though I note that creating safe Rust abstractions around unsafe code is pretty tricky and there are a bunch of subtle rules. This page is a good starting point:

https://doc.rust-lang.org/reference/behavior-considered-undefined.html