More precise pool shares (#87)

* More precise account for pools shares after shutdown + force unwinding if assets < ink * No more dapp * Minor changes * Rename test * Add exit tests * Fix exit and exit_revert specs * Fix exec_normal_revert expect with the new changes * Improve exit tests * Improving exec_normal spec * Fix comment * Enforce solc 0.8.14 in make test and use it for CI * Fix test rounding error * Remove unnecessary tests * 1e18 to wad and aave rounding (#88) * switch compound tests from 1e18 to WAD * fix aave test rounding issues * gonza fixing my assertEq dyslexia Co-authored-by: Gonzalo Balabasquer <[email protected]> Co-authored-by: Gonzalo Balabasquer <[email protected]> Co-authored-by: Chris Smith <[email protected]>
makerdao · Oct 3, 2022 · c89c426 · c89c426
1 parent add9f47
commit c89c426
Show file tree

Hide file tree

Showing 22 changed files with 444 additions and 327 deletions.
diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml
@@ -15,4 +15,6 @@ jobs:
         run: forge install
 
       - name: Run tests
-        run: forge test -v --force --fork-url ${{ secrets.ETH_RPC_URL }} --use 0.8.14
+        run: make test
+        env:
+          ETH_RPC_URL: ${{ secrets.ETH_RPC_URL }}
diff --git a/.gitignore b/.gitignore
@@ -3,6 +3,7 @@
 resource_errors.json
 
 # certora
+*autoFinder_*
 .*certora*
 .last_confs/
 *.zip

diff --git a/Makefile b/Makefile
@@ -1,6 +1,5 @@
-all    		 :; DAPP_BUILD_OPTIMIZE=1 DAPP_BUILD_OPTIMIZE_RUNS=200 dapp --use solc:0.8.14 build
-clean  		 :; dapp clean
-test   		 :; ./test.sh match="$(match)" optimizer=1
-test-dev   	 :; ./test.sh match="$(match)" optimizer=0
-test-forge 	 :; ./test-forge.sh match="$(match)" block="$(block)" match-test="$(match-test)" match-contract="$(match-contract)"
-certora-hub  :; PATH=~/.solc-select/artifacts:${PATH} certoraRun --solc_map D3MHub=solc-0.8.14,Vat=solc-0.5.12,DaiJoin=solc-0.5.12,Dai=solc-0.5.12,End=solc-0.5.12,D3MTestPlan=solc-0.8.14,D3MTestPool=solc-0.8.14,D3MTestGem=solc-0.8.14 --rule_sanity basic src/D3MHub.sol certora/dss/Vat.sol certora/dss/DaiJoin.sol certora/dss/Dai.sol certora/dss/End.sol src/tests/stubs/D3MTestPlan.sol src/tests/stubs/D3MTestPool.sol src/tests/stubs/D3MTestGem.sol --link D3MHub:vat=Vat D3MHub:daiJoin=DaiJoin D3MHub:end=End DaiJoin:vat=Vat DaiJoin:dai=Dai End:vat=Vat D3MTestPlan:dai=Dai D3MTestPool:hub=D3MHub D3MTestPool:vat=Vat D3MTestPool:dai=Dai D3MTestPool:share=D3MTestGem --verify D3MHub:certora/D3MHub.spec --settings -mediumTimeout=180 --staging$(if $(short), --short_output,)$(if $(rule), --rule $(rule),)$(if $(multi), --multi_assert_check,)
+all			:; DAPP_BUILD_OPTIMIZE=1 DAPP_BUILD_OPTIMIZE_RUNS=200 dapp --use solc:0.8.14 build
+clean		:; dapp clean
+test		:; ./test.sh solc="0.8.14" match="$(match)" block="$(block)" match-test="$(match-test)" match-contract="$(match-contract)"
+test-forge	:; make test
+certora-hub	:; PATH=~/.solc-select/artifacts:${PATH} certoraRun --solc_map D3MHub=solc-0.8.14,Vat=solc-0.5.12,DaiJoin=solc-0.5.12,Dai=solc-0.5.12,End=solc-0.5.12,D3MTestPlan=solc-0.8.14,D3MTestPool=solc-0.8.14,D3MTestGem=solc-0.8.14 --rule_sanity basic src/D3MHub.sol certora/dss/Vat.sol certora/dss/DaiJoin.sol certora/dss/Dai.sol certora/dss/End.sol src/tests/stubs/D3MTestPlan.sol src/tests/stubs/D3MTestPool.sol src/tests/stubs/D3MTestGem.sol --link D3MHub:vat=Vat D3MHub:daiJoin=DaiJoin D3MHub:end=End DaiJoin:vat=Vat DaiJoin:dai=Dai End:vat=Vat D3MTestPlan:dai=Dai D3MTestPool:hub=D3MHub D3MTestPool:vat=Vat D3MTestPool:dai=Dai D3MTestPool:share=D3MTestGem --verify D3MHub:certora/D3MHub.spec --settings -mediumTimeout=180 --staging$(if $(short), --short_output,)$(if $(rule), --rule $(rule),)$(if $(multi), --multi_assert_check,)
diff --git a/certora/D3MHub.spec b/certora/D3MHub.spec
diff --git a/src/D3MHub.sol b/src/D3MHub.sol
@@ -136,6 +136,7 @@ contract D3MHub {
     }
 
     // --- Math ---
+    uint256 internal constant WAD = 10 ** 18;
     uint256 internal constant RAY = 10 ** 27;
     uint256 internal constant MAXINT256 = uint256(type(int256).max);
     uint256 internal constant SAFEMAX = MAXINT256 / RAY;
@@ -283,8 +284,10 @@ contract D3MHub {
         uint256 toUnwind;
         uint256 toWind;
 
-        // Determine if it needs to fully unwind due to D3M ilk being caged (but not culled) or plan is not active
-        if (ilks[ilk].tic != 0 || !ilks[ilk].plan.active()) {
+        // Determine if it needs to fully unwind due to D3M ilk being caged (but not culled), plan is not active or something
+        // wrong is going with the third party and we are entering in the ilegal situation of having less assets than registered
+        // It's adding up `WAD` due possible rounding errors
+        if (ilks[ilk].tic != 0 || !ilks[ilk].plan.active() || currentAssets + WAD < ink) {
             toUnwind = maxWithdraw;
         } else {
             uint256 Line = vat.Line();
@@ -406,7 +409,7 @@ contract D3MHub {
     function exit(bytes32 ilk, address usr, uint256 wad) external lock {
         require(wad <= MAXINT256, "D3MHub/overflow");
         vat.slip(ilk, msg.sender, -int256(wad));
-        ilks[ilk].pool.transfer(usr, wad);
+        ilks[ilk].pool.exit(usr, wad);
         emit Exit(ilk, usr, wad);
     }
 

diff --git a/src/pools/D3MAavePool.sol b/src/pools/D3MAavePool.sol
@@ -32,6 +32,11 @@ interface VatLike {
 
 interface D3mHubLike {
     function vat() external view returns (address);
+    function end() external view returns (EndLike);
+}
+
+interface EndLike {
+    function Art(bytes32) external view returns (uint256);
 }
 
 // aDai: https://etherscan.io/address/0x028171bCA77440897B824Ca71D1c56caC55b68A3
@@ -72,7 +77,9 @@ contract D3MAavePool is ID3MPool {
     mapping (address => uint256) public wards;
     address                      public hub;
     address                      public king; // Who gets the rewards
+    uint256                      public exited;
 
+    bytes32         public immutable ilk;
     VatLike         public immutable vat;
     LendingPoolLike public immutable pool;
     ATokenLike      public immutable stableDebt;
@@ -86,7 +93,8 @@ contract D3MAavePool is ID3MPool {
     event File(bytes32 indexed what, address data);
     event Collect(address indexed king, address indexed gift, uint256 amt);
 
-    constructor(address hub_, address dai_, address pool_) {
+    constructor(bytes32 ilk_, address hub_, address dai_, address pool_) {
+        ilk = ilk_;
         dai = TokenLike(dai_);
         pool = LendingPoolLike(pool_);
 
@@ -173,8 +181,11 @@ contract D3MAavePool is ID3MPool {
         require(dai.balanceOf(msg.sender) == prevDai + wad, "D3MAavePool/incorrect-dai-balance-received");
     }
 
-    function transfer(address dst, uint256 wad) external override onlyHub {
-        require(adai.transfer(dst, wad), "D3MAavePool/transfer-failed");
+    function exit(address dst, uint256 wad) external override onlyHub {
+        uint256 exited_ = exited;
+        exited = exited_ + wad;
+        uint256 amt = wad * assetBalance() / (D3mHubLike(hub).end().Art(ilk) - exited_);
+        require(adai.transfer(dst, amt), "D3MAavePool/transfer-failed");
     }
 
     function quit(address dst) external override auth {

diff --git a/src/pools/D3MCompoundPool.sol b/src/pools/D3MCompoundPool.sol
@@ -32,6 +32,11 @@ interface VatLike {
 
 interface D3mHubLike {
     function vat() external view returns (address);
+    function end() external view returns (EndLike);
+}
+
+interface EndLike {
+    function Art(bytes32) external view returns (uint256);
 }
 
 // cDai - https://etherscan.io/token/0x5d3a536E4D6DbD6114cc1Ead35777bAB948E3643
@@ -58,7 +63,9 @@ contract D3MCompoundPool is ID3MPool {
     mapping (address => uint256) public wards;
     address                      public hub;
     address                      public king; // Who gets the rewards
+    uint256                      public exited;
 
+    bytes32         public immutable ilk;
     VatLike         public immutable vat;
     ComptrollerLike public immutable comptroller;
     TokenLike       public immutable comp;
@@ -71,7 +78,8 @@ contract D3MCompoundPool is ID3MPool {
     event File(bytes32 indexed what, address data);
     event Collect(address indexed king, address indexed gift, uint256 amt);
 
-    constructor(address hub_, address cDai_) {
+    constructor(bytes32 ilk_, address hub_, address cDai_) {
+        ilk         = ilk_;
         cDai        = CErc20Like(cDai_);
         dai         = TokenLike(cDai.underlying());
         comptroller = ComptrollerLike(cDai.comptroller());
@@ -152,8 +160,11 @@ contract D3MCompoundPool is ID3MPool {
         require(dai.balanceOf(msg.sender) == prevDai + wad, "D3MCompoundPool/incorrect-dai-balance-received");
     }
 
-    function transfer(address dst, uint256 wad) external override onlyHub {
-        require(cDai.transfer(dst, _wdiv(wad, cDai.exchangeRateCurrent())), "D3MCompoundPool/transfer-failed");
+    function exit(address dst, uint256 wad) external override onlyHub {
+        uint256 exited_ = exited;
+        exited = exited_ + wad;
+        uint256 amt = wad * cDai.balanceOf(address(this)) / (D3mHubLike(hub).end().Art(ilk) - exited_);
+        require(cDai.transfer(dst, amt), "D3MCompoundPool/transfer-failed");
     }
 
     function quit(address dst) external override auth {

diff --git a/src/pools/ID3MPool.sol b/src/pools/ID3MPool.sol
@@ -44,14 +44,14 @@ interface ID3MPool {
     function withdraw(uint256 wad) external;
 
      /**
-        @notice Transfer shares.
+        @notice Exit proportional amount of shares.
         @dev If the external pool/token contract requires a different amount to be
-        passed in the conversion should occur here as the Hub passes Dai [wad]
+        passed in the conversion should occur here as the Hub passes Gem [wad]
         amounts. msg.sender must be the hub.
         @param dst address that should receive the redeemable tokens
-        @param wad amount in Dai terms that we want to withdraw
+        @param wad amount in Gem terms that we want to withdraw
     */
-    function transfer(address dst, uint256 wad) external;
+    function exit(address dst, uint256 wad) external;
 
     /**
         @notice Transfer all shares from this pool.

diff --git a/src/tests/D3MHub.t.sol b/src/tests/D3MHub.t.sol
@@ -42,8 +42,6 @@ interface Hevm {
 }
 
 contract D3MHubTest is DSSTest {
-    Hevm hevm;
-
     VatLike vat;
     EndLike end;
     D3MTestRewards rewardsClaimer;
@@ -63,10 +61,6 @@ contract D3MHubTest is DSSTest {
     D3MOracle pip;
 
     function setUp() public override {
-        hevm = Hevm(
-            address(bytes20(uint160(uint256(keccak256("hevm cheat code")))))
-        );
-
         vat = VatLike(0x35D1b3F3D7966A1DFe207aa4514C12a259A0492B);
         end = EndLike(0x0e2e8F1D1326A4B9633D96222Ce399c708B19c28);
         dai = DaiLike(0x6B175474E89094C44Da98b954EedeAC495271d0F);
@@ -76,7 +70,7 @@ contract D3MHubTest is DSSTest {
         vow = 0xA950524441892A31ebddF91d3cEEFa04Bf454466;
         pauseProxy = 0xBE8E3e3618f7474F8cB1d074A26afFef007E98FB;
 
-        // Force give admin access to these contracts via hevm magic
+        // Force give admin access to these contracts via vm magic
         _giveAuthAccess(address(vat), address(this));
         _giveAuthAccess(address(end), address(this));
         _giveAuthAccess(address(spot), address(this));
@@ -136,11 +130,11 @@ contract D3MHubTest is DSSTest {
 
         for (int256 i = 0; i < 100; i++) {
             // Scan the storage for the ward storage slot
-            bytes32 prevValue = hevm.load(
+            bytes32 prevValue = vm.load(
                 address(base),
                 keccak256(abi.encode(target, uint256(i)))
             );
-            hevm.store(
+            vm.store(
                 address(base),
                 keccak256(abi.encode(target, uint256(i))),
                 bytes32(uint256(1))
@@ -150,7 +144,7 @@ contract D3MHubTest is DSSTest {
                 return;
             } else {
                 // Keep going after restoring the original value
-                hevm.store(
+                vm.store(
                     address(base),
                     keccak256(abi.encode(target, uint256(i))),
                     prevValue
@@ -176,11 +170,11 @@ contract D3MHubTest is DSSTest {
 
         for (int256 i = 0; i < 100; i++) {
             // Scan the storage for the balance storage slot
-            bytes32 prevValue = hevm.load(
+            bytes32 prevValue = vm.load(
                 address(token),
                 keccak256(abi.encode(address(usr), uint256(i)))
             );
-            hevm.store(
+            vm.store(
                 address(token),
                 keccak256(abi.encode(address(usr), uint256(i))),
                 bytes32(amount)
@@ -190,7 +184,7 @@ contract D3MHubTest is DSSTest {
                 return;
             } else {
                 // Keep going after restoring the original value
-                hevm.store(
+                vm.store(
                     address(token),
                     keccak256(abi.encode(address(usr), uint256(i))),
                     prevValue
@@ -1088,7 +1082,7 @@ contract D3MHubTest is DSSTest {
         // with auth we can cull anytime
         d3mHub.deny(address(this));
         // but with enough time, anyone can cull
-        hevm.warp(block.timestamp + 7 days);
+        vm.warp(block.timestamp + 7 days);
 
         (uint256 pink, uint256 part) = vat.urns(ilk, address(d3mTestPool));
         assertEq(pink, 50 * WAD);
@@ -1126,7 +1120,7 @@ contract D3MHubTest is DSSTest {
         _windSystem();
         d3mHub.cage(ilk);
         d3mHub.deny(address(this));
-        hevm.warp(block.timestamp + 6 days);
+        vm.warp(block.timestamp + 6 days);
 
         assertRevert(address(d3mHub), abi.encodeWithSignature("cull(bytes32)", ilk), "D3MHub/unauthorized-cull");
     }
@@ -1596,21 +1590,21 @@ contract D3MHubTest is DSSTest {
 
     function test_exec_lock_protection() public {
         // Store memory slot 0x3
-        hevm.store(address(d3mHub), bytes32(uint256(3)), bytes32(uint256(1)));
+        vm.store(address(d3mHub), bytes32(uint256(3)), bytes32(uint256(1)));
         assertEq(d3mHub.locked(), 1);
 
         assertRevert(address(d3mHub), abi.encodeWithSignature("exec(bytes32)", ilk), "D3MHub/system-locked");
     }
 
     function test_exit_lock_protection() public {
         // Store memory slot 0x3
-        hevm.store(address(d3mHub), bytes32(uint256(3)), bytes32(uint256(1)));
+        vm.store(address(d3mHub), bytes32(uint256(3)), bytes32(uint256(1)));
         assertEq(d3mHub.locked(), 1);
 
         assertRevert(address(d3mHub), abi.encodeWithSignature("exit(bytes32,address,uint256)", ilk, address(this), 1), "D3MHub/system-locked");
     }
 
-    function test_wind_limited_by_pool_loss() public {
+    function test_unwind_due_to_by_pool_loss() public {
         _windSystem(); // winds to 50 * WAD
 
         // Set debt ceiling to 60 to limit loss
@@ -1623,21 +1617,21 @@ contract D3MHubTest is DSSTest {
         assertEq(testGem.balanceOf(address(d3mTestPool)), 50 * WAD);
         assertEq(d3mTestPool.assetBalance(), 50 * WAD);
 
-        _giveTokens(TokenLike(address(testGem)), address(d3mTestPool), 0);
+        _giveTokens(TokenLike(address(testGem)), address(d3mTestPool), 20 * WAD); // Lost 30 tokens
 
-        assertEq(testGem.balanceOf(address(d3mTestPool)), 0);
-        assertEq(d3mTestPool.assetBalance(), 0);
+        assertEq(testGem.balanceOf(address(d3mTestPool)), 20 * WAD);
+        assertEq(d3mTestPool.assetBalance(), 20 * WAD);
         (ink, art) = vat.urns(ilk, address(d3mTestPool));
         assertEq(ink, 50 * WAD);
         assertEq(art, 50 * WAD);
 
-        // This should only fill another 10 because the debt ceiling
+        // This should force unwind
         d3mHub.exec(ilk);
 
-        assertEq(d3mTestPool.assetBalance(), 10 * WAD);
+        assertEq(d3mTestPool.assetBalance(), 0);
         (ink, art) = vat.urns(ilk, address(d3mTestPool));
-        assertEq(ink, 60 * WAD);
-        assertEq(art, 60 * WAD);
+        assertEq(ink, 30 * WAD);
+        assertEq(art, 30 * WAD);
     }
 
     function test_exec_fixInk_full_under_debt_ceiling() public {
@@ -1672,7 +1666,7 @@ contract D3MHubTest is DSSTest {
         _giveTokens(TokenLike(address(testGem)), 10 * WAD);
         testGem.transfer(address(d3mTestPool), 10 * WAD); // Simulates 10 WAD of interest accumulated
         assertEq(testGem.balanceOf(address(d3mTestPool)), 60 * WAD);
-        hevm.store(
+        vm.store(
             address(dai),
             keccak256(abi.encode(address(testGem), uint256(2))),
             bytes32(uint256(0))
@@ -1702,7 +1696,7 @@ contract D3MHubTest is DSSTest {
         _giveTokens(TokenLike(address(testGem)), 10 * WAD);
         testGem.transfer(address(d3mTestPool), 10 * WAD); // Simulates 10 WAD of interest accumulated
         assertEq(testGem.balanceOf(address(d3mTestPool)), 60 * WAD);
-        hevm.store(
+        vm.store(
             address(dai),
             keccak256(abi.encode(address(testGem), uint256(2))),
             bytes32(uint256(3 * WAD))
@@ -1757,7 +1751,7 @@ contract D3MHubTest is DSSTest {
         _giveTokens(TokenLike(address(testGem)), 10 * WAD);
         testGem.transfer(address(d3mTestPool), 10 * WAD); // Simulates 10 WAD of interest accumulated
         assertEq(testGem.balanceOf(address(d3mTestPool)), 60 * WAD);
-        hevm.store(
+        vm.store(
             address(dai),
             keccak256(abi.encode(address(testGem), uint256(2))),
             bytes32(uint256(0))
@@ -1787,7 +1781,7 @@ contract D3MHubTest is DSSTest {
         _giveTokens(TokenLike(address(testGem)), 10 * WAD);
         testGem.transfer(address(d3mTestPool), 10 * WAD); // Simulates 10 WAD of interest accumulated
         assertEq(testGem.balanceOf(address(d3mTestPool)), 60 * WAD);
-        hevm.store(
+        vm.store(
             address(dai),
             keccak256(abi.encode(address(testGem), uint256(2))),
             bytes32(uint256(3 * WAD))
@@ -1817,7 +1811,7 @@ contract D3MHubTest is DSSTest {
         _giveTokens(TokenLike(address(testGem)), 10 * WAD);
         testGem.transfer(address(d3mTestPool), 10 * WAD); // Simulates 10 WAD of interest accumulated
         assertEq(testGem.balanceOf(address(d3mTestPool)), 60 * WAD);
-        hevm.store(
+        vm.store(
             address(dai),
             keccak256(abi.encode(address(testGem), uint256(2))),
             bytes32(uint256(10 * WAD))
@@ -1847,7 +1841,7 @@ contract D3MHubTest is DSSTest {
         _giveTokens(TokenLike(address(testGem)), 10 * WAD);
         testGem.transfer(address(d3mTestPool), 10 * WAD); // Simulates 10 WAD of interest accumulated
         assertEq(testGem.balanceOf(address(d3mTestPool)), 60 * WAD);
-        hevm.store(
+        vm.store(
             address(dai),
             keccak256(abi.encode(address(testGem), uint256(2))),
             bytes32(uint256(0))
@@ -1877,7 +1871,7 @@ contract D3MHubTest is DSSTest {
         _giveTokens(TokenLike(address(testGem)), 10 * WAD);
         testGem.transfer(address(d3mTestPool), 10 * WAD); // Simulates 10 WAD of interest accumulated
         assertEq(testGem.balanceOf(address(d3mTestPool)), 60 * WAD);
-        hevm.store(
+        vm.store(
             address(dai),
             keccak256(abi.encode(address(testGem), uint256(2))),
             bytes32(uint256(3 * WAD))

diff --git a/src/tests/D3MMom.t.sol b/src/tests/D3MMom.t.sol
@@ -35,16 +35,10 @@ interface Hevm {
 }
 
 contract D3MMomTest is DSSTest {
-    Hevm hevm;
-
     D3MTestPlan d3mTestPlan;
     D3MMom d3mMom;
 
     function setUp() public override {
-        hevm = Hevm(
-            address(bytes20(uint160(uint256(keccak256("hevm cheat code")))))
-        );
-
         d3mTestPlan = new D3MTestPlan(address(123));
 
         d3mTestPlan.file("maxBar_", type(uint256).max);