drift-labs / protocol-v2

On-chain perpetuals dex with multiple liquidity mechanisms
Apache License 2.0
174 stars 91 forks source link

Tests about size of SpotMarket and PerpMarket are failing #891

Open JakkuSakura opened 4 months ago

JakkuSakura commented 4 months ago

I'm using commit 79fdcab4ab4be6199de58ebb634c9aa3ab82691b, with cargo test These tests are failing, rendering the programs unusable

    #[test]
    fn perp_market() {
        let expected_size = std::mem::size_of::<PerpMarket>() + 8;
        let actual_size = PerpMarket::SIZE;
        assert_eq!(actual_size, expected_size);
    }
    #[test]
    fn spot_market() {
        let expected_size = std::mem::size_of::<SpotMarket>() + 8;
        let actual_size = SpotMarket::SIZE;
        assert_eq!(actual_size, expected_size);
    }
assertion `left == right` failed
  left: 1216
 right: 1240

Left:  1216
Right: 1240

assertion `left == right` failed
  left: 776
 right: 808

Left:  776
Right: 808
JakkuSakura commented 4 months ago

I observed there is some end padding with the repr(C) struct

Padding is also invisible, and sometimes tricky to spot, like padding at the end of struct caused by alignment. It's also hard to check when using nested structs or type aliases.

JakkuSakura commented 4 months ago

https://internals.rust-lang.org/t/auto-trait-for-stable-layout-types-with-no-padding/20339/2 I tried to adjust padding of PerpMarket, but failed to get 1216, I only got 1208 or 1224

0xbigz commented 4 months ago
  1. can you share rust/anchor/solana version? @crispheaney should chime in
  2. theres a 'slop' check on AMM https://github.com/drift-labs/protocol-v2/blob/f33f49b8bf8b35e82f901a21078ab69fd4c5d687/programs/drift/src/state/perp_market.rs#L550. do you think this padding mismatch is related to this?
JakkuSakura commented 4 months ago

info: The currently active rustc version is rustc 1.78.0-nightly (a84bb95a1 2024-02-13) for anchor and solana I used the project default.

solana is 1.14.16 anchor-lang = "0.27.0"

For assert_no_slop, I had a glance at the code. It looks fine, and bytemuck is doing similar check with #[zero_copy] but not with #[zero_copy(unsafe)]. I tried to remove these unsafe and rustc says there are padding.

However, only usage of assert_no_slop is AAM, but other types, including the whole PerpMarket and SpotMarket is not asserted

crispheaney commented 4 months ago

solana uses 64 bit architecture, i'm guessing you're using 128 bit architecture. if you're using apple, i'd use x86_64 toolchain

JakkuSakura commented 4 months ago

I never heard of any machine that runs 128bit architecture. my triplet: aarch64-apple-darwin or x86_64-unknown-linux-gnu None of them worked

jordy25519 commented 4 months ago

@JakkuSakura the issue is from solana program memory layout not matching aarch64 layout of m-series macs, it breaks some de/serialization logic. see e.g. https://solana.stackexchange.com/questions/10273/compiling-and-testing-solana-programs-on-apple-silicon-m1-m2-m3

as @crispheaney mentioned build for x86_64 will fix it

softwareupdate --install-rosetta
rustup install 1.76.0-x86_64-apple-darwin
rustup override set 1.76.0-x86_64-apple-darwin
JakkuSakura commented 4 months ago

Thanks for the update. However, the link provided doesn't work.

➜  drift git:(master) ✗ cargo test --package drift --lib state::traits::tests::size::spot_market --target x86_64-apple-darwin -- --exact 
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.11s
     Running unittests src/lib.rs (/Users/jack/Dev/protocol-v2/target/x86_64-apple-darwin/debug/deps/drift-834034f62b24db11)

running 1 test
test state::traits::tests::size::spot_market ... FAILED

failures:

---- state::traits::tests::size::spot_market stdout ----
thread 'state::traits::tests::size::spot_market' panicked at programs/drift/src/state/traits/tests.rs:29:9:
assertion `left == right` failed
  left: 776
 right: 808
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Moreover, I have also tested on x86_64 Archlinux and arm64 Fedora, on neither of which the test passes with default triplet

jordy25519 commented 3 months ago

I think this issue is caused by memory layout change with rust compiler >= v1.77.0 see: https://github.com/rust-lang/rust/pull/116672/ and https://github.com/rust-lang/compiler-team/issues/683

rustc built against LLVM < 18 that isn't from Rust's tree will mostly work with mainline rustc, with the exception of in-memory passing

deserializing program types relies on these layouts matching and it does not anymore (program built on ~1.62.0-ish). seems like it may be broader issue for solana programs. best to use <= v1.76.0 for now

JakkuSakura commented 2 months ago

Seems like aligning i128 to 32 bytes can solve this issue. But not sure about a compatible way with older compilers

tgross35 commented 2 months ago

Just fyi there is some more info about compatibility in the blog post https://blog.rust-lang.org/2024/03/30/i128-layout-update.html