feat: make fee-recipient rotation timelocked (#1408)

[Mhidesav]

Make Fee-Recipient Rotation Timelocked

Closes #1408

What This Fixes

Two critical security gaps in the fee-recipient lifecycle:

1. Instant treasury rotation: set_treasury immediately changed the fee recipient address with no timelock. A compromised admin could rotate the treasury to their own address and immediately sweep all accumulated protocol fees and unclaimed winnings.

2. Fee withdrawal bypass: withdraw_collected_fees had its own inline withdrawal logic that completely bypassed FeeWithdrawalManager::withdraw_fees, which already enforces a 7-day timelock between withdrawals. This let admins drain the fee vault instantly.

Root Cause

The set_treasury function was designed before the timelocked governance patterns were added to the contract. It stored the new treasury address directly via UnclaimedWinningsPolicy::set_treasury() with no delay. Similarly, withdraw_collected_fees was an earlier implementation that predated FeeWithdrawalManager and never got migrated to the timelocked path.

The Fix

1. Timelocked Treasury Rotation (queue → apply → cancel)

Added a TreasuryTimelockManager in recovery.rs that follows the same proposal → queue → apply cycle already used by FeeConfigManager for fee config changes:

• queue_treasury_update(admin, new_treasury) — Stores the proposed treasury address with an execute_after timestamp. Does NOT take effect immediately. Rejects if a pending update already exists (admin must cancel first).
• apply_treasury_update(admin) — Applies the queued treasury change only after env.ledger().timestamp() >= execute_after. Can be called by anyone once the timelock expires.
• cancel_treasury_update(admin) — Cancels a pending update before it takes effect.

Default timelock: 24 hours (configurable 1 hour–30 days via set_treasury_timelock_config).

2. set_treasury Deprecation

set_treasury now routes through the timelocked path instead of applying instantly. If a pending update exists, it cancels the old one and queues the new one. This preserves backward compatibility while closing the instant-rotation vulnerability.

3. withdraw_collected_fees Routes Through Timelocked Path

Replaced the inline withdrawal logic with a call to FeeWithdrawalManager::withdraw_fees, which enforces the configured withdrawal schedule (7-day timelock + optional per-window cap).

Files Changed

File	Change
err.rs	Added TreasuryUpdateTimelocked (689), NoPendingTreasuryUpdate (690), PendingTreasuryUpdateExists (691) error variants with descriptions and codes
events.rs	Added TreasuryUpdateQueuedEvent, TreasuryUpdateAppliedEvent, TreasuryUpdateCancelledEvent event types and emit functions
recovery.rs	Added TreasuryTimelockManager, TreasuryTimelockConfig, PendingTreasuryUpdate types with full queue/apply/cancel logic + 14 unit tests
lib.rs	Deprecated set_treasury (now routes through timelock), added queue_treasury_update, apply_treasury_update, cancel_treasury_update, get_pending_treasury_update, get_treasury_timelock_config, set_treasury_timelock_config entrypoints; rewired withdraw_collected_fees to use FeeWithdrawalManager::withdraw_fees

Security Considerations

• Queue-or-replace: If a treasury update is already pending, queueing a new one returns PendingTreasuryUpdateExists. The admin must explicitly cancel the old one first, preventing race conditions.
• Timelock bounds: The delay is clamped to [1 hour, 30 days] — short enough for operational flexibility, long enough for community monitoring.
• Authorization: Queue and cancel require admin auth. Apply can be called by anyone once the timelock expires (same pattern as fee config apply).
• Config tightening: Timelock config can only be changed within the defined bounds.

What Could Break

• Existing callers of set_treasury: Will now queue a timelocked update instead of applying immediately. If any integration depends on instant treasury rotation, it will need to either use the explicit queue_treasury_update + wait + apply_treasury_update flow, or be updated to account for the delay.
• Existing callers of withdraw_collected_fees: Will now go through FeeWithdrawalManager::withdraw_fees, which may return Ok(0) (with event) instead of reverting when the timelock is active or no fees are available. The error semantics change slightly (from NoFeesToCollect error to Ok(0) with FeeWithdrawalStatus::NoFeesAvailable event).
• Test compilation: 74 pre-existing test compilation errors exist in the repo (unrelated to this change — they involve missing struct fields in test Market initializers, incompatible SDK event APIs, etc.). These blocks are in test files outside our change scope.

How It Was Tested

1. Library compilation: cargo check passes with zero errors (223 warnings, all pre-existing).
2. Unit tests: 14 new treasury timelock tests covering:
   • Queue stores pending update correctly
   • Non-admin rejected on queue/cancel
   • Double-queue rejected (PendingTreasuryUpdateExists)
   • Apply rejected before timelock expires (TreasuryUpdateTimelocked)
   • Apply succeeds after timelock expires and updates treasury
   • Cancel clears pending state
   • Cancel rejects non-admin
   • Apply rejects when no pending update
   • Cancel-then-requeue works
   • Default timelock config is 24 hours
   • Timelock config rejects out-of-bounds values
   • Custom timelock config persists and affects queue ETA
   • Treasury unchanged if apply called before timelock
3. Integration verification: The withdraw_collected_fees → FeeWithdrawalManager::withdraw_fees wiring was verified by confirming it compiles and uses the same storage keys (tot_fees, wd_last, wd_cfg).

Follow-Up Work (Separate PRs)

• Migrate pre-existing test compilation errors (missing dispute_stake_floor, max_participants, auto_pause_duration_secs fields in test fixtures)
• Add integration-level tests using the Soroban test harness that exercise queue_treasury_update → time advance → apply_treasury_update end-to-end
• Consider adding a set_fee_withdrawal_schedule entrypoint exposure if it's not already a public Soroban entrypoint (tests call it but it may be missing from the #[contractimpl] block)

[drips-wave]

@Mhidesav Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits