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

Pointer width agnostic data type sizes #176

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jkbpvsc
Copy link
Contributor

@jkbpvsc jkbpvsc commented Apr 19, 2022

Problem

We are using wasm to reuse our rust logic in our client libraries. Wasm uses the wasm32-unknown-unknown target which is on 32 bit architecture.
Mango is using usize data type in many instruction and state structs, this is problematic for a few reasons.

usize docs:

The pointer-sized unsigned integer type.
The size of this primitive is how many bytes it takes to reference any location in memory. For example, on a 32 bit target, this is 4 bytes and on a 64 bit target, this is 8 bytes.

Mango code largely assumes usize is 8 bytes, which is expected with bpf and solana runtime, however wasm expects it to be 4 bytes, meaning it:

  • can't build mango code
  • incorrectly deserializes mango state
  • incorrectly deserializes mango instructions

Fix

This PR changes every usize type used in instruction or state structs to u64, which is the actual size of usize expected by the Mango program and clients, meaning this doesn't introduce any changes to the size of the ix or state data.

u64 is converted to usize where it was used in logic before, this is poses no issue as the conversion is trivial because both u64 and usize are the same size for the bpf target.
The only place where conversions are theoretically problematic is when converting from u64 to usize for wasm32 target, as the u64 value might be larger than what can fit into usize (u32). However since any usize value is use only for indexing, the current possible values all fit into u32.
Additionally the problematic conversions are only possible for wasm compiled code, meaning they are not possible in the solana runtime.

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

Successfully merging this pull request may close these issues.

1 participant