Post audit changes

README.md

@@ -96,7 +96,7 @@ Upon calling `open`, an `urn` contract is deployed for each position. The `urn`
 
 The following functions are called from the LockstakeClipper (see below) throughout the liquidation process.
 
-* `onKick(address urn, uint256 wad)` - Undelegate and unstake the entire `urn`'s MKR amount.
+* `onKick(address urn, uint256 wad)` - Undelegate and unstake the entire `urn`'s MKR amount. Users need to manually delegate and stake again if there are leftovers after liquidation finishes.
 * `onTake(address urn, address who, uint256 wad)` - Transfer MKR to the liquidation auction buyer.
 * `onRemove(address urn, uint256 sold, uint256 left)` - Burn a proportional amount of the MKR which was bought in the auction and return the rest to the `urn`.
 
@@ -166,8 +166,9 @@ Note that the increased gas cost should be taken into consideration when determi
 ## 3. Vote Delegation
 ### 3.a. VoteDelegate
 
-The LSE integrates with the current [VoteDelegate](https://github.com/makerdao/vote-delegate/blob/c2345b78376d5b0bb24749a97f82fe9171b53394/src/VoteDelegate.sol) contracts almost as is. However, there are two changes done:
-* In order to support long-term locking the delegate's expiration functionality needs to be removed.
+The LSE integrates with the current [VoteDelegate](https://github.com/makerdao/vote-delegate/blob/c2345b78376d5b0bb24749a97f82fe9171b53394/src/VoteDelegate.sol) contracts almost as is. However, there are three changes done:
+* In order to support long-term locking, the delegate's expiration functionality needs to be removed.
+* In order to simplify the logic, the IOU tokens generated by DSChief are kept in the new VoteDelegate contract.
 * In order to protect against an attack vector of delaying liquidations or blocking freeing of MKR, an on-demand window where locking MKR is blocked is introduced. The need for this stems from the Chief's flash loan protection, which doesn't allow to free MKR from a delegate in case MKR locking was already done in the same block.
 
 ### 3.b. VoteDelegateFactory

deploy/LockstakeInit.sol

@@ -131,6 +131,7 @@ library LockstakeInit {
     uint256 constant internal RATES_ONE_HUNDRED_PCT = 1000000021979553151239153027;
     uint256 constant internal WAD = 10**18;
     uint256 constant internal RAY = 10**27;
+    uint256 constant internal RAD = 10**45;
 
     function initLockstake(
         DssInstance        memory dss,
@@ -158,12 +159,14 @@ library LockstakeInit {
         require(clipper.dog()                == address(dss.dog),          "Clipper dog mismatch");
         require(clipper.spotter()            == address(dss.spotter),      "Clipper spotter mismatch");
 
+        require(cfg.gap <= cfg.maxLine, "gap greater than max line");
         require(cfg.dust <= cfg.hole, "dust greater than hole");
         require(cfg.duty >= RAY && cfg.duty <= RATES_ONE_HUNDRED_PCT, "duty out of boundaries");
         require(cfg.mat >= RAY && cfg.mat < 10 * RAY, "mat out of boundaries");
         require(cfg.buf >= RAY && cfg.buf < 10 * RAY, "buf out of boundaries");
         require(cfg.cusp < RAY, "cusp negative drop value");
         require(cfg.chip < WAD, "chip equal or greater than 100%");
+        require(cfg.tip <= 1_000 * RAD, "tip out of boundaries");
         require(cfg.chop >= WAD && cfg.chop < 2 * WAD, "chop out of boundaries");
         require(cfg.tolerance < RAY, "tolerance equal or greater than 100%");
 

src/LockstakeEngine.sol

@@ -66,15 +66,15 @@ interface MkrNgtLike {
 contract LockstakeEngine is Multicall {
     // --- storage variables ---
 
-    mapping(address => uint256)                     public wards;            // usr => 1 == access
-    mapping(address => FarmStatus)                  public farms;            // farm => FarmStatus
-    mapping(address => uint256)                     public usrAmts;          // usr => urns amount
-    mapping(address => address)                     public urnOwners;        // urn => owner
-    mapping(address => mapping(address => uint256)) public urnCan;           // urn => usr => allowed (1 = yes, 0 = no)
-    mapping(address => address)                     public urnVoteDelegates; // urn => current associated voteDelegate
-    mapping(address => address)                     public urnFarms;         // urn => current selected farm
-    mapping(address => uint256)                     public urnAuctions;      // urn => amount of ongoing liquidations
-    JugLike                                         public jug;
+    mapping(address usr  => uint256 allowed)                         public wards;
+    mapping(address farm => FarmStatus)                              public farms;
+    mapping(address usr  => uint256 urnsCount)                       public usrAmts;
+    mapping(address urn  => address owner)                           public urnOwners;
+    mapping(address urn  => mapping(address usr => uint256 allowed)) public urnCan;
+    mapping(address urn  => address voteDelegate)                    public urnVoteDelegates;
+    mapping(address urn  => address farm)                            public urnFarms;
+    mapping(address urn  => uint256 auctionsCount)                   public urnAuctions;
+    JugLike                                                          public jug;
 
     // --- constants and enums ---
 
@@ -218,6 +218,7 @@ contract LockstakeEngine is Multicall {
 
     // --- getters ---
 
+    // NOTE: this function will succeed returning the address even if the urn for the specified index hasn't been created yet
     function getUrn(address owner, uint256 index) external view returns (address urn) {
         uint256 salt = uint256(keccak256(abi.encode(owner, index)));
         bytes32 codeHash = keccak256(abi.encodePacked(_initCode()));

src/Multicall.sol

@@ -12,12 +12,10 @@ abstract contract Multicall  {
             (bool success, bytes memory result) = address(this).delegatecall(data[i]);
 
             if (!success) {
-                // Next 5 lines from https://ethereum.stackexchange.com/a/83577
-                if (result.length < 68) revert();
-                assembly {
-                    result := add(result, 0x04)
+                if (result.length == 0) revert("multicall failed");
+                assembly ("memory-safe") {
+                    revert(add(32, result), mload(result))
                 }
-                revert(abi.decode(result, (string)));
             }
 
             results[i] = result;

test/LockstakeEngine.t.sol

@@ -941,6 +941,15 @@ contract LockstakeEngineTest is DssTest {
         assertEq(_ink(ilk, urn), 100_000 * 10**18);
         assertEq(farm.balanceOf(address(urn)), 100_000 * 10**18);
         assertEq(lsmkr.balanceOf(address(farm)), 100_000 * 10**18);
+
+        bytes[] memory revertExecute = new bytes[](1);
+        revertExecute[0] = abi.encodeWithSignature("open(uint256)", 2);
+        vm.expectRevert("LockstakeEngine/wrong-urn-index");
+        engine.multicall(revertExecute);
+
+        revertExecute[0] = abi.encodeWithSignature("onRemove(address,uint256,uint256)", urn, uint256(0), uint256(0));
+        vm.expectRevert(stdError.arithmeticError);
+        vm.prank(pauseProxy); engine.multicall(revertExecute);
     }
 
     function testGetReward() public {

test/mocks/VoteDelegateMock.sol

@@ -34,7 +34,7 @@ contract VoteDelegateMock {
 
     function lock(uint256 wad) external {
         gov.transferFrom(msg.sender, address(this), wad);
-        stake[msg.sender] = stake[msg.sender] + wad;
+        stake[msg.sender] += wad;
     }
 
     function free(uint256 wad) external {