Skip to content

Commit

Permalink
chore: running forge update and fixing OZ related methods (#98)
Browse files Browse the repository at this point in the history
As a second step from #97 in order to improve contracts inheritability
from an external repo, this PR aims to update OZ to the latest version
(5.0.2), which brought incompatibilities with repos using this version
(as Math rounding enum was modified, for example).

**Proposed changes**:
- run `forge update` to fetch latest packages (OZ and UniV2)
- replaced `safeApprove` with `forceApprove` (note: different behaviour)
- replaced `Math.Rounding.Up/Down` for `Ceil/Floor`

**Pending**:
- [x] `testDeploymentRevertsIfApprovalReverts` test is failing with a
different expectation than written on the test, this must come from the
different behaviour that `forceApprove` has over `safeApprove`, this
test expectation should be fixed before merging
  • Loading branch information
wei3erHase authored Jul 11, 2024
1 parent e94c9d3 commit 6566128
Show file tree
Hide file tree
Showing 11 changed files with 26 additions and 16 deletions.
1 change: 1 addition & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@
[submodule "lib/openzeppelin"]
path = lib/openzeppelin
url = https://github.com/OpenZeppelin/openzeppelin-contracts
branch = release-v5.0
2 changes: 1 addition & 1 deletion lib/openzeppelin
Submodule openzeppelin updated 575 files
2 changes: 1 addition & 1 deletion lib/uniswap-v2-core
2 changes: 1 addition & 1 deletion src/ConstantProduct.sol
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,6 @@ contract ConstantProduct is IERC1271 {
* @param spender The address that can transfer on behalf of this contract.
*/
function approveUnlimited(IERC20 token, address spender) internal {
OZIERC20(address(token)).safeApprove(spender, type(uint256).max);
OZIERC20(address(token)).forceApprove(spender, type(uint256).max);
}
}
4 changes: 2 additions & 2 deletions src/libraries/GetTradeableOrder.sol
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,12 @@ library GetTradeableOrder {
sellToken = params.token0;
buyToken = params.token1;
sellAmount = selfReserve0 / 2 - Math.ceilDiv(selfReserve1TimesPriceNumerator, 2 * params.priceDenominator);
buyAmount = Math.mulDiv(sellAmount, selfReserve1, selfReserve0 - sellAmount, Math.Rounding.Up);
buyAmount = Math.mulDiv(sellAmount, selfReserve1, selfReserve0 - sellAmount, Math.Rounding.Ceil);
} else {
sellToken = params.token1;
buyToken = params.token0;
sellAmount = selfReserve1 / 2 - Math.ceilDiv(selfReserve0TimesPriceDenominator, 2 * params.priceNumerator);
buyAmount = Math.mulDiv(sellAmount, selfReserve0, selfReserve1 - sellAmount, Math.Rounding.Up);
buyAmount = Math.mulDiv(sellAmount, selfReserve0, selfReserve1 - sellAmount, Math.Rounding.Ceil);
}

order_ = GPv2Order.Data(
Expand Down
4 changes: 2 additions & 2 deletions src/oracles/BalancerWeightedPoolPriceOracle.sol
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ contract BalancerWeightedPoolPriceOracle is IPriceOracle {
} else {
(max, min) = (num2, num1);
}
uint256 logMax = Math.log2(max, Math.Rounding.Up);
uint256 logMin = Math.log2(min, Math.Rounding.Down);
uint256 logMax = Math.log2(max, Math.Rounding.Ceil);
uint256 logMin = Math.log2(min, Math.Rounding.Floor);

if ((logMax <= 128) || (logMin <= TOLERANCE)) {
return (num1, num2);
Expand Down
2 changes: 1 addition & 1 deletion test/ConstantProduct/ConstantProductTestHarness.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ pragma solidity ^0.8.24;

import {BaseComposableCoWTest} from "lib/composable-cow/test/ComposableCoW.base.t.sol";

import {ConstantProduct, GPv2Order, IERC20} from "src/ConstantProduct.sol";
import {ConstantProduct, GPv2Order, IERC20, SafeERC20} from "src/ConstantProduct.sol";
import {UniswapV2PriceOracle, IUniswapV2Pair} from "src/oracles/UniswapV2PriceOracle.sol";
import {ISettlement} from "src/interfaces/ISettlement.sol";

Expand Down
7 changes: 4 additions & 3 deletions test/ConstantProduct/DeploymentParamsTest.sol
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-3.0
pragma solidity ^0.8.24;

import {ConstantProductTestHarness, ConstantProduct, IERC20} from "./ConstantProductTestHarness.sol";
import {ConstantProductTestHarness, ConstantProduct, IERC20, SafeERC20} from "./ConstantProductTestHarness.sol";

abstract contract DeploymentParamsTest is ConstantProductTestHarness {
function testSetsDeploymentParameters() public {
Expand Down Expand Up @@ -40,7 +40,8 @@ abstract contract DeploymentParamsTest is ConstantProductTestHarness {
mockZeroAllowance(token, expectedDeploymentAddress(), spenderBadApproval);
vm.mockCallRevert(
address(token),
abi.encodeCall(IERC20.approve, (spenderBadApproval, type(uint256).max)),
// Notice: we intentionally don't match the approved amount.
abi.encodeWithSelector(IERC20.approve.selector, spenderBadApproval),
"mock revert on approval"
);
vm.mockCallRevert(address(token), hex"", abi.encode("Unexpected call to token contract"));
Expand Down Expand Up @@ -93,7 +94,7 @@ abstract contract DeploymentParamsTest is ConstantProductTestHarness {
);

vm.prank(defaultDeployer());
vm.expectRevert("SafeERC20: ERC20 operation did not succeed");
vm.expectRevert(abi.encodeWithSelector(SafeERC20.SafeERC20FailedOperation.selector, falseOnApproval));
new ConstantProduct(solutionSettler, falseOnApproval, regular);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
// SPDX-License-Identifier: GPL-3.0
pragma solidity ^0.8.24;

import {ConstantProductFactory, ConstantProduct, GPv2Order, ISettlement, IERC20} from "src/ConstantProductFactory.sol";
import {
ConstantProductFactory,
ConstantProduct,
GPv2Order,
ISettlement,
IERC20,
SafeERC20
} from "src/ConstantProductFactory.sol";

import {ConstantProductTestHarness} from "test/ConstantProduct/ConstantProductTestHarness.sol";

Expand Down
4 changes: 2 additions & 2 deletions test/ConstantProductFactory/Deposit.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ pragma solidity ^0.8.24;

import {IERC20} from "src/ConstantProductFactory.sol";

import {ConstantProductFactoryTestHarness} from "./ConstantProductFactoryTestHarness.sol";
import {ConstantProductFactoryTestHarness, SafeERC20} from "./ConstantProductFactoryTestHarness.sol";

abstract contract Deposit is ConstantProductFactoryTestHarness {
function testAnyoneCanDeposit() public {
Expand Down Expand Up @@ -54,7 +54,7 @@ abstract contract Deposit is ConstantProductFactoryTestHarness {
abi.encode(false)
);

vm.expectRevert("SafeERC20: ERC20 operation did not succeed");
vm.expectRevert(abi.encodeWithSelector(SafeERC20.SafeERC20FailedOperation.selector, token1));
constantProductFactory.deposit(constantProduct, amount0, amount1);
}

Expand Down
5 changes: 3 additions & 2 deletions test/ConstantProductFactory/Withdraw.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import {ConstantProductFactory, IERC20} from "src/ConstantProductFactory.sol";

import {
EditableOwnerConstantProductFactory,
ConstantProductFactoryTestHarness
ConstantProductFactoryTestHarness,
SafeERC20
} from "./ConstantProductFactoryTestHarness.sol";

abstract contract Withdraw is ConstantProductFactoryTestHarness {
Expand Down Expand Up @@ -64,7 +65,7 @@ abstract contract Withdraw is ConstantProductFactoryTestHarness {
token1, abi.encodeCall(IERC20.transferFrom, (address(constantProduct), owner, amount1)), abi.encode(false)
);

vm.expectRevert("SafeERC20: ERC20 operation did not succeed");
vm.expectRevert(abi.encodeWithSelector(SafeERC20.SafeERC20FailedOperation.selector, token1));
vm.prank(owner);
factory.withdraw(constantProduct, amount0, amount1);
}
Expand Down

0 comments on commit 6566128

Please sign in to comment.