From e7e83fd335c398682a4a51a70889e0b232d7815b Mon Sep 17 00:00:00 2001 From: kl456123 Date: Thu, 26 Sep 2024 04:34:08 +0800 Subject: [PATCH 01/10] add funcs for delayed upgrade --- .../contracts/aux/access/LoopringV3Owner.sol | 17 ++++++++ .../contracts/aux/access/ProxyOwner.sol | 25 ++++++++++++ .../contracts/core/iface/ExchangeData.sol | 10 +++++ .../contracts/core/iface/IExchangeV3.sol | 4 ++ .../contracts/core/impl/ExchangeV3.sol | 39 +++++++++++++++++++ .../contracts/core/impl/LoopringV3.sol | 4 ++ .../impl/libexchange/ExchangeDeposits.sol | 8 +++- 7 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 packages/loopring_v3/contracts/aux/access/LoopringV3Owner.sol create mode 100644 packages/loopring_v3/contracts/aux/access/ProxyOwner.sol diff --git a/packages/loopring_v3/contracts/aux/access/LoopringV3Owner.sol b/packages/loopring_v3/contracts/aux/access/LoopringV3Owner.sol new file mode 100644 index 000000000..8f01c2d09 --- /dev/null +++ b/packages/loopring_v3/contracts/aux/access/LoopringV3Owner.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2017 Loopring Technology Limited. +pragma solidity ^0.7.0; + +import "../../core/iface/ILoopringV3.sol"; +import "./DelayedOwner.sol"; + +/// @title LoopringOwner +/// @author Brecht Devos - +contract LoopringV3Owner is DelayedOwner { + constructor( + ILoopringV3 loopringV3 + ) DelayedOwner(address(loopringV3), 3 days) { + setFunctionDelay(loopringV3.updateSettings.selector, 7 days); + setFunctionDelay(loopringV3.updateProtocolFeeSettings.selector, 1 days); + } +} diff --git a/packages/loopring_v3/contracts/aux/access/ProxyOwner.sol b/packages/loopring_v3/contracts/aux/access/ProxyOwner.sol new file mode 100644 index 000000000..238351242 --- /dev/null +++ b/packages/loopring_v3/contracts/aux/access/ProxyOwner.sol @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2017 Loopring Technology Limited. +pragma solidity ^0.7.0; + +import "../../core/iface/ILoopringV3.sol"; +import "./DelayedOwner.sol"; + +interface IProxy { + function transferProxyOwnership(address newOwner) external; + function upgradeTo(address implementation) external; + function upgradeToAndCall( + address implementation, + bytes memory data + ) external payable; +} + +/// @title LoopringOwner +/// @author Brecht Devos - +contract ProxyOwner is DelayedOwner { + constructor(IProxy proxy) DelayedOwner(address(proxy), 3 days) { + setFunctionDelay(proxy.transferProxyOwnership.selector, 7 days); + setFunctionDelay(proxy.upgradeTo.selector, 7 days); + setFunctionDelay(proxy.upgradeToAndCall.selector, 7 days); + } +} diff --git a/packages/loopring_v3/contracts/core/iface/ExchangeData.sol b/packages/loopring_v3/contracts/core/iface/ExchangeData.sol index 8bc062c8a..b8d29597a 100644 --- a/packages/loopring_v3/contracts/core/iface/ExchangeData.sol +++ b/packages/loopring_v3/contracts/core/iface/ExchangeData.sol @@ -124,6 +124,8 @@ library ExchangeData // This is the prime number that is used for the alt_bn128 elliptic curve, see EIP-196. uint public constant SNARK_SCALAR_FIELD = 21888242871839275222246405745257275088548364400416034343698204186575808495617; + // 0.1% of pending deposited amount + uint public constant MIN_DEPOSIT_PERCENTAGE = 1; uint public constant MAX_OPEN_FORCED_REQUESTS = 4096; uint public constant MAX_AGE_FORCED_REQUEST_UNTIL_WITHDRAW_MODE = 15 days; uint public constant TIMESTAMP_HALF_WINDOW_SIZE_IN_SECONDS = 7 days; @@ -187,6 +189,11 @@ library ExchangeData uint txIndex; } + struct CachedLoopringSetting { + address loopringAddr; + uint nextEffectiveTime; + } + // Represents the entire exchange state except the owner of the exchange. struct State { @@ -263,5 +270,8 @@ library ExchangeData // owner => minter => NFT type => token address => nftID => amount withdrawable // This is only used when the automatic distribution of the withdrawal failed. mapping (address => mapping (address => mapping (NftType => mapping (address => mapping(uint256 => uint))))) amountWithdrawableNFT; + + // cached loopring address to delayed upgrade + CachedLoopringSetting cachedLoopringSetting; } } diff --git a/packages/loopring_v3/contracts/core/iface/IExchangeV3.sol b/packages/loopring_v3/contracts/core/iface/IExchangeV3.sol index 6371ef681..527c2c323 100644 --- a/packages/loopring_v3/contracts/core/iface/IExchangeV3.sol +++ b/packages/loopring_v3/contracts/core/iface/IExchangeV3.sol @@ -131,6 +131,10 @@ abstract contract IExchangeV3 is Claimable virtual external; + function getLoopring() external view virtual returns (address); + function upgradeLoopring(address _loopringAddr) external virtual; + function applyLoopringUpgrade() external virtual; + /// @dev Initialized the agent registry contract used by the exchange. /// Can only be called by the exchange owner once. /// @param agentRegistry The agent registry contract to be used diff --git a/packages/loopring_v3/contracts/core/impl/ExchangeV3.sol b/packages/loopring_v3/contracts/core/impl/ExchangeV3.sol index b6eb9705e..964df102a 100644 --- a/packages/loopring_v3/contracts/core/impl/ExchangeV3.sol +++ b/packages/loopring_v3/contracts/core/impl/ExchangeV3.sol @@ -102,6 +102,45 @@ contract ExchangeV3 is IExchangeV3, ReentrancyGuard, ERC1155Holder, ERC721Holder ); } + function upgradeLoopring( + address _loopringAddr + ) external override nonReentrant onlyOwner { + require(_loopringAddr != address(0), "ZERO_ADDRESS"); + require( + _loopringAddr != state.cachedLoopringSetting.loopringAddr, + "ALREADY_SET" + ); + + state.cachedLoopringSetting = ExchangeData.CachedLoopringSetting({ + loopringAddr: _loopringAddr, + nextEffectiveTime: block.timestamp + + ExchangeData.SETTING_UPDATE_DELAY + }); + } + + function applyLoopringUpgrade() external override nonReentrant onlyOwner { + require( + state.cachedLoopringSetting.nextEffectiveTime > 0, + "NO_ANY_UPDATES" + ); + require( + state.cachedLoopringSetting.nextEffectiveTime <= block.timestamp, + "NOT_ENABLED_YET" + ); + + // update loopring + state.loopringAddr = state.cachedLoopringSetting.loopringAddr; + state.loopring = ILoopringV3(state.loopringAddr); + + // clean cache + state.cachedLoopringSetting.nextEffectiveTime = 0; + state.cachedLoopringSetting.loopringAddr = address(0); + } + + function getLoopring() external view override returns (address) { + return state.loopringAddr; + } + function setAgentRegistry(address _agentRegistry) external override diff --git a/packages/loopring_v3/contracts/core/impl/LoopringV3.sol b/packages/loopring_v3/contracts/core/impl/LoopringV3.sol index 127015215..279ce3ba5 100644 --- a/packages/loopring_v3/contracts/core/impl/LoopringV3.sol +++ b/packages/loopring_v3/contracts/core/impl/LoopringV3.sol @@ -165,6 +165,10 @@ contract LoopringV3 is ILoopringV3, ReentrancyGuard { require(address(0) != _protocolFeeVault, "ZERO_ADDRESS"); require(address(0) != _blockVerifierAddress, "ZERO_ADDRESS"); + require( + _forcedWithdrawalFee <= 0.5 ether, + "FORCED_WITHDRAWAL_FEE_TOO_HIGH" + ); protocolFeeVault = _protocolFeeVault; blockVerifierAddress = _blockVerifierAddress; diff --git a/packages/loopring_v3/contracts/core/impl/libexchange/ExchangeDeposits.sol b/packages/loopring_v3/contracts/core/impl/libexchange/ExchangeDeposits.sol index ff121c0d4..a0b1ff231 100644 --- a/packages/loopring_v3/contracts/core/impl/libexchange/ExchangeDeposits.sol +++ b/packages/loopring_v3/contracts/core/impl/libexchange/ExchangeDeposits.sol @@ -57,6 +57,13 @@ library ExchangeDeposits // Allow depositing with amount == 0 to allow updating the deposit timestamp uint16 tokenID = S.getTokenID(tokenAddress); + ExchangeData.Deposit memory _deposit = S.pendingDeposits[to][tokenID]; + // prevent from attackers to deposit too little tokens + require( + amount * 1000 >= + _deposit.amount * ExchangeData.MIN_DEPOSIT_PERCENTAGE, + "DEPOSIT_TOO_LITTLE" + ); if (tokenID == 0 && amount == 0) { require(msg.value == 0), "INVALID_ETH_DEPOSIT"); @@ -71,7 +78,6 @@ library ExchangeDeposits ); // Add the amount to the deposit request and reset the time the operator has to process it - ExchangeData.Deposit memory _deposit = S.pendingDeposits[to][tokenID]; _deposit.timestamp = uint64(block.timestamp); _deposit.amount = _deposit.amount.add(amountDeposited); S.pendingDeposits[to][tokenID] = _deposit; From 0f48e3ee7a04a2a841676e383c55b1b17c297e34 Mon Sep 17 00:00:00 2001 From: kl456123 Date: Thu, 26 Sep 2024 05:12:46 +0800 Subject: [PATCH 02/10] add delayed selector manager --- .../DelayedSelectorBasedAccessManager.sol | 69 +++++++++++++++++++ .../aux/access/LoopringIOExchangeOwner.sol | 8 ++- .../contracts/core/iface/ExchangeData.sol | 8 --- .../contracts/core/iface/IExchangeV3.sol | 3 +- .../contracts/core/impl/ExchangeV3.sol | 36 ++-------- 5 files changed, 80 insertions(+), 44 deletions(-) create mode 100644 packages/loopring_v3/contracts/aux/access/DelayedSelectorBasedAccessManager.sol diff --git a/packages/loopring_v3/contracts/aux/access/DelayedSelectorBasedAccessManager.sol b/packages/loopring_v3/contracts/aux/access/DelayedSelectorBasedAccessManager.sol new file mode 100644 index 000000000..4c0bff4bc --- /dev/null +++ b/packages/loopring_v3/contracts/aux/access/DelayedSelectorBasedAccessManager.sol @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2017 Loopring Technology Limited. +pragma solidity ^0.7.0; + +import "../../core/iface/IExchangeV3.sol"; +import "../../lib/Claimable.sol"; +import "../../thirdparty/BytesUtil.sol"; +import "./DelayedTransaction.sol"; + +/// @title SelectorBasedAccessManager +/// @author Daniel Wang - +contract DelayedSelectorBasedAccessManager is DelayedTransaction, Claimable { + using BytesUtil for bytes; + + event PermissionUpdate( + address indexed user, + bytes4 indexed selector, + bool allowed + ); + + address public target; + mapping(address => mapping(bytes4 => bool)) public permissions; + + modifier withAccess(bytes4 selector) { + require(hasAccessTo(msg.sender, selector), "PERMISSION_DENIED"); + _; + } + + constructor( + address _target, + uint _timeToLive + ) DelayedTransaction(_timeToLive) { + require(_target != address(0), "ZERO_ADDRESS"); + target = _target; + } + + function grantAccess( + address user, + bytes4 selector, + bool granted + ) external onlyOwner { + require(permissions[user][selector] != granted, "INVALID_VALUE"); + permissions[user][selector] = granted; + emit PermissionUpdate(user, selector, granted); + } + + receive() external payable {} + + fallback() external payable { + transactInternal(target, msg.value, msg.data); + } + + function hasAccessTo( + address user, + bytes4 selector + ) public view returns (bool) { + return user == owner || permissions[user][selector]; + } + + function isAuthorizedForTransactions( + address sender + ) internal view override returns (bool) { + return hasAccessTo(sender, msg.data.toBytes4(0)); + } + + function setFunctionDelay(bytes4 functionSelector, uint delay) internal { + setFunctionDelay(target, functionSelector, delay); + } +} diff --git a/packages/loopring_v3/contracts/aux/access/LoopringIOExchangeOwner.sol b/packages/loopring_v3/contracts/aux/access/LoopringIOExchangeOwner.sol index 8384a40fa..585fd7f8a 100644 --- a/packages/loopring_v3/contracts/aux/access/LoopringIOExchangeOwner.sol +++ b/packages/loopring_v3/contracts/aux/access/LoopringIOExchangeOwner.sol @@ -12,11 +12,11 @@ import "../../lib/Drainable.sol"; import "../../lib/ERC1271.sol"; import "../../lib/MathUint.sol"; import "../../lib/SignatureUtil.sol"; -import "./SelectorBasedAccessManager.sol"; +import "./DelayedSelectorBasedAccessManager.sol"; import "./IBlockReceiver.sol"; -contract LoopringIOExchangeOwner is SelectorBasedAccessManager, ERC1271, Drainable +contract LoopringIOExchangeOwner is DelayedSelectorBasedAccessManager, ERC1271, Drainable { using AddressUtil for address; using AddressUtil for address payable; @@ -53,8 +53,10 @@ contract LoopringIOExchangeOwner is SelectorBasedAccessManager, ERC1271, Drainab constructor( address _exchange ) - SelectorBasedAccessManager(_exchange) + DelayedSelectorBasedAccessManager(_exchange, 3 days) { + setFunctionDelay(IExchangeV3.upgradeLoopring.selector, 7 days); + setFunctionDelay(IExchangeV3.refreshBlockVerifier.selector, 7 days); } function openAccessToSubmitBlocks(bool _open) diff --git a/packages/loopring_v3/contracts/core/iface/ExchangeData.sol b/packages/loopring_v3/contracts/core/iface/ExchangeData.sol index b8d29597a..3a0131b2b 100644 --- a/packages/loopring_v3/contracts/core/iface/ExchangeData.sol +++ b/packages/loopring_v3/contracts/core/iface/ExchangeData.sol @@ -189,11 +189,6 @@ library ExchangeData uint txIndex; } - struct CachedLoopringSetting { - address loopringAddr; - uint nextEffectiveTime; - } - // Represents the entire exchange state except the owner of the exchange. struct State { @@ -270,8 +265,5 @@ library ExchangeData // owner => minter => NFT type => token address => nftID => amount withdrawable // This is only used when the automatic distribution of the withdrawal failed. mapping (address => mapping (address => mapping (NftType => mapping (address => mapping(uint256 => uint))))) amountWithdrawableNFT; - - // cached loopring address to delayed upgrade - CachedLoopringSetting cachedLoopringSetting; } } diff --git a/packages/loopring_v3/contracts/core/iface/IExchangeV3.sol b/packages/loopring_v3/contracts/core/iface/IExchangeV3.sol index 527c2c323..2ad47b57d 100644 --- a/packages/loopring_v3/contracts/core/iface/IExchangeV3.sol +++ b/packages/loopring_v3/contracts/core/iface/IExchangeV3.sol @@ -132,8 +132,7 @@ abstract contract IExchangeV3 is Claimable external; function getLoopring() external view virtual returns (address); - function upgradeLoopring(address _loopringAddr) external virtual; - function applyLoopringUpgrade() external virtual; + function upgradeLoopring(address loopringAddr) external virtual; /// @dev Initialized the agent registry contract used by the exchange. /// Can only be called by the exchange owner once. diff --git a/packages/loopring_v3/contracts/core/impl/ExchangeV3.sol b/packages/loopring_v3/contracts/core/impl/ExchangeV3.sol index 964df102a..dc2f5fd56 100644 --- a/packages/loopring_v3/contracts/core/impl/ExchangeV3.sol +++ b/packages/loopring_v3/contracts/core/impl/ExchangeV3.sol @@ -102,39 +102,13 @@ contract ExchangeV3 is IExchangeV3, ReentrancyGuard, ERC1155Holder, ERC721Holder ); } - function upgradeLoopring( - address _loopringAddr + function upgradeLoopring( + address loopringAddr ) external override nonReentrant onlyOwner { - require(_loopringAddr != address(0), "ZERO_ADDRESS"); - require( - _loopringAddr != state.cachedLoopringSetting.loopringAddr, - "ALREADY_SET" - ); - - state.cachedLoopringSetting = ExchangeData.CachedLoopringSetting({ - loopringAddr: _loopringAddr, - nextEffectiveTime: block.timestamp + - ExchangeData.SETTING_UPDATE_DELAY - }); - } - - function applyLoopringUpgrade() external override nonReentrant onlyOwner { - require( - state.cachedLoopringSetting.nextEffectiveTime > 0, - "NO_ANY_UPDATES" - ); - require( - state.cachedLoopringSetting.nextEffectiveTime <= block.timestamp, - "NOT_ENABLED_YET" - ); - + require(loopringAddr != address(0), "ZERO_ADDRESS"); // update loopring - state.loopringAddr = state.cachedLoopringSetting.loopringAddr; - state.loopring = ILoopringV3(state.loopringAddr); - - // clean cache - state.cachedLoopringSetting.nextEffectiveTime = 0; - state.cachedLoopringSetting.loopringAddr = address(0); + state.loopringAddr = loopringAddr; + state.loopring = ILoopringV3(loopringAddr); } function getLoopring() external view override returns (address) { From 816a3daa68196d1f34b2787f8eb0da6835d41bfb Mon Sep 17 00:00:00 2001 From: kl456123 Date: Thu, 26 Sep 2024 10:30:34 +0800 Subject: [PATCH 03/10] delayed to transfer ownership --- packages/loopring_v3/contracts/aux/access/LoopringV3Owner.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/loopring_v3/contracts/aux/access/LoopringV3Owner.sol b/packages/loopring_v3/contracts/aux/access/LoopringV3Owner.sol index 8f01c2d09..e09c936f6 100644 --- a/packages/loopring_v3/contracts/aux/access/LoopringV3Owner.sol +++ b/packages/loopring_v3/contracts/aux/access/LoopringV3Owner.sol @@ -11,6 +11,7 @@ contract LoopringV3Owner is DelayedOwner { constructor( ILoopringV3 loopringV3 ) DelayedOwner(address(loopringV3), 3 days) { + setFunctionDelay(loopringV3.transferOwnership.selector, 7 days); setFunctionDelay(loopringV3.updateSettings.selector, 7 days); setFunctionDelay(loopringV3.updateProtocolFeeSettings.selector, 1 days); } From 990d50dbd37d73fbb5fd748d9c5664e7ea97d041 Mon Sep 17 00:00:00 2001 From: kl456123 Date: Thu, 26 Sep 2024 18:21:17 +0800 Subject: [PATCH 04/10] fix bug of permission check --- .../aux/access/DelayedSelectorBasedAccessManager.sol | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/loopring_v3/contracts/aux/access/DelayedSelectorBasedAccessManager.sol b/packages/loopring_v3/contracts/aux/access/DelayedSelectorBasedAccessManager.sol index 4c0bff4bc..92fddda22 100644 --- a/packages/loopring_v3/contracts/aux/access/DelayedSelectorBasedAccessManager.sol +++ b/packages/loopring_v3/contracts/aux/access/DelayedSelectorBasedAccessManager.sol @@ -47,7 +47,13 @@ contract DelayedSelectorBasedAccessManager is DelayedTransaction, Claimable { receive() external payable {} fallback() external payable { - transactInternal(target, msg.value, msg.data); + transact(msg.data); + } + + function transact( + bytes memory data + ) public payable withAccess(data.toBytes4(0)) { + transactInternal(target, msg.value, data); } function hasAccessTo( From 7f00c3d4aa13b45f60db865c86cd9bf7eefbf2eb Mon Sep 17 00:00:00 2001 From: kl456123 Date: Fri, 11 Oct 2024 08:17:47 +0800 Subject: [PATCH 05/10] fix compilation error --- .../contracts/core/impl/libexchange/ExchangeDeposits.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/loopring_v3/contracts/core/impl/libexchange/ExchangeDeposits.sol b/packages/loopring_v3/contracts/core/impl/libexchange/ExchangeDeposits.sol index a0b1ff231..cad63df25 100644 --- a/packages/loopring_v3/contracts/core/impl/libexchange/ExchangeDeposits.sol +++ b/packages/loopring_v3/contracts/core/impl/libexchange/ExchangeDeposits.sol @@ -66,7 +66,7 @@ library ExchangeDeposits ); if (tokenID == 0 && amount == 0) { - require(msg.value == 0), "INVALID_ETH_DEPOSIT"); + require(msg.value == 0, "INVALID_ETH_DEPOSIT"); } // Transfer the tokens to this contract From 121991efa88b062e5775850d146333f9f7e72ce0 Mon Sep 17 00:00:00 2001 From: kl456123 Date: Fri, 11 Oct 2024 09:16:21 +0800 Subject: [PATCH 06/10] fix title and author of code --- .../aux/access/DelayedSelectorBasedAccessManager.sol | 4 ++-- packages/loopring_v3/contracts/aux/access/LoopringV3Owner.sol | 4 ++-- packages/loopring_v3/contracts/aux/access/ProxyOwner.sol | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/loopring_v3/contracts/aux/access/DelayedSelectorBasedAccessManager.sol b/packages/loopring_v3/contracts/aux/access/DelayedSelectorBasedAccessManager.sol index 92fddda22..ec56d0fa5 100644 --- a/packages/loopring_v3/contracts/aux/access/DelayedSelectorBasedAccessManager.sol +++ b/packages/loopring_v3/contracts/aux/access/DelayedSelectorBasedAccessManager.sol @@ -7,8 +7,8 @@ import "../../lib/Claimable.sol"; import "../../thirdparty/BytesUtil.sol"; import "./DelayedTransaction.sol"; -/// @title SelectorBasedAccessManager -/// @author Daniel Wang - +/// @title DelayedSelectorBasedAccessManager +/// @author Break Xiong - contract DelayedSelectorBasedAccessManager is DelayedTransaction, Claimable { using BytesUtil for bytes; diff --git a/packages/loopring_v3/contracts/aux/access/LoopringV3Owner.sol b/packages/loopring_v3/contracts/aux/access/LoopringV3Owner.sol index e09c936f6..ff10d15cf 100644 --- a/packages/loopring_v3/contracts/aux/access/LoopringV3Owner.sol +++ b/packages/loopring_v3/contracts/aux/access/LoopringV3Owner.sol @@ -5,8 +5,8 @@ pragma solidity ^0.7.0; import "../../core/iface/ILoopringV3.sol"; import "./DelayedOwner.sol"; -/// @title LoopringOwner -/// @author Brecht Devos - +/// @title LoopringV3Owner +/// @author Break Xiong - contract LoopringV3Owner is DelayedOwner { constructor( ILoopringV3 loopringV3 diff --git a/packages/loopring_v3/contracts/aux/access/ProxyOwner.sol b/packages/loopring_v3/contracts/aux/access/ProxyOwner.sol index 238351242..2f74830e6 100644 --- a/packages/loopring_v3/contracts/aux/access/ProxyOwner.sol +++ b/packages/loopring_v3/contracts/aux/access/ProxyOwner.sol @@ -14,8 +14,8 @@ interface IProxy { ) external payable; } -/// @title LoopringOwner -/// @author Brecht Devos - +/// @title ProxyOwner +/// @author Break Xiong - contract ProxyOwner is DelayedOwner { constructor(IProxy proxy) DelayedOwner(address(proxy), 3 days) { setFunctionDelay(proxy.transferProxyOwnership.selector, 7 days); From 942ed69c8ff0a4ec7e223fc11bfb557ca7d97722 Mon Sep 17 00:00:00 2001 From: kl456123 Date: Fri, 11 Oct 2024 09:23:13 +0800 Subject: [PATCH 07/10] fix function names and code format --- .../contracts/aux/access/ProxyOwner.sol | 2 ++ .../contracts/core/iface/IExchangeV3.sol | 2 +- .../contracts/core/impl/ExchangeV3.sol | 18 +++++++++++++----- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/packages/loopring_v3/contracts/aux/access/ProxyOwner.sol b/packages/loopring_v3/contracts/aux/access/ProxyOwner.sol index 2f74830e6..c51839d13 100644 --- a/packages/loopring_v3/contracts/aux/access/ProxyOwner.sol +++ b/packages/loopring_v3/contracts/aux/access/ProxyOwner.sol @@ -5,6 +5,8 @@ pragma solidity ^0.7.0; import "../../core/iface/ILoopringV3.sol"; import "./DelayedOwner.sol"; +/// @title IProxy +/// @author Break Xiong - interface IProxy { function transferProxyOwnership(address newOwner) external; function upgradeTo(address implementation) external; diff --git a/packages/loopring_v3/contracts/core/iface/IExchangeV3.sol b/packages/loopring_v3/contracts/core/iface/IExchangeV3.sol index 2ad47b57d..68ef72e91 100644 --- a/packages/loopring_v3/contracts/core/iface/IExchangeV3.sol +++ b/packages/loopring_v3/contracts/core/iface/IExchangeV3.sol @@ -132,7 +132,7 @@ abstract contract IExchangeV3 is Claimable external; function getLoopring() external view virtual returns (address); - function upgradeLoopring(address loopringAddr) external virtual; + function setLoopring(address loopringAddr) external virtual; /// @dev Initialized the agent registry contract used by the exchange. /// Can only be called by the exchange owner once. diff --git a/packages/loopring_v3/contracts/core/impl/ExchangeV3.sol b/packages/loopring_v3/contracts/core/impl/ExchangeV3.sol index dc2f5fd56..d107fcd13 100644 --- a/packages/loopring_v3/contracts/core/impl/ExchangeV3.sol +++ b/packages/loopring_v3/contracts/core/impl/ExchangeV3.sol @@ -102,16 +102,24 @@ contract ExchangeV3 is IExchangeV3, ReentrancyGuard, ERC1155Holder, ERC721Holder ); } - function upgradeLoopring( - address loopringAddr - ) external override nonReentrant onlyOwner { + function setLoopring(address loopringAddr) + external + override + nonReentrant + onlyOwner + { require(loopringAddr != address(0), "ZERO_ADDRESS"); - // update loopring + // set loopring state.loopringAddr = loopringAddr; state.loopring = ILoopringV3(loopringAddr); } - function getLoopring() external view override returns (address) { + function getLoopring() + external + view + override + returns (address) + { return state.loopringAddr; } From 3390715426a8bbd4f5d2bb58f303b4eaaa406ff1 Mon Sep 17 00:00:00 2001 From: kl456123 Date: Fri, 11 Oct 2024 09:31:09 +0800 Subject: [PATCH 08/10] fix bug of new func name --- .../contracts/aux/access/LoopringIOExchangeOwner.sol | 2 +- packages/loopring_v3/contracts/core/impl/ExchangeV3.sol | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/loopring_v3/contracts/aux/access/LoopringIOExchangeOwner.sol b/packages/loopring_v3/contracts/aux/access/LoopringIOExchangeOwner.sol index 585fd7f8a..bade3fc92 100644 --- a/packages/loopring_v3/contracts/aux/access/LoopringIOExchangeOwner.sol +++ b/packages/loopring_v3/contracts/aux/access/LoopringIOExchangeOwner.sol @@ -55,7 +55,7 @@ contract LoopringIOExchangeOwner is DelayedSelectorBasedAccessManager, ERC1271, ) DelayedSelectorBasedAccessManager(_exchange, 3 days) { - setFunctionDelay(IExchangeV3.upgradeLoopring.selector, 7 days); + setFunctionDelay(IExchangeV3.setLoopring.selector, 7 days); setFunctionDelay(IExchangeV3.refreshBlockVerifier.selector, 7 days); } diff --git a/packages/loopring_v3/contracts/core/impl/ExchangeV3.sol b/packages/loopring_v3/contracts/core/impl/ExchangeV3.sol index d107fcd13..12eafa90e 100644 --- a/packages/loopring_v3/contracts/core/impl/ExchangeV3.sol +++ b/packages/loopring_v3/contracts/core/impl/ExchangeV3.sol @@ -102,7 +102,7 @@ contract ExchangeV3 is IExchangeV3, ReentrancyGuard, ERC1155Holder, ERC721Holder ); } - function setLoopring(address loopringAddr) + function setLoopring(address _loopringAddr) external override nonReentrant @@ -110,8 +110,8 @@ contract ExchangeV3 is IExchangeV3, ReentrancyGuard, ERC1155Holder, ERC721Holder { require(loopringAddr != address(0), "ZERO_ADDRESS"); // set loopring - state.loopringAddr = loopringAddr; - state.loopring = ILoopringV3(loopringAddr); + state.loopringAddr = _loopringAddr; + state.loopring = ILoopringV3(_loopringAddr); } function getLoopring() From 2dbee73faf75c0b8e405dc3f9e81c747cef8f52a Mon Sep 17 00:00:00 2001 From: kl456123 Date: Thu, 17 Oct 2024 09:35:06 +0800 Subject: [PATCH 09/10] force to deprecate transact in loopringIOExchangeOwner --- .../aux/access/DelayedSelectorBasedAccessManager.sol | 7 +++++++ .../contracts/aux/access/DelayedTransaction.sol | 3 ++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/loopring_v3/contracts/aux/access/DelayedSelectorBasedAccessManager.sol b/packages/loopring_v3/contracts/aux/access/DelayedSelectorBasedAccessManager.sol index ec56d0fa5..5b426ff8c 100644 --- a/packages/loopring_v3/contracts/aux/access/DelayedSelectorBasedAccessManager.sol +++ b/packages/loopring_v3/contracts/aux/access/DelayedSelectorBasedAccessManager.sol @@ -56,6 +56,13 @@ contract DelayedSelectorBasedAccessManager is DelayedTransaction, Claimable { transactInternal(target, msg.value, data); } + function transact( + address, + bytes calldata + ) external payable override nonReentrant onlyAuthorized { + revert("Deprecated"); + } + function hasAccessTo( address user, bytes4 selector diff --git a/packages/loopring_v3/contracts/aux/access/DelayedTransaction.sol b/packages/loopring_v3/contracts/aux/access/DelayedTransaction.sol index bd19dac61..e726f7677 100644 --- a/packages/loopring_v3/contracts/aux/access/DelayedTransaction.sol +++ b/packages/loopring_v3/contracts/aux/access/DelayedTransaction.sol @@ -49,9 +49,10 @@ abstract contract DelayedTransaction is IDelayedTransaction, ReentrancyGuard bytes calldata data ) external + payable + virtual override nonReentrant - payable onlyAuthorized { transactInternal(to, msg.value, data); From 1d19626c9f9e7ed1148784df665e04445da80643 Mon Sep 17 00:00:00 2001 From: kl456123 Date: Fri, 25 Oct 2024 10:03:14 +0800 Subject: [PATCH 10/10] fix audit bugs --- .../DelayedSelectorBasedAccessManager.sol | 21 +++++++++---------- .../contracts/aux/access/ProxyOwner.sol | 1 - .../contracts/core/impl/ExchangeV3.sol | 2 +- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/packages/loopring_v3/contracts/aux/access/DelayedSelectorBasedAccessManager.sol b/packages/loopring_v3/contracts/aux/access/DelayedSelectorBasedAccessManager.sol index 5b426ff8c..5d1d5ee4a 100644 --- a/packages/loopring_v3/contracts/aux/access/DelayedSelectorBasedAccessManager.sol +++ b/packages/loopring_v3/contracts/aux/access/DelayedSelectorBasedAccessManager.sol @@ -2,7 +2,6 @@ // Copyright 2017 Loopring Technology Limited. pragma solidity ^0.7.0; -import "../../core/iface/IExchangeV3.sol"; import "../../lib/Claimable.sol"; import "../../thirdparty/BytesUtil.sol"; import "./DelayedTransaction.sol"; @@ -12,15 +11,15 @@ import "./DelayedTransaction.sol"; contract DelayedSelectorBasedAccessManager is DelayedTransaction, Claimable { using BytesUtil for bytes; + address public immutable target; + mapping(address => mapping(bytes4 => bool)) public permissions; + event PermissionUpdate( address indexed user, bytes4 indexed selector, bool allowed ); - address public target; - mapping(address => mapping(bytes4 => bool)) public permissions; - modifier withAccess(bytes4 selector) { require(hasAccessTo(msg.sender, selector), "PERMISSION_DENIED"); _; @@ -34,6 +33,12 @@ contract DelayedSelectorBasedAccessManager is DelayedTransaction, Claimable { target = _target; } + receive() external payable {} + + fallback() external payable { + transact(msg.data); + } + function grantAccess( address user, bytes4 selector, @@ -44,14 +49,8 @@ contract DelayedSelectorBasedAccessManager is DelayedTransaction, Claimable { emit PermissionUpdate(user, selector, granted); } - receive() external payable {} - - fallback() external payable { - transact(msg.data); - } - function transact( - bytes memory data + bytes calldata data ) public payable withAccess(data.toBytes4(0)) { transactInternal(target, msg.value, data); } diff --git a/packages/loopring_v3/contracts/aux/access/ProxyOwner.sol b/packages/loopring_v3/contracts/aux/access/ProxyOwner.sol index c51839d13..66e336011 100644 --- a/packages/loopring_v3/contracts/aux/access/ProxyOwner.sol +++ b/packages/loopring_v3/contracts/aux/access/ProxyOwner.sol @@ -2,7 +2,6 @@ // Copyright 2017 Loopring Technology Limited. pragma solidity ^0.7.0; -import "../../core/iface/ILoopringV3.sol"; import "./DelayedOwner.sol"; /// @title IProxy diff --git a/packages/loopring_v3/contracts/core/impl/ExchangeV3.sol b/packages/loopring_v3/contracts/core/impl/ExchangeV3.sol index 12eafa90e..a05c8d2d7 100644 --- a/packages/loopring_v3/contracts/core/impl/ExchangeV3.sol +++ b/packages/loopring_v3/contracts/core/impl/ExchangeV3.sol @@ -108,7 +108,7 @@ contract ExchangeV3 is IExchangeV3, ReentrancyGuard, ERC1155Holder, ERC721Holder nonReentrant onlyOwner { - require(loopringAddr != address(0), "ZERO_ADDRESS"); + require(_loopringAddr != address(0), "ZERO_ADDRESS"); // set loopring state.loopringAddr = _loopringAddr; state.loopring = ILoopringV3(_loopringAddr);