[Bug] Incorrect safety invariant

[SozinM]

Component

rpc

What version of Alloy are you on?

v0.7.0/master

Operating System

None

Describe the bug

Safety invariant in the unwrap function is incorrect

alloy/crates/rpc-types-engine/src/payload.rs

Line 476 in 67576f9

// SAFETY: Same repr and size

Structs

alloy/crates/rpc-types-engine/src/payload.rs

Line 460 in 67576f9

struct BlobsBundleV1Ssz {

And

alloy/crates/rpc-types-engine/src/payload.rs

Line 449 in 67576f9

pub struct BlobsBundleV1 {

Have default repr (Rust)
Rust repr does not guarantee that this structs would have the same memory layout (https://doc.rust-lang.org/nomicon/repr-rust.html).
Also transmute highlights that it should not be used with Rust repr

When transmuting between different compound types, you have to make sure they are laid out the same way! If layouts differ, the wrong fields are going to get filled with the wrong data, which will make you unhappy and can also be Undefined Behavior (see above).

So how do you know if the layouts are the same? For repr(C) types and repr(transparent) types, layout is precisely defined. But for your run-of-the-mill repr(Rust), it is not.

https://doc.rust-lang.org/nomicon/transmutes.html

To fix the problem we could either change unwrap or add repr(C) to structs (but we will need to align them manually)

wrap_ref have similar issue too, but it's not easy to fix this as the operation itself is not safe (only with repr(C) ) so it's up to discussion

[DaniPopes]

this is still present in wrap_ref

[SozinM]

@DaniPopes Yes, any idea on how to address this?
Because ref conversion between differently aligned types is UB so in's not a way to go, so maybe little refactoring is needed?
It's quite hard to work around in the safe code (at least i don't see easy way) because both BlobsBundleV1Ssz and BlobsBundleV1 have owned fields so i can't as_ref helper, for example.

Bad working solution would be to do clone, but it will be slow, obviously.

I think it's possible to use https://github.com/google/zerocopy to do the job (as both types have the same type fields), wdyt?

[DaniPopes]

I think we can avoid the wrapper type altogether seeing that Bytes48 and Blob are just type aliases to FixedBytes, so the SSZ derive should work on the main type directly.