Ondo Bridge Registrar & USDon Converter

Description

The BridgeRegistrar.register() function only grants MINTER_ROLE to the newly deployed OFT contract but fails to grant the required BURNER_ROLE. This causes all outbound cross-chain token transfers to fail until the tokenAdmin manually grants the BURNER_ROLE to the OFT contract.

When a user attempts to send tokens cross-chain via the OFT's send() function, the _debit() internal function is called:

function _debit(    address _from,    uint256 _amountLD,    uint256 _minAmountLD,    uint32 _dstEid) internal returns (uint256 amountSentLD, uint256 amountReceivedLD) {    (amountSentLD, amountReceivedLD) = _debitView(_amountLD, _minAmountLD, _dstEid);    // Burns tokens from the caller.    innerToken.burn(_from, amountSentLD);}

The innerToken.burn() call attempts to burn tokens from the GMToken contract, which has the following access control:

function burn(address from, uint256 amount) external onlyRole(BURNER_ROLE) {    _burn(from, amount);}

Since the OFT lacks BURNER_ROLE, the burn operation will revert with an access control error, completely preventing users from bridging tokens out of the chain.

Recommendation

Consider implementing either of the following suggestions:

1. Replace the burn function with burnFrom function in the OndoOFT.sol contract:

function _debit(    address _from,    uint256 _amountLD,    uint256 _minAmountLD,    uint32 _dstEid) internal returns (uint256 amountSentLD, uint256 amountReceivedLD) {   (amountSentLD, amountReceivedLD) = _debitView(_amountLD, _minAmountLD, _dstEid);   // Burns tokens from the caller.-  innerToken.burn(_from, amountSentLD);+  innerToken.burnFrom(_from, amountSentLD);}

2. Grant both MINTER_ROLE and BURNER_ROLE to the OFT contract in the register() function:

function register(address token) external override onlyRole(TOKEN_FACTORY_ROLE) whenNotPaused {    ....+   IAccessControlEnumerable(token).grantRole(keccak256("BURNER_ROLE"), oft);    ....}

Description

The redeem() function in the USDonConverter.sol contract suffers from precision loss when converting USDon (18 decimals) to USDC (6 decimals) due to Solidity's integer division truncation.

function redeem(    uint256 rwaAmount,    address receivingToken,    uint256 minimumTokenReceived) external override returns (uint256 receiveTokenAmount) {    ....    usdon.safeTransferFrom(gmTokenManager, wallet, rwaAmount);    receiveTokenAmount = rwaAmount / USDC_TO_USDON_CONVERSION_RATE; // --> precision loss    ...}

The conversion divides rwaAmount (18 decimals) by USDC_TO_USDON_CONVERSION_RATE (1e12). Any remainder from this division is truncated due to Solidity's integer division behavior. This means any USDon amount with non-zero digits in the last 12 decimal places will lose that fractional value.

Recommendation

Consider one of the following approaches:

1. Require exact divisibility:

function redeem(    uint256 rwaAmount,    address receivingToken,    uint256 minimumTokenReceived) external override returns (uint256 receiveTokenAmount) {+  if (rwaAmount % USDC_TO_USDON_CONVERSION_RATE != 0) revert InvalidRedeemAmount();   ....}

2. Return dust to user:

function redeem(   uint256 rwaAmount,   address receivingToken,   uint256 minimumTokenReceived) external override returns (uint256 receiveTokenAmount) { ....+  uint256 dust = rwaAmount % USDC_TO_USDON_CONVERSION_RATE;+  rwaAmount = rwaAmount - dust;  /// here the dust remains in the GMTokenManager contract; refunds should be handled there.  ....}

Ondo

We are aware of the truncation that will occur when decimals of the stablecoin < decimals of USDon; however, the truncation favors the protocol, so there is no additional risk here. Regardless, though, the dust that the user could be refunded would not be enough to offset the additional gas cost incurred from the additional refund logic.

Description

The USDonConverter.sol contract uses a hardcoded USDC_TO_USDON_CONVERSION_RATE of 1e12, which assumes USDC always has 6 decimals. However, on certain chains, such as BNB Chain (BSC), USDC has 18 decimals instead of 6.

This discrepancy will cause severe calculation errors and affect the smart contract functionality on those chains (or) force redeployment.

Recommendation

Consider querying decimals dynamically during deployment to set the USDC_TO_USDON_CONVERSION_RATE variable, instead of hardcoding it.

Description

In the register() function in BridgeRegistrar.sol contract, uses abi.encodePacked() to generate a deterministic tokenId by hashing the token's symbol and name:

bytes32 tokenId = keccak256(  abi.encodePacked(    IERC20Metadata(token).symbol(),    IERC20Metadata(token).name()  ));

abi.encodePacked() concatenates dynamic types (strings) without including length information or padding, which can lead to hash collisions. Different combinations of symbol and name can produce identical hashes.

Using the same tokenId again will cause reverts in the Messenger.sol contract because each tokenId can only be registered once. This primarily prevents valid tokens with similar token IDs (resulting from collisions) from being registered.

Recommendation

Replace abi.encodePacked() with abi.encode() to eliminate collision risks:

bytes32 tokenId = keccak256(-  abi.encodePacked(+  abi.encode(    IERC20Metadata(token).symbol(),    IERC20Metadata(token).name()  ));

Description

The constructor of BridgeRegistrar.sol takes a guardian parameter but does not verify that it isn't the zero address (i.e.,'address(0)') Deploying with address(0) may render the contract unusable, potentially requiring redeployment.

Recommendation

Consider adding a zero address check as follows:

constructor(address guardian, address _ondoBridgeOwner) {+  if (guardian == address(0)) revert OndoGuardianCantBeZero();   /// rest of the code}

Description

The USDonConverter.sol contract relies on a Chainlink oracle to validate USDC pricing before allowing subscription and redemption operations.

The contract hardcodes the expected oracle price format to 8 decimals (MINIMUM_USDC_PRICE = 0.98e8) but fails to validate that the provided oracle actually returns prices in this format during deployment.

Recommendation

Add a validation check in the constructor to ensure the oracle returns prices with exactly 8 decimals:

+ error InvalidOracleDecimals();
constructor(  address _gmTokenManager,  address _wallet,  address _usdon,  address _usdc,  address _usdcOracle) {   /// rest of the code+  if (usdcOracle.decimals() != 8) {+    revert InvalidOracleDecimals();+  }}

Description

The constructor's NatSpec documentation is incomplete and missing documentation for the _usdcOracle parameter.

Recommendation

Add the missing @param documentation for the _usdcOracle parameter:

/**   * @notice Constructs the USDonConverter contract   * @param _gmTokenManager The GMTokenManager address authorized to call functions   * @param _wallet         The wallet address that holds token reserves   * @param _usdon          The USDon token address   * @param _usdc           The USDC token address+  * @param _usdcOracle     The Chainlink USDC/USD price feed address   */  constructor(    address _gmTokenManager,    address _wallet,    address _usdon,    address _usdc,    address _usdcOracle  ) {    // ...  }

Description

The USDonConverter.sol contract sets MAX_ORACLE_DATA_AGE to 30 hours to validate the freshness of Chainlink oracle data:

uint256 public constant MAX_ORACLE_DATA_AGE = 30 hours;
function _assertTokenMinimumUSDCPrice() internal view {    (, int price, , uint256 updatedAt, ) = usdcOracle.latestRoundData();    if (updatedAt < block.timestamp - MAX_ORACLE_DATA_AGE)      revert OraclePriceOutdated();    // ...}

This 30-hour window is huge and defeats the purpose of Oracle staleness checks.

Recommendation

Reduce the MAX_ORACLE_DATA_AGE to a more reasonable value (< 24 hours)