Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add delayed upgrade #2735

Open
wants to merge 1 commit into
base: release_loopring_3.6.3
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions packages/loopring_v3/contracts/core/iface/ExchangeData.sol
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,15 @@ 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;

uint public constant MIN_DEPOSIT_PERCENTAGE = 1; // 0.1% of pending deposited amount
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;
uint public constant MAX_NUM_ACCOUNTS = 2 ** 32;
uint public constant MAX_NUM_TOKENS = 2 ** 16;
uint public constant MIN_AGE_PROTOCOL_FEES_UNTIL_UPDATED = 7 days;
uint public constant MIN_TIME_IN_SHUTDOWN = 30 days;
uint public constant SETTING_UPDATE_DELAY = 7 days;
// The amount of bytes each rollup transaction uses in the block data for data-availability.
// This is the maximum amount of bytes of all different transaction types.
uint32 public constant MAX_AGE_DEPOSIT_UNTIL_WITHDRAWABLE_UPPERBOUND = 15 days;
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -263,5 +270,7 @@ 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;
}
}
10 changes: 10 additions & 0 deletions packages/loopring_v3/contracts/core/iface/IBlockVerifier.sol
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,16 @@ abstract contract IBlockVerifier is Claimable
external
virtual;

/// @dev Enables the use of the specified circuit.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// @dev Enables the use of the specified circuit.
/// @dev Enables the use of a specified circuit.

/// @param blockType The type of the block
/// @param blockSize The number of requests handled in the block
/// @param blockVersion The block version (i.e. which circuit version needs to be used)
function enableCircuit(
uint8 blockType,
uint16 blockSize,
uint8 blockVersion
) external virtual;

/// @dev Verifies blocks with the given public data and proofs.
/// Verifying a block makes sure all requests handled in the block
/// are correctly handled by the operator.
Expand Down
6 changes: 6 additions & 0 deletions packages/loopring_v3/contracts/core/iface/IExchangeV3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,12 @@ abstract contract IExchangeV3 is Claimable
virtual
external;

/// get loopring address
function getLoopring() external view virtual returns (address);
/// delayed upgrade loopring contract
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
Expand Down
13 changes: 12 additions & 1 deletion packages/loopring_v3/contracts/core/iface/ILoopringV3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,14 @@ abstract contract ILoopringV3 is Claimable
event ExchangeStakeDeposited(address exchangeAddr, uint amount);
event ExchangeStakeWithdrawn(address exchangeAddr, uint amount);
event ExchangeStakeBurned(address exchangeAddr, uint amount);
event SettingsUpdated(uint time);
event SettingsUpdated(uint time, uint effectTime);
event SettingsApplied(uint time);

// == Public Variables ==
uint256 internal constant SETTING_UPDATE_DELAY = 7 days;
mapping (address => uint) internal exchangeStake;

uint public nextEffectiveTime;
Copy link

@dantaik dantaik Sep 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will change the data storage layout table and break upgradeability!!!!!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you have contract LoopringV3 is ILoopringV3, ReentrancyGuard, changing any storage layout in IloopringV3 will break upgradabilty.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

loopringv3 is not deployed using proxy mode

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because it has no initialize function

uint public totalStake;
address public blockVerifierAddress;
uint public forcedWithdrawalFee;
Expand All @@ -29,6 +32,14 @@ abstract contract ILoopringV3 is Claimable

address payable public protocolFeeVault;

struct CachedSettings {
address protocolFeeVault;
address blockVerifierAddress;
uint forcedWithdrawalFee;
}

CachedSettings internal cachedSettings;

// == Public Functions ==

/// @dev Returns the LRC token address
Expand Down
24 changes: 23 additions & 1 deletion packages/loopring_v3/contracts/core/impl/BlockVerifier.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@ contract BlockVerifier is ReentrancyGuard, IBlockVerifier
struct Circuit
{
bool registered;
uint registeredTime;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will you deploy a new BlockVerifier or upgrade an existing deployment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blockverifier is the same case

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But you are modifying the code, so what did you mean "is the same"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for my confused statement. I mean that block verifier also cannot be deployed by using proxy mode like in loopringv3 so that only way to upgrade smart contract is to deploy a new one. so the disorder of storage of it doesn't matter.

bool enabled;
uint[18] verificationKey;
}

mapping (uint8 => mapping (uint16 => mapping (uint8 => Circuit))) public circuits;
uint internal constant CIRCUIT_DELAYED_EFFECT = 7 days;

constructor() Claimable() {}

Expand All @@ -44,7 +46,8 @@ contract BlockVerifier is ReentrancyGuard, IBlockVerifier
circuit.verificationKey[i] = vk[i];
}
circuit.registered = true;
circuit.enabled = true;
circuit.registeredTime = block.timestamp;
// NOTE: circuit is disabled by default

emit CircuitRegistered(
blockType,
Expand Down Expand Up @@ -77,6 +80,25 @@ contract BlockVerifier is ReentrancyGuard, IBlockVerifier
);
}

function enableCircuit(
uint8 blockType,
uint16 blockSize,
uint8 blockVersion
) external override nonReentrant onlyOwner {
Circuit storage circuit = circuits[blockType][blockSize][blockVersion];
require(circuit.registered == true, "NOT_REGISTERED");
require(circuit.enabled == false, "ALREADY_ENABLED");
require(
block.timestamp >= circuit.registeredTime + CIRCUIT_DELAYED_EFFECT,
"NOT_EFFECT_YET"
);

// Enable the circuit
circuit.enabled = true;

emit CircuitDisabled(blockType, blockSize, blockVersion);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is enabled ,can add an event

}

function verifyProofs(
uint8 blockType,
uint16 blockSize,
Expand Down
39 changes: 39 additions & 0 deletions packages/loopring_v3/contracts/core/impl/ExchangeV3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
50 changes: 42 additions & 8 deletions packages/loopring_v3/contracts/core/impl/LoopringV3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ contract LoopringV3 is ILoopringV3, ReentrancyGuard

lrcAddress = _lrcAddress;

updateSettingsInternal(_protocolFeeVault, _blockVerifierAddress, 0);
updateSettingsInternal(_protocolFeeVault, _blockVerifierAddress, 0, 0);
}

// == Public Functions ==
Expand All @@ -51,7 +51,8 @@ contract LoopringV3 is ILoopringV3, ReentrancyGuard
updateSettingsInternal(
_protocolFeeVault,
_blockVerifierAddress,
_forcedWithdrawalFee
_forcedWithdrawalFee,
SETTING_UPDATE_DELAY + block.timestamp
);
}

Expand All @@ -67,7 +68,7 @@ contract LoopringV3 is ILoopringV3, ReentrancyGuard
protocolTakerFeeBips = _protocolTakerFeeBips;
protocolMakerFeeBips = _protocolMakerFeeBips;

emit SettingsUpdated(block.timestamp);
emit SettingsUpdated(block.timestamp, 0);
}

function getExchangeStake(
Expand Down Expand Up @@ -159,17 +160,50 @@ contract LoopringV3 is ILoopringV3, ReentrancyGuard
function updateSettingsInternal(
address payable _protocolFeeVault,
address _blockVerifierAddress,
uint _forcedWithdrawalFee
uint _forcedWithdrawalFee,
uint _nextEffectiveTime
)
private
{
require(address(0) != _protocolFeeVault, "ZERO_ADDRESS");
require(address(0) != _blockVerifierAddress, "ZERO_ADDRESS");
require(
_forcedWithdrawalFee <= 0.5 ether,
"FORCED_WITHDRAWAL_FEE_TOO_HIGH"
);

if (_nextEffectiveTime == 0) {
// effect immediately
protocolFeeVault = _protocolFeeVault;
blockVerifierAddress = _blockVerifierAddress;
forcedWithdrawalFee = _forcedWithdrawalFee;
} else {
// delayed effect
cachedSettings = CachedSettings({
protocolFeeVault: _protocolFeeVault,
blockVerifierAddress: _blockVerifierAddress,
forcedWithdrawalFee: _forcedWithdrawalFee
});
nextEffectiveTime = _nextEffectiveTime;
}

emit SettingsUpdated(block.timestamp, _nextEffectiveTime);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should not modify current event, can add another event which only update but not effect

}

function applyUpdate() external onlyOwner {
// TODO(add ownership check)
require(nextEffectiveTime > 0, "NO_ANY_UPDATES");
require(nextEffectiveTime <= block.timestamp, "NOT_ENABLED_YET");

protocolFeeVault = payable(cachedSettings.protocolFeeVault);
blockVerifierAddress = cachedSettings.blockVerifierAddress;
forcedWithdrawalFee = cachedSettings.forcedWithdrawalFee;

// clear updates
nextEffectiveTime = 0;

protocolFeeVault = _protocolFeeVault;
blockVerifierAddress = _blockVerifierAddress;
forcedWithdrawalFee = _forcedWithdrawalFee;
delete cachedSettings;

emit SettingsUpdated(block.timestamp);
emit SettingsApplied(block.timestamp);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,16 @@ 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");
require(msg.value == 0, "INVALID_ETH_DEPOSIT");
}

// Transfer the tokens to this contract
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// SPDX-License-Identifier: GPL-2.0-or-later
// Copyright 2017 Loopring Technology Limited.
pragma solidity ^0.8.2;

import "@openzeppelin/contracts/access/Ownable.sol";

/**
* @title DelayedImplementationManager
* @author Kongliang Zhong - <[email protected]>
*/
contract DelayedImplementationManagerV2 is Ownable {
address public currImpl;
address public nextImpl;
uint public nextEffectiveTime;

event UpgradeScheduled(address nextImpl, uint effectiveTime);
event UpgradeCancelled(address nextImpl);
event ImplementationChanged(address newImpl);

constructor(address initImpl) {
require(initImpl != address(0), "ZERO_ADDRESS");
currImpl = initImpl;
}

/**
* @dev Allows the proxy owner to upgrade the current version of the proxy.
* @param _nextImpl representing the address of the next implementation to be set.
* @param _daysToDelay representing the amount of days after the next implementation take effect.
*/
function delayedUpgradeTo(
address _nextImpl,
uint _daysToDelay
) public onlyOwner {
if (_nextImpl == address(0)) {
require(
nextImpl != address(0) && _daysToDelay == 0,
"INVALID_ARGS"
);
emit UpgradeCancelled(nextImpl);
nextImpl = address(0);
} else {
// remove it only for test
// require(_daysToDelay >= 1, "INVALID_DAYS");
// solhint-disable-next-line not-rely-on-time
uint _nextEffectiveTime = block.timestamp + _daysToDelay * 1 days;
nextImpl = _nextImpl;
nextEffectiveTime = _nextEffectiveTime;
emit UpgradeScheduled(_nextImpl, _nextEffectiveTime);
}
}

/**
* @dev Allows everyone to replace implementation after effective time.
*/
function executeUpgrade() public {
require(
// solhint-disable-next-line not-rely-on-time
nextImpl != address(0) && block.timestamp >= nextEffectiveTime,
"NOT_IN_EFFECT"
);
currImpl = nextImpl;
nextImpl = address(0);
emit ImplementationChanged(currImpl);
}
}
23 changes: 23 additions & 0 deletions packages/loopring_v3/contracts/thirdparty/proxies/ForwardProxy.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// SPDX-License-Identifier: GPL-2.0-or-later
// Copyright 2017 Loopring Technology Limited.
pragma solidity ^0.8.2;

import "@openzeppelin/contracts/proxy/Proxy.sol";
import "./DelayedImplementationManager.sol";

/**
* @title ForwardProxy
* @author Kongliang Zhong - <[email protected]>
*/
contract ForwardProxyV2 is Proxy {
DelayedImplementationManagerV2 public immutable implManager;

constructor(address _implManager) {
require(_implManager != address(0), "ZERO_ADDRESS");
implManager = DelayedImplementationManagerV2(_implManager);
}

function _implementation() internal view override returns (address) {
return implManager.currImpl();
}
}