fix: remove repay timelock mechanism

[maxencerb]

Fixes 3F-66

Summary

Removes the repay timelock mechanism entirely. Guardian signatures (3F-69) now provide stronger, proactive security by requiring quorum approval before setRequest can be called — making the reactive timelock redundant.

The timelock also blocked flash loan request contracts that need atomic setRequest + repay in the same transaction.

Changes

• Remove repayTimelock from FacilityStorageData storage struct
• Remove requestSetAt from Intent struct (no longer needed without timelock)
• Remove repayAvailableAt(), setRepayTimelock(), _setRepayTimelock() functions
• Remove timelock enforcement in repay()
• Remove RepayTimelockSet event and RepayTimelockActive error
• Remove repayTimelock_ param from initialize()
• Update facilityConfig() and getIntent() return values
• Delete FacilityRepayTimelockPoC.t.sol and all timelock-related tests
• Remove FAC-11 and FAC-12 invariants (timelock enforcement and requestSetAt consistency)

Test plan

• All 1232 tests pass (0 failed)
• 100% coverage on all modified source files
• Build passes with FOUNDRY_PROFILE=ci forge build

[linear]

3F-66 [CS-GRUNT-006] Facilitator Theft With Malicious Request

ChainSecurity I-11 — CRITICAL

Source: ChainSecurity Audit
Severity: Critical — Complete theft of all user deposits by FACILITATOR_ROLE
Location: FacilityIntents.sol:81-84 (lock), FacilityIntents.sol:156-182 (setRequest), FacilityRequests.sol:47-63 (repay), RequestFactory.sol:111-131 (createRequest), VaultController.sol:237-244 (burnAll)

---

Description

If an intent is in DEPOSITING state, the Facilitator can steal all deposits within two blocks. The attack exploits the fact that the Facilitator controls the entire intent lifecycle: lock, setRequest, repay, and resolve — with no checks that the Request is legitimate or that repaid funds flow back to depositors.

Attack Sequence

Block N (single transaction via multicall or contract):

1. lock(intentId) — transitions intent from DEPOSITING to RESOLVING
2. RequestFactory.createRequest(facilitator, facilitator, facilitator, asset, ..., block.timestamp + 12) — creates a malicious Request where the Facilitator is owner, puller, AND consumer. repaymentDeadline set to 12 seconds (next block)
3. authorizeMinting(self, 1, 1) then mint() on the Request — deposits 1 wei of the asset, mints 1 PT + 1 YT. Facilitator now holds 100% of PT/YT supply
4. setRequest(intentId, maliciousRequest) — links the malicious Request to the intent. setRequest uses getIntent(id) (no state restriction), and the only validation is that the Request's asset matches the PM's debt asset
5. repay(intentId, totalFacilityBalance) — the Facility transfers ALL deposited funds to the malicious Request via safeTransferFrom. This works because the repayment deadline hasn't passed yet (same block)

Block N+1 (12 seconds later):

6. burnAll(self, self) — the repaymentDeadline has passed, so _syncWithdrawalStatus() returns true, enabling withdrawals. The Facilitator burns their 1 PT + 1 YT and receives:
   • pAssets = min(totalBalance, ptSupply) = min(allFunds + 1, 1) = 1
   • yAssets = totalBalance - pAssets = allFunds
   • Total extracted = allFunds + 1 (net theft = allFunds)

Root Causes

1. setRequest() has no validation on Request origin — accepts any contract, not just protocol-deployed Requests. The factory is permissionless, so the Facilitator can create their own Request with arbitrary parameters.
2. repay() has no bounds — the Facility can transfer its entire balance to a Request with no cap relative to the intent's actual deposits or pulled amounts.
3. Facilitator controls the full lifecycle — a single role (FACILITATOR_ROLE) can execute lock → setRequest → repay → resolve with no guardian oversight, timelock, or multi-sig requirement.
4. repaymentDeadline acts as an unconditional withdrawal enabler — once the deadline passes, _syncWithdrawalStatus() sets repaid = true regardless of whether actual repayment occurred. Combined with the Facilitator holding 100% of PT/YT supply, this allows full extraction.
5. No separation of concerns — the same role that can move funds out of the Facility can also create and link the destination. There's no independent verification that funds were legitimately borrowed and repaid.

Impact

Complete loss of all user deposits in any intent where the Facilitator is malicious or compromised. The attack:

• Requires only 1 wei of capital from the attacker
• Completes in 2 blocks (24 seconds)
• Leaves depositors with worthless LP tokens (the Facility's balance is drained)
• Is undetectable until after execution (no timelock, no guardian signatures)

Proof of Concept Trace

State: Intent has 1,000,000 USDC deposited by users

Block N:
  lock(id)                          → Intent: RESOLVING
  factory.createRequest(            → Creates Request owned by Facilitator
    facilitator, facilitator, facilitator,
    USDC, "X", "X", block.timestamp + 12
  )
  request.authorizeMinting(self, 1, 1)
  request.mint()                    → Facilitator deposits 1 USDC, gets 1 PT + 1 YT
  setRequest(id, request)           → Links malicious Request to intent
  repay(id, 1_000_000e6)           → Facility transfers 1M USDC to Request
                                     Request balance: 1_000_001 USDC

Block N+1:
  request.burnAll(self, self)       → Deadline passed, withdrawals enabled
                                     pAssets = min(1_000_001, 1) = 1
                                     yAssets = 1_000_001 - 1 = 1_000_000
                                     Facilitator receives: 1_000_001 USDC
                                     Net theft: 1_000_000 USDC

Recommended Fixes

Option 1 — Restrict Request linking (most impactful):

• Only allow setRequest() for Requests created by a trusted, protocol-managed factory
• Verify the factory address on-chain: require(trustedFactory.isRequest(newRequest))
• Ensure the factory enforces that Request owner/puller/consumer are protocol-controlled addresses

Option 2 — Require Request before lock:

• setRequest() should only be callable in DEPOSITING state, so depositors can see the Request before committing
• This gives users transparency — they know which Request their funds will flow to before they deposit

Option 3 — Bound repay amounts:

• Track the total amount pull()-ed from a Request and cap repay() to that amount
• Prevent the Facility from sending MORE funds to a Request than were originally pulled from it

Option 4 (Preferred) — Add timelock on setRequest + repay:

• Require a delay between setRequest() and the first repay() call
• This gives guardians and depositors time to review and potentially pause

Option 5 — Require guardian signatures for repay:

• Leverage the existing quorum/guardian system for repay() operations, similar to how swaps require guardian signatures

These fixes are not mutually exclusive and should ideally be combined for defense-in-depth.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Free

Run ID: 0f474860-ad6f-4b20-9534-349edb82d905

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

---

Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}