fix: oracle written timestamp

* fix: write current block timestamp instead of previous timestamp for oracle observation * fix: change arg in internal func sig to reflect correct timestamp * ci: update solc version * lint: fix issues * ci: update hashes * ci: remove pin on foundry version * test: add assert to make sure latest timestamp written * test: test case to make sure latest timestamp is written * fix: failing tests * fix: bug in test case * lib: bump `forge-std` to v1.7.6 * ci: update gas snapshot
reservoir-labs · Jan 23, 2024 · c066d02 · c066d02
1 parent 573a2db
commit c066d02
Show file tree

Hide file tree

Showing 16 changed files with 255 additions and 233 deletions.
diff --git a/.gas-snapshot b/.gas-snapshot
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -16,7 +16,7 @@ jobs:
           submodules: recursive
       - uses: foundry-rs/foundry-toolchain@v1
         with:
-          version: nightly-e15e33a07c0920189fc336391f538c3dad53da73
+          version: nightly
       - run: forge build
 
   lint:
@@ -41,7 +41,7 @@ jobs:
           submodules: recursive
       - uses: foundry-rs/foundry-toolchain@v1
         with:
-          version: nightly-e15e33a07c0920189fc336391f538c3dad53da73
+          version: nightly
       - run: npm run test:unit
 
   test-integration:
@@ -52,7 +52,7 @@ jobs:
           submodules: recursive
       - uses: foundry-rs/foundry-toolchain@v1
         with:
-          version: nightly-e15e33a07c0920189fc336391f538c3dad53da73
+          version: nightly
       - run: npm run test:integration
 
   test-e2e:
@@ -63,7 +63,7 @@ jobs:
           submodules: recursive
       - uses: foundry-rs/foundry-toolchain@v1
         with:
-          version: nightly-e15e33a07c0920189fc336391f538c3dad53da73
+          version: nightly
       - run: npm run test:e2e
 
   test-uniswap-v2:
@@ -75,7 +75,7 @@ jobs:
           submodules: recursive
       - uses: foundry-rs/foundry-toolchain@v1
         with:
-          version: nightly-e15e33a07c0920189fc336391f538c3dad53da73
+          version: nightly
       - uses: actions/setup-node@v3
         with:
           node-version-file: ".nvmrc"
@@ -93,7 +93,7 @@ jobs:
           submodules: recursive
       - uses: foundry-rs/foundry-toolchain@v1
         with:
-          version: nightly-e15e33a07c0920189fc336391f538c3dad53da73
+          version: nightly
       - run: npm run install:balancer
       - run: npm run build:balancer
       - run: npm run test:balancer
@@ -108,7 +108,7 @@ jobs:
           submodules: recursive
       - uses: foundry-rs/foundry-toolchain@v1
         with:
-          version: nightly-e15e33a07c0920189fc336391f538c3dad53da73
+          version: nightly
       - run: forge snapshot --check
         env:
           FOUNDRY_PROFILE: default
@@ -122,7 +122,7 @@ jobs:
           submodules: recursive
       - uses: foundry-rs/foundry-toolchain@v1
         with:
-          version: nightly-e15e33a07c0920189fc336391f538c3dad53da73
+          version: nightly
       - run: ./script/coverage_patch_deployer.sh && forge coverage --report lcov
         env:
           FOUNDRY_PROFILE: coverage

diff --git a/README.md b/README.md
@@ -68,7 +68,8 @@ V1.0
 Are you interested in helping us build the future of Reservoir?
 Contribute in these ways:
 
-- For SECURITY related or sensitive bugs, please get in touch with the team at [email protected] or on discord instead of opening an issue on github.
+- For SECURITY related or sensitive bugs, please get in touch with the team
+at [email protected] or on discord instead of opening an issue on github.
 
 - If you find bugs or code errors, you can open a new
 [issue ticket here.](https://github.com/reservoir-labs/amm-core/issues/new)

diff --git a/foundry.toml b/foundry.toml
@@ -1,5 +1,5 @@
 [profile.default]
-solc = "0.8.19"
+solc = "0.8.23"
 #via_ir = true
 bytecode_hash = "ipfs"
 optimizer_runs = 1_000_000

diff --git a/lib/forge-std b/lib/forge-std
diff --git a/script/optimized-deployer-meta b/script/optimized-deployer-meta
@@ -1,6 +1,6 @@
 {
-  "constant_product_hash": "0xde6d8452b97c785350ce9f82090dfb5962933aa5467827310798925c8725013b",
-  "factory_hash": "0xaafa0bd4694a22c24f8ef26cfd45864f11be2102e269ee6a831d2c659b30f08f",
-  "oracle_caller_hash": "0x1dcb29ca6399be1a5f8a4b3f168a83ff20697718c3d5434ddeeb4a8050fdc615",
-  "stable_hash": "0x1289a8879d4d1992d308d2a4d8457a5ca8d57e07effe092f2a1f21b0b9b10619"
+  "constant_product_hash": "0x7f3a12e644606d9d166691742af07487155a1b86cc1a97a9ca21a5b474c3fd14",
+  "factory_hash": "0x0aa0c77b70a77a082be061cb9565cbe92d8ee7b8a74007287d597c000c0d4ec4",
+  "oracle_caller_hash": "0x9c8ae1fb85485b08e88ceed1c47608205b81b6c7b438c1770abd5dedf6188423",
+  "stable_hash": "0x044e7b5a6e582680c9a2dfcb895bd9993c9462c3cf17048f408a3c714600e37d"
 }
diff --git a/script/unoptimized-deployer-meta b/script/unoptimized-deployer-meta
@@ -1,6 +1,6 @@
 {
-  "constant_product_hash": "0x71f0d385cb55f83e9dd571b118cb5ee391542d27ff471998ea316d3693b21308",
-  "factory_hash": "0xa61785bbfa1f668e1ad6e2aff9c6fc4ed961b95ae7cc54c6463d49b259b2e984",
-  "oracle_caller_hash": "0x6e5b6d511ec5f70fdae7be4b74be5a313df4168f1b50dee9fc571e4414697354",
-  "stable_hash": "0xa17d9c01054e0d89b5da9d84936c6960c6f82f7981951978cc2aff275d943f5a"
+  "constant_product_hash": "0xf2204f73059bda42b0910c810137b13b403a9087928123890443c76b866d4135",
+  "factory_hash": "0x64c369a012abcc9c156cce47a6a1a0916225795135bec914ac0083327650b949",
+  "oracle_caller_hash": "0x99643805c6f1cf50d6f68ecdaba00a6dc4694001423319968799766504a28e1b",
+  "stable_hash": "0x76047e8e6a3e3291cd4334776733b188fb11d2d83851f3ffe0861cc90afa3925"
 }
diff --git a/src/ReservoirDeployer.sol b/src/ReservoirDeployer.sol
@@ -17,12 +17,12 @@ contract ReservoirDeployer {
     uint256 public step = 0;
 
     // Bytecode hashes.
-    bytes32 public constant FACTORY_HASH = bytes32(0xaafa0bd4694a22c24f8ef26cfd45864f11be2102e269ee6a831d2c659b30f08f);
+    bytes32 public constant FACTORY_HASH = bytes32(0x0aa0c77b70a77a082be061cb9565cbe92d8ee7b8a74007287d597c000c0d4ec4);
     bytes32 public constant CONSTANT_PRODUCT_HASH =
-        bytes32(0xde6d8452b97c785350ce9f82090dfb5962933aa5467827310798925c8725013b);
-    bytes32 public constant STABLE_HASH = bytes32(0x1289a8879d4d1992d308d2a4d8457a5ca8d57e07effe092f2a1f21b0b9b10619);
+        bytes32(0x7f3a12e644606d9d166691742af07487155a1b86cc1a97a9ca21a5b474c3fd14);
+    bytes32 public constant STABLE_HASH = bytes32(0x044e7b5a6e582680c9a2dfcb895bd9993c9462c3cf17048f408a3c714600e37d);
     bytes32 public constant ORACLE_CALLER_HASH =
-        bytes32(0x1dcb29ca6399be1a5f8a4b3f168a83ff20697718c3d5434ddeeb4a8050fdc615);
+        bytes32(0x9c8ae1fb85485b08e88ceed1c47608205b81b6c7b438c1770abd5dedf6188423);
 
     // Deployment addresses.
     GenericFactory public factory;

diff --git a/src/ReservoirPair.sol b/src/ReservoirPair.sol
@@ -156,7 +156,7 @@ abstract contract ReservoirPair is IAssetManagedPair, ReservoirERC20 {
             lTimeElapsed = lBlockTimestamp - aBlockTimestampLast;
         }
         if (lTimeElapsed > 0 && aReserve0 != 0 && aReserve1 != 0) {
-            _updateOracle(aReserve0, aReserve1, lTimeElapsed, aBlockTimestampLast);
+            _updateOracle(aReserve0, aReserve1, lTimeElapsed, lBlockTimestamp);
         }
 
         // update reserves to match latest balances
@@ -544,7 +544,7 @@ abstract contract ReservoirPair is IAssetManagedPair, ReservoirERC20 {
         }
     }
 
-    function _updateOracle(uint256 aReserve0, uint256 aReserve1, uint32 aTimeElapsed, uint32 aTimestampLast)
+    function _updateOracle(uint256 aReserve0, uint256 aReserve1, uint32 aTimeElapsed, uint32 aCurrentTimestamp)
         internal
         virtual;
 }
diff --git a/src/curve/constant-product/ConstantProductPair.sol b/src/curve/constant-product/ConstantProductPair.sol
@@ -223,7 +223,7 @@ contract ConstantProductPair is ReservoirPair {
                                 ORACLE METHODS
     //////////////////////////////////////////////////////////////////////////*/
 
-    function _updateOracle(uint256 aReserve0, uint256 aReserve1, uint32 aTimeElapsed, uint32 aTimestampLast)
+    function _updateOracle(uint256 aReserve0, uint256 aReserve1, uint32 aTimeElapsed, uint32 aCurrentTimestamp)
         internal
         override
     {
@@ -246,7 +246,7 @@ contract ConstantProductPair is ReservoirPair {
                 previous.logAccClampedPrice + int56(currLogClampedPrice) * int56(int256(uint256(aTimeElapsed)));
             int56 logAccLiq = previous.logAccLiquidity + int56(lCurrLogLiq) * int56(int256(uint256(aTimeElapsed)));
             _slot0.index += 1;
-            _observations[_slot0.index] = Observation(logAccRawPrice, logAccClampedPrice, logAccLiq, aTimestampLast);
+            _observations[_slot0.index] = Observation(logAccRawPrice, logAccClampedPrice, logAccLiq, aCurrentTimestamp);
         }
     }
 }
diff --git a/src/curve/stable/StablePair.sol b/src/curve/stable/StablePair.sol
@@ -279,7 +279,7 @@ contract StablePair is ReservoirPair {
                                 ORACLE METHODS
     //////////////////////////////////////////////////////////////////////////*/
 
-    function _updateOracle(uint256 aReserve0, uint256 aReserve1, uint32 aTimeElapsed, uint32 aTimestampLast)
+    function _updateOracle(uint256 aReserve0, uint256 aReserve1, uint32 aTimeElapsed, uint32 aCurrentTimestamp)
         internal
         override
     {
@@ -301,7 +301,7 @@ contract StablePair is ReservoirPair {
                 previous.logAccClampedPrice + int56(currLogClampedPrice) * int56(int256(uint256(aTimeElapsed)));
             int56 logAccLiq = previous.logAccLiquidity + int56(currLogLiq) * int56(int256(uint256(aTimeElapsed)));
             _slot0.index += 1;
-            _observations[_slot0.index] = Observation(logAccRawPrice, logAccClampedPrice, logAccLiq, aTimestampLast);
+            _observations[_slot0.index] = Observation(logAccRawPrice, logAccClampedPrice, logAccLiq, aCurrentTimestamp);
         }
     }
 }
diff --git a/test/__fixtures/BaseTest.sol b/test/__fixtures/BaseTest.sol
@@ -17,6 +17,7 @@ import { ConstantProductPair } from "src/curve/constant-product/ConstantProductP
 import { StablePair, AmplificationData } from "src/curve/stable/StablePair.sol";
 import { StableMintBurn } from "src/curve/stable/StableMintBurn.sol";
 
+// solhint-disable-next-line max-states-count
 abstract contract BaseTest is Test {
     using FactoryStoreLib for GenericFactory;
 

diff --git a/test/unit/AssetManagedPair.t.sol b/test/unit/AssetManagedPair.t.sol
@@ -208,8 +208,8 @@ contract AssetManagedPairTest is BaseTest {
 
     function testMint_AfterLoss(uint256 aNewManagedBalance0, uint256 aNewManagedBalance1) external allPairs {
         // assume
-        uint256 lNewManagedBalance0 = bound(aNewManagedBalance0, 1, 10e18);
-        uint256 lNewManagedBalance1 = bound(aNewManagedBalance1, 1, 10e18);
+        uint256 lNewManagedBalance0 = bound(aNewManagedBalance0, 1, 9e18);
+        uint256 lNewManagedBalance1 = bound(aNewManagedBalance1, 1, 9e18);
 
         // arrange
         vm.prank(address(_factory));
@@ -234,8 +234,8 @@ contract AssetManagedPairTest is BaseTest {
 
     function testBurn_AfterLoss(uint256 aNewManagedBalance0, uint256 aNewManagedBalance1) external allPairs {
         // assume
-        uint256 lNewManagedBalance0 = bound(aNewManagedBalance0, 1, 10e18);
-        uint256 lNewManagedBalance1 = bound(aNewManagedBalance1, 1, 10e18);
+        uint256 lNewManagedBalance0 = bound(aNewManagedBalance0, 1, 9e18);
+        uint256 lNewManagedBalance1 = bound(aNewManagedBalance1, 1, 9e18);
 
         // arrange
         vm.prank(address(_factory));
@@ -259,7 +259,7 @@ contract AssetManagedPairTest is BaseTest {
 
     function testSwap_AfterLoss(uint256 aNewManagedBalance0) external allPairs {
         // assume
-        uint256 lNewManagedBalance0 = bound(aNewManagedBalance0, 1, 10e18);
+        uint256 lNewManagedBalance0 = bound(aNewManagedBalance0, 1, 9e18);
 
         // arrange
         int256 lSwapAmt = 1e18;

diff --git a/test/unit/OracleWriter.t.sol b/test/unit/OracleWriter.t.sol
@@ -40,29 +40,6 @@ contract OracleWriterTest is BaseTest {
         _pair.observation(lIndex);
     }
 
-    function testUpdateOracle_WriteOldReservesNotNew() external allPairs {
-        // arrange
-        uint256 lJumpAhead = 10;
-        (uint104 lReserve0,,,) = _pair.getReserves();
-        assertEq(lReserve0, Constants.INITIAL_MINT_AMOUNT);
-        _tokenA.mint(address(_pair), 10e18);
-
-        // act - call sync to trigger a write to the oracle
-        _stepTime(lJumpAhead);
-        _pair.sync();
-
-        // assert - make sure that the written observation is of the previous reserves, not the new reserves
-        (uint256 lNewReserve0,,, uint16 lIndex) = _pair.getReserves();
-
-        Observation memory lObs = _oracleCaller.observation(_pair, lIndex);
-        assertEq(lNewReserve0, 110e18);
-        assertApproxEqRel(
-            LogCompression.fromLowResLog(lObs.logAccLiquidity / int56(int256(lJumpAhead))),
-            Constants.INITIAL_MINT_AMOUNT,
-            0.0001e18
-        );
-    }
-
     function testUpdateOracleCaller() external allPairs {
         // arrange
         address lNewOracleCaller = address(0x555);
@@ -122,6 +99,47 @@ contract OracleWriterTest is BaseTest {
         _pair.setMaxChangeRate(lMaxChangeRate);
     }
 
+    function testUpdateOracle_WriteOldReservesNotNew() external allPairs {
+        // arrange
+        uint256 lJumpAhead = 10;
+        (uint104 lReserve0,,,) = _pair.getReserves();
+        assertEq(lReserve0, Constants.INITIAL_MINT_AMOUNT);
+        _tokenA.mint(address(_pair), 10e18);
+
+        // act - call sync to trigger a write to the oracle
+        _stepTime(lJumpAhead);
+        _pair.sync();
+
+        // assert - make sure that the written observation is of the previous reserves, not the new reserves
+        (uint256 lNewReserve0,,, uint16 lIndex) = _pair.getReserves();
+
+        Observation memory lObs = _oracleCaller.observation(_pair, lIndex);
+        assertEq(lNewReserve0, 110e18);
+        assertApproxEqRel(
+            LogCompression.fromLowResLog(lObs.logAccLiquidity / int56(int256(lJumpAhead))),
+            Constants.INITIAL_MINT_AMOUNT,
+            0.0001e18
+        );
+    }
+
+    function testUpdateOracle_LatestTimestampWritten(uint256 aJumpAhead) external allPairs {
+        // assume
+        uint256 lJumpAhead = bound(aJumpAhead, 10, type(uint16).max);
+
+        // arrange
+        uint256 lStartingTimestamp = block.timestamp;
+        _stepTime(lJumpAhead);
+
+        // act
+        _tokenA.mint(address(_pair), 5e18);
+        _pair.swap(5e18, true, address(this), "");
+
+        // assert
+        (,,, uint16 lIndex) = _pair.getReserves();
+        Observation memory lObs = _oracleCaller.observation(_pair, lIndex);
+        assertEq(lObs.timestamp, lStartingTimestamp + lJumpAhead);
+    }
+
     function testOracle_CompareLiquidityTwoCurves_Balanced(uint32 aNewStartTime)
         external
         randomizeStartTime(aNewStartTime)

diff --git a/test/unit/StablePair.t.sol b/test/unit/StablePair.t.sol
@@ -415,7 +415,7 @@ contract StablePairTest is BaseTest {
         (uint256 lExpectedPlatformFee, uint256 lGrowthInLiq) =
             _calcExpectedPlatformFee(lPlatformFee, lPair, lReserve0, lReserve1, lTotalSupply, lOldLiq);
         assertEq(lPair.balanceOf(_platformFeeTo), lExpectedPlatformFee);
-        if (aPlatformFee > 0) {
+        if (lPlatformFee > 0) {
             assertGt(lPair.balanceOf(_platformFeeTo), 0);
         }
         assertApproxEqRel(

diff --git a/test/unit/gas/ConstantProductPair.gas.sol b/test/unit/gas/ConstantProductPair.gas.sol
@@ -37,6 +37,7 @@ contract ConstantProductPairGas is BaseTest {
         _oraclePair.burn(address(this));
 
         // This pair will let a user swap without writing the oracle.
+        // solhint-disable-next-line reentrancy
         _simplePair = ConstantProductPair(_factory.createPair(IERC20(address(_tokenA)), IERC20(address(_tokenC)), 0));
         _tokenA.mint(address(_simplePair), 100e18);
         _tokenC.mint(address(_simplePair), 100e18);
+1 −0		.gitattributes
+48 −6		.github/workflows/ci.yml
+29 −0		.github/workflows/sync.yml
+1 −1		foundry.toml
+1 −1		package.json
+635 −0		scripts/vm.py
+5 −3		src/Base.sol
+4 −3		src/Script.sol
+31 −16		src/StdChains.sol
+195 −17		src/StdCheats.sol
+16 −1		src/StdInvariant.sol
+18 −14		src/StdJson.sol
+54 −3		src/StdStorage.sol
+2 −2		src/StdStyle.sol
+71 −34		src/StdUtils.sol
+4 −3		src/Test.sol
+1,002 −321		src/Vm.sol
+394 −382		src/console2.sol
+216 −0		src/mocks/MockERC20.sol
+221 −0		src/mocks/MockERC721.sol
+13,248 −0		src/safeconsole.sol
+93 −77		test/StdAssertions.t.sol
+114 −58		test/StdChains.t.sol
+236 −44		test/StdCheats.t.sol
+13 −11		test/StdError.t.sol
+27 −12		test/StdMath.t.sol
+66 −34		test/StdStorage.t.sol
+4 −4		test/StdStyle.t.sol
+80 −35		test/StdUtils.t.sol
+15 −0		test/Vm.t.sol
+441 −0		test/mocks/MockERC20.t.sol
+721 −0		test/mocks/MockERC721.t.sol