Gate withdraw_protocol_fees behind the emergency flag in addition to the pause flag

[mikewheeleer]

Gate withdraw_protocol_fees behind the emergency flag in addition to the pause flag

Description

withdraw_protocol_fees in contracts/escrow/src/lib.rs blocks withdrawal while DataKey::Paused is set, but it inspects only the Paused flag directly rather than going through the shared require_not_paused guard that the rest of the contract uses. Because the guard helper distinguishes the emergency case (EmergencyActive) from the ordinary pause case (ContractPaused), bypassing it here makes fee withdrawal inconsistent with every other money-flow entrypoint: the error surfaced during an emergency is the generic ContractPaused, and any future code path that sets Emergency without also setting Paused would leave fee withdrawal unexpectedly open.

This issue routes withdraw_protocol_fees through the same require_not_paused gate, so an active emergency reports EmergencyActive and the freeze semantics match the rest of the contract.

Requirements and context

• Repository scope: Talenttrust/Talenttrust-Contracts only.
• Replace the ad-hoc Paused-only check in withdraw_protocol_fees with the shared Self::require_not_paused(&env) guard.
• Confirm require_not_paused emits EmergencyActive when emergency is active and ContractPaused when only paused; keep that distinction observable to callers.
• Preserve admin require_auth, the amount > 0 check, the InsufficientAccumulatedFees check, and the typed SettlementTokenNotConfigured path.
• Do not change the accumulated-fee accounting or the fee withdraw event payload.

Suggested execution

• Fork the repo and create a branch
• git checkout -b security/contracts-fee-withdraw-emergency-gate
• Implement changes
  • Write code in: contracts/escrow/src/lib.rs — swap the inline pause check in withdraw_protocol_fees for require_not_paused.
  • Write comprehensive tests in: contracts/escrow/src/test/protocol_fees.rs — assert withdrawal reverts with ContractPaused when paused and EmergencyActive when emergency is active, and succeeds once resolved.
  • Add documentation: note the emergency gating in the entrypoint doc comment.
  • Include NatSpec-style doc comments (///) reflecting both freeze errors.
  • Validate security assumptions: no fee drain during pause or emergency.
• Test and commit

Test and commit

• Run cargo fmt --all -- --check, cargo build, and cargo test.
• Cover edge cases and failure paths: paused, emergency-active, and post-resolution withdrawal.
• Include the full cargo test output and a short security notes section in the PR description.

Example commit message

fix: gate withdraw_protocol_fees behind the shared pause-and-emergency guard

Guidelines

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

Community & contribution rewards

• 💬 Join the TalentTrust community on Discord for questions, reviews, and faster merges: https://discord.gg/WqnGpcPx
• ⭐ 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.

[GBOYEE]

withdraw_protocol_fees checks DataKey::Paused directly instead of using Self::require_not_paused(&env). This means during an emergency, fee withdrawal shows ContractPaused instead of EmergencyActive, and future code paths that set Emergency without Paused would leave withdrawal open.

Plan:

• Replace the inline Paused check in withdraw_protocol_fees with Self::require_not_paused(&env)
• Confirm error output is EmergencyActive during emergency, ContractPaused otherwise
• Add tests: withdrawal blocked with EmergencyActive during emergency, ContractPaused when only paused, succeeds when neither is set
• Preserve admin auth, amount validation, and SettlementTokenNotConfigured checks

Requesting assignment, PR same day.

[YfengJ]

Hi maintainers, I would like to apply for this issue if it is still available.

If assigned, I will keep the change focused on withdraw_protocol_fees: route the pause/emergency validation through the existing guard semantics, preserve current protocol-fee behavior, and add regression coverage for normal, paused, and emergency-active cases.