Strengthen revert tests (#30)

* Strengthen revert tests * Rename test functions * Add not enough calldata test
ScopeLift · Jun 15, 2023 · 1ee0d62 · 1ee0d62
1 parent c885bfb
commit 1ee0d62
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 14 deletions.
diff --git a/src/PerpetualPositionRouter.sol b/src/PerpetualPositionRouter.sol
@@ -14,6 +14,9 @@ contract PerpetualPositionRouter {
   /// @dev Thrown when calldata provides a function ID that does not exist.
   error FunctionDoesNotExist();
 
+  /// @dev Thrown when calldata is invalid for the provided function id.
+  error InvalidCalldata();
+
   /// @dev Thrown when a user tries to take an action on a position that does not exist.
   error NoExistingPosition();
 
@@ -195,11 +198,13 @@ contract PerpetualPositionRouter {
     uint256 deadline;
     uint160 sqrtPriceLimitX96;
     if (funcId != 5) {
+      if (msg.data.length != 49) revert InvalidCalldata();
       sqrtPriceLimitX96 = uint160(bytes20(msg.data[1:21]));
       deadline = uint256(uint32(bytes4(msg.data[21:25])));
       amount = uint256(uint96(bytes12(msg.data[25:37])));
       oppositeAmountBound = uint256(uint96(bytes12(msg.data[37:49])));
     } else {
+      if (msg.data.length != 37) revert InvalidCalldata();
       sqrtPriceLimitX96 = uint160(bytes20(msg.data[1:21]));
       deadline = uint256(uint32(bytes4(msg.data[21:25])));
       oppositeAmountBound = uint256(uint96(bytes12(msg.data[25:37])));

diff --git a/test/PerpetualPositionRouter.t.sol b/test/PerpetualPositionRouter.t.sol
@@ -38,11 +38,7 @@ contract PositionRouterTest is Test, PerpetualContracts {
     );
     (bool okTwo,) = payable(vethPositionRouterAddr).call(
       abi.encodePacked(
-        uint8(5),
-        uint160(sqrtPriceLimitX96),
-        uint32(block.timestamp),
-        uint96(0),
-        uint96(oppositeAmountBound)
+        uint8(5), uint160(sqrtPriceLimitX96), uint32(block.timestamp), uint96(oppositeAmountBound)
       )
     );
     assertTrue(ok);
@@ -220,37 +216,54 @@ contract Fallback is PositionRouterTest {
     assertEq(info.takerPositionSize, 0);
   }
 
-  function testFork_FailedCallWhenExtraCalldataArgument() public {
+  function testFork_RevertIf_CallWhenExtraCalldataArgument() public {
     delegateApproval.approve(vethPositionRouterAddr, 1);
+
+    vm.expectRevert(PerpetualPositionRouter.InvalidCalldata.selector);
     (bool ok,) = payable(vethPositionRouterAddr).call(
-      abi.encodePacked(uint8(4), uint160(0), uint32(block.timestamp), uint96(0), uint96(100))
+      abi.encodePacked(
+        uint8(4), uint160(0), uint32(block.timestamp), uint96(10), uint96(100), uint96(0)
+      )
     );
-    assertFalse(ok);
+    assertTrue(ok);
   }
 
-  function testFork_FailedClosePositionCallWithWrongArguments() public {
+  function testFork_RevertIf_CallWhenNotEnoughCalldata() public {
     delegateApproval.approve(vethPositionRouterAddr, 1);
+
+    vm.expectRevert(PerpetualPositionRouter.InvalidCalldata.selector);
+    (bool ok,) = payable(vethPositionRouterAddr).call(
+      abi.encodePacked(uint8(4), uint160(0), uint32(block.timestamp), uint96(10), uint8(1))
+    );
+    assertTrue(ok);
+  }
+
+  function testFork_RevertIf_ClosePositionCallWithWrongArguments() public {
+    delegateApproval.approve(vethPositionRouterAddr, 1);
+    vm.expectRevert(PerpetualPositionRouter.InvalidCalldata.selector);
     (bool ok,) = payable(vethPositionRouterAddr).call(
       abi.encodePacked(uint8(5), uint160(0), uint32(block.timestamp), uint96(1 ether), uint96(0))
     );
-    assertFalse(ok);
+    assertTrue(ok);
   }
 
-  function testFork_FailedFallbackWithZeroFuncId() public {
+  function testFork_RevertIf_FallbackWithZeroFuncId() public {
     delegateApproval.approve(vethPositionRouterAddr, 1);
+    vm.expectRevert(PerpetualPositionRouter.FunctionDoesNotExist.selector);
     (bool ok,) = payable(vethPositionRouterAddr).call(
       abi.encodePacked(uint8(0), uint160(0), uint32(block.timestamp), uint96(1 ether), uint96(0))
     );
-    assertFalse(ok);
+    assertTrue(ok);
   }
 
-  function testFork_FailedDeadlineHasExpired() public {
+  function testFork_RevertIf_DeadlineHasExpired() public {
     delegateApproval.approve(vethPositionRouterAddr, 1);
+    vm.expectRevert(bytes("CH_TE"));
     (bool ok,) = payable(vethPositionRouterAddr).call(
       abi.encodePacked(
         uint8(1), uint160(0), uint32(block.timestamp - 1000), uint96(1 ether), uint96(0)
       )
     );
-    assertFalse(ok);
+    assertTrue(ok);
   }
 }