Ondo Finance

Ondo: rwa-internal

Cantina Security Report

Organization

@Ondofinance

Engagement Type

Cantina Solo

Period

-

Researchers


Findings

Informational

3 findings

3 fixed

0 acknowledged

Gas Optimizations

2 findings

2 fixed

0 acknowledged


Informational3 findings

  1. Initialize issuer proxy with contract deployment

    Severity

    Severity: Informational

    Submitted by

    HickupHH3


    Description

    The GMIssuerManager proxy is created with empty data and initialization is a follow-up call. Foundry broadcasts those as two transactions, so the proxy sits uninitialized in between. On a public mempool, a frontrunner can invoke initialize with their own admin after the proxy transaction and before the deployer.

    Passing the calldata into the proxy constructor not only mitigates this, it also removes the need to broadcast another transaction, thus saving cost.

    Recommendation

    new TransparentUpgradeableProxy(     impl,     proxyAdmin,     abi.encodeCall(          GMIssuerManager.initialize,          (gmTokenManager, defaultAdmin, network, name, version)     ));
  2. Comment & Natspec Improvements

    Severity

    Severity: Informational

    Submitted by

    HickupHH3


    Description & Recommendation

    The referenced comments are stale, incomplete or can be clarified:

    1. consumeGlobalNet has no skip-if-inactive

      consume() already says that token and net limits are optional. consumeToken() repeats that on the wrapper, while consumeGlobalNet() does not. Consider add a @dev comment on the optionality (skip if inactive).

    2. setConfig resetting decay accounting is no-op for defaultUser

      defaultUser.capacityUsed & lastUpdated params are unused, as usage lives on each user’s own bucket. A lowered default therefore does not grant fresh per-user capacity. Consider providing clarification on this case.

      - * @dev    Resets `capacityUsed`; a lowered limit grants instant fresh capacity+ * @dev    For rate limit types besides DEFAULT_USER, resets `capacityUsed`; a lowered limit grants instant fresh capacity
    3. Misleading @dev comment for setUserMint/RedeemRateLimit() functions

      resolve to userId 0 and share that override; unset is confusing because requireEligible() reverts with UserNotRegistered for 0 userId. Consider dropping this portion of the comment.

  3. Redundant Import

    Severity

    Severity: Informational

    Submitted by

    HickupHH3


    Description & Recommendation

    The referenced line is a redundant import and can be deleted.

Gas Optimizations2 findings

  1. Cache storage reads into local variables

    Severity

    Severity: Gas optimization

    Submitted by

    HickupHH3


    Description

    1. Cache rl.limit and rl.window as they are used (multiple) times.
    2. Return capacityUsed in checkAndUpdateRateLimit as it will be used for the event emission in the parent function.

    POC

    Total tests: 119, ↑ 8, ↓ 63, ━ 48Overall gas change: -9426 (-0.019%)

    Recommendation

    diff --git a/contracts/globalMarkets/issuer/rateLimit/RateLimitLib.sol b/contracts/globalMarkets/issuer/rateLimit/RateLimitLib.solindex 2a2dda00..3c5e71fd 100644--- a/contracts/globalMarkets/issuer/rateLimit/RateLimitLib.sol+++ b/contracts/globalMarkets/issuer/rateLimit/RateLimitLib.sol@@ -222,10 +222,11 @@ library RateLimitLib {   function consumeGlobal(Direction direction, uint256 usd) internal {     RateLimit storage rl = dirLimits(direction).global;     if (!rl.active) revert RateLimitNotConfigured(RateLimitType.GLOBAL, direction);-    (bool ok, uint256 available) = checkAndUpdateRateLimit(rl, rl.limit, rl.window, usd);+    (uint256 limit, uint48 window) = (rl.limit, rl.window);+    (bool ok, uint256 available, uint256 capacityUsed) = checkAndUpdateRateLimit(rl, limit, window, usd);     if (!ok) revert RateLimited(RateLimitType.GLOBAL, direction, usd, available);     emit RateLimitConsumed(-      RateLimitType.GLOBAL, direction, bytes32(0), usd, rl.capacityUsed, rl.limit - rl.capacityUsed+      RateLimitType.GLOBAL, direction, bytes32(0), usd, capacityUsed, limit - capacityUsed     );   } @@ -234,15 +235,16 @@ library RateLimitLib {   function consumeToken(Direction direction, address token, uint256 usd) internal {     RateLimit storage rl = dirLimits(direction).token[token];     if (!rl.active) return;-    (bool ok, uint256 available) = checkAndUpdateRateLimit(rl, rl.limit, rl.window, usd);+    (uint256 limit, uint48 window) = (rl.limit, rl.window);+    (bool ok, uint256 available, uint256 capacityUsed) = checkAndUpdateRateLimit(rl, limit, window, usd);     if (!ok) revert RateLimited(RateLimitType.TOKEN, direction, usd, available);     emit RateLimitConsumed(       RateLimitType.TOKEN,       direction,       bytes32(uint256(uint160(token))),       usd,-      rl.capacityUsed,-      rl.limit - rl.capacityUsed+      capacityUsed,+      limit - capacityUsed     );   } @@ -251,10 +253,10 @@ library RateLimitLib {   function consumeUser(Direction direction, bytes32 user, uint256 usd) internal {     (RateLimit storage rl, uint256 limit, uint48 window, bool active) = userConfig(direction, user);     if (!active) revert RateLimitNotConfigured(RateLimitType.USER, direction);-    (bool ok, uint256 available) = checkAndUpdateRateLimit(rl, limit, window, usd);+    (bool ok, uint256 available, uint256 capacityUsed) = checkAndUpdateRateLimit(rl, limit, window, usd);     if (!ok) revert RateLimited(RateLimitType.USER, direction, usd, available);     emit RateLimitConsumed(-      RateLimitType.USER, direction, user, usd, rl.capacityUsed, limit - rl.capacityUsed+      RateLimitType.USER, direction, user, usd, capacityUsed, limit - capacityUsed     );   } @@ -265,11 +267,12 @@ library RateLimitLib {   function consumeUserOnly(Direction direction, bytes32 user, uint256 usd) internal {     RateLimit storage rl = dirLimits(direction).user[user];     if (!rl.active) revert RateLimitNotConfigured(RateLimitType.USER, direction);-    (bool ok, uint256 available) = checkAndUpdateRateLimit(rl, rl.limit, rl.window, usd);+    (uint256 limit, uint48 window) = (rl.limit, rl.window);+    (bool ok, uint256 available, uint256 capacityUsed) = checkAndUpdateRateLimit(rl, limit, window, usd);     if (!ok) revert RateLimited(RateLimitType.USER, direction, usd, available);     // Only the USER tier emits — a privileged tx has no GLOBAL/TOKEN/NET checkpoint.     emit RateLimitConsumed(-      RateLimitType.USER, direction, user, usd, rl.capacityUsed, rl.limit - rl.capacityUsed+      RateLimitType.USER, direction, user, usd, capacityUsed, limit - capacityUsed     );   } @@ -348,13 +351,13 @@ library RateLimitLib {     uint256 limit,     uint48 window,     uint256 usd-  ) internal returns (bool ok, uint256 available) {+  ) internal returns (bool ok, uint256 available, uint256 capacityUsed) {     uint256 used;     (used, available) = _calculateDecay(state, limit, window);-    if (usd > available) return (false, available);-    state.capacityUsed = used + usd;+    if (usd > available) return (false, available, type(uint256).max);+    state.capacityUsed = capacityUsed = used + usd;     state.lastUpdated = block.timestamp;-    return (true, available);+    return (true, available, capacityUsed);   }    // ─────────────────────────────────────────────────────────────────────────────
  2. Redundant conditional check

    Severity

    Severity: Gas optimization

    Submitted by

    HickupHH3


    Description

    The 1st zero cap condition is redundant, since the check above enforces usd > 0, it's covered by the 2nd.

    Recommendation

    - if (cap == 0 || usd > cap)+ if (usd > cap)