Escrow

executeYearlyIncrementChange updates yearlyIncrementUsd immediately, but the contract only settles elapsed yearly growth later inside _applyYearlyIncrements. This means already-accrued time is repriced at the new increment rate when the next claim occurs instead of at the rate that was active while the time accrued.

function executeYearlyIncrementChange(uint256 _yearlyIncrementUsd) external onlyAdmin {
    _execute(keccak256(abi.encode("yearlyIncrement", _yearlyIncrementUsd)));
    yearlyIncrementUsd = _yearlyIncrementUsd; // new rate is stored immediately
    emit YearlyIncrementChanged(_yearlyIncrementUsd);
}

function _applyYearlyIncrements() internal {
    if (yearlyIncrementUsd == 0) return;
    uint256 yrs = (block.timestamp - lastIncrementTimestamp) / 365 days;
    if (yrs > 0) {
        maxPeriodUsd += yrs * yearlyIncrementUsd; // elapsed years use the latest rate
        lastIncrementTimestamp += yrs * 365 days;
        emit YearlyIncrementApplied(maxPeriodUsd, yrs);
    }
}

Impact:

• Raising yearlyIncrementUsd can retroactively over-credit the claimant for past years, which can be gamed by delaying claim.
• Lowering yearlyIncrementUsd can claw back claim capacity that had already accrued economically but was not yet materialized on-chain.
• Setting yearlyIncrementUsd to 0 can wipe out all previously accrued but unsettled yearly growth.

Example:

• maxPeriodUsd starts at $1,000 and yearlyIncrementUsd starts at $100.
• Three full years pass without a claim, so under the original rate the cap would have grown to $1,300.
• Before the claimant makes the next claim, the admin executes executeYearlyIncrementChange with $200.
• The claimant waits until after that update to submit the delayed claim.
• _applyYearlyIncrements now applies 3 * $200, so the cap becomes $1,600 instead of $1,300.

The result is that the delayed config update changes the meaning of past elapsed time, not just future elapsed time.

Before writing a new yearlyIncrementUsd, first settle any accrued growth under the old rate. One approach is to checkpoint the pending increase in executeYearlyIncrementChange before updating the variable. Another approach is to store an effective from timestamp so time before the change uses the old rate and time after the change uses the new rate.

executeMinPeriodChange updates minPeriod immediately, and _rollPeriod uses the current minPeriod when deciding whether the current claim window has expired. Because of that, increasing minPeriod can retroactively extend a period that would already have expired under the old setting.

function executeMinPeriodChange(uint256 _minPeriod) external onlyAdmin {
    if (_minPeriod < 7 days) revert MinPeriodTooShort();
    _execute(keccak256(abi.encode("minPeriod", _minPeriod)));
    minPeriod = _minPeriod; // applies to the current live period immediately
    emit MinPeriodSet(_minPeriod);
}

function _rollPeriod() internal {
    if (block.timestamp >= periodStart + minPeriod) {
        claimedThisPeriod = 0;
        periodStart = block.timestamp;
    }
}

Impact:

• The claimant can lose access to a fresh period even after the previous waiting window has already elapsed.
• The admin can delay the claimant’s next withdrawal by extending minPeriod shortly before the claimant makes the first post-expiry claim.

Example:

• The claimant uses the full $1,000 budget with minPeriod = 7 days.
• After 7 days pass, the claimant should be eligible for a fresh period.
• Before the claimant calls claim, the admin executes executeMinPeriodChange to raise minPeriod to 30 days.
• _rollPeriod now checks periodStart + 30 days, so the contract still treats the old period as active and the new claim reverts.

The result is that the delayed config update changes the status of the current period instead of only affecting future periods.

Make minPeriod changes take effect only for the next period boundary. For example, keep the current live period under the old minPeriod and apply the new value only after _rollPeriod starts a new period. Another option is to settle the current period at execution time and then start a new period using the new duration.

The constructor does not prevent admin and claimant from being set to the same address. When that happens, the contract's two-party approval model degrades because _callerBit returns ADMIN_BIT first, so the shared address can never contribute CLAIMANT_BIT.

function _callerBit() internal view returns (uint8) {
    if (msg.sender == admin) return ADMIN_BIT;
    if (msg.sender == claimant) return CLAIMANT_BIT;
    return 0;
}

As a result, express role-change execution cannot be reached because roleApprovals[hash] can never become BOTH_BITS. Similarly, cancelAction can never fully cancel a pending action because cancelApprovals[hash] can never reach BOTH_BITS.

Reject deployments where admin == claimant, or handle the shared-role case explicitly.

The contract does not carry forward unused allowance from one period to the next. When _rollPeriod detects that the old period has expired, it resets claimedThisPeriod to 0 and starts a new period at the current timestamp. Any unclaimed portion of maxPeriodUsd from the prior period is discarded.

function _rollPeriod() internal {
    if (block.timestamp >= periodStart + minPeriod) {
        claimedThisPeriod = 0; // prior-period usage is cleared with no rollover accounting
        periodStart = block.timestamp;
    }
}

This behavior may be intentional, but a claimant could reasonably expect missed allowance to accumulate or at least remain claimable until explicitly forfeited. So if the claimant forgets to call claim during a period, the unused portion of that period’s budget is lost.

Document this behavior explicitly. If rollover is desired, add separate accounting for unspent allowance rather than resetting only claimedThisPeriod.

claim lets the claimant choose a _usdAmount, but it does not let them enforce a minimum acceptable token amount.

function claim(address _token, uint256 _usdAmount) external onlyClaimant {
    ...
    uint256 tokenAmount = _usdToTokens(_usdAmount, cfg); // amount is fixed only at execution time
    claimedThisPeriod += _usdAmount;

    IERC20(_token).safeTransfer(claimant, tokenAmount);
    emit Claimed(claimant, _token, tokenAmount, _usdAmount);
}

Example:

• The claimant checks getTokenAmount and sees that claiming $1,000 should return 0.5 WETH.
• Before the transaction is mined, the oracle price updates upward (or receives a bad oracle update).
• claim executes using the new higher price, so the claimant receives only 0.45 WETH.
• The full $1,000 is still counted against claimedThisPeriod.

Extend claim to accept a claimant-specified minimum token amount, for example minTokenAmount, and revert when the oracle-priced output falls below that floor.

The contract uses a single ACTION_DELAY for all delayed operations, including role changes, emergency withdrawals, and routine configuration updates. This keeps the implementation simple, but it also means actions with very different risk profiles all share the same review window.

For example, signalAdminChange, signalWithdraw, and signalPriceFeedChange all mature after the same 7 days. In practice, a bad role change or withdrawal is much more security-sensitive than a routine oracle update. A single delay forces the system to choose one compromise between security response time and operational flexibility.

As a result,

• High-impact actions such as role changes and withdraw do not get a longer reaction window than routine parameter updates.
• Operational changes such as price feed or cap updates cannot be expedited without also shortening the delay for more dangerous actions.
• The timelock cannot be tuned per action type to match the actual security and operational needs of the escrow.

Consider separating delays by action class. A common split is to use a longer delay for role changes and emergency withdrawals, and a shorter delay for routine configuration changes such as feed or cap updates.

_getPrice hardcodes a single 24 hours freshness window for every configured Chainlink feed. That assumes all supported assets share the same acceptable staleness threshold, but in practice feeds can have different heartbeats.

function _getPrice(TokenConfig memory cfg) internal view returns (uint256) {
    (, int256 answer,, uint256 updatedAt,) = cfg.priceFeed.latestRoundData();
    if (answer <= 0) revert InvalidPrice();
    if (block.timestamp - updatedAt > 24 hours) revert StalePrice(); // fixed threshold for every feed
    return uint256(answer);
}

As a result, feeds whose expected freshness is shorter than 24 hours can still be accepted even after they should be treated as stale, allowing claim to settle against outdated prices. Similarly, feeds whose heartbeat is longer than 24 hours can become unusable even when the oracle is behaving as configured, blocking otherwise valid claims.

Store a per-token maximum price age alongside each feed configuration and check updatedAt against that token-specific value instead of a universal 24 hours. If the escrow is intended for L2 deployment, also consider gating claims on the relevant sequencer uptime feed.

The escrow treats admin and claimant as equal co-signers for recovery and cancellation. Either role can initiate replacement of the other after ACTION_DELAY, and cancelAction requires approvals from both roles. As a result, the contract's safety model is bilateral: the claimant is not only a capped beneficiary, but also a participant in privileged recovery flow.

function cancelAction(bytes32 actionHash) external onlyAdminOrClaimant {
    cancelApprovals[actionHash] |= _callerBit();
    if (cancelApprovals[actionHash] == BOTH_BITS) {
        delete pendingActions[actionHash]; // both roles must participate
    }
}

In a key-compromise scenario, admin and claimant must work together to cancel hostile pending actions before execution. ACTION_DELAY provides notice and time to coordinate, but it does not create a unilateral admin-side veto.

For example:

• An attacker uses a leaked claimant key to call signalAdminChange(attackerAdmin).
• The honest admin cannot clear that action alone, because cancelAction requires both role approvals.
• If the honest claimant still has practical control of the leaked key, the admin and claimant can coordinate to cancel the action before ACTION_DELAY expires.
• If they fail to coordinate in time, attackerAdmin can execute the rotation after the delay.

Document the trust assumptions explicitly: claimant is an equal co-signer in recovery and cancellation flows. Operationally, both roles should be prepared to act together if a key is leaked, because hostile pending actions cannot be cancelled by one side alone. If this bilateral model is not desired, recovery powers should be concentrated on an admin-side multisig instead of being shared with the claimant.