Skip to content

Commit 1fc085b

Browse files
thomas-waitethomas-waite
committed
refactor: remove some config params, add test
1 parent 17cf5d1 commit 1fc085b

File tree

5 files changed

+46
-75
lines changed

5 files changed

+46
-75
lines changed

‎contracts/oracle/uniswap/UniswapV3OracleWrapper.sol‎

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -35,20 +35,16 @@ contract UniswapV3OracleWrapper is IOracle, CoreRef {
3535

3636
/// @notice Mean Ethereum block time. Used in estimating the number of observations
3737
/// required on the Uniswap pool to support a particular TWAP period.
38-
uint32 private constant meanBlockTime = 13 seconds;
38+
uint32 public constant meanBlockTime = 13 seconds;
3939

4040
/// @notice Type for the oracle configuration parameters
4141
/// @param twapPeriod Time period of which the time weighted average price is calculated
42-
/// @param minTwapPeriod Safety parameter, minimum time period for which twap can be determined
43-
/// @param maxTwapPeriod Safety parameter, maximum time period for which twap can be determined
4442
/// @param minPoolLiquidity Safety parameter, minimum liquidity that must be present in the pool
4543
/// for the oracle reading to be permitted
4644
/// @param uniswapPool The Uniswap pool on which the oracle is based
4745
/// @param precision Number of decimal places to which the oracle price should be reported
4846
struct OracleConfig {
4947
uint32 twapPeriod;
50-
uint32 minTwapPeriod;
51-
uint32 maxTwapPeriod;
5248
uint128 minPoolLiquidity;
5349
address uniswapPool;
5450
uint256 precision;
@@ -79,7 +75,7 @@ contract UniswapV3OracleWrapper is IOracle, CoreRef {
7975

8076
validatePoolLiquidity(_oracleConfig.uniswapPool, _oracleConfig.minPoolLiquidity);
8177
validateTokensInPool(_oracleConfig.uniswapPool, _inputToken, _outputToken);
82-
validateTwapPeriod(_oracleConfig.twapPeriod, _oracleConfig.minTwapPeriod, _oracleConfig.maxTwapPeriod);
78+
validateTwapPeriod(_oracleConfig.twapPeriod);
8379

8480
pool = _oracleConfig.uniswapPool;
8581
inputToken = _inputToken;
@@ -157,22 +153,14 @@ contract UniswapV3OracleWrapper is IOracle, CoreRef {
157153

158154
/// @notice Validate that the TWAP period is within appropriate bounds
159155
/// @param _twapPeriod Time period over which the mean price is calculated
160-
/// @param _minTwapPeriod Minimum time period for which TWAP can be determined
161-
/// @param _maxTwapPeriod Maximum time period for which TWAP can be determined
162-
function validateTwapPeriod(uint32 _twapPeriod, uint32 _minTwapPeriod, uint32 _maxTwapPeriod) internal pure {
156+
function validateTwapPeriod(uint32 _twapPeriod) internal pure {
163157
require(_twapPeriod > 0, "Twap period must be greater than 0");
164-
require(
165-
_twapPeriod >= _minTwapPeriod && _twapPeriod <= _maxTwapPeriod,
166-
"TWAP period out of bounds"
167-
);
168158
}
169159

170160
/// @notice Increase pool observation cardinality to support requested TWAP period
171161
function addSupportForPool(address _pool, uint32 _twapPeriod, uint32 _meanBlockTime) internal {
172-
uint16 requiredCardinality = uint16(
173-
uint256(_twapPeriod).toUint16() / uint256(_meanBlockTime).toUint16()
174-
) + uint16(10); // Add additional number of slots to ensure available
175-
162+
uint16 requiredCardinality = uint16(_twapPeriod / _meanBlockTime) + uint16(10); // Add additional number of slots to ensure available
163+
176164
IUniswapV3Pool(_pool).increaseObservationCardinalityNext(requiredCardinality);
177165
emit AddPoolSupport(_pool, _twapPeriod, requiredCardinality);
178166
}
@@ -192,7 +180,7 @@ contract UniswapV3OracleWrapper is IOracle, CoreRef {
192180
/// @notice Change the time period over which the TWAP price is calculated
193181
/// @param _twapPeriod Period of time over which time weighted average price is calculated
194182
function setTwapPeriod(uint32 _twapPeriod) external onlyGuardianOrGovernor {
195-
validateTwapPeriod(_twapPeriod, oracleConfig.minTwapPeriod, oracleConfig.maxTwapPeriod);
183+
validateTwapPeriod(_twapPeriod);
196184

197185
uint32 oldTwapPeriod = oracleConfig.twapPeriod;
198186
oracleConfig.twapPeriod = _twapPeriod;

‎contracts/test/integration/backupOracles.sol‎

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ pragma solidity ^0.8.4;
33

44
import {IUniswapV3Pool} from "@uniswap/v3-core/contracts/interfaces/IUniswapV3Pool.sol";
55
import {IUniswapWrapper} from "../../oracle/uniswap/IUniswapWrapper.sol";
6-
import {ICore} from "../../core/ICore.sol";
76
import {Decimal} from "../../oracle/IOracle.sol";
7+
import {ICore} from "../../core/ICore.sol";
88

99
import {UniswapV3OracleWrapper} from "../../oracle/uniswap/UniswapV3OracleWrapper.sol";
1010

@@ -56,8 +56,6 @@ contract UniswapV3OracleIntegrationTest is DSTest, StdLib {
5656
UniswapV3OracleWrapper.OracleConfig memory daiUsdOracleConfig = UniswapV3OracleWrapper.OracleConfig({
5757
twapPeriod: twapPeriod,
5858
uniswapPool: address(daiUsdcPool),
59-
minTwapPeriod: 0,
60-
maxTwapPeriod: 50000,
6159
minPoolLiquidity: daiUsdcPool.liquidity(),
6260
precision: precision
6361
});
@@ -74,8 +72,6 @@ contract UniswapV3OracleIntegrationTest is DSTest, StdLib {
7472
UniswapV3OracleWrapper.OracleConfig memory usdcEthOracleConfig = UniswapV3OracleWrapper.OracleConfig({
7573
twapPeriod: twapPeriod,
7674
uniswapPool: address(usdcEthPool),
77-
minTwapPeriod: 0,
78-
maxTwapPeriod: 50000,
7975
minPoolLiquidity: usdcEthPool.liquidity(),
8076
precision: precision
8177
});
@@ -93,8 +89,6 @@ contract UniswapV3OracleIntegrationTest is DSTest, StdLib {
9389
UniswapV3OracleWrapper.OracleConfig memory wbtcUsdcOracleConfig = UniswapV3OracleWrapper.OracleConfig({
9490
twapPeriod: twapPeriod,
9591
uniswapPool: address(wbtcUsdcPool),
96-
minTwapPeriod: 0,
97-
maxTwapPeriod: 50000,
9892
minPoolLiquidity: wbtcUsdcPool.liquidity(),
9993
precision: precision
10094
});
@@ -111,8 +105,6 @@ contract UniswapV3OracleIntegrationTest is DSTest, StdLib {
111105
UniswapV3OracleWrapper.OracleConfig memory usdtAvinocOracleConfig = UniswapV3OracleWrapper.OracleConfig({
112106
twapPeriod: twapPeriod,
113107
uniswapPool: address(usdtAvinocPool),
114-
minTwapPeriod: 0,
115-
maxTwapPeriod: 50000,
116108
minPoolLiquidity: usdtAvinocPool.liquidity(),
117109
precision: precision
118110
});
@@ -133,8 +125,6 @@ contract UniswapV3OracleIntegrationTest is DSTest, StdLib {
133125
UniswapV3OracleWrapper.OracleConfig memory daiUsdOracleConfig = UniswapV3OracleWrapper.OracleConfig({
134126
twapPeriod: twapPeriod,
135127
uniswapPool: address(daiUsdcPool),
136-
minTwapPeriod: 0,
137-
maxTwapPeriod: 50000,
138128
minPoolLiquidity: daiUsdcPool.liquidity(),
139129
precision: precision
140130
});
@@ -152,6 +142,31 @@ contract UniswapV3OracleIntegrationTest is DSTest, StdLib {
152142
);
153143
}
154144

145+
function testAddPoolSupport() public {
146+
uint32 highTwapPeriod = 4000;
147+
148+
UniswapV3OracleWrapper.OracleConfig memory highTwapPeriodConfig = UniswapV3OracleWrapper.OracleConfig({
149+
twapPeriod: highTwapPeriod,
150+
uniswapPool: address(daiUsdcPool),
151+
minPoolLiquidity: daiUsdcPool.liquidity(),
152+
precision: precision
153+
});
154+
155+
UniswapV3OracleWrapper highTwapOracle = new UniswapV3OracleWrapper(
156+
address(core),
157+
dai,
158+
usdc,
159+
uniswapMathWrapper,
160+
highTwapPeriodConfig
161+
);
162+
163+
( , , , , uint16 observationCardinalityNext, , ) = daiUsdcPool.slot0();
164+
165+
uint32 meanBlockTime = highTwapOracle.meanBlockTime();
166+
uint16 expectedCardinalityForMaxTwapPeriod = uint16(highTwapPeriod / meanBlockTime) + uint16(10);
167+
assertEq(observationCardinalityNext, expectedCardinalityForMaxTwapPeriod);
168+
}
169+
155170
// Note: These price tests themselves are not strict as price changes
156171
// Exact price is logged and checked manually against spot prices on Uniswap
157172
function testPriceForDaiUsdc() public {

‎contracts/test/unit/oracle/UniswapV3OracleWrapper.t.sol‎

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ contract UniswapV3OracleTest is DSTest, StdLib {
2222
address private uniswapMathWrapper;
2323
ICore core;
2424

25-
uint32 private twapPeriod = 61; // Min TWAP period is 60 seconds
25+
uint32 private twapPeriod = 61;
2626
uint256 private precision = 10**18;
2727

2828
UniswapV3OracleWrapper private oracle;
@@ -42,8 +42,6 @@ contract UniswapV3OracleTest is DSTest, StdLib {
4242
UniswapV3OracleWrapper.OracleConfig memory oracleConfig = UniswapV3OracleWrapper.OracleConfig({
4343
twapPeriod: twapPeriod,
4444
uniswapPool: address(mockUniswapPool),
45-
minTwapPeriod: 0,
46-
maxTwapPeriod: 50000,
4745
minPoolLiquidity: mockUniswapPool.liquidity(),
4846
precision: precision
4947
});
@@ -73,8 +71,6 @@ contract UniswapV3OracleTest is DSTest, StdLib {
7371
UniswapV3OracleWrapper.OracleConfig memory zeroTwapConfig = UniswapV3OracleWrapper.OracleConfig({
7472
twapPeriod: 0,
7573
uniswapPool: address(mockUniswapPool),
76-
minTwapPeriod: 0,
77-
maxTwapPeriod: 50000,
7874
minPoolLiquidity: mockUniswapPool.liquidity(),
7975
precision: precision
8076
});
@@ -95,8 +91,6 @@ contract UniswapV3OracleTest is DSTest, StdLib {
9591
function testMinLiquidity() public {
9692
UniswapV3OracleWrapper.OracleConfig memory highMinLiquidityConfig = UniswapV3OracleWrapper.OracleConfig({
9793
twapPeriod: twapPeriod,
98-
minTwapPeriod: 0,
99-
maxTwapPeriod: 50000,
10094
minPoolLiquidity: mockUniswapPool.liquidity() + 1,
10195
uniswapPool: address(mockUniswapPool),
10296
precision: precision
@@ -113,17 +107,6 @@ contract UniswapV3OracleTest is DSTest, StdLib {
113107
highMinLiquidityConfig
114108
);
115109
}
116-
117-
function testTwapPeriodBounds() public {
118-
(, , uint32 maxTwapPeriod, , , ) = oracle.oracleConfig();
119-
uint32 newTwapPeriod = maxTwapPeriod + 1;
120-
121-
vm.prank(addresses.governorAddress);
122-
vm.expectRevert(
123-
bytes("TWAP period out of bounds")
124-
);
125-
oracle.setTwapPeriod(newTwapPeriod);
126-
}
127110

128111
function testUpdateNoop() public {
129112
assertFalse(oracle.isOutdated());

‎protocol-configuration/backupOracleConfig.ts‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,26 @@
11
import { ethers } from 'hardhat';
2+
import { BigNumberish } from 'ethers';
23

34
const toBN = ethers.BigNumber.from;
45
const BNe18 = (x: any) => ethers.constants.WeiPerEther.mul(toBN(x));
56

67
export const daiUsdcBackupOracleConfig: BackupOracleConfig = {
7-
twapPeriod: 600, // 10 minutes
8-
minTwapPeriod: 600, // 10 minutes
9-
maxTwapPeriod: 86400, // 1 day
8+
twapPeriod: 600, // 30 minutes
109
minPoolLiquidity: 50e6, // 50m
1110
uniswapPool: '0x5777d92f208679DB4b9778590Fa3CAB3aC9e2168',
1211
precision: BNe18(1)
1312
};
1413

1514
export const ethUsdcBackupOracleConfig: BackupOracleConfig = {
16-
twapPeriod: 1800, // 30 minutes
17-
minTwapPeriod: 1800, // 30 minutes
18-
maxTwapPeriod: 172800, // 2 days
15+
twapPeriod: 600, // 10 minutes
1916
minPoolLiquidity: 50e6, // 50m
2017
uniswapPool: '0x8ad599c3A0ff1De082011EFDDc58f1908eb6e6D8',
2118
precision: BNe18(1)
2219
};
2320

2421
export type BackupOracleConfig = {
2522
twapPeriod: number;
26-
minTwapPeriod: number;
27-
maxTwapPeriod: number;
2823
minPoolLiquidity: number;
2924
uniswapPool: string;
30-
precision: ethers.BigNumber;
25+
precision: BigNumberish;
3126
};

‎test/integration/tests/backupOracles.ts‎

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,14 @@ import CBN from 'chai-bn';
55
import { solidity } from 'ethereum-waffle';
66
import { BigNumber } from 'ethers';
77
import { ethers } from 'hardhat';
8-
import { NamedAddresses, NamedContracts } from '@custom-types/types';
9-
import { expectApprox, expectRevert, getImpersonatedSigner, resetFork } from '@test/helpers';
8+
import { NamedContracts } from '@custom-types/types';
9+
import { expectApprox, getImpersonatedSigner, resetFork } from '@test/helpers';
1010
import proposals from '@test/integration/proposals_config';
1111
import { TestEndtoEndCoordinator } from '../setup';
1212

13+
const toBN = ethers.BigNumber.from;
14+
const BNe18 = (x: any) => ethers.constants.WeiPerEther.mul(toBN(x));
15+
1316
describe('Backup Oracles', function () {
1417
let contracts: NamedContracts;
1518
let deployAddress: SignerWithAddress;
@@ -50,11 +53,7 @@ describe('Backup Oracles', function () {
5053

5154
// Add backup oracle to DAI PSM
5255
daiPSM = contracts.daiFixedPricePSM as PegStabilityModule;
53-
console.log('dai psm address: ', daiPSM.address);
54-
5556
daiUsdcBackupOracle = contracts.daiUsdcTwapOracle as UniswapV3OracleWrapper;
56-
57-
console.log('dai oracle address: ', daiUsdcBackupOracle.address);
5857
await daiPSM.connect(daoSigner).setBackupOracle(daiUsdcBackupOracle.address);
5958

6059
// Add backup oracle to ETH PSM
@@ -86,26 +85,17 @@ describe('Backup Oracles', function () {
8685
expect(ethBackupOracle).to.equal(ethUsdcBackupOracle.address);
8786
});
8887

89-
it.only('should read reasonable Uniswap V3 TWAP oracle price', async () => {
90-
// Can call the oracle directly to query price
88+
it('should read reasonable Uniswap V3 TWAP oracle price', async () => {
9189
const [daiPrice, isValid] = await daiUsdcBackupOracle.read();
92-
93-
console.log({ isValid });
94-
console.log('daiPrice: ', daiPrice.toString());
95-
9690
expect(isValid).to.be.true;
97-
expect(daiPrice.toString()).to.be.equal('1000000000000000000');
91+
expectApprox(daiPrice.toString(), BNe18(1).toString());
9892
});
9993

100-
it.only('should read Uniswap V3 TWAP oracle price comparable to primary oracle price', async () => {
94+
it('should read Uniswap V3 TWAP oracle price comparable to primary oracle price', async () => {
10195
const primaryChainlinkOracle = contracts.chainlinkDaiUsdOracleWrapper;
102-
10396
const primaryOraclePrice = await primaryChainlinkOracle.read();
10497
const backupOraclePrice = await daiUsdcBackupOracle.read();
10598

106-
console.log('primary oracle price: ', primaryOraclePrice.toString());
107-
console.log('backup oracle price: ', backupOraclePrice.toString());
108-
109-
expect(primaryOraclePrice.toString()).to.equal(backupOraclePrice.toString());
99+
expectApprox(primaryOraclePrice.toString(), backupOraclePrice.toString());
110100
});
111101
});

0 commit comments

Comments
 (0)