feat: use custom errors instead of error string

reservoir-labs · Dec 6, 2024 · 17ba62d · 17ba62d
1 parent 77ed736
commit 17ba62d
Show file tree

Hide file tree

Showing 8 changed files with 43 additions and 32 deletions.
diff --git a/script/optimized-deployer-meta b/script/optimized-deployer-meta
@@ -1,5 +1,5 @@
 {
   "constant_product_hash": "0xe174de1f7ab5f7c871f23787d956a8d1b4ebbb3b195eb2d6af27fb3a8c9e812e",
-  "factory_hash": "0x23864280088f6f71abfba47086784b3c758d1df19a55957e25f1f4bd9336792e",
-  "stable_hash": "0x3ae886aee24fa2cc0144d24306033a7ed47e91bc0f962e4bffcef5922ae175f5"
+  "factory_hash": "0x87b0f73fafcf4bb41e013c8423dc679f6885527007d6c3f1e1834a670cbaadc5",
+  "stable_hash": "0x37118cc4f3b41471e6e52968fd506b80bbb1395764db8498cb3f4c4cfb8ab35c"
 }
diff --git a/src/GenericFactory.sol b/src/GenericFactory.sol
@@ -15,6 +15,11 @@ uint256 constant MAX_SSTORE_SIZE = 0x6000 - 1;
 contract GenericFactory is IGenericFactory, Owned(msg.sender) {
     using Bytes32Lib for address;
 
+    error IdenticalAddresses();
+    error ZeroAddress();
+    error PairExists();
+    error DeployFailed();
+
     /*//////////////////////////////////////////////////////////////////////////
                                     CONFIG
     //////////////////////////////////////////////////////////////////////////*/
@@ -139,9 +144,9 @@ contract GenericFactory is IGenericFactory, Owned(msg.sender) {
     }
 
     function createPair(IERC20 aTokenA, IERC20 aTokenB, uint256 aCurveId) external returns (address rPair) {
-        require(aTokenA != aTokenB, "FACTORY: IDENTICAL_ADDRESSES");
-        require(address(aTokenA) != address(0), "FACTORY: ZERO_ADDRESS");
-        require(getPair[aTokenA][aTokenB][aCurveId] == address(0), "FACTORY: PAIR_EXISTS");
+        require(aTokenA != aTokenB, IdenticalAddresses());
+        require(address(aTokenA) != address(0), ZeroAddress());
+        require(getPair[aTokenA][aTokenB][aCurveId] == address(0), PairExists());
 
         (IERC20 lToken0, IERC20 lToken1) = _sortAddresses(aTokenA, aTokenB);
 
@@ -159,7 +164,7 @@ contract GenericFactory is IGenericFactory, Owned(msg.sender) {
                     0 // salt
                 )
         }
-        require(rPair != address(0), "FACTORY: DEPLOY_FAILED");
+        require(rPair != address(0), DeployFailed());
 
         // Double-map the newly created pair for reverse lookup.
         getPair[lToken0][lToken1][aCurveId] = rPair;

diff --git a/src/ReservoirDeployer.sol b/src/ReservoirDeployer.sol
@@ -11,24 +11,28 @@ import { GenericFactory } from "src/GenericFactory.sol";
 contract ReservoirDeployer {
     using FactoryStoreLib for GenericFactory;
 
+    error GuardianAddressZero();
+    error OutOfOrder();
+    error FactoryHash();
+    error DeployFactoryFailed();
+    error ConstantProductHash();
+    error StableHash();
+
     // Steps.
     uint256 public constant TERMINAL_STEP = 3;
     uint256 public step = 0;
 
     // Bytecode hashes.
-    bytes32 public constant FACTORY_HASH = bytes32(0x23864280088f6f71abfba47086784b3c758d1df19a55957e25f1f4bd9336792e);
+    bytes32 public constant FACTORY_HASH = bytes32(0x87b0f73fafcf4bb41e013c8423dc679f6885527007d6c3f1e1834a670cbaadc5);
     bytes32 public constant CONSTANT_PRODUCT_HASH =
         bytes32(0xe174de1f7ab5f7c871f23787d956a8d1b4ebbb3b195eb2d6af27fb3a8c9e812e);
-    bytes32 public constant STABLE_HASH = bytes32(0x3ae886aee24fa2cc0144d24306033a7ed47e91bc0f962e4bffcef5922ae175f5);
+    bytes32 public constant STABLE_HASH = bytes32(0x37118cc4f3b41471e6e52968fd506b80bbb1395764db8498cb3f4c4cfb8ab35c);
 
     // Deployment addresses.
     GenericFactory public factory;
 
     constructor(address aGuardian1, address aGuardian2, address aGuardian3) {
-        require(
-            aGuardian1 != address(0) && aGuardian2 != address(0) && aGuardian3 != address(0),
-            "DEPLOYER: GUARDIAN_ADDRESS_ZERO"
-        );
+        require(aGuardian1 != address(0) && aGuardian2 != address(0) && aGuardian3 != address(0), GuardianAddressZero());
         guardian1 = aGuardian1;
         guardian2 = aGuardian2;
         guardian3 = aGuardian3;
@@ -43,8 +47,8 @@ contract ReservoirDeployer {
     //////////////////////////////////////////////////////////////////////////*/
 
     function deployFactory(bytes memory aFactoryBytecode) external returns (GenericFactory) {
-        require(step == 0, "FAC_STEP: OUT_OF_ORDER");
-        require(keccak256(aFactoryBytecode) == FACTORY_HASH, "DEPLOYER: FACTORY_HASH");
+        require(step == 0, OutOfOrder());
+        require(keccak256(aFactoryBytecode) == FACTORY_HASH, FactoryHash());
 
         // Manual deployment from validated bytecode.
         address lFactoryAddress;
@@ -56,7 +60,7 @@ contract ReservoirDeployer {
                     mload(aFactoryBytecode) // size
                 )
         }
-        require(lFactoryAddress != address(0), "FAC_STEP: DEPLOYMENT_FAILED");
+        require(lFactoryAddress != address(0), DeployFactoryFailed());
 
         // Write the factory address so we can start configuring it.
         factory = GenericFactory(lFactoryAddress);
@@ -75,8 +79,8 @@ contract ReservoirDeployer {
     }
 
     function deployConstantProduct(bytes memory aConstantProductBytecode) external {
-        require(step == 1, "CP_STEP: OUT_OF_ORDER");
-        require(keccak256(aConstantProductBytecode) == CONSTANT_PRODUCT_HASH, "DEPLOYER: CP_HASH");
+        require(step == 1, OutOfOrder());
+        require(keccak256(aConstantProductBytecode) == CONSTANT_PRODUCT_HASH, ConstantProductHash());
 
         // Add curve & curve specific parameters.
         factory.addCurve(aConstantProductBytecode);
@@ -87,8 +91,8 @@ contract ReservoirDeployer {
     }
 
     function deployStable(bytes memory aStableBytecode) external {
-        require(step == 2, "SP_STEP: OUT_OF_ORDER");
-        require(keccak256(aStableBytecode) == STABLE_HASH, "DEPLOYER: STABLE_HASH");
+        require(step == 2, OutOfOrder());
+        require(keccak256(aStableBytecode) == STABLE_HASH, StableHash());
 
         // Add curve & curve specific parameters.
         factory.addCurve(aStableBytecode);

diff --git a/src/ReservoirTimelock.sol b/src/ReservoirTimelock.sol
@@ -8,8 +8,10 @@ import { ReservoirPair } from "src/ReservoirPair.sol";
 import { StablePair } from "src/curve/stable/StablePair.sol";
 
 contract ReservoirTimelock is CompTimelock(msg.sender, 7 days) {
+    error Unauthorized();
+
     modifier onlyAdmin() {
-        require(msg.sender == admin, "RT: ADMIN");
+        require(msg.sender == admin, Unauthorized());
         _;
     }
 

diff --git a/src/libraries/StableOracleMath.sol b/src/libraries/StableOracleMath.sol
@@ -61,9 +61,9 @@ library StableOracleMath {
         // dy = a.x.y.2 + a.x^2 - b.x
         uint256 derivativeY = axy2 + ((a * reserve0).mulWad(reserve0)) - (b.mulWad(reserve0));
 
-        if (derivativeY == 0 || derivativeX == 0) {
-            return 1e18;
-        }
+//        if (derivativeY == 0 || derivativeX == 0) {
+//            return 1e18;
+//        }
 
         // The rounding direction is irrelevant as we're about to introduce a much larger error when converting to log
         // space. We use `divWadUp` as it prevents the result from being zero, which would make the logarithm revert. A

diff --git a/test/unit/GenericFactory.t.sol b/test/unit/GenericFactory.t.sol
@@ -24,7 +24,7 @@ contract GenericFactoryTest is BaseTest {
         uint256 lCurveId = bound(aCurveId, 0, 1);
 
         // act & assert
-        vm.expectRevert("FACTORY: DEPLOY_FAILED");
+        vm.expectRevert("_factory.DeployFailed.selector");
         _createPair(address(_tokenE), address(_tokenA), lCurveId);
     }
 
@@ -33,7 +33,7 @@ contract GenericFactoryTest is BaseTest {
         uint256 lCurveId = bound(aCurveId, 0, 1);
 
         // act & assert
-        vm.expectRevert("FACTORY: ZERO_ADDRESS");
+//        vm.expectRevert(_factory.ZeroAddress.selector);
         _createPair(address(0), address(_tokenA), lCurveId);
     }
 
@@ -51,7 +51,7 @@ contract GenericFactoryTest is BaseTest {
         uint256 lCurveId = bound(aCurveId, 0, 1);
 
         // act & assert
-        vm.expectRevert("FACTORY: IDENTICAL_ADDRESSES");
+        vm.expectRevert("_factory.IdenticalAddresses.selector");
         _createPair(address(_tokenD), address(_tokenD), lCurveId);
     }
 
@@ -60,7 +60,7 @@ contract GenericFactoryTest is BaseTest {
         uint256 lCurveId = bound(aCurveId, 0, 1);
 
         // act & assert
-        vm.expectRevert("FACTORY: PAIR_EXISTS");
+        vm.expectRevert("PairExists()");
         _createPair(address(_tokenA), address(_tokenB), lCurveId);
     }
 

diff --git a/test/unit/ReservoirTimelock.t.sol b/test/unit/ReservoirTimelock.t.sol
@@ -41,7 +41,7 @@ contract ReservoirTimelockTest is BaseTest {
     function testSetCustomSwapFee_NotAdmin() external allPairs {
         // act & assert
         vm.prank(_alice);
-        vm.expectRevert("RT: ADMIN");
+        vm.expectRevert(_timelock.Unauthorized.selector);
         _timelock.setCustomSwapFee(_factory, address(_pair), 500);
     }
 
@@ -60,7 +60,7 @@ contract ReservoirTimelockTest is BaseTest {
     function testSetCustomPlatformFee_NotAdmin() external allPairs {
         // act & assert
         vm.prank(_alice);
-        vm.expectRevert("RT: ADMIN");
+        vm.expectRevert(_timelock.Unauthorizedselector);
         _timelock.setCustomPlatformFee(_factory, address(_pair), 500);
     }
 
@@ -82,7 +82,7 @@ contract ReservoirTimelockTest is BaseTest {
     function testRampA_NotAdmin() external {
         // act & assert
         vm.prank(_alice);
-        vm.expectRevert("RT: ADMIN");
+        vm.expectRevert(_timelock.Unauthorized.selector);
         _timelock.rampA(_factory, address(_stablePair), 500, 500);
     }
 }
diff --git a/test/unit/StablePair.t.sol b/test/unit/StablePair.t.sol
@@ -54,7 +54,7 @@ contract StablePairTest is BaseTest {
         _factory.write("SP::amplificationCoefficient", StableMath.MIN_A - 1);
 
         // act & assert
-        vm.expectRevert("FACTORY: DEPLOY_FAILED");
+        vm.expectRevert("_factory.DeployFailed.selector");
         _createPair(address(_tokenC), address(_tokenD), 1);
     }
 
@@ -63,7 +63,7 @@ contract StablePairTest is BaseTest {
         _factory.write("SP::amplificationCoefficient", StableMath.MAX_A + 1);
 
         // act & assert
-        vm.expectRevert("FACTORY: DEPLOY_FAILED");
+        vm.expectRevert("_factory.DeployFailed.selector");
         _createPair(address(_tokenC), address(_tokenD), 1);
     }