feat: custom errors for factory and deployer

* feat: use custom errors instead of error string * test: change expect * ci: update hash * gas: update snapshot * gas: update snapshot * gas: update snapshot
reservoir-labs · Dec 10, 2024 · 3985f40 · 3985f40
1 parent a690b5c
commit 3985f40
Show file tree

Hide file tree

Showing 9 changed files with 101 additions and 88 deletions.
diff --git a/.gas-snapshot b/.gas-snapshot
diff --git a/script/optimized-deployer-meta b/script/optimized-deployer-meta
@@ -1,5 +1,5 @@
 {
   "constant_product_hash": "0xe174de1f7ab5f7c871f23787d956a8d1b4ebbb3b195eb2d6af27fb3a8c9e812e",
-  "factory_hash": "0x23864280088f6f71abfba47086784b3c758d1df19a55957e25f1f4bd9336792e",
+  "factory_hash": "0x87b0f73fafcf4bb41e013c8423dc679f6885527007d6c3f1e1834a670cbaadc5",
   "stable_hash": "0x3ae886aee24fa2cc0144d24306033a7ed47e91bc0f962e4bffcef5922ae175f5"
 }
diff --git a/script/unoptimized-deployer-meta b/script/unoptimized-deployer-meta
@@ -1,5 +1,5 @@
 {
   "constant_product_hash": "0x89ba88e63d531f6343d8f30d0baf35d10eb900e4e9e5cf64e857a17d63c27863",
-  "factory_hash": "0x4106b8ba3fea1679610d16c7270d5d7e41404bbf3aed56e9c5f042e8eb827f8b",
+  "factory_hash": "0x09a9ce1ed77c95be4842dddd771939e048b8bfe2837863be3a2766b1c13ea5a2",
   "stable_hash": "0x2556755e769639b4fbeff548907a3a675f94209a24a97c7fca31eedb76567c2f"
 }
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,12 +11,21 @@ import { GenericFactory } from "src/GenericFactory.sol";
 contract ReservoirDeployer {
     using FactoryStoreLib for GenericFactory;
 
+    error GuardianAddressZero();
+    error OutOfOrder();
+    error FactoryHash();
+    error DeployFactoryFailed();
+    error ConstantProductHash();
+    error StableHash();
+    error Threshold();
+    error NotOwner();
+
     // 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);
@@ -25,10 +34,7 @@ contract ReservoirDeployer {
     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 +49,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 +62,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 +81,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 +93,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);
@@ -125,7 +131,7 @@ contract ReservoirDeployer {
         uint256 lGuardian3Support = proposals[guardian3][msg.sender];
 
         uint256 lSupport = lGuardian1Support + lGuardian2Support + lGuardian3Support;
-        require(lSupport >= GUARDIAN_THRESHOLD, "DEPLOYER: THRESHOLD");
+        require(lSupport >= GUARDIAN_THRESHOLD, Threshold());
 
         owner = msg.sender;
     }
@@ -137,7 +143,7 @@ contract ReservoirDeployer {
     address public owner = address(0);
 
     modifier onlyOwner() {
-        require(msg.sender == owner, "DEPLOYER: NOT_OWNER");
+        require(msg.sender == owner, NotOwner());
         _;
     }
 

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/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(GenericFactory.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(GenericFactory.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(GenericFactory.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(GenericFactory.PairExists.selector);
         _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(ReservoirTimelock.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(ReservoirTimelock.Unauthorized.selector);
         _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(ReservoirTimelock.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(GenericFactory.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(GenericFactory.DeployFailed.selector);
         _createPair(address(_tokenC), address(_tokenD), 1);
     }