Harden the fund-moving path against reentrancy and unsafe call ordering
Description
The router in src/lib.rs is on a roadmap to move real SAC value (see the planned on-chain route work). Today compute_route_fee performs its storage writes (counter, timestamp) before doing any work, which is fine while no external calls exist — but once a token::Client::transfer is introduced, the checks-effects-interactions ordering and reentrancy posture become security-critical. This issue establishes an explicit reentrancy guard and a documented effects-before-interactions discipline so the fund-moving path is safe by construction before it ships.
Requirements and context
- Repository scope: StableRoute-Org/Stableroute-contracts only.
- Add a
DataKey::ReentrancyLock (bool) plus a private nonreentrant guard helper that sets the lock on entry and clears it on exit of any entrypoint making external token calls.
- Document and enforce checks-effects-interactions: validate, write effects (counter/timestamp/liquidity), then perform external transfers last.
- Add an append-only
ReentrantCall error returned when the lock is already held.
- Provide a test that simulates a malicious token callback attempting to re-enter and asserts it is rejected (use a mock token contract that calls back).
Suggested execution
- Fork the repo and create a branch
git checkout -b security/contracts-42-reentrancy-guard
- Implement changes
- Write code in:
src/lib.rs — DataKey::ReentrancyLock, the nonreentrant helper, ReentrantCall, and the ordering discipline.
- Write comprehensive tests in:
src/lib.rs #[cfg(test)] mod test — a mock re-entering token contract; assert the second entry panics with ReentrantCall and the lock is released on normal exit.
- Add documentation: add a "Reentrancy & call ordering" section to
README.md.
- Include NatSpec-style doc comments (
///) on the guard helper.
- Validate security assumptions: lock always released on success and on the guarded panic paths; no entrypoint can be re-entered mid-transfer.
- Test and commit
Test and commit
- Run
cargo fmt --all -- --check, cargo build, and cargo test.
- Cover edge cases and failure paths: normal call releases lock, re-entrant call rejected, nested distinct entrypoints, lock state after panic.
- Include the full
cargo test output and a short security notes section in the PR description (threat model + mitigations).
Example commit message
feat: add reentrancy guard and checks-effects-interactions discipline
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.
Harden the fund-moving path against reentrancy and unsafe call ordering
Description
The router in
src/lib.rsis on a roadmap to move real SAC value (see the planned on-chainroutework). Todaycompute_route_feeperforms its storage writes (counter, timestamp) before doing any work, which is fine while no external calls exist — but once atoken::Client::transferis introduced, the checks-effects-interactions ordering and reentrancy posture become security-critical. This issue establishes an explicit reentrancy guard and a documented effects-before-interactions discipline so the fund-moving path is safe by construction before it ships.Requirements and context
DataKey::ReentrancyLock(bool) plus a privatenonreentrantguard helper that sets the lock on entry and clears it on exit of any entrypoint making external token calls.ReentrantCallerror returned when the lock is already held.Suggested execution
git checkout -b security/contracts-42-reentrancy-guardsrc/lib.rs—DataKey::ReentrancyLock, thenonreentranthelper,ReentrantCall, and the ordering discipline.src/lib.rs#[cfg(test)] mod test— a mock re-entering token contract; assert the second entry panics withReentrantCalland the lock is released on normal exit.README.md.///) on the guard helper.Test and commit
cargo fmt --all -- --check,cargo build, andcargo test.cargo testoutput and a short security notes section in the PR description (threat model + mitigations).Example commit message
feat: add reentrancy guard and checks-effects-interactions disciplineGuidelines
Community & contribution rewards