Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions contracts/order_handler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -724,11 +724,21 @@ impl OrderHandler {
panic_with_error!(&env, Error::ZeroSizeDelta);
}

// Position manager authorization:
// Position manager authorization:
// For position orders, verify caller is either the owner OR an authorized manager for this market.
// If caller is a manager, receiver must be the owner (cannot redirect funds).
//
// ISSUE #385: This logic is currently REVERSED and needs fixing.
// Current code incorrectly calls get_position_manager(&caller, market) which looks up
// "who is the manager FOR caller" — but we need to check "is caller a manager FOR the owner".
//
// REQUIRED FIX: Add on_behalf_of: Option<Address> field to CreateOrderParams.
// Then verify: if on_behalf_of is present, check get_position_manager(&on_behalf_of, market) == Some(caller).
// When absent, caller must be the owner.
//
// For now, this preserves existing behavior but the logic is inverted and needs the refactor above.
let (actual_owner, actual_receiver) = if is_position_order {
// Check if caller is an authorized manager for this market
// TODO(#385): This logic is reversed. See comment above for required fix.
match ds.get_position_manager(&caller, &params.market) {
Some(owner) => {
// Caller is a manager; position owner is stored in data_store
Expand Down
40 changes: 40 additions & 0 deletions libs/math/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -559,4 +559,44 @@ mod tests {
assert_eq!(mul_div_wide_up(&env, 12345, FLOAT_PRECISION, 0), 0);
assert_eq!(mul_div(12345, FLOAT_PRECISION, 0), 0);
}

/// mul_div_wide saturates to i128::MAX when result exceeds i128 range (issue #384).
#[test]
fn mul_div_wide_overflow_saturates_to_max() {
let env = Env::default();
let result = mul_div_wide(&env, i128::MAX, i128::MAX, 1);
assert_eq!(result, i128::MAX, "mul_div_wide overflow should saturate to i128::MAX");
}

/// mul_div_wide saturates to i128::MIN for negative overflow (issue #384).
#[test]
fn mul_div_wide_negative_overflow_saturates_to_min() {
let env = Env::default();
let result = mul_div_wide(&env, i128::MIN, i128::MAX, 1);
assert_eq!(result, i128::MIN, "mul_div_wide negative overflow should saturate to i128::MIN");
}

/// mul_div_wide_up saturates when result exceeds i128 range (issue #384).
#[test]
fn mul_div_wide_up_overflow_saturates_to_max() {
let env = Env::default();
let result = mul_div_wide_up(&env, i128::MAX, i128::MAX, 1);
assert_eq!(result, i128::MAX, "mul_div_wide_up overflow should saturate to i128::MAX");
}

/// mul_div_wide_up saturates to i128::MIN for negative overflow (issue #384).
#[test]
fn mul_div_wide_up_negative_overflow_saturates_to_min() {
let env = Env::default();
let result = mul_div_wide_up(&env, i128::MIN, i128::MAX, 1);
assert_eq!(result, i128::MIN, "mul_div_wide_up negative overflow should saturate to i128::MIN");
}

/// mul_div_wide handles large but safe values correctly (issue #384).
#[test]
fn mul_div_wide_large_but_safe_values() {
let env = Env::default();
let result = mul_div_wide(&env, FLOAT_PRECISION, FLOAT_PRECISION, FLOAT_PRECISION);
assert_eq!(result, FLOAT_PRECISION, "mul_div_wide(FP*FP)/FP must equal FP");
}
}
Loading