odx-contracts

Description

_mint() first calculates the USD value of the amount tokens to mint using _calculateMintValue(). If the maximum remaining limit of the epoch is lower than the USD value of amount tokens, then it returns a lower value.

_mint() function then updates the limit of the minter and witnesses by this value, but still mints the full amount to the destination.

Thus, in the worst case, even if _calculateMintValue() returns 0, the full amount is minted.

Recommendation

Either revert if minting the entire amount exceeds the limit or mint a lower amount.

ODX: Fixed in 95fddb8.

Cantina: Fix verified.

Description

Controller and Oracle contract are upgradeable contracts. Upgradeable contracts should initialize their storage variables only in initializer() since any initialization of these variables in constructor doesn't affect the proxy storage.

However, these contracts call AccessControlDefaultAdminRules() and Ownable() constructor setting key variables.

Recommendation

• Use the upgradeable version of all the OZ contracts from https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable.
• Add _disableInitializers() (imported from Initializable contract) to the constructors to block anyone to call initializer() on these implementation contracts.

ODX: Fixed in e6b7594.

Cantina: Fix verified.

Description

The _calculateMintValue function forces the merchant and each witness to have exactly the same remaining “USD mint budget” for the requested amount whenever one of them surpasses the mint limit. If any participant has a smaller budget, the function reverts and the entire mint operation fails. This design means there is no mechanism for partial fulfilment of a mint request if one signer has a lower limit.

For example:

• Minting amount: 1000 USD
• Merchant remaining mint limit for current epoch: 900
• Witness #1 mint limit for current epoch: 900
• Witness #2 mint limit for current epoch: 950

The request will revert with a MintLimitReached error instead of allowing the mint of 900 USD worth in tokens.

Recommendation

If the goal is to prevent any partial minting and ensure full alignment across all signers, no code change is needed. However, if partial mints are a desired feature (for example, allowing the merchant to mint only what each witness can also mint without surpassing their limit), consider modifying _calculateMintValue to handle partial amounts. This might involve returning and minting only the smallest leftover budget among the participants (the merchant and all the witnesses) rather than reverting entirely.

ODX: Acknowledged. We want to prevent any partial minting, since the system around this is not setup to handle partial mints

Description

The request(Request memory req, bytes[] memory witnessSigs) function checks hasRole(MERCHANT_ROLE, msg.sender) but does not enforce req.account == msg.sender. Consequently, a merchant (MerchantA) can call request using req.account = MerchantB (as long as MerchantB also has the MERCHANT_ROLE). Although this does not allow MerchantA to mint funds in MerchantB’s name (because the minted tokens still go to req.account specified in the request), it can be used to “front-run” or otherwise interfere with MerchantB’s intended batch operation if MerchantB was about to call the same request.

A potential problem with this implementation is that if the front-running merchant was able to have access somehow to the witness signatures given to the legit merchant, the front-running merchant could submit the request call in an epoch where both (the legit merchant and all the witnesses) are over the mint limit. This way the signatures would be invalidated but the legit merchant would not receive any minted tokens (it would be minted 0 tokens). Whenever witnessVal or mintVal is equal to 0 the transaction should revert to avoid this scenario from happening

Recommendation

If the design requires that each merchant only submit requests for their own account, consider adding require(req.account == msg.sender) within the request function implementation. That ensures no one can front-run or submit a request “on behalf” of another merchant. On the other hand, consider reverting whenever witnessVal or mintVal is equal to 0.

ODX: Fixed in e3c9f4e.

Cantina: Fix verified.

Description

Within the Controller._validateRequest function, userd for signature verification, the code snippet

function _validateRequest(Request memory req, bytes[] memory witnessSig) internal returns (address[] memory) {    uint256 operationsLength = req.operations.length;    uint256 witnessLength = witnessSig.length;    bytes32 digest = hashRequest(req);    uint256 nonce = req.nonce;    address merchant = req.account;    address[] memory witnesses = new address[](witnessLength);
    if (req.expiration < block.timestamp) revert SignatureExpired();    if (req.expiration > block.timestamp + EPOCH_DURATION) revert SignatureTooFarInFuture();    if (witnessLength != witnessThreshold) revert InvalidWitnessCount();    if (operationsLength == 0) revert NoOperations();
    _checkRoleAndPaused(merchant, MERCHANT_ROLE);    _useUnorderedNonce(merchant, nonce);
    for (uint256 i = 0; i < witnessSig.length; ++i) {        address witness = digest.recover(witnessSig[i]); // <-----------------------------        _checkRoleAndPaused(witness, WITNESS_ROLE);        _useUnorderedNonce(witness, nonce);        witnesses[i] = witness;    }
    for (uint256 i = 0; i < operationsLength; ++i) {        Operation memory op = req.operations[i];        _validateOperation(op);    }    return witnesses;}

relies on ECDSA.recover which only supports externally owned accounts (EOAs). As a result, contract wallets (including multisigs) cannot generate valid signatures under this scheme. This restriction excludes a significant user base that employs smart contract wallets for automation, treasury management and advanced DeFi interactions. Any operator behind a multisig will not be able to provide a valid signature and operate as a witness.

Recommendation

Use a more flexible approach that accommodates both EOAs and contract wallets. For instance, OpenZeppelin’s SignatureChecker.isValidSignatureNow can verify signatures from EOAs while also supporting ERC1271 for contract accounts. By switching to a signature verification method compatible with contract wallets, you ensure that operators behind a multisig could provide their signature to operate as a witness.

ODX: Acknowledged. At this point in time, we are not looking to support smart-contract wallet signatures.

Description

Since ODX contracts are supposed to be used as proxy implementations, introducing new storage variables may misalign the existing storage variables with the storage slots they currently occupy.

Recommendation

1. Add uint[50] private __gap to each contract.
2. If introducing a new storage variable, declare it just before __gap declaration, and decrement the size of _gap by 1.

ODX

Fixed in 066ff4d and 16aab29.

Cantina

Fix verified.

Description

Within ODXOracle, the contract calls priceFeed.latestRoundData() to obtain the Chainlink aggregator’s latest price. However, it does not verify how recently or frequently the price feed has been updated. In other words, the contract does not check for staleness (e.g., by comparing the aggregator’s updatedAt time to the current block timestamp). If an oracle is not updating properly for an extended period, the system may continue relying on outdated price data.

While it may be acceptable for an approximate USD conversion in this system, stale price data could cause inaccurate calculations of value. If the feed is stuck at an old price, merchants or witnesses might either overestimate or underestimate the actual USD value tied to tokens, skewing the system’s mint limit checks.

Recommendation

If a high degree of price accuracy is crucial, consider including a staleness check by inspecting the aggregator’s latest timestamp (e.g., updatedAt). Revert or take other precautions if the data is older than a configured threshold. If minor deviations are acceptable, you can keep this as is but add external monitoring to ensure the feed remains active.

ODX: Acknowledged. As already stated, we are using this oracle just to get an approximate USD value. Nothing precise, hence choosing to consider this issue for the next version.

Description

Within the ODXOracle, the contract calls _checkSequencer prior to fetching price data from the Chainlink aggregator. _checkSequencer verifies the L2 sequencer is active and that the configured grace period has passed. This step is commonly required on optimistic rollups (e.g., Optimism) to ensure that data feeds are reliable after the sequencer recovers from downtime. However, if this contract is deployed on a chain that does not rely on such a sequencer, this check may be an unnecessary overhead.

Recommendation

If the design specifically requires L2 downtime checks (e.g., for an Optimistic Rollup environment), keep _checkSequencer as is. Otherwise, consider making the sequencer check optional or configurable so it can be disabled on networks where no sequencer-based downtime risk exists.

ODX: Fixed in 3651f23.

Cantina: Fix verified.

Description

Several functions in the codebase modify critical state variables without emitting any corresponding event. Specifically:

• src/Controller.sol:277: The line witnessThreshold = threshold; changes the threshold of witness signatures required but does not emit an event to notify off-chain systems.
• src/Oracle.sol:92: The line feeds[asset] = Feed(feedType, feedData); updates or sets the feed configuration for a given asset but does not emit an event to track these changes.
• src/WrappedAsset.sol:86: The function setUserBlacklist(address user, bool blacklist) modifies the blacklist status of a user but does not emit an event reflecting that change.

Without events, external monitors, user interfaces, or analytics tools have to rely on raw transaction logs or chain state diffs to detect these updates. Emitting events at critical points is a best practice for transparency, debugging and integration with off-chain services.

Recommendation

Add dedicated event declarations for each state-modifying function above and emit them whenever changes occur.

ODX: Fixed in 24ebd41.

Cantina: Fix verified.

Description

Certain parts of the codebase declare imports or custom errors that are never referenced within the contracts, becoming “dead code.” Specifically:

• src/Controller.sol:12 imports IERC20 from “openzeppelin-contracts/token/ERC20/IERC20.sol” but never actually uses it in the contract.
• src/Oracle.sol:12 declares the custom error InvalidFeedType() which is never thrown or referenced within the code.

Leaving unused imports or custom errors in the code does not typically pose a security risk. However, it can lead to confusion, clutter the codebase, and require unnecessary attention during future maintenance or audits.

Recommendation

Remove the unused IERC20 import and InvalidFeedType error.

ODX: Fixed in 470bf73.

Cantina: Fix verified.

Description

The highlighted code returns the minimum of value and limitVal. Since OZ Math is already imported, Math.min() can be used instead.

Recommendation

Return the min value using:

return Math.min(value, limitVal);

ODX: Fixed in 5325995.

Cantina: Fix verified.

Description

Unit tests are important to catch bugs and also future proof the code from the same bugs. We highly recommend spending time on adding unit and fuzz tests testing for usual and unusual behavior.

Recommendation

Add tests aiming for high coverage.

ODX: Acknowledged. We have currently an internal team writing tests for this. It's not in this same repository. Tests will be formalized and added to this repository in the next 14-30 days.

Description

By rearranging, the if conditions in the highlighted code, the nested code can be simplified.

Recommendation

Refactor the code as follows:

if (value + epoch.minted <= limitVal) {    return value;}if (epoch.minted < limitVal) {    return limitVal - epoch.minted;} else {    return 0;}

ODX: Fixed in 129bb7d.

Cantina: Fix verified.