Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Audit Fixes: QS-11 #41

Merged
merged 3 commits into from
Nov 29, 2023
Merged

Audit Fixes: QS-11 #41

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c1eea16
added maperiod validations
TechnoGeek01 Nov 25, 2023
1715c20
formatting fix
TechnoGeek01 Nov 25, 2023
975318d
added internal constants and solidity time units instead of direct nu…
TechnoGeek01 Nov 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions contracts/oracle/BaseUniOracle.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ interface IMasterPriceOracle {
abstract contract BaseUniOracle is Ownable {
address public constant UNISWAP_FACTORY = 0x1F98431c8aD98523631AE4a59f267346ea31F984;
address public constant UNISWAP_UTILS = 0xd2Aa19D3B7f8cdb1ea5B782c5647542055af415e;
uint256 internal constant MA_PERIOD_LOWER_BOUND = 10 minutes;
uint256 internal constant MA_PERIOD_UPPER_BOUND = 2 hours;

address public masterOracle; // Address of the master price oracle
address public pool; // Address of the uniswap pool for the token and quoteToken
Expand All @@ -39,6 +41,7 @@ abstract contract BaseUniOracle is Ownable {
error QuoteTokenFeedMissing();
error FeedUnavailable();
error InvalidAddress();
error InvalidMaPeriod();

/// @notice A function to get price
/// @return (uint256, uint256) Returns price and price precision
Expand Down Expand Up @@ -74,6 +77,11 @@ abstract contract BaseUniOracle is Ownable {
revert QuoteTokenFeedMissing();
}

// Validate if maPeriod is between 10 minutes and 2 hours
if (_maPeriod < MA_PERIOD_LOWER_BOUND || _maPeriod > MA_PERIOD_UPPER_BOUND) {
revert InvalidMaPeriod();
}

pool = uniOraclePool;
quoteToken = _quoteToken;
quoteTokenPrecision = uint128(10) ** ERC20(quoteToken).decimals();
Expand Down
4 changes: 1 addition & 3 deletions test/oracle/ChainlinkOracle.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ contract ChainlinkOracleTest is BaseTest {
super.setUp();
setArbitrumFork();
vm.prank(USDS_OWNER);
chainlinkOracle = new ChainlinkOracle(
new ChainlinkOracle.SetupTokenData[](0)
);
chainlinkOracle = new ChainlinkOracle(new ChainlinkOracle.SetupTokenData[](0));
}

function _getTokenData() internal pure returns (ChainlinkOracle.SetupTokenData[] memory) {
Expand Down
18 changes: 11 additions & 7 deletions test/oracle/SPAOracle.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,7 @@ contract SPAOracleTest is BaseUniOracleTest {
function setUp() public override {
super.setUp();
vm.startPrank(USDS_OWNER);
spaOracle = new SPAOracle(
masterOracle,
USDCe,
FEE_TIER,
MA_PERIOD,
WEIGHT_DIA
);
spaOracle = new SPAOracle(masterOracle, USDCe, FEE_TIER, MA_PERIOD, WEIGHT_DIA);
spaOracle.updateDIAParams(WEIGHT_DIA, type(uint128).max);
vm.stopPrank();
}
Expand All @@ -98,6 +92,8 @@ contract Test_FetchPrice is SPAOracleTest {
}

contract Test_setUniMAPriceData is SPAOracleTest {
error InvalidMaPeriod();

function test_revertsWhen_notOwner() public {
vm.expectRevert("Ownable: caller is not the owner");
spaOracle.setUniMAPriceData(SPA, USDCe, 10000, 600);
Expand All @@ -113,6 +109,14 @@ contract Test_setUniMAPriceData is SPAOracleTest {
emit UniMAPriceDataChanged(USDCe, 10000, 700);
spaOracle.setUniMAPriceData(SPA, USDCe, 10000, 700);
}

function test_revertsWhen_invalidMaPeriod() public useKnownActor(USDS_OWNER) {
vm.expectRevert(abi.encodeWithSelector(InvalidMaPeriod.selector));
spaOracle.setUniMAPriceData(SPA, USDCe, 10000, 599);

vm.expectRevert(abi.encodeWithSelector(InvalidMaPeriod.selector));
spaOracle.setUniMAPriceData(SPA, USDCe, 10000, 7201);
}
}

contract Test_updateMasterOracle is SPAOracleTest {
Expand Down
4 changes: 1 addition & 3 deletions test/utils/DeploymentSetup.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,7 @@ abstract contract PreMigrationSetup is Setup {
CollateralManager collateralManager = new CollateralManager(VAULT);

ORACLE = address(new MasterPriceOracle());
FeeCalculator feeCalculator = new FeeCalculator(
address(collateralManager)
);
FeeCalculator feeCalculator = new FeeCalculator(address(collateralManager));
FEE_CALCULATOR = address(feeCalculator);

COLLATERAL_MANAGER = address(collateralManager);
Expand Down
6 changes: 1 addition & 5 deletions test/utils/UpgradeUtil.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,7 @@ contract UpgradeUtil {
}

function deployErc1967Proxy(address impl) public returns (address) {
TransparentUpgradeableProxy proxy = new TransparentUpgradeableProxy(
impl,
address(proxyAdmin),
""
);
TransparentUpgradeableProxy proxy = new TransparentUpgradeableProxy(impl, address(proxyAdmin), "");
return address(proxy);
}
}