Skip to content

Commit

Permalink
fix: impl fix for attempting to use a composite route as an intermedi…
Browse files Browse the repository at this point in the history
…ate route
  • Loading branch information
xenide committed Dec 11, 2024
1 parent df7f0dd commit 6ce37dd
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 1 deletion.
3 changes: 3 additions & 0 deletions src/ReservoirPriceOracle.sol
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar

// Calculate the storage slot for this intermediate segment and read it to see if there is an existing
// route. If there isn't an existing route, we create one as well.
// Will revert if it attempts to use an intermediate route that is not a simple route.
function _checkAndPopulateIntermediateRoute(address aTokenA, address aTokenB, uint64 aBpMaxReward) private {
(address lToken0, address lToken1) = Utils.sortTokens(aTokenA, aTokenB);

Expand All @@ -310,6 +311,8 @@ contract ReservoirPriceOracle is IPriceOracle, Owned(msg.sender), ReentrancyGuar
uint64[] memory lRewardThreshold = new uint64[](1);
lRewardThreshold[0] = aBpMaxReward;
setRoute(lToken0, lToken1, lIntermediateRoute, lRewardThreshold);
} else {
require(lData.isSimplePrice(), OracleErrors.AttemptToUseCompositeRouteAsIntermediateRoute());
}
}

Expand Down
1 change: 1 addition & 0 deletions src/libraries/OracleErrors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ pragma solidity ^0.8.0;
/// @dev Collection of all oracle related errors.
library OracleErrors {
// config errors
error AttemptToUseCompositeRouteAsIntermediateRoute();
error IncorrectTokensDesignatePair();
error InvalidRewardThreshold();
error InvalidRewardThresholdsLength();
Expand Down
84 changes: 83 additions & 1 deletion test/unit/ReservoirPriceOracle.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ contract ReservoirPriceOracleTest is BaseTest {
_oracle.setRoute(address(_tokenA), address(_tokenB), lRoute, lRewardThreshold);
}

function testName() external {
function testName() external view {
// act & assert
assertEq(_oracle.name(), "RESERVOIR PRICE ORACLE");
}
Expand Down Expand Up @@ -809,6 +809,58 @@ contract ReservoirPriceOracleTest is BaseTest {
assertEq(_oracle.route(lIntermediate2, lEnd), lIntermediateRoute3);
}

function testSetRoute_ReplaceExistingCompositeWithSimple() external {
// arrange - set route A-B-C
address lStart = address(_tokenA);
address lIntermediate = address(_tokenB);
address lEnd = address(_tokenC);
address[] memory lRoute = new address[](3);
lRoute[0] = lStart;
lRoute[1] = lIntermediate;
lRoute[2] = lEnd;
uint64[] memory lRewardThreshold = new uint64[](2);
lRewardThreshold[0] = lRewardThreshold[1] = Constants.WAD;
_oracle.setRoute(lStart, lEnd, lRoute, lRewardThreshold);

// act - overwrite by A-C-D
_oracle.clearRoute(lStart, lEnd);
lRoute[1] = address(_tokenC);
lRoute[2] = address(_tokenD);

_oracle.setRoute(address(_tokenA), address(_tokenD), lRoute, lRewardThreshold);

// assert
address[] memory lRouteAC = new address[](2);
lRouteAC[0] = address(_tokenA);
lRouteAC[1] = address(_tokenC);

// intermediate routes are all intact
assertEq(_oracle.route(address(_tokenA), address(_tokenC)), lRouteAC);
assertEq(_oracle.route(address(_tokenA), address(_tokenD)), lRoute);
assertEq(_oracle.route(address(_tokenA), address(_tokenB)).length, 2);
assertEq(_oracle.route(address(_tokenB), address(_tokenC)).length, 2);
assertEq(_oracle.route(address(_tokenC), address(_tokenD)).length, 2);
}

function testSetRoute_ReplaceExistingSimpleWithComposite() external {
// arrange
address lStart = address(_tokenA);
address lIntermediate = address(_tokenC);
address lEnd = address(_tokenB);
address[] memory lRoute = new address[](3);
lRoute[0] = lStart;
lRoute[1] = lIntermediate;
lRoute[2] = lEnd;
uint64[] memory lRewardThreshold = new uint64[](2);
lRewardThreshold[0] = lRewardThreshold[1] = Constants.WAD;

// act - replace existing A-B with A-C-B
_oracle.setRoute(lStart, lEnd, lRoute, lRewardThreshold);

// assert
assertEq(_oracle.route(address(_tokenA), address(_tokenB)).length, 3);
}

function testClearRoute() external {
// arrange
address lToken0 = address(_tokenB);
Expand Down Expand Up @@ -1146,6 +1198,36 @@ contract ReservoirPriceOracleTest is BaseTest {
);
}

// modified from test case supplied by Cantina auditors
function testSetRoute_OverwriteExisting_UsingCompositeAsIntermediate() external {
// arrange - set pair A/C with route [A->B->C]
address lStart = address(_tokenA);
address lIntermediate1 = address(_tokenB);
address lEnd = address(_tokenC);
address[] memory lRoute = new address[](3);
lRoute[0] = lStart;
lRoute[1] = lIntermediate1;
lRoute[2] = lEnd;
uint64[] memory lRewardThreshold = new uint64[](2);
lRewardThreshold[0] = lRewardThreshold[1] = Constants.WAD;
_oracle.setRoute(lStart, lEnd, lRoute, lRewardThreshold);

// Test if A/C slot contains 2-HOP route as expected
bytes32 AC_Slot = Utils.calculateSlot(address(_tokenA), address(_tokenC));
bytes32 AC_SlotValue = vm.load(address(_oracle), AC_Slot);
assertEq(AC_SlotValue.is2HopRoute(), true);

// act - Overwrite with pair A/D with route [A->C->D]
lEnd = address(_tokenD);
lRoute = new address[](3);
lRoute[0] = lStart;
lRoute[1] = address(_tokenC);
lRoute[2] = lEnd = address(_tokenD);

vm.expectRevert(OracleErrors.AttemptToUseCompositeRouteAsIntermediateRoute.selector);
_oracle.setRoute(lStart, lEnd, lRoute, lRewardThreshold);
}

function testUpdateRewardGasAmount_NotOwner() external {
// act & assert
vm.prank(address(123));
Expand Down

0 comments on commit 6ce37dd

Please sign in to comment.