From 4760c7983cf89c44004f0a1310236834a8a68082 Mon Sep 17 00:00:00 2001 From: t11s Date: Fri, 10 Dec 2021 21:57:26 -0800 Subject: [PATCH 1/6] perf: optimize + inline isContract --- contracts/PrimitiveManager.sol | 4 ++-- contracts/base/ERC1155.sol | 4 ++-- contracts/base/MarginManager.sol | 2 +- contracts/base/SwapManager.sol | 2 +- contracts/libraries/EngineAddress.sol | 16 ---------------- 5 files changed, 6 insertions(+), 22 deletions(-) diff --git a/contracts/PrimitiveManager.sol b/contracts/PrimitiveManager.sol index ba26a01..e0c5866 100644 --- a/contracts/PrimitiveManager.sol +++ b/contracts/PrimitiveManager.sol @@ -50,7 +50,7 @@ contract PrimitiveManager is IPrimitiveManager, Multicall, CashManager, SelfPerm ) { address engine = EngineAddress.computeAddress(factory, risky, stable); - if (EngineAddress.isContract(engine) == false) revert EngineAddress.EngineNotDeployedError(); + if (engine.code.length == 0) revert EngineAddress.EngineNotDeployedError(); if (delLiquidity == 0) revert ZeroLiquidityError(); @@ -86,7 +86,7 @@ contract PrimitiveManager is IPrimitiveManager, Multicall, CashManager, SelfPerm uint256 minLiquidityOut ) external payable override lock returns (uint256 delLiquidity) { _engine = EngineAddress.computeAddress(factory, risky, stable); - if (EngineAddress.isContract(_engine) == false) revert EngineAddress.EngineNotDeployedError(); + if (_engine.code.length == 0) revert EngineAddress.EngineNotDeployedError(); if (delRisky == 0 && delStable == 0) revert ZeroLiquidityError(); diff --git a/contracts/base/ERC1155.sol b/contracts/base/ERC1155.sol index 758af5d..c8c7fba 100644 --- a/contracts/base/ERC1155.sol +++ b/contracts/base/ERC1155.sol @@ -415,7 +415,7 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { uint256 amount, bytes memory data ) private { - if (to.isContract()) { + if (to.code.length != 0) { try IERC1155Receiver(to).onERC1155Received(operator, from, id, amount, data) returns (bytes4 response) { if (response != IERC1155Receiver.onERC1155Received.selector) { revert("ERC1155: ERC1155Receiver rejected tokens"); @@ -436,7 +436,7 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { uint256[] memory amounts, bytes memory data ) private { - if (to.isContract()) { + if (to.code.length != 0) { try IERC1155Receiver(to).onERC1155BatchReceived(operator, from, ids, amounts, data) returns ( bytes4 response ) { diff --git a/contracts/base/MarginManager.sol b/contracts/base/MarginManager.sol index ea0c2cb..895be7a 100644 --- a/contracts/base/MarginManager.sol +++ b/contracts/base/MarginManager.sol @@ -31,7 +31,7 @@ abstract contract MarginManager is IMarginManager, CashManager { if (delRisky == 0 && delStable == 0) revert ZeroDelError(); address engine = EngineAddress.computeAddress(factory, risky, stable); - if (EngineAddress.isContract(engine) == false) revert EngineAddress.EngineNotDeployedError(); + if (engine.code.length == 0) revert EngineAddress.EngineNotDeployedError(); IPrimitiveEngineActions(engine).deposit( address(this), diff --git a/contracts/base/SwapManager.sol b/contracts/base/SwapManager.sol index c93dcc7..9e29eef 100644 --- a/contracts/base/SwapManager.sol +++ b/contracts/base/SwapManager.sol @@ -34,7 +34,7 @@ abstract contract SwapManager is ISwapManager, CashManager, MarginManager { }); address engine = EngineAddress.computeAddress(factory, params.risky, params.stable); - if (EngineAddress.isContract(engine) == false) revert EngineAddress.EngineNotDeployedError(); + if (engine.code.length == 0) revert EngineAddress.EngineNotDeployedError(); IPrimitiveEngineActions(engine).swap( params.toMargin ? address(this) : params.recipient, diff --git a/contracts/libraries/EngineAddress.sol b/contracts/libraries/EngineAddress.sol index 9d46437..7e0b39a 100644 --- a/contracts/libraries/EngineAddress.sol +++ b/contracts/libraries/EngineAddress.sol @@ -31,20 +31,4 @@ library EngineAddress { ) ); } - - /// @notice Checks if the target address is a contract, this function is used - /// to verify if a PrimitiveEngine was deployed before calling it - /// @param target Address of the contract to check - /// @return True if the target is a contract - function isContract(address target) internal view returns (bool) { - // This method relies on extcodesize, which returns 0 for contracts in - // construction, since the code is only stored at the end of the - // constructor execution. - - uint256 size; - assembly { - size := extcodesize(target) - } - return size > 0; - } } From 7f508c3d55e7f3ac7994956ba16166c1006ed853 Mon Sep 17 00:00:00 2001 From: t11s Date: Fri, 10 Dec 2021 21:58:42 -0800 Subject: [PATCH 2/6] perf: optimize > 0 to != 0 --- contracts/PrimitiveManager.sol | 8 ++++---- contracts/base/CashManager.sol | 6 +++--- contracts/base/MarginManager.sol | 4 ++-- contracts/base/SwapManager.sol | 4 ++-- contracts/libraries/HexStrings.sol | 2 +- contracts/libraries/Margin.sol | 8 ++++---- 6 files changed, 16 insertions(+), 16 deletions(-) diff --git a/contracts/PrimitiveManager.sol b/contracts/PrimitiveManager.sol index e0c5866..fa7cb59 100644 --- a/contracts/PrimitiveManager.sol +++ b/contracts/PrimitiveManager.sol @@ -145,8 +145,8 @@ contract PrimitiveManager is IPrimitiveManager, Multicall, CashManager, SelfPerm address engine = EngineAddress.computeAddress(factory, decoded.risky, decoded.stable); if (msg.sender != engine) revert NotEngineError(); - if (delRisky > 0) pay(decoded.risky, decoded.payer, msg.sender, delRisky); - if (delStable > 0) pay(decoded.stable, decoded.payer, msg.sender, delStable); + if (delRisky != 0) pay(decoded.risky, decoded.payer, msg.sender, delRisky); + if (delStable != 0) pay(decoded.stable, decoded.payer, msg.sender, delStable); } /// @inheritdoc IPrimitiveLiquidityCallback @@ -160,7 +160,7 @@ contract PrimitiveManager is IPrimitiveManager, Multicall, CashManager, SelfPerm address engine = EngineAddress.computeAddress(factory, decoded.risky, decoded.stable); if (msg.sender != engine) revert NotEngineError(); - if (delRisky > 0) pay(decoded.risky, decoded.payer, msg.sender, delRisky); - if (delStable > 0) pay(decoded.stable, decoded.payer, msg.sender, delStable); + if (delRisky != 0) pay(decoded.risky, decoded.payer, msg.sender, delRisky); + if (delStable != 0) pay(decoded.stable, decoded.payer, msg.sender, delStable); } } diff --git a/contracts/base/CashManager.sol b/contracts/base/CashManager.sol index 4660c92..0e9d7ef 100644 --- a/contracts/base/CashManager.sol +++ b/contracts/base/CashManager.sol @@ -31,7 +31,7 @@ abstract contract CashManager is ICashManager, ManagerBase { if (balance < amountMin) revert BalanceTooLowError(balance, amountMin); - if (balance > 0) { + if (balance != 0) { IWETH9(WETH9).withdraw(balance); TransferHelper.safeTransferETH(recipient, balance); } @@ -46,14 +46,14 @@ abstract contract CashManager is ICashManager, ManagerBase { uint256 balance = IERC20(token).balanceOf(address(this)); if (balance < amountMin) revert BalanceTooLowError(balance, amountMin); - if (balance > 0) { + if (balance != 0) { TransferHelper.safeTransfer(token, recipient, balance); } } /// @inheritdoc ICashManager function refundETH() external payable override { - if (address(this).balance > 0) TransferHelper.safeTransferETH(msg.sender, address(this).balance); + if (address(this).balance != 0) TransferHelper.safeTransferETH(msg.sender, address(this).balance); } /// @dev Pays {value} of {token] to {recipient} from {payer} wallet diff --git a/contracts/base/MarginManager.sol b/contracts/base/MarginManager.sol index 895be7a..8608215 100644 --- a/contracts/base/MarginManager.sol +++ b/contracts/base/MarginManager.sol @@ -89,7 +89,7 @@ abstract contract MarginManager is IMarginManager, CashManager { address engine = EngineAddress.computeAddress(factory, decoded.risky, decoded.stable); if (msg.sender != engine) revert NotEngineError(); - if (delStable > 0) pay(decoded.stable, decoded.payer, msg.sender, delStable); - if (delRisky > 0) pay(decoded.risky, decoded.payer, msg.sender, delRisky); + if (delStable != 0) pay(decoded.stable, decoded.payer, msg.sender, delStable); + if (delRisky != 0) pay(decoded.risky, decoded.payer, msg.sender, delRisky); } } diff --git a/contracts/base/SwapManager.sol b/contracts/base/SwapManager.sol index 9e29eef..8f17a3c 100644 --- a/contracts/base/SwapManager.sol +++ b/contracts/base/SwapManager.sol @@ -87,7 +87,7 @@ abstract contract SwapManager is ISwapManager, CashManager, MarginManager { address engine = EngineAddress.computeAddress(factory, decoded.risky, decoded.stable); if (msg.sender != engine) revert NotEngineError(); - if (delRisky > 0) pay(decoded.risky, decoded.payer, msg.sender, delRisky); - if (delStable > 0) pay(decoded.stable, decoded.payer, msg.sender, delStable); + if (delRisky != 0) pay(decoded.risky, decoded.payer, msg.sender, delRisky); + if (delStable != 0) pay(decoded.stable, decoded.payer, msg.sender, delStable); } } diff --git a/contracts/libraries/HexStrings.sol b/contracts/libraries/HexStrings.sol index 7c871e8..41a51ef 100644 --- a/contracts/libraries/HexStrings.sol +++ b/contracts/libraries/HexStrings.sol @@ -22,7 +22,7 @@ library HexStrings { function toHexStringNoPrefix(uint256 value, uint256 length) internal pure returns (string memory) { bytes memory buffer = new bytes(2 * length); - for (uint256 i = buffer.length; i > 0; i--) { + for (uint256 i = buffer.length; i != 0; i--) { buffer[i - 1] = ALPHABET[value & 0xf]; value >>= 4; } diff --git a/contracts/libraries/Margin.sol b/contracts/libraries/Margin.sol index 6b85bcf..86a5e29 100644 --- a/contracts/libraries/Margin.sol +++ b/contracts/libraries/Margin.sol @@ -23,8 +23,8 @@ library Margin { uint256 delRisky, uint256 delStable ) internal { - if (delRisky > 0) margin.balanceRisky += delRisky.toUint128(); - if (delStable > 0) margin.balanceStable += delStable.toUint128(); + if (delRisky != 00) margin.balanceRisky += delRisky.toUint128(); + if (delStable != 00) margin.balanceStable += delStable.toUint128(); } /// @notice Removes risky and stable token balance from an internal margin account @@ -36,7 +36,7 @@ library Margin { uint256 delRisky, uint256 delStable ) internal { - if (delRisky > 0) margin.balanceRisky -= delRisky.toUint128(); - if (delStable > 0) margin.balanceStable -= delStable.toUint128(); + if (delRisky != 00) margin.balanceRisky -= delRisky.toUint128(); + if (delStable != 00) margin.balanceStable -= delStable.toUint128(); } } From e2f457cc9466930ecdb46827be22c947ff6f8462 Mon Sep 17 00:00:00 2001 From: t11s Date: Fri, 10 Dec 2021 22:01:53 -0800 Subject: [PATCH 3/6] perf: optimize nonce increment --- contracts/base/ERC1155Permit.sol | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/contracts/base/ERC1155Permit.sol b/contracts/base/ERC1155Permit.sol index f9f4e3c..852e1a4 100644 --- a/contracts/base/ERC1155Permit.sol +++ b/contracts/base/ERC1155Permit.sol @@ -30,17 +30,18 @@ contract ERC1155Permit is ERC1155, IERC1155Permit, EIP712 { ) external override { if (block.timestamp > deadline) revert SigExpiredError(); - bytes32 structHash = keccak256( - abi.encode(_PERMIT_TYPEHASH, owner, operator, approved, nonces[owner], deadline) - ); + unchecked { + bytes32 structHash = keccak256( + abi.encode(_PERMIT_TYPEHASH, owner, operator, approved, nonces[owner]++, deadline) + ); - bytes32 hash = _hashTypedDataV4(structHash); - address signer = ECDSA.recover(hash, v, r, s); + bytes32 hash = _hashTypedDataV4(structHash); + address signer = ECDSA.recover(hash, v, r, s); - if (signer != owner) revert InvalidSigError(); + if (signer != owner) revert InvalidSigError(); + } _setApprovalForAll(owner, operator, approved); - nonces[owner] += 1; } /// @inheritdoc IERC1155Permit From ec64477dd0098b8d11e9994f8a68fa1507118b15 Mon Sep 17 00:00:00 2001 From: t11s Date: Fri, 10 Dec 2021 22:03:56 -0800 Subject: [PATCH 4/6] perf: optimize lock --- contracts/base/Reentrancy.sol | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/contracts/base/Reentrancy.sol b/contracts/base/Reentrancy.sol index 705f314..4529fba 100644 --- a/contracts/base/Reentrancy.sol +++ b/contracts/base/Reentrancy.sol @@ -8,14 +8,15 @@ contract Reentrancy { /// @notice Thrown when a call to the contract is made during a locked state error LockedError(); - uint256 private _unlocked = 1; + /// @dev Reentrancy guard initialized to state + uint256 private locked = 1; /// @notice Locks the contract to prevent reentrancy modifier lock() { - if (_unlocked != 1) revert LockedError(); + if (locked == 1) revert LockedError(); - _unlocked = 0; + locked = 2; _; - _unlocked = 1; - } + locked = 1; + } From f20f89ba2bb79b644b88d638fc77728369303615 Mon Sep 17 00:00:00 2001 From: t11s Date: Fri, 10 Dec 2021 22:04:42 -0800 Subject: [PATCH 5/6] fix: lock operator mixup --- contracts/base/Reentrancy.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/base/Reentrancy.sol b/contracts/base/Reentrancy.sol index 4529fba..6307274 100644 --- a/contracts/base/Reentrancy.sol +++ b/contracts/base/Reentrancy.sol @@ -13,7 +13,7 @@ contract Reentrancy { /// @notice Locks the contract to prevent reentrancy modifier lock() { - if (locked == 1) revert LockedError(); + if (locked != 1) revert LockedError(); locked = 2; _; From 159c17a3e524c2578bf2a68c437bef39a63c5bf2 Mon Sep 17 00:00:00 2001 From: t11s Date: Fri, 10 Dec 2021 22:50:52 -0800 Subject: [PATCH 6/6] fix: double 0s --- contracts/libraries/Margin.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/contracts/libraries/Margin.sol b/contracts/libraries/Margin.sol index 86a5e29..e28088f 100644 --- a/contracts/libraries/Margin.sol +++ b/contracts/libraries/Margin.sol @@ -23,8 +23,8 @@ library Margin { uint256 delRisky, uint256 delStable ) internal { - if (delRisky != 00) margin.balanceRisky += delRisky.toUint128(); - if (delStable != 00) margin.balanceStable += delStable.toUint128(); + if (delRisky != 0) margin.balanceRisky += delRisky.toUint128(); + if (delStable != 0) margin.balanceStable += delStable.toUint128(); } /// @notice Removes risky and stable token balance from an internal margin account @@ -36,7 +36,7 @@ library Margin { uint256 delRisky, uint256 delStable ) internal { - if (delRisky != 00) margin.balanceRisky -= delRisky.toUint128(); - if (delStable != 00) margin.balanceStable -= delStable.toUint128(); + if (delRisky != 0) margin.balanceRisky -= delRisky.toUint128(); + if (delStable != 0) margin.balanceStable -= delStable.toUint128(); } }