Fix: Replace raw u128 with layout-stable U128Bytes in zero-copy structs#292
Open
tom9000 wants to merge 1 commit into
Open
Fix: Replace raw u128 with layout-stable U128Bytes in zero-copy structs#292tom9000 wants to merge 1 commit into
u128 with layout-stable U128Bytes in zero-copy structs#292tom9000 wants to merge 1 commit into
Conversation
Native u128 alignment varies by platform and Rust version (8 bytes on x86_64 with Rust <= 1.77, 16 bytes on x86_64 with Rust >= 1.78, and 16 bytes on aarch64 with all Rust versions). When alignment is 16, the compiler inserts padding that inflates zero-copy struct sizes beyond their expected values, causing compile-time assertion failures (e.g., NODE_SIZE = 88 becomes 96). This prevents building on any modern stable Rust toolchain or on aarch64 targets (including Apple Silicon contributor machines). Introduce U128Bytes, a #[repr(C)] wrapper over [u8; 16] with guaranteed 1-byte alignment and 16-byte size on all platforms. Replace all raw u128 fields in persisted zero-copy account structs with U128Bytes, and provide inline accessor methods to convert to/from native u128 for algorithm code. The stored byte representation is identical (little-endian u128), so existing on-chain accounts are read correctly without migration. ### New type (nodes.rs) - Add U128Bytes: #[repr(C)] wrapper over [u8; 16] - from_u128() / to_u128() using to_le_bytes() / from_le_bytes() - wrapping_add_assign() for += patterns on accumulator fields - From<u128>, PartialEq<u128>, Display impls for ergonomic use - Compile-time assertions: size == 16, align == 1 ### Struct field changes InnerNode (nodes.rs): - key: u128 → key: U128Bytes - Add key(&self) -> u128 accessor method LeafNode (nodes.rs): - key: u128 → key: U128Bytes - Add key(&self) -> u128 accessor method - Update price_data() to use key() accessor Market (market.rs): - fees_accrued: u128 → U128Bytes - fees_to_referrers: u128 → U128Bytes - maker_volume: u128 → U128Bytes - taker_volume_wo_oo: u128 → U128Bytes Position (open_orders_account.rs): - maker_volume: u128 → U128Bytes - taker_volume: u128 → U128Bytes OpenOrder (open_orders_account.rs): - id: u128 → U128Bytes ### Call-site updates ordertree.rs: - All direct .key field accesses on LeafNode/InnerNode replaced with .key() method calls (remove_by_key, insert_leaf, verify_order_tree_ invariant, verify_order_tree_iteration) bookside.rs: - worse.node.key → worse.node.key() - order.node.key → order.node.key() (test println) bookside_iterator.rs: - f.1.key / o.1.key → f.1.key() / o.1.key() in rank_orders book.rs: - best_opposing.node.key → best_opposing.node.key() (3 occurrences) - oo.id → oo.id.to_u128() in cancel_all_orders - market.fees_accrued += → .wrapping_add_assign() - market.taker_volume_wo_oo += → .wrapping_add_assign() open_orders_account.rs: - oo.id == order_id → oo.id.to_u128() == order_id - pa.maker_volume += → .wrapping_add_assign() - pa.taker_volume += → .wrapping_add_assign() - market.maker_volume += → .wrapping_add_assign() - market.fees_accrued += → .wrapping_add_assign() - pa.maker_volume / pa.taker_volume in log emissions → .to_u128() - Default impls: 0 → U128Bytes::from_u128(0) cancel_order.rs: - oo.id → oo.id.to_u128() create_market.rs: - Literal 0 initializers → U128Bytes::from_u128(0) settle_funds.rs: - market.fees_to_referrers += → .wrapping_add_assign() mod.rs (orderbook): - nodes module visibility: mod → pub mod (for U128Bytes import) - Test code: .id → .id.to_u128(), .key → .key() - market.fees_accrued as i64 → market.fees_accrued.to_u128() as i64 AnyNode::key() (nodes.rs): - inner.key / leaf.key → inner.key() / leaf.key() ### Integration test updates test.rs: - order_id_to_cancel → order_id_to_cancel.to_u128() (2 occurrences) test_oracle_peg.rs: - order.id → order.id.to_u128() ### Dependency update Cargo.lock: - time: 0.3.30 → 0.3.36 (0.3.30 fails to compile on Rust >= 1.78 due to a type inference change; unrelated to the layout fix but necessary for stable toolchain compatibility)
Author
|
Hello @mschneider @ckamm @metaproph3t I would appreciate a review of this when you get a chance, and let me know if you have any feedback. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replace all raw
u128fields in zero-copy persisted account structs withU128Bytes, a#[repr(C)]wrapper over[u8; 16]that guarantees stablealignment on all platforms and Rust versions. This fixes compile failures on
modern stable Rust and aarch64 targets without changing on-chain layout,
protocol behavior, or account compatibility.
Closes #291 and related #281
Previous Changes
PR #260 ("Ensure all ordertree nodes share an alignment") by @ckamm added
force_align: u64fields toFreeNodeandAnyNodeto ensure all node typesshare the same alignment. That fix addressed the alignment matching between
node variants, but did not address the underlying issue: raw
u128fields causeplatform-dependent struct inflation. This PR is the logical next step.
Problem
Native
u128alignment is not guaranteed by Rust. It changed from 8 to 16bytes on x86_64 starting in Rust 1.78 (via rust-lang/rust#116672,
adopting LLVM 18's data layout), and has always been 16 on aarch64.
When alignment is 16, the compiler inserts padding that inflates zero-copy
structs beyond their expected sizes, failing the existing
const_assert_eq!size checks.
The project currently works only on one specific combination: x86_64 + Rust
1.70. Every other combination fails to compile:
Solution
The
U128Bytestypeu128)[u8; 16]under#[repr(C)])u128on all Solana targetsfrom_u128()/to_u128()—#[inline(always)], zero-cost on LE targetsAffected structs
InnerNodekeyLeafNodekeyMarketfees_accrued,fees_to_referrers,maker_volume,taker_volume_wo_ooPositionmaker_volume,taker_volumeOpenOrderidChange pattern
Every change follows the same mechanical pattern:
field: u128→field: U128Bytesnode.key→node.key()(returnsu128)U128Bytes::from_u128(value)field += x as u128→field.wrapping_add_assign(x as u128)field→field.to_u128()No algorithm logic, control flow, or protocol semantics were changed.
Why these are the minimal necessary changes
The fix touches only the storage representation of
u128values insidepersisted structs. Specifically:
u128via accessorsU128Bytesuses onlybytemuckandanchor_langwhich are already dependenciesThe
nodesmodule was changed frommodtopub modsolely to allowU128Bytesto be imported bymarket.rsandopen_orders_account.rs.Dependency note
Cargo.lockupdatestimefrom 0.3.30 to 0.3.36. This is unrelated to thelayout fix —
time0.3.30 fails to compile on Rust >= 1.78 due to a typeinference change. Version 0.3.36 is compatible with both Rust 1.70 and modern
stable, and the lockfile format remains v3 (compatible with Rust 1.70's Cargo).
Why this does not introduce regression risk
1. Byte-level compatibility
The
U128Bytesencoding usesu128::to_le_bytes()/u128::from_le_bytes().On little-endian targets (all Solana validators are x86_64-LE), this produces
the exact same byte sequence as the compiler's native
u128representation.The struct field offsets are unchanged because
U128Bytesoccupies the same16 bytes at the same position — the only difference is that the compiler no
longer inserts alignment padding before the field.
2. Algorithm preservation
All tree operations — crit-bit traversal, key comparison, XOR/leading_zeros,
insertion, removal, expiry propagation — continue to operate on native
u128values obtained via
key()accessors. The accessor methods are#[inline(always)]and compile away to direct memory reads + byte reordering(which is a no-op on LE targets).
3. Comprehensive test coverage
The existing test suite was run in full — both the unit test subset and the
complete CI-faithful
cargo test-sbfpipeline.Testing results
CI-faithful testing (
cargo test-sbf, the project's actual CI command)Environment: x86_64 Ubuntu (OrbStack), Solana CLI 1.16.1, Rust 1.70.0
The fix produces identical test results to the unfixed baseline on the
project's CI toolchain. All 44 tests pass, including 11 integration tests
that execute the program through the Solana BPF runtime.
Extended testing (
cargo test --lib, unit tests only)Tests exercised
The test suite covers the full order book behavioral surface:
Tree correctness (ordertree.rs):
order_tree_expiry_manual— deterministic insert/remove/expiry sequencesorder_tree_expiry_random— randomized insert/remove with invariant verificationverify_order_tree_invariant— crit-bit prefix correctness on every mutationverify_order_tree_iteration— ascending/descending traversal orderingBookSide behavior (bookside.rs):
bookside_iteration_random— combined fixed + oracle-pegged orderingbookside_order_filtering— invalid/expired/peg-limited order filteringbookside_remove_worst— worst-order removalOrder book operations (mod.rs):
book_bids_full— capacity pressure and worst-order replacementbook_new_order— matching, fees, maker/taker settlementbook_max_quote_lots— quote lot accountingIntegration tests (test-sbf, 11 additional tests):
Like-for-like comparison methodology
To ensure the layout fix is tested in isolation:
timecrate (0.3.36)Cargo.lockformat v3 (compatible with Rust 1.70)timeupdate,confirming it still fails with layout assertions (proving the layout fix
is the operative change)
Files changed