Fix math::basis_points_of — Produces Mathematically Wrong Results for Negative Amounts

[Samuel1505]

Description

basis_points_of is documented as computing basis_points / 10_000 of amount "safely without intermediate overflow" using a split identity: (amount / 10_000) * bps + (amount % 10_000) * bps / 10_000. The implementation mixes two different division semantics for the two halves of that identity — checked_div (which truncates toward zero) for the "whole" part, and checked_rem_euclid (which always returns a non-negative remainder) for the "remainder" part:

pub fn basis_points_of(amount: i128, basis_points: u32) -> Result<i128, ContractError> {
    if basis_points > 10_000 {
        return Err(ContractError::InvalidInput);
    }
    if basis_points == 0 {
        return Ok(0);
    }
    let bps_i = basis_points as i128;
    let whole = amount
        .checked_div(10_000)
        .ok_or(ContractError::InvalidInput)?
        .checked_mul(bps_i)
        .ok_or(ContractError::InvalidInput)?;
    let remainder = amount
        .checked_rem_euclid(10_000)
        .ok_or(ContractError::InvalidInput)?
        .checked_mul(bps_i)
        .ok_or(ContractError::InvalidInput)?
        / 10_000;
    whole
        .checked_add(remainder)
        .ok_or(ContractError::InvalidInput)
}

For non-negative amount this identity is correct (as the existing tests confirm), but for negative amount the two divisions disagree about which multiple of 10,000 to subtract, and the result is wrong — not just off by a rounding unit, but wrong in both magnitude and sign. Concretely, basis_points_of(-100, 2500) should equal -100 * 2500 / 10_000 = -25, but the actual computation is:

• whole = (-100).checked_div(10_000) = 0 (truncating division), so whole = 0 * 2500 = 0
• remainder = (-100).checked_rem_euclid(10_000) = 9_900 (Euclidean remainder is always non-negative), so remainder = 9_900 * 2500 / 10_000 = 2_475
• whole + remainder = 2_475

basis_points_of(-100, 2500) returns Ok(2475) instead of the mathematically correct -25. Neither basis_points_of nor proportional_share (which delegates to it) documents or enforces a non-negative precondition on amount, and no test in the #[cfg(test)] module exercises a negative amount. This is a generic, crate-wide "safe math" utility (also used by fees::calculate_fee via crate::math::basis_points_of), so any future caller that passes a signed delta or a value derived from a subtraction without first validating its sign will silently get a nonsensical result instead of a clean error.

Technical Requirements

Files to update

• contracts/contracts/stellar-grants/src/math.rs (basis_points_of, lines 16-38)

Fix direction

Either explicitly reject negative amount up front (if amount < 0 { return Err(ContractError::InvalidInput); }, matching the existing style of validating inputs before doing the split-identity math), or make the whole/remainder split internally consistent for negative values (e.g. use checked_div/checked_rem — both truncating — consistently instead of mixing checked_div with checked_rem_euclid).

Acceptance Criteria

• basis_points_of either returns the mathematically correct value for negative amount, or returns Err(ContractError::InvalidInput) for negative amount — no longer silently returns a wrong positive value.
• A test asserts the new negative-amount behavior (e.g. assert_eq!(basis_points_of(-100, 2500), Err(ContractError::InvalidInput)) or the corrected Ok(-25), depending on the chosen fix direction).
• Existing non-negative test cases (test_basis_points_of_ok, test_basis_points_of_large_amount_no_overflow, test_basis_points_of_max_bps, test_proportional_share_overflow, etc.) continue to pass.
• cargo test passes.

Estimated Effort

Beginner: 3 hours
Intermediate: 1.5 hours
Expert: 1 hour

How to work this issue

1. Read contracts/ContributionGuide.md for the contribution workflow.
2. Comment on the issue to claim it before starting.
3. Branch: fix/issue-120-math-basis-points-negative-amount.
4. Run cargo fmt, cargo clippy -- -D warnings, cargo test before opening your PR.
5. Use a Conventional Commit message, e.g. fix: correct basis_points_of behavior for negative amounts.

Before you start

If you find this project interesting, please consider starring the repository on GitHub. It helps the project gain visibility and supports the Drips Wave program that rewards contributors for merged fixes like this one.