Use safecast for any downcasting (#2306)

* use safecasts for any downcasting * use safecast when downcasting from int256. Also fix tests * prettier * clean up implementation
OriginProtocol · Nov 13, 2024 · 0ab7bc8 · 0ab7bc8
1 parent 5cbcf4c
commit 0ab7bc8
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 26 deletions.
diff --git a/contracts/contracts/token/OUSD.sol b/contracts/contracts/token/OUSD.sol
@@ -8,6 +8,7 @@ pragma solidity ^0.8.0;
  * @author Origin Protocol Inc
  */
 import { Governable } from "../governance/Governable.sol";
+import { SafeCast } from "@openzeppelin/contracts/utils/math/SafeCast.sol";
 
 /**
  * NOTE that this is an ERC20 token but the invariant that the sum of
@@ -16,6 +17,9 @@ import { Governable } from "../governance/Governable.sol";
  */
 
 contract OUSD is Governable {
+    using SafeCast for int256;
+    using SafeCast for uint256;
+
     event TotalSupplyUpdatedHighres(
         uint256 totalSupply,
         uint256 rebasingCredits,
@@ -257,11 +261,11 @@ contract OUSD is Governable {
         (
             int256 fromRebasingCreditsDiff,
             int256 fromNonRebasingSupplyDiff
-        ) = _adjustAccount(_from, -int256(_value));
+        ) = _adjustAccount(_from, -_value.toInt256());
         (
             int256 toRebasingCreditsDiff,
             int256 toNonRebasingSupplyDiff
-        ) = _adjustAccount(_to, int256(_value));
+        ) = _adjustAccount(_to, _value.toInt256());
 
         _adjustGlobals(
             fromRebasingCreditsDiff + toRebasingCreditsDiff,
@@ -274,22 +278,21 @@ contract OUSD is Governable {
         returns (int256 rebasingCreditsDiff, int256 nonRebasingSupplyDiff)
     {
         RebaseOptions state = rebaseState[account];
-        int256 currentBalance = int256(balanceOf(account));
-        uint256 newBalance = uint256(
-            int256(currentBalance) + int256(balanceChange)
-        );
-        if (newBalance < 0) {
+        int256 currentBalance = balanceOf(account).toInt256();
+        if (currentBalance + balanceChange < 0) {
             revert("Transfer amount exceeds balance");
         }
+        uint256 newBalance = (currentBalance + balanceChange).toUint256();
+
         if (state == RebaseOptions.YieldDelegationSource) {
             address target = yieldTo[account];
             uint256 targetOldBalance = balanceOf(target);
             uint256 targetNewCredits = _balanceToRebasingCredits(
                 targetOldBalance + newBalance
             );
             rebasingCreditsDiff =
-                int256(targetNewCredits) -
-                int256(_creditBalances[target]);
+                targetNewCredits.toInt256() -
+                _creditBalances[target].toInt256();
 
             _creditBalances[account] = newBalance;
             _creditBalances[target] = targetNewCredits;
@@ -299,8 +302,8 @@ contract OUSD is Governable {
                 newBalance + _creditBalances[yieldFrom[account]]
             );
             rebasingCreditsDiff =
-                int256(newCredits) -
-                int256(_creditBalances[account]);
+                newCredits.toInt256() -
+                _creditBalances[account].toInt256();
             _creditBalances[account] = newCredits;
         } else if (_isNonRebasingAccount(account)) {
             nonRebasingSupplyDiff = balanceChange;
@@ -309,8 +312,8 @@ contract OUSD is Governable {
         } else {
             uint256 newCredits = _balanceToRebasingCredits(newBalance);
             rebasingCreditsDiff =
-                int256(newCredits) -
-                int256(_creditBalances[account]);
+                newCredits.toInt256() -
+                _creditBalances[account].toInt256();
             _creditBalances[account] = newCredits;
         }
     }
@@ -320,20 +323,18 @@ contract OUSD is Governable {
         int256 nonRebasingSupplyDiff
     ) internal {
         if (rebasingCreditsDiff != 0) {
-            if (uint256(int256(_rebasingCredits) + rebasingCreditsDiff) < 0) {
+            if (_rebasingCredits.toInt256() + rebasingCreditsDiff < 0) {
                 revert("rebasingCredits underflow");
             }
-            _rebasingCredits = uint256(
-                int256(_rebasingCredits) + rebasingCreditsDiff
-            );
+            _rebasingCredits = (_rebasingCredits.toInt256() +
+                rebasingCreditsDiff).toUint256();
         }
         if (nonRebasingSupplyDiff != 0) {
-            if (int256(nonRebasingSupply) + nonRebasingSupplyDiff < 0) {
+            if (nonRebasingSupply.toInt256() + nonRebasingSupplyDiff < 0) {
                 revert("nonRebasingSupply underflow");
             }
-            nonRebasingSupply = uint256(
-                int256(nonRebasingSupply) + nonRebasingSupplyDiff
-            );
+            nonRebasingSupply = (nonRebasingSupply.toInt256() +
+                nonRebasingSupplyDiff).toUint256();
         }
     }
 
@@ -388,7 +389,7 @@ contract OUSD is Governable {
         (
             int256 toRebasingCreditsDiff,
             int256 toNonRebasingSupplyDiff
-        ) = _adjustAccount(_account, int256(_amount));
+        ) = _adjustAccount(_account, _amount.toInt256());
         // Globals
         _adjustGlobals(toRebasingCreditsDiff, toNonRebasingSupplyDiff);
         _totalSupply = _totalSupply + _amount;
@@ -425,7 +426,7 @@ contract OUSD is Governable {
         (
             int256 toRebasingCreditsDiff,
             int256 toNonRebasingSupplyDiff
-        ) = _adjustAccount(_account, -int256(_amount));
+        ) = _adjustAccount(_account, -_amount.toInt256());
         // Globals
         _adjustGlobals(toRebasingCreditsDiff, toNonRebasingSupplyDiff);
         _totalSupply = _totalSupply - _amount;

diff --git a/contracts/test/flipper/flipper.js b/contracts/test/flipper/flipper.js
@@ -62,7 +62,9 @@ describe("Flipper", function () {
           // eslint-disable-next-line
           `buyOusdWith${titleName}`
         ](ousdUnits("1"));
-        await expect(call).to.be.revertedWith("Transfer greater than balance");
+        await expect(call).to.be.revertedWith(
+          "Transfer amount exceeds balance"
+        );
       }
     );
   });

diff --git a/contracts/test/vault/oeth-vault.js b/contracts/test/vault/oeth-vault.js
@@ -1428,7 +1428,7 @@ describe("OETH Vault", function () {
             .connect(josh)
             .requestWithdrawal(dataBefore.userOeth.add(1));
 
-          await expect(tx).to.revertedWith("Remove exceeds balance");
+          await expect(tx).to.revertedWith("Transfer amount exceeds balance");
         });
         it("capital is paused", async () => {
           const { oethVault, governor, josh } = fixture;

diff --git a/contracts/test/vault/redeem.js b/contracts/test/vault/redeem.js
@@ -154,7 +154,7 @@ describe("Vault Redeem", function () {
     // Try to withdraw more than balance
     await expect(
       vault.connect(anna).redeem(ousdUnits("100.0"), 0)
-    ).to.be.revertedWith("Remove exceeds balance");
+    ).to.be.revertedWith("Transfer amount exceeds balance");
   });
 
   it("Should only allow Governor to set a redeem fee", async () => {