Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests about size of SpotMarket and PerpMarket are failing #891

Open
JakkuSakura opened this issue Feb 18, 2024 · 11 comments
Open

Tests about size of SpotMarket and PerpMarket are failing #891

JakkuSakura opened this issue Feb 18, 2024 · 11 comments

Comments

@JakkuSakura
Copy link

I'm using commit 79fdcab, 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
Copy link
Author

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
Copy link
Author

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
Copy link
Member

0xbigz commented Feb 19, 2024

  1. can you share rust/anchor/solana version? @crispheaney should chime in
  2. theres a 'slop' check on AMM . do you think this padding mismatch is related to this?

@JakkuSakura
Copy link
Author

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
Copy link
Member

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
Copy link
Author

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
Copy link

jordy25519 commented Mar 6, 2024

@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
Copy link
Author

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
Copy link

jordy25519 commented Apr 2, 2024

I think this issue is caused by memory layout change with rust compiler >= v1.77.0
see: rust-lang/rust#116672 and rust-lang/compiler-team#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
Copy link
Author

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

@tgross35
Copy link

tgross35 commented May 1, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants