Skip to content

Commit c066d02

Browse files
authored
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
1 parent 573a2db commit c066d02

16 files changed

+255
-233
lines changed

.gas-snapshot

Lines changed: 176 additions & 175 deletions
Large diffs are not rendered by default.

.github/workflows/ci.yml

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ jobs:
1616
submodules: recursive
1717
- uses: foundry-rs/foundry-toolchain@v1
1818
with:
19-
version: nightly-e15e33a07c0920189fc336391f538c3dad53da73
19+
version: nightly
2020
- run: forge build
2121

2222
lint:
@@ -41,7 +41,7 @@ jobs:
4141
submodules: recursive
4242
- uses: foundry-rs/foundry-toolchain@v1
4343
with:
44-
version: nightly-e15e33a07c0920189fc336391f538c3dad53da73
44+
version: nightly
4545
- run: npm run test:unit
4646

4747
test-integration:
@@ -52,7 +52,7 @@ jobs:
5252
submodules: recursive
5353
- uses: foundry-rs/foundry-toolchain@v1
5454
with:
55-
version: nightly-e15e33a07c0920189fc336391f538c3dad53da73
55+
version: nightly
5656
- run: npm run test:integration
5757

5858
test-e2e:
@@ -63,7 +63,7 @@ jobs:
6363
submodules: recursive
6464
- uses: foundry-rs/foundry-toolchain@v1
6565
with:
66-
version: nightly-e15e33a07c0920189fc336391f538c3dad53da73
66+
version: nightly
6767
- run: npm run test:e2e
6868

6969
test-uniswap-v2:
@@ -75,7 +75,7 @@ jobs:
7575
submodules: recursive
7676
- uses: foundry-rs/foundry-toolchain@v1
7777
with:
78-
version: nightly-e15e33a07c0920189fc336391f538c3dad53da73
78+
version: nightly
7979
- uses: actions/setup-node@v3
8080
with:
8181
node-version-file: ".nvmrc"
@@ -93,7 +93,7 @@ jobs:
9393
submodules: recursive
9494
- uses: foundry-rs/foundry-toolchain@v1
9595
with:
96-
version: nightly-e15e33a07c0920189fc336391f538c3dad53da73
96+
version: nightly
9797
- run: npm run install:balancer
9898
- run: npm run build:balancer
9999
- run: npm run test:balancer
@@ -108,7 +108,7 @@ jobs:
108108
submodules: recursive
109109
- uses: foundry-rs/foundry-toolchain@v1
110110
with:
111-
version: nightly-e15e33a07c0920189fc336391f538c3dad53da73
111+
version: nightly
112112
- run: forge snapshot --check
113113
env:
114114
FOUNDRY_PROFILE: default
@@ -122,7 +122,7 @@ jobs:
122122
submodules: recursive
123123
- uses: foundry-rs/foundry-toolchain@v1
124124
with:
125-
version: nightly-e15e33a07c0920189fc336391f538c3dad53da73
125+
version: nightly
126126
- run: ./script/coverage_patch_deployer.sh && forge coverage --report lcov
127127
env:
128128
FOUNDRY_PROFILE: coverage

README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ V1.0
6868
Are you interested in helping us build the future of Reservoir?
6969
Contribute in these ways:
7070

71-
- For SECURITY related or sensitive bugs, please get in touch with the team at security@reservoir.fi or on discord instead of opening an issue on github.
71+
- For SECURITY related or sensitive bugs, please get in touch with the team
72+
at security@reservoir.fi or on discord instead of opening an issue on github.
7273

7374
- If you find bugs or code errors, you can open a new
7475
[issue ticket here.](https://github.com/reservoir-labs/amm-core/issues/new)

foundry.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
[profile.default]
2-
solc = "0.8.19"
2+
solc = "0.8.23"
33
#via_ir = true
44
bytecode_hash = "ipfs"
55
optimizer_runs = 1_000_000

script/optimized-deployer-meta

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
2-
"constant_product_hash": "0xde6d8452b97c785350ce9f82090dfb5962933aa5467827310798925c8725013b",
3-
"factory_hash": "0xaafa0bd4694a22c24f8ef26cfd45864f11be2102e269ee6a831d2c659b30f08f",
4-
"oracle_caller_hash": "0x1dcb29ca6399be1a5f8a4b3f168a83ff20697718c3d5434ddeeb4a8050fdc615",
5-
"stable_hash": "0x1289a8879d4d1992d308d2a4d8457a5ca8d57e07effe092f2a1f21b0b9b10619"
2+
"constant_product_hash": "0x7f3a12e644606d9d166691742af07487155a1b86cc1a97a9ca21a5b474c3fd14",
3+
"factory_hash": "0x0aa0c77b70a77a082be061cb9565cbe92d8ee7b8a74007287d597c000c0d4ec4",
4+
"oracle_caller_hash": "0x9c8ae1fb85485b08e88ceed1c47608205b81b6c7b438c1770abd5dedf6188423",
5+
"stable_hash": "0x044e7b5a6e582680c9a2dfcb895bd9993c9462c3cf17048f408a3c714600e37d"
66
}

script/unoptimized-deployer-meta

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
2-
"constant_product_hash": "0x71f0d385cb55f83e9dd571b118cb5ee391542d27ff471998ea316d3693b21308",
3-
"factory_hash": "0xa61785bbfa1f668e1ad6e2aff9c6fc4ed961b95ae7cc54c6463d49b259b2e984",
4-
"oracle_caller_hash": "0x6e5b6d511ec5f70fdae7be4b74be5a313df4168f1b50dee9fc571e4414697354",
5-
"stable_hash": "0xa17d9c01054e0d89b5da9d84936c6960c6f82f7981951978cc2aff275d943f5a"
2+
"constant_product_hash": "0xf2204f73059bda42b0910c810137b13b403a9087928123890443c76b866d4135",
3+
"factory_hash": "0x64c369a012abcc9c156cce47a6a1a0916225795135bec914ac0083327650b949",
4+
"oracle_caller_hash": "0x99643805c6f1cf50d6f68ecdaba00a6dc4694001423319968799766504a28e1b",
5+
"stable_hash": "0x76047e8e6a3e3291cd4334776733b188fb11d2d83851f3ffe0861cc90afa3925"
66
}

src/ReservoirDeployer.sol

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,12 @@ contract ReservoirDeployer {
1717
uint256 public step = 0;
1818

1919
// Bytecode hashes.
20-
bytes32 public constant FACTORY_HASH = bytes32(0xaafa0bd4694a22c24f8ef26cfd45864f11be2102e269ee6a831d2c659b30f08f);
20+
bytes32 public constant FACTORY_HASH = bytes32(0x0aa0c77b70a77a082be061cb9565cbe92d8ee7b8a74007287d597c000c0d4ec4);
2121
bytes32 public constant CONSTANT_PRODUCT_HASH =
22-
bytes32(0xde6d8452b97c785350ce9f82090dfb5962933aa5467827310798925c8725013b);
23-
bytes32 public constant STABLE_HASH = bytes32(0x1289a8879d4d1992d308d2a4d8457a5ca8d57e07effe092f2a1f21b0b9b10619);
22+
bytes32(0x7f3a12e644606d9d166691742af07487155a1b86cc1a97a9ca21a5b474c3fd14);
23+
bytes32 public constant STABLE_HASH = bytes32(0x044e7b5a6e582680c9a2dfcb895bd9993c9462c3cf17048f408a3c714600e37d);
2424
bytes32 public constant ORACLE_CALLER_HASH =
25-
bytes32(0x1dcb29ca6399be1a5f8a4b3f168a83ff20697718c3d5434ddeeb4a8050fdc615);
25+
bytes32(0x9c8ae1fb85485b08e88ceed1c47608205b81b6c7b438c1770abd5dedf6188423);
2626

2727
// Deployment addresses.
2828
GenericFactory public factory;

src/ReservoirPair.sol

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ abstract contract ReservoirPair is IAssetManagedPair, ReservoirERC20 {
156156
lTimeElapsed = lBlockTimestamp - aBlockTimestampLast;
157157
}
158158
if (lTimeElapsed > 0 && aReserve0 != 0 && aReserve1 != 0) {
159-
_updateOracle(aReserve0, aReserve1, lTimeElapsed, aBlockTimestampLast);
159+
_updateOracle(aReserve0, aReserve1, lTimeElapsed, lBlockTimestamp);
160160
}
161161

162162
// update reserves to match latest balances
@@ -544,7 +544,7 @@ abstract contract ReservoirPair is IAssetManagedPair, ReservoirERC20 {
544544
}
545545
}
546546

547-
function _updateOracle(uint256 aReserve0, uint256 aReserve1, uint32 aTimeElapsed, uint32 aTimestampLast)
547+
function _updateOracle(uint256 aReserve0, uint256 aReserve1, uint32 aTimeElapsed, uint32 aCurrentTimestamp)
548548
internal
549549
virtual;
550550
}

src/curve/constant-product/ConstantProductPair.sol

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ contract ConstantProductPair is ReservoirPair {
223223
ORACLE METHODS
224224
//////////////////////////////////////////////////////////////////////////*/
225225

226-
function _updateOracle(uint256 aReserve0, uint256 aReserve1, uint32 aTimeElapsed, uint32 aTimestampLast)
226+
function _updateOracle(uint256 aReserve0, uint256 aReserve1, uint32 aTimeElapsed, uint32 aCurrentTimestamp)
227227
internal
228228
override
229229
{
@@ -246,7 +246,7 @@ contract ConstantProductPair is ReservoirPair {
246246
previous.logAccClampedPrice + int56(currLogClampedPrice) * int56(int256(uint256(aTimeElapsed)));
247247
int56 logAccLiq = previous.logAccLiquidity + int56(lCurrLogLiq) * int56(int256(uint256(aTimeElapsed)));
248248
_slot0.index += 1;
249-
_observations[_slot0.index] = Observation(logAccRawPrice, logAccClampedPrice, logAccLiq, aTimestampLast);
249+
_observations[_slot0.index] = Observation(logAccRawPrice, logAccClampedPrice, logAccLiq, aCurrentTimestamp);
250250
}
251251
}
252252
}

src/curve/stable/StablePair.sol

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ contract StablePair is ReservoirPair {
279279
ORACLE METHODS
280280
//////////////////////////////////////////////////////////////////////////*/
281281

282-
function _updateOracle(uint256 aReserve0, uint256 aReserve1, uint32 aTimeElapsed, uint32 aTimestampLast)
282+
function _updateOracle(uint256 aReserve0, uint256 aReserve1, uint32 aTimeElapsed, uint32 aCurrentTimestamp)
283283
internal
284284
override
285285
{
@@ -301,7 +301,7 @@ contract StablePair is ReservoirPair {
301301
previous.logAccClampedPrice + int56(currLogClampedPrice) * int56(int256(uint256(aTimeElapsed)));
302302
int56 logAccLiq = previous.logAccLiquidity + int56(currLogLiq) * int56(int256(uint256(aTimeElapsed)));
303303
_slot0.index += 1;
304-
_observations[_slot0.index] = Observation(logAccRawPrice, logAccClampedPrice, logAccLiq, aTimestampLast);
304+
_observations[_slot0.index] = Observation(logAccRawPrice, logAccClampedPrice, logAccLiq, aCurrentTimestamp);
305305
}
306306
}
307307
}

test/__fixtures/BaseTest.sol

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { ConstantProductPair } from "src/curve/constant-product/ConstantProductP
1717
import { StablePair, AmplificationData } from "src/curve/stable/StablePair.sol";
1818
import { StableMintBurn } from "src/curve/stable/StableMintBurn.sol";
1919

20+
// solhint-disable-next-line max-states-count
2021
abstract contract BaseTest is Test {
2122
using FactoryStoreLib for GenericFactory;
2223

test/unit/AssetManagedPair.t.sol

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -208,8 +208,8 @@ contract AssetManagedPairTest is BaseTest {
208208

209209
function testMint_AfterLoss(uint256 aNewManagedBalance0, uint256 aNewManagedBalance1) external allPairs {
210210
// assume
211-
uint256 lNewManagedBalance0 = bound(aNewManagedBalance0, 1, 10e18);
212-
uint256 lNewManagedBalance1 = bound(aNewManagedBalance1, 1, 10e18);
211+
uint256 lNewManagedBalance0 = bound(aNewManagedBalance0, 1, 9e18);
212+
uint256 lNewManagedBalance1 = bound(aNewManagedBalance1, 1, 9e18);
213213

214214
// arrange
215215
vm.prank(address(_factory));
@@ -234,8 +234,8 @@ contract AssetManagedPairTest is BaseTest {
234234

235235
function testBurn_AfterLoss(uint256 aNewManagedBalance0, uint256 aNewManagedBalance1) external allPairs {
236236
// assume
237-
uint256 lNewManagedBalance0 = bound(aNewManagedBalance0, 1, 10e18);
238-
uint256 lNewManagedBalance1 = bound(aNewManagedBalance1, 1, 10e18);
237+
uint256 lNewManagedBalance0 = bound(aNewManagedBalance0, 1, 9e18);
238+
uint256 lNewManagedBalance1 = bound(aNewManagedBalance1, 1, 9e18);
239239

240240
// arrange
241241
vm.prank(address(_factory));
@@ -259,7 +259,7 @@ contract AssetManagedPairTest is BaseTest {
259259

260260
function testSwap_AfterLoss(uint256 aNewManagedBalance0) external allPairs {
261261
// assume
262-
uint256 lNewManagedBalance0 = bound(aNewManagedBalance0, 1, 10e18);
262+
uint256 lNewManagedBalance0 = bound(aNewManagedBalance0, 1, 9e18);
263263

264264
// arrange
265265
int256 lSwapAmt = 1e18;

test/unit/OracleWriter.t.sol

Lines changed: 41 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -40,29 +40,6 @@ contract OracleWriterTest is BaseTest {
4040
_pair.observation(lIndex);
4141
}
4242

43-
function testUpdateOracle_WriteOldReservesNotNew() external allPairs {
44-
// arrange
45-
uint256 lJumpAhead = 10;
46-
(uint104 lReserve0,,,) = _pair.getReserves();
47-
assertEq(lReserve0, Constants.INITIAL_MINT_AMOUNT);
48-
_tokenA.mint(address(_pair), 10e18);
49-
50-
// act - call sync to trigger a write to the oracle
51-
_stepTime(lJumpAhead);
52-
_pair.sync();
53-
54-
// assert - make sure that the written observation is of the previous reserves, not the new reserves
55-
(uint256 lNewReserve0,,, uint16 lIndex) = _pair.getReserves();
56-
57-
Observation memory lObs = _oracleCaller.observation(_pair, lIndex);
58-
assertEq(lNewReserve0, 110e18);
59-
assertApproxEqRel(
60-
LogCompression.fromLowResLog(lObs.logAccLiquidity / int56(int256(lJumpAhead))),
61-
Constants.INITIAL_MINT_AMOUNT,
62-
0.0001e18
63-
);
64-
}
65-
6643
function testUpdateOracleCaller() external allPairs {
6744
// arrange
6845
address lNewOracleCaller = address(0x555);
@@ -122,6 +99,47 @@ contract OracleWriterTest is BaseTest {
12299
_pair.setMaxChangeRate(lMaxChangeRate);
123100
}
124101

102+
function testUpdateOracle_WriteOldReservesNotNew() external allPairs {
103+
// arrange
104+
uint256 lJumpAhead = 10;
105+
(uint104 lReserve0,,,) = _pair.getReserves();
106+
assertEq(lReserve0, Constants.INITIAL_MINT_AMOUNT);
107+
_tokenA.mint(address(_pair), 10e18);
108+
109+
// act - call sync to trigger a write to the oracle
110+
_stepTime(lJumpAhead);
111+
_pair.sync();
112+
113+
// assert - make sure that the written observation is of the previous reserves, not the new reserves
114+
(uint256 lNewReserve0,,, uint16 lIndex) = _pair.getReserves();
115+
116+
Observation memory lObs = _oracleCaller.observation(_pair, lIndex);
117+
assertEq(lNewReserve0, 110e18);
118+
assertApproxEqRel(
119+
LogCompression.fromLowResLog(lObs.logAccLiquidity / int56(int256(lJumpAhead))),
120+
Constants.INITIAL_MINT_AMOUNT,
121+
0.0001e18
122+
);
123+
}
124+
125+
function testUpdateOracle_LatestTimestampWritten(uint256 aJumpAhead) external allPairs {
126+
// assume
127+
uint256 lJumpAhead = bound(aJumpAhead, 10, type(uint16).max);
128+
129+
// arrange
130+
uint256 lStartingTimestamp = block.timestamp;
131+
_stepTime(lJumpAhead);
132+
133+
// act
134+
_tokenA.mint(address(_pair), 5e18);
135+
_pair.swap(5e18, true, address(this), "");
136+
137+
// assert
138+
(,,, uint16 lIndex) = _pair.getReserves();
139+
Observation memory lObs = _oracleCaller.observation(_pair, lIndex);
140+
assertEq(lObs.timestamp, lStartingTimestamp + lJumpAhead);
141+
}
142+
125143
function testOracle_CompareLiquidityTwoCurves_Balanced(uint32 aNewStartTime)
126144
external
127145
randomizeStartTime(aNewStartTime)

test/unit/StablePair.t.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ contract StablePairTest is BaseTest {
415415
(uint256 lExpectedPlatformFee, uint256 lGrowthInLiq) =
416416
_calcExpectedPlatformFee(lPlatformFee, lPair, lReserve0, lReserve1, lTotalSupply, lOldLiq);
417417
assertEq(lPair.balanceOf(_platformFeeTo), lExpectedPlatformFee);
418-
if (aPlatformFee > 0) {
418+
if (lPlatformFee > 0) {
419419
assertGt(lPair.balanceOf(_platformFeeTo), 0);
420420
}
421421
assertApproxEqRel(

test/unit/gas/ConstantProductPair.gas.sol

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ contract ConstantProductPairGas is BaseTest {
3737
_oraclePair.burn(address(this));
3838

3939
// This pair will let a user swap without writing the oracle.
40+
// solhint-disable-next-line reentrancy
4041
_simplePair = ConstantProductPair(_factory.createPair(IERC20(address(_tokenA)), IERC20(address(_tokenC)), 0));
4142
_tokenA.mint(address(_simplePair), 100e18);
4243
_tokenC.mint(address(_simplePair), 100e18);

0 commit comments

Comments
 (0)