Skip to content

Commit

Permalink
chore: upgrade openzeppelin lib to v5.0.2
Browse files Browse the repository at this point in the history
  • Loading branch information
alex0207s committed Jul 2, 2024
1 parent 703d6b9 commit 6b0dd6d
Show file tree
Hide file tree
Showing 12 changed files with 38 additions and 29 deletions.
4 changes: 2 additions & 2 deletions .gitmodules
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[submodule "lib/openzeppelin-contracts"]
path = lib/openzeppelin-contracts
url = https://github.com/openzeppelin/openzeppelin-contracts
branch = v4.9.2
url = https://github.com/OpenZeppelin/openzeppelin-contracts
branch = v5.0.2
[submodule "lib/forge-std"]
path = lib/forge-std
url = https://github.com/foundry-rs/forge-std
Expand Down
2 changes: 1 addition & 1 deletion contracts/AllowanceTarget.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ pragma solidity 0.8.26;

import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import { Pausable } from "@openzeppelin/contracts/security/Pausable.sol";
import { Pausable } from "@openzeppelin/contracts/utils/Pausable.sol";

import { Ownable } from "./abstracts/Ownable.sol";
import { IAllowanceTarget } from "./interfaces/IAllowanceTarget.sol";
Expand Down
2 changes: 1 addition & 1 deletion contracts/LimitOrderSwap.sol
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;

import { ReentrancyGuard } from "@openzeppelin/contracts/security/ReentrancyGuard.sol";
import { ReentrancyGuard } from "@openzeppelin/contracts/utils/ReentrancyGuard.sol";

import { TokenCollector } from "./abstracts/TokenCollector.sol";
import { Ownable } from "./abstracts/Ownable.sol";
Expand Down
2 changes: 1 addition & 1 deletion contracts/abstracts/AdminManagement.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ abstract contract AdminManagement is Ownable {
function approveTokens(address[] calldata tokens, address[] calldata spenders) external onlyOwner {
for (uint256 i = 0; i < tokens.length; ++i) {
for (uint256 j = 0; j < spenders.length; ++j) {
IERC20(tokens[i]).safeApprove(spenders[j], type(uint256).max);
IERC20(tokens[i]).forceApprove(spenders[j], type(uint256).max);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion contracts/libraries/SignatureValidator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ library SignatureValidator {
* @return True if the address recovered from the provided signature matches the input signer address.
*/
function validateSignature(address _signerAddress, bytes32 _hash, bytes memory _signature) internal view returns (bool) {
if (_signerAddress.isContract()) {
if (_signerAddress.code.length > 0) {
return ERC1271_MAGICVALUE == IERC1271Wallet(_signerAddress).isValidSignature(_hash, _signature);
} else {
return _signerAddress == ECDSA.recover(_hash, _signature);
Expand Down
2 changes: 1 addition & 1 deletion lib/openzeppelin-contracts
5 changes: 3 additions & 2 deletions test/AllowanceTarget.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ pragma solidity 0.8.26;

import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import { Pausable } from "@openzeppelin/contracts/utils/Pausable.sol";

import { IAllowanceTarget } from "contracts/interfaces/IAllowanceTarget.sol";
import { AllowanceTarget } from "contracts/AllowanceTarget.sol";
Expand Down Expand Up @@ -64,7 +65,7 @@ contract AllowanceTargetTest is BalanceUtil {

function testCannotSpendFromUserInsufficientBalanceWithReturnFalseToken() public {
uint256 userBalance = noRevertERC20.balanceOf(user);
vm.expectRevert("SafeERC20: ERC20 operation did not succeed");
vm.expectRevert(abi.encodeWithSelector(SafeERC20.SafeERC20FailedOperation.selector, address(noRevertERC20)));
vm.prank(authorized);
allowanceTarget.spendFromUserTo(user, address(noRevertERC20), recipient, userBalance + 1);
}
Expand All @@ -86,7 +87,7 @@ contract AllowanceTargetTest is BalanceUtil {
vm.prank(allowanceTargetOwner, allowanceTargetOwner);
allowanceTarget.pause();

vm.expectRevert("Pausable: paused");
vm.expectRevert(Pausable.EnforcedPause.selector);
allowanceTarget.spendFromUserTo(user, address(mockERC20), recipient, 1234);
}

Expand Down
5 changes: 3 additions & 2 deletions test/forkMainnet/LimitOrderSwap/Fill.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ pragma solidity 0.8.26;

import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import { Address } from "@openzeppelin/contracts/utils/Address.sol";

import { ILimitOrderSwap } from "contracts/interfaces/ILimitOrderSwap.sol";
import { Constant } from "contracts/libraries/Constant.sol";
Expand Down Expand Up @@ -473,10 +474,10 @@ contract FillTest is LimitOrderSwapTest {
mockStrategy.setOutputAmountAndRecipient(defaultOrder.takerTokenAmount - 1, payable(randomTaker));

vm.startPrank(randomTaker);
IERC20(defaultOrder.takerToken).safeApprove(address(limitOrderSwap), type(uint256).max);
IERC20(defaultOrder.takerToken).forceApprove(address(limitOrderSwap), type(uint256).max);

// the final step transferFrom will fail since taker doesn't have enough balance to fill
vm.expectRevert("SafeERC20: low-level call failed");
vm.expectRevert(Address.FailedInnerCall.selector);
limitOrderSwap.fillLimitOrder({
order: defaultOrder,
makerSignature: defaultMakerSig,
Expand Down
31 changes: 19 additions & 12 deletions test/forkMainnet/TokenCollector.t.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;

import { ERC20Permit } from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Permit.sol";
import { IERC20Errors } from "@openzeppelin/contracts/interfaces/draft-IERC6093.sol";

import { AllowanceTarget } from "contracts/AllowanceTarget.sol";
import { TokenCollector } from "contracts/abstracts/TokenCollector.sol";
import { IUniswapPermit2 } from "contracts/interfaces/IUniswapPermit2.sol";
Expand All @@ -19,6 +22,7 @@ contract Strategy is TokenCollector {
contract TestTokenCollector is Addresses, Permit2Helper {
uint256 otherPrivateKey = uint256(123);
uint256 userPrivateKey = uint256(1);
address other = vm.addr(otherPrivateKey);
address user = vm.addr(userPrivateKey);
address allowanceTargetOwner = makeAddr("allowanceTargetOwner");

Expand All @@ -34,7 +38,7 @@ contract TestTokenCollector is Addresses, Permit2Helper {
Strategy strategy = new Strategy(address(permit2), address(allowanceTarget));

function setUp() public {
token.mint(user, 10000 * 1e18);
token.mint(user, 10000 ether);

// get permit2 nonce and compose PermitSingle for AllowanceTransfer
uint256 expiration = block.timestamp + 1 days;
Expand All @@ -54,12 +58,12 @@ contract TestTokenCollector is Addresses, Permit2Helper {
function testCannotCollectByTokenApprovalWhenAllowanceIsNotEnough() public {
bytes memory data = abi.encodePacked(TokenCollector.Source.Token);

vm.expectRevert("ERC20: insufficient allowance");
vm.expectRevert(abi.encodeWithSelector(IERC20Errors.ERC20InsufficientAllowance.selector, address(strategy), 0, 1));
strategy.collect(address(token), user, address(this), 1, data);
}

function testCollectByTokenApproval() public {
uint256 amount = 100 * 1e18;
uint256 amount = 100 ether;

vm.prank(user);
token.approve(address(strategy), amount);
Expand All @@ -74,12 +78,14 @@ contract TestTokenCollector is Addresses, Permit2Helper {
function testCannotCollectByAllowanceTargetIfNoPriorApprove() public {
bytes memory data = abi.encodePacked(TokenCollector.Source.TokenlonAllowanceTarget);

vm.expectRevert("ERC20: insufficient allowance");
vm.expectRevert(abi.encodeWithSelector(IERC20Errors.ERC20InsufficientAllowance.selector, address(allowanceTarget), 0, 1));
vm.startPrank(user);
strategy.collect(address(token), user, address(this), 1, data);
vm.stopPrank();
}

function testCollectByAllowanceTarget() public {
uint256 amount = 100 * 1e18;
uint256 amount = 100 ether;

vm.prank(user);
token.approve(address(allowanceTarget), amount);
Expand All @@ -98,7 +104,7 @@ contract TestTokenCollector is Addresses, Permit2Helper {
token: address(token),
owner: user,
spender: address(strategy),
amount: 100 * 1e18,
amount: 100 ether,
nonce: token.nonces(user),
deadline: block.timestamp + 1 days
});
Expand Down Expand Up @@ -132,7 +138,7 @@ contract TestTokenCollector is Addresses, Permit2Helper {
(uint8 v, bytes32 r, bytes32 s) = vm.sign(otherPrivateKey, permitHash);

bytes memory data = encodeTokenPermitData(permit, v, r, s);
vm.expectRevert("ERC20Permit: invalid signature");
vm.expectRevert(abi.encodeWithSelector(ERC20Permit.ERC2612InvalidSigner.selector, other, permit.owner));
strategy.collect(address(token), permit.owner, address(this), permit.amount, data);
}

Expand All @@ -145,7 +151,7 @@ contract TestTokenCollector is Addresses, Permit2Helper {
(uint8 v, bytes32 r, bytes32 s) = vm.sign(userPrivateKey, permitHash);

bytes memory data = encodeTokenPermitData(permit, v, r, s);
vm.expectRevert("ERC20: insufficient allowance");
vm.expectRevert(abi.encodeWithSelector(IERC20Errors.ERC20InsufficientAllowance.selector, address(strategy), 0, permit.amount));
strategy.collect(address(token), permit.owner, address(this), permit.amount, data);
}

Expand All @@ -158,7 +164,7 @@ contract TestTokenCollector is Addresses, Permit2Helper {
(uint8 v, bytes32 r, bytes32 s) = vm.sign(userPrivateKey, permitHash);

bytes memory data = encodeTokenPermitData(permit, v, r, s);
vm.expectRevert("ERC20: insufficient allowance");
vm.expectRevert(abi.encodeWithSelector(IERC20Errors.ERC20InsufficientAllowance.selector, address(strategy), permit.amount, invalidAmount));
strategy.collect(address(token), permit.owner, address(this), invalidAmount, data);
}

Expand All @@ -169,9 +175,10 @@ contract TestTokenCollector is Addresses, Permit2Helper {

bytes32 permitHash = getTokenPermitHash(permit);
(uint8 v, bytes32 r, bytes32 s) = vm.sign(userPrivateKey, permitHash);
address recoveredAddress = 0x5d6b650146e111D930C9F97570876A12F568D2B5;

bytes memory data = encodeTokenPermitData(permit, v, r, s);
vm.expectRevert("ERC20Permit: invalid signature");
vm.expectRevert(abi.encodeWithSelector(ERC20Permit.ERC2612InvalidSigner.selector, recoveredAddress, permit.owner));
strategy.collect(address(token), permit.owner, address(this), permit.amount, data);
}

Expand All @@ -184,7 +191,7 @@ contract TestTokenCollector is Addresses, Permit2Helper {
(uint8 v, bytes32 r, bytes32 s) = vm.sign(userPrivateKey, permitHash);

bytes memory data = encodeTokenPermitData(permit, v, r, s);
vm.expectRevert("ERC20Permit: expired deadline");
vm.expectRevert(abi.encodeWithSelector(ERC20Permit.ERC2612ExpiredSignature.selector, permit.deadline));
strategy.collect(address(token), permit.owner, address(this), permit.amount, data);
}

Expand Down Expand Up @@ -289,7 +296,7 @@ contract TestTokenCollector is Addresses, Permit2Helper {

IUniswapPermit2.PermitTransferFrom DEFAULT_PERMIT_TRANSFER =
IUniswapPermit2.PermitTransferFrom({
permitted: IUniswapPermit2.TokenPermissions({ token: address(token), amount: 100 * 1e18 }),
permitted: IUniswapPermit2.TokenPermissions({ token: address(token), amount: 100 ether }),
nonce: 0,
deadline: block.timestamp + 1 days
});
Expand Down
6 changes: 3 additions & 3 deletions test/libraries/SignatureValidator.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,14 @@ contract SignatureValidatorTest is Test {
// should have 96 bytes signature
bytes memory signature = abi.encodePacked(r, s, v);
// will be reverted in OZ ECDSA lib
vm.expectRevert("ECDSA: invalid signature length");
vm.expectRevert(abi.encodeWithSelector(ECDSA.ECDSAInvalidSignatureLength.selector, signature.length));
SignatureValidator.validateSignature(vm.addr(userPrivateKey), digest, signature);
}

function testEIP712WithEmptySignature() public {
bytes memory signature;
// will be reverted in OZ ECDSA lib
vm.expectRevert("ECDSA: invalid signature length");
vm.expectRevert(abi.encodeWithSelector(ECDSA.ECDSAInvalidSignatureLength.selector, signature.length));
SignatureValidator.validateSignature(vm.addr(userPrivateKey), digest, signature);
}

Expand All @@ -79,7 +79,7 @@ contract SignatureValidatorTest is Test {
bytes memory signature = abi.encodePacked(r, s, uint8(10));
// OZ ECDSA lib will handle the zero address case and throw error instead
// so the zero address will never be matched
vm.expectRevert("ECDSA: invalid signature");
vm.expectRevert(ECDSA.ECDSAInvalidSignature.selector);
this._isValidSignatureWrap(address(0), digest, signature);
}

Expand Down
4 changes: 2 additions & 2 deletions test/mocks/MockERC1271Wallet.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ contract MockERC1271Wallet is IERC1271Wallet {

function setAllowance(address[] memory _tokenList, address _spender) external onlyOperator {
for (uint256 i = 0; i < _tokenList.length; i++) {
IERC20(_tokenList[i]).safeApprove(_spender, MAX_UINT);
IERC20(_tokenList[i]).forceApprove(_spender, MAX_UINT);
}
}

function closeAllowance(address[] memory _tokenList, address _spender) external onlyOperator {
for (uint256 i = 0; i < _tokenList.length; i++) {
IERC20(_tokenList[i]).safeApprove(_spender, 0);
IERC20(_tokenList[i]).forceApprove(_spender, 0);
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/utils/BalanceUtil.sol
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ contract BalanceUtil is Test {

function approveERC20(address token, address user, address spender) internal {
vm.startPrank(user);
IERC20(token).safeApprove(spender, type(uint256).max);
IERC20(token).forceApprove(spender, type(uint256).max);
vm.stopPrank();
}
}

0 comments on commit 6b0dd6d

Please sign in to comment.