Ondo Finance - castle_chain's results

Institutional-Grade Finance. On-Chain. For Everyone.

General Information

Platform: Code4rena

Start Date: 01/09/2023

Pot Size: $36,500 USDC

Total HM: 4

Participants: 70

Period: 6 days

Judge: kirk-baird

Id: 281

League: ETH

Ondo Finance

Findings Distribution

Researcher Performance

Rank: 31/70

Findings: 3

Award: $116.18

QA:
grade-a
Gas:
grade-b
Analysis:
grade-b

🌟 Selected for report: 0

🚀 Solo Findings: 0

Awards

87.5807 USDC - $87.58

Labels

bug
grade-a
high quality report
QA (Quality Assurance)
sponsor acknowledged
edited-by-warden
Q-26

External Links

destination bridge

1) rescueTokens() function should prevent the withdrawal of USDY tokens .

the function rescueTokens()should make sure that the token to be withdrawn is not the core token of the bridge , but any tokens that had been sent by mistake , consider add this check :

  function rescueTokens(address _token) external onlyOwner {
+   if(_token == address(TOKEN)) revert();
    uint256 balance = IRWALike(_token).balanceOf(address(this));
    IRWALike(_token).transfer(owner(), balance);
  }

code snippet https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/bridge/DestinationBridge.sol#L322C1-L325C4

2) if the numOfApprovals is 0 this will freeze the execute function add prevent receiving the messages .

in the function setThresholds() there is no check if the numberOfApprovers is equal to 0 which be able to prevent receiving a messages of this amount . consider add a check to make sure that the number of approvers in greater than zero .

    for (uint256 i = 0; i < amounts.length; ++i) {
+   if (numOfApprovers[i] == 0 ) revert();
      if (i == 0) {
        chainToThresholds[srcChain].push(
          Threshold(amounts[i], numOfApprovers[i])
        );
      } else {
        if (chainToThresholds[srcChain][i - 1].amount > amounts[i]) {
          revert ThresholdsNotInAscendingOrder();
        }
        chainToThresholds[srcChain].push(
          Threshold(amounts[i], numOfApprovers[i])
        );
      }
    }

code snippet https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/bridge/DestinationBridge.sol#L255-L279

3) consider adding a function to remove a chain from the mapping chainToApprovedSender to allow the owner to deactivate receiving messages from a specific chain .

there is only the method to add chain and there is no removing mechanism code snippet

https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/bridge/DestinationBridge.sol#L234-L240

4) add a check to prevent the approvers to approve a deleted transaction and mint to address(zero) .

the function approve() allow approving a deleted transaction, and if the number reach the threshold the function will mint zero tokens to the zero address , so it is better to prevent this from happen . should make sure that if(txn.sender == address(0)) revert();

function approve(bytes32 txnHash) external { if (!approvers[msg.sender]) { revert NotApprover(); } _approve(txnHash); _mintIfThresholdMet(txnHash); }

code snippet https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/bridge/DestinationBridge.sol#L197-L203

5) using getNumApproved() function instead of getting the length of the array , to incease the modularity and readability of the code

first need to make the function getNumApproved() public function , then

  function _checkThresholdMet(bytes32 txnHash) internal view returns (bool) {
-   TxnThreshold memory t = txnToThresholdSet[txnHash];
-   if (t.numberOfApprovalsNeeded <= t.approvers.length) {
+   if (t.numberOfApprovalsNeeded <= getNumApproved(txnHash)) {
      return true;
    } else {
      return false;
    }
  }

code snippet

https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/bridge/DestinationBridge.sol#L177C1-L184C4

6) check for zero value , otherwise the minting on the desination chain will stop

the _mintLimit variable should be greater than zero to allow minting tokens on the destination chain , so the check for that should be exist.

code snippet

https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/bridge/DestinationBridge.sol#L61 https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/bridge/DestinationBridge.sol#L64-L71

7) crucial value txnHash should be emitted to allow user to get information about the transaction .

-  emit MessageReceived(srcChain, srcSender, amt, nonce);
+  emit MessageReceived(srcChain, srcSender, amt, nonce , txnHash );

https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/bridge/DestinationBridge.sol#L113

8) typo

  • the name of the mapping txnToThresholdSet should be txnHashToThresholdSet because it maps from the hash of the transaction to the threshold .
-   mapping(bytes32 => TxnThreshold) public txnToThresholdSet;
+   mapping(bytes32 => TxnThreshold) public txnHashToThresholdSet;

code snippet

https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/bridge/DestinationBridge.sol#L51

  • the comment say public functions but the following function is internal function
/*////////////////////////////////////////////////////////////// Public Functions //////////////////////////////////////////////////////////////*/

the comment should be internal function or should move the internal function to another position .

https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/bridge/DestinationBridge.sol#L327-L329

RWADynamicOracle

1) when passing firstRangeStart and firstRangeEnd in the constructor , the firstRangeStart should be equal or less than the current timeStamp , and the firstRangeEnd should be greater than the current timeStamp

this will garantee that the oracle will return the right prices after the deployment immediately

  constructor(
    address admin,
    address setter,
    address pauser,
    uint256 firstRangeStart,
    uint256 firstRangeEnd,
    uint256 dailyIR,
    uint256 startPrice
  ) {

+   if ( firstRangeStart < block.timestamp || firstRangeEnd <= block.timestamp) revert(); 
    _grantRole(DEFAULT_ADMIN_ROLE, admin);
    _grantRole(PAUSER_ROLE, pauser);
    _grantRole(SETTER_ROLE, setter);


    if (firstRangeStart >= firstRangeEnd) revert InvalidRange();
    uint256 trueStart = (startPrice * ONE) / dailyIR;
    ranges.push(Range(firstRangeStart, firstRangeEnd, dailyIR, trueStart));
  }

code snippet

https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/rwaOracles/RWADynamicOracle.sol#L30-L46

rUSDY contract

1) wrong values are emitted can mislead the monitoring mechanism or the frontent or any sytem that relay on the events

in the contract rUSDY.sol , the function wrap() emit two events Transfer and TransferShares , the event Transfer emits the number of rUSDY tokens by calling the function getRUSDYByShares(_USDYAmount) which take the shares and returns the amount of tokens , but in this event the amount of shares is wrong , the correct number of shares is : _USDYAmount * BPS_DENOMINATOR , also the event TransferShares should emit the number of shares but it also emit the wrong value emit TransferShares(address(0), msg.sender, _USDYAmount);

Recommendations

consider emitting the right values of shares .

  function wrap(uint256 _USDYAmount) external whenNotPaused {
    require(_USDYAmount > 0, "rUSDY: can't wrap zero USDY tokens");
    _mintShares(msg.sender, _USDYAmount * BPS_DENOMINATOR);
    usdy.transferFrom(msg.sender, address(this), _USDYAmount);
-   emit Transfer(address(0), msg.sender, getRUSDYByShares(_USDYAmount));
-   emit TransferShares(address(0), msg.sender, _USDYAmount);
+   emit Transfer(address(0), msg.sender, getRUSDYByShares(_USDYAmount * BPS_DENOMINATOR));
+   emit TransferShares(address(0), msg.sender, _USDYAmount * BPS_DENOMINATOR );

2) allow the user to specify a recipient address

allow the user to specify address to receive the tokens in the function wrap() and unwrap() .

 - function wrap(uint256 _USDYAmount) external whenNotPaused {
 + function wrap(uint256 _USDYAmount , address recipient) external whenNotPaused {
    require(_USDYAmount > 0, "rUSDY: can't wrap zero USDY tokens");
-   _mintShares(msg.sender, _USDYAmount * BPS_DENOMINATOR);
+   _mintShares(recipient, _USDYAmount * BPS_DENOMINATOR);
    usdy.transferFrom(msg.sender, address(this), _USDYAmount);
    emit Transfer(address(0), msg.sender, getRUSDYByShares(_USDYAmount));
    emit TransferShares(address(0), msg.sender, _USDYAmount);

code snippet

https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/usdy/rUSDY.sol#L434-L439 https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/usdy/rUSDY.sol#L449-L456

3) allow the user to pass minAmountOut of the tokens in the functions unwrap() to protect him from any slippage

allow user to specify the minimum amount expected to be received after the unwrapping to protect him from slippage

-  function unwrap(uint256 _rUSDYAmount) external whenNotPaused {
+  function unwrap(uint256 _rUSDYAmount , uint256 minAmountOut) external whenNotPaused {
    require(_rUSDYAmount > 0, "rUSDY: can't unwrap zero rUSDY tokens");
    uint256 usdyAmount = getSharesByRUSDY(_rUSDYAmount);
    if (usdyAmount < BPS_DENOMINATOR) revert UnwrapTooSmall();
    _burnShares(msg.sender, usdyAmount);
+   if ((usdyAmount / BPS_DENOMINATOR) <= minAmountOut ) revert();
    usdy.transfer(msg.sender, usdyAmount / BPS_DENOMINATOR);
    emit TokensBurnt(msg.sender, _rUSDYAmount);
  }

code snippet

https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/usdy/rUSDY.sol#L449-L456

4) unused returned value can be removed in the functions _mintShares() and _burnShares()

the functions _mintShares() and _burnShares() return the totalShares as returned value which is not been used when those functions been called and totalShares is state variable so it is accessable globaly so there is no need to be returned

  function _mintShares(
    address _recipient,
    uint256 _sharesAmount
-   ) internal whenNotPaused returns (uint256) {
+   ) internal whenNotPaused  { 

    require(_recipient != address(0), "MINT_TO_THE_ZERO_ADDRESS");


    _beforeTokenTransfer(address(0), _recipient, _sharesAmount);


    totalShares += _sharesAmount;


    shares[_recipient] = shares[_recipient] + _sharesAmount;


-   return totalShares;

code snippet

https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/usdy/rUSDY.sol#L543-L555

https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/usdy/rUSDY.sol#L575-L601

5) unused arguments of function _beforeTokenTransfer() can be removed .

  function _beforeTokenTransfer(
    address from,
    address to,
-   uint256
  ) internal view {
    // Check constraints when `transferFrom` is called to facliitate
    // a transfer between two parties that are not `from` or `to`.
    if (from != msg.sender && to != msg.sender) {
      require(!_isBlocked(msg.sender), "rUSDY: 'sender' address blocked");
      require(!_isSanctioned(msg.sender), "rUSDY: 'sender' address sanctioned");
      require(
        _isAllowed(msg.sender),
        "rUSDY: 'sender' address not on allowlist"
      );
    }

code snippet https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/usdy/rUSDY.sol#L629

6) avoid shadowing the state variable by change the names of the parameters and and "_" prefix the name

code snippet https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/usdy/rUSDY.sol#L109-L118

#0 - c4-pre-sort

2023-09-08T08:09:25Z

raymondfam marked the issue as high quality report

#1 - ypatil12

2023-09-12T16:24:17Z

Solid report

#2 - c4-sponsor

2023-09-13T15:24:42Z

tom2o17 (sponsor) acknowledged

#3 - tom2o17

2023-09-13T15:24:45Z

Looks good

#4 - c4-judge

2023-09-21T10:01:42Z

kirk-baird marked the issue as grade-a

Awards

9.7506 USDC - $9.75

Labels

bug
G (Gas Optimization)
grade-b
sufficient quality report
G-19

External Links

Gas optimization report

DestinationBridge

1) unnecessary check inside the for loop that waste gas can be optimized .

the function setThresholds() uses for loop to assign the thresholds to the amounts , the for loop always check

for (uint256 i = 0; i < amounts.length; ++i) { if (i == 0) { chainToThresholds[srcChain].push( Threshold(amounts[i], numOfApprovers[i]) ); }

, and this condition will be true only once in the loop , so it can be executed outside of the loop to avoid unnecessary check and save much gas . the optimized code should be

  function setThresholds(
    string calldata srcChain,
    uint256[] calldata amounts,
    uint256[] calldata numOfApprovers
  ) external onlyOwner {
    if (amounts.length != numOfApprovers.length) {
      revert ArrayLengthMismatch();
    }
    delete chainToThresholds[srcChain];

-  for (uint256 i = 0; i < amounts.length; ++i) {
-    if (i == 0) {
-      chainToThresholds[srcChain].push(
-        Threshold(amounts[i], numOfApprovers[i])
-     );
-    } else {
+      chainToThresholds[srcChain].push(
+       Threshold(amounts[0], numOfApprovers[0])
+     );
+    for (uint256 i = 1; i < amounts.length; ++i) {
        if (chainToThresholds[srcChain][i - 1].amount > amounts[i]) {
          revert ThresholdsNotInAscendingOrder();
        }
        chainToThresholds[srcChain].push(
          Threshold(amounts[i], numOfApprovers[i])
        );
      }
    }
    emit ThresholdSet(srcChain, amounts, numOfApprovers);
  }

code snippet

https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/bridge/DestinationBridge.sol#L255-L279

2) read from the storage and make a copy of the struct to the memory before the check will waste gas in case the check did not pass .

the function _mintIfThresholdMet() read from the storage to get the Transaction struct corresponding to the txnHash and then create a copy on the memory , then checks if the threshold is met or not , in case of the threshold has not been met yet ,the function will not use the copy of the struct , so it is considered wasting gas .

function _mintIfThresholdMet(bytes32 txnHash) internal { bool thresholdMet = _checkThresholdMet(txnHash); --> Transaction memory txn = txnHashToTransaction[txnHash]; --> if (thresholdMet) { _checkAndUpdateInstantMintLimit(txn.amount); if (!ALLOWLIST.isAllowed(txn.sender)) { ALLOWLIST.setAccountStatus( txn.sender, ALLOWLIST.getValidTermIndexes()[0], true ); } TOKEN.mint(txn.sender, txn.amount); delete txnHashToTransaction[txnHash]; emit BridgeCompleted(txn.sender, txn.amount); } }

so the optimized code should include the reading from storage and creating the copy inside the if condition .

  function _mintIfThresholdMet(bytes32 txnHash) internal {
    bool thresholdMet = _checkThresholdMet(txnHash);
-  Transaction memory txn = txnHashToTransaction[txnHash];
    if (thresholdMet) {
+   Transaction memory txn = txnHashToTransaction[txnHash];
      _checkAndUpdateInstantMintLimit(txn.amount);
      if (!ALLOWLIST.isAllowed(txn.sender)) {
        ALLOWLIST.setAccountStatus(
          txn.sender,
          ALLOWLIST.getValidTermIndexes()[0],
          true
        );
      }
      TOKEN.mint(txn.sender, txn.amount);
      delete txnHashToTransaction[txnHash];
      emit BridgeCompleted(txn.sender, txn.amount);
    }
  }

another instance : the function setRange() execute some code,which is reading from storage and creating copy into the memory , before the check ,so including this code after the check will save the gas in case of reversion .

  function setRange(
    uint256 endTimestamp,
    uint256 dailyInterestRate
  ) external onlyRole(SETTER_ROLE) {
-    Range memory lastRange = ranges[ranges.length - 1];

    // Check that the endTimestamp is greater than the last range's end time
    if (lastRange.end >= endTimestamp) revert InvalidRange();
+    Range memory lastRange = ranges[ranges.length - 1];
    uint256 prevClosePrice = derivePrice(lastRange, lastRange.end - 1);
    ranges.push(
      Range(lastRange.end, endTimestamp, dailyInterestRate, prevClosePrice)
    );
    emit RangeSet(
      ranges.length - 1,
      lastRange.end,
      endTimestamp,
      dailyInterestRate,
      prevClosePrice
    );
  }

gas saved : about 200

code snippet

https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/bridge/DestinationBridge.sol#L337-L353

https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/rwaOracles/RWADynamicOracle.sol#L151C1-L171C4

3) start the function with the checks will save gas in case of reversion

consider implement the checks first before the body of the function to save gas in case of revertion , as shown here

  constructor(
    address admin,
    address setter,
    address pauser,
    uint256 firstRangeStart,
    uint256 firstRangeEnd,
    uint256 dailyIR,
    uint256 startPrice
  ) {
+   if (firstRangeStart >= firstRangeEnd) revert InvalidRange();
    _grantRole(DEFAULT_ADMIN_ROLE, admin);
    _grantRole(PAUSER_ROLE, pauser);
    _grantRole(SETTER_ROLE, setter);


-    if (firstRangeStart >= firstRangeEnd) revert InvalidRange();
    uint256 trueStart = (startPrice * ONE) / dailyIR;
    ranges.push(Range(firstRangeStart, firstRangeEnd, dailyIR, trueStart));
  }

code snippet

https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/rwaOracles/RWADynamicOracle.sol#L30-L46

rUSDY contract

1) remove the unneseccary and redundant modifier whenNotPaused will save gas .

the functions wrap() and unwrap() uses the modifier whenNotPaused which exist also in the internal functions _mintShares() and _burnShares() that both of the functions wrap() and unwrap() ,

function _mintShares(address _recipient, uint256 _sharesAmount) internal whenNotPaused returns (uint256) {

and the _burnShares()

function _burnShares(address _account, uint256 _sharesAmount) internal whenNotPaused returns (uint256) {

one of the two modifier can be removed and it will perform the same functionallity of the modifier , and this the same implementation that used by transferShares() which does not use the modifier explicitly but the internal function _transferShares() which has the whenNotPaused modifier so the optimized code will be :

-   function wrap(uint256 _USDYAmount) external whenNotPaused {
+   function wrap(uint256 _USDYAmount) external  {

-     function unwrap(uint256 _rUSDYAmount) external whenNotPaused {
+     function unwrap(uint256 _rUSDYAmount) external {

code snippet

https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/usdy/rUSDY.sol#L434 https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/usdy/rUSDY.sol#L449

https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/usdy/rUSDY.sol#L543-L546

https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/usdy/rUSDY.sol#L575-L578

2) unused returned value can be removed in the functions _mintShares() and _burnShares() and save gas .

the functions _mintShares() and _burnShares() return the totalShares as returned value which is not been used when those functions been called and totalShares is state variable so it is accessable globaly so there is no need to be returned , and this will waste gas every call to this function .

  function _mintShares(
    address _recipient,
    uint256 _sharesAmount
-   ) internal whenNotPaused returns (uint256) {
+   ) internal whenNotPaused  { 

    require(_recipient != address(0), "MINT_TO_THE_ZERO_ADDRESS");


    _beforeTokenTransfer(address(0), _recipient, _sharesAmount);


    totalShares += _sharesAmount;


    shares[_recipient] = shares[_recipient] + _sharesAmount;


-   return totalShares;

code snippet

https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/usdy/rUSDY.sol#L543-L555

https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/usdy/rUSDY.sol#L575-L601

#0 - c4-pre-sort

2023-09-08T14:25:49Z

raymondfam marked the issue as sufficient quality report

#1 - c4-judge

2023-09-24T06:08:00Z

kirk-baird marked the issue as grade-b

Findings Information

Awards

18.8458 USDC - $18.85

Labels

analysis-advanced
grade-b
sufficient quality report
edited-by-warden
A-17

External Links

bridge overview

Architecture improvments

1) implement a minimum limit to amount of token burnt on the source chain .

the protocol use a bridge between two chains , source and destination chain . On the destination chain, there is a limit for the amount of tokens that can be minted over a specified duration, causing the mint function to revert if this limit is exceeded. Conversely, the source chain imposes no such limit, allowing users to burn and bridge an unlimited quantity of tokens without reversion. As a result, if a user burns and bridges tokens exceeding the destination chain's minting limit, the call will revert on the destination chain or the message will reaches the destination chain but the user will not get his minted tokens even if the number of approvers exceeds the threshold so the user will not be able to mint his tokens because of the limit on minting amount of tokens .

Recommendation

implement the same limit of minting and burning tokens on the source and destination chains to prevent such an issue .

2) pre-determined quantity of gas

The primary issue revolves around the lack of predetermined gas limits, which allows users to specify any gas amount, leading to transaction failures and unrecoverable token burns. according to axelar docs , the way to recover the transaction if it has an insufficient gas issue is to use the UI or the SDK which need the transaction hash to determine which transaction to be recovered , but it will not be a good solution to force the user to moniter the transaction maually , and it is better to predetermine the gas amount to get it from the user

Recommendations

  1. using external oracle such as chainLink fast gas oracle which provide the gas price on chain and will allow the protocol to pre-determine the gas cost and take them from the user , which will reduce the risk of insufficient amount of gas error .
  2. Implementation of Chainlink's CCIP: according to chainlink CCIP docs Chainlink's Cross-Chain Interoperability Protocol (CCIP) brings critical capabilities for seamless communication and interaction between different blockchains , which provide Arbitrary Messaging: This allows for the transmission of custom data to different blockchain smart contracts, the message transmission will be sufficient to execute the minting tokens proccess on the destination chain .

The advantages of chainlink CCIP are :

  • Pre-determines the gas needed for transactions, eliminating user-specified gas issues.
  • Automatically returns transaction hash (messageId) to users, enabling real-time transaction monitoring.

3) keep tracking of the tokens that had been burnt .

according to the axelar docs , the relayer is not always guaranteed to work ,

These relayer services are a free, operational convenience Axelar provides , anyone can create his own relayer

and if the relayer works the bridge can go down for any reason , or even the transaction on the destination chain can revert due to multiple reasons such as (there is no number of approvers set to this amount so the transaction will revert) , so it will be more efficient to keep track of the burned amount of each user and add mint function to allow the owner to re-mint the amount of tokens that had been burnt and it supposed to be minted on the destination chain .

RWA Dynamic Oracle

1) implement maximum duration for each range to avoid exponential increasing in the price and unexpected value of price .

the formula that calculates the price in the oracle uses the time as exponential variable and this formula can be represented on this graph a : dailyRate , b : prevClosePrice , x : elapsedDays , the price increases exponentially over time which mean for one (constant) specified dailyInterestRate and prevClosePrice , the slope of the curve varies and increases continuously and this will affects the price and will result in unexpected and inflated prices .

currentPrice = (Range.dailyInterestRate ** (Days Elapsed + 1)) * Range.lastSetPrice

as you can see in the graph , the price increase exponentially not linear as shown in the graph in the README of the contest , so if the duration has no limits and the DaysElapsed increase over the time , the price can be inflated and be unexpected due to the exponential increasing .

Recomendations

  1. set maximum duration per range or set limits to the start and the end time of the range .
  2. use this formula instead (use multiplication instead of exponent)
currentPrice = (Range.dailyInterestRate * (Days Elapsed + 1)) * Range.lastSetPrice .

rUSDY

add a slippage protection mechanism as the protocols that relay on oracles and external bridges

the protocol should allow the users to specify the minimun amount of tokens that they receive in order to protect them from slippage that can happen due to alot of issues such as wrong parameters that the admin had set , or one of the bridges on a specific chain goes down or even due to any oracle manipulation , so it will be better to add those slippage protection to ensure better user experience .

Recommendations

  • add the slippage allowance argument in the function wrap() and unWarp()

Time spent

40 hours

Time spent:

40 hours

#0 - c4-pre-sort

2023-09-08T14:41:41Z

raymondfam marked the issue as sufficient quality report

#1 - c4-judge

2023-09-24T07:18:01Z

kirk-baird marked the issue as grade-b

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter