Move the cooldown and bounds checks ahead of the liquidity debit in compute_route_fee to avoid spending liquidity on a rejected route

[mikewheeleer]

Reorder compute_route_fee so the cooldown check runs before liquidity is debited

Description

In compute_route_fee (src/lib.rs) the CHECKS/EFFECTS ordering is subtly wrong: the liquidity debit (and its liq_used event) happens before the per-pair cooldown check. If a route passes the min/max/liquidity gates but is then rejected by RouteCooldownActive, the whole transaction reverts — which is correct — but the code has already written PairLiquidity and emitted liq_used earlier in the same body. While Soroban's atomic rollback saves correctness today, the ordering violates the CHECKS-then-EFFECTS discipline the contract documents elsewhere and is fragile: any future early-return-without-revert (or partial-effect refactor) would leak a liquidity decrement for a route that never happened. This issue reorders all read-only CHECKS (including cooldown) ahead of every EFFECT (liquidity debit, counters, timestamp, events).

Requirements and context

• Repository scope: StableRoute-Org/Stableroute-contracts only.
• Move the cooldown check so it runs in the CHECKS phase, before the liquidity decrement and the liq_used emission.
• Keep the existing semantics: registered, positive amount, min, max, liquidity sufficiency, and cooldown must all pass before any storage write or event.
• This is distinct from the issues-4 "release the reentrancy lock on every panic path" item — that is about the lock; this is about the CHECKS/EFFECTS ordering of liquidity vs cooldown.
• Add an inline comment documenting the strict CHECKS-then-EFFECTS contract so the ordering is not accidentally regressed.

Suggested execution

• Fork the repo and create a branch
• git checkout -b security/contracts-checks-before-effects-ordering
• Implement changes
  • Write code in: src/lib.rs — reorder compute_route_fee so the cooldown gate precedes the liquidity debit and all other effects.
  • Write comprehensive tests in: src/lib.rs (inline #[cfg(test)] mod test) — assert that a cooldown-blocked route leaves PairLiquidity, the route counter, the volume, and the timestamp unchanged, and emits no liq_used or route event.
  • Add documentation: note the CHECKS-then-EFFECTS guarantee in SECURITY.md.
  • Include NatSpec-style doc comments (///) updating compute_route_fee's ordering description, matching the existing style in lib.rs.
  • Validate security assumptions: no effect is applied before all guards pass; no event leaks on a rejected route.
• Test and commit

Test and commit

• Run cargo fmt --all -- --check, cargo build, and cargo test.
• Cover edge cases: cooldown-blocked route, first-ever route, and a route that passes all gates.
• Include the full cargo test output and a short security notes section in the PR description.

Example commit message

security: order all CHECKS before liquidity debit in compute_route_fee with tests

Guidelines

• Minimum 95 percent test coverage for impacted modules.
• Clear, reviewer-focused documentation.
• Timeframe: 96 hours.

Community & contribution rewards

• 💬 Join the StableRoute community on Discord for questions, reviews, and faster merges: https://discord.gg/37aCpusvx
• ⭐ This is a GrantFox OSS / Official Campaign task and may be rewarded. When your PR is merged you'll be prompted to rate the project — if this issue and the maintainers helped you ship, we'd be grateful for a 5-star rating. Clear questions in Discord and tidy, well-tested PRs are the fastest path to a merge and a reward.