From 69924ebe48bf54af3186252332700142d56c6f1a Mon Sep 17 00:00:00 2001 From: Charles Jhong Date: Mon, 16 Jan 2023 23:45:18 +0800 Subject: [PATCH 1/3] add timelock for limit order fee factors --- contracts/LimitOrder.sol | 29 ++++++++++++++++++--- contracts/test/forkMainnet/LimitOrder.t.sol | 14 ++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/contracts/LimitOrder.sol b/contracts/LimitOrder.sol index 9024973f..064bb144 100644 --- a/contracts/LimitOrder.sol +++ b/contracts/LimitOrder.sol @@ -25,6 +25,7 @@ contract LimitOrder is ILimitOrder, BaseLibEIP712, SignatureValidator, Reentranc using SafeERC20 for IERC20; string public constant version = "1.0.0"; + uint256 public immutable factorActivateDelay; IPermanentStorage public immutable permStorage; address public immutable userProxy; IWETH public immutable weth; @@ -40,9 +41,13 @@ contract LimitOrder is ILimitOrder, BaseLibEIP712, SignatureValidator, Reentranc address public feeCollector; // Factors + uint256 public factorsTimeLock; uint16 public makerFeeFactor = 0; + uint16 public pendingMakerFeeFactor; uint16 public takerFeeFactor = 0; + uint16 public pendingTakerFeeFactor; uint16 public profitFeeFactor = 0; + uint16 public pendingProfitFeeFactor; constructor( address _operator, @@ -51,6 +56,7 @@ contract LimitOrder is ILimitOrder, BaseLibEIP712, SignatureValidator, Reentranc ISpender _spender, IPermanentStorage _permStorage, IWETH _weth, + uint256 _factorActivateDelay, address _uniswapV3RouterAddress, address _sushiswapRouterAddress, address _feeCollector @@ -61,6 +67,7 @@ contract LimitOrder is ILimitOrder, BaseLibEIP712, SignatureValidator, Reentranc spender = _spender; permStorage = _permStorage; weth = _weth; + factorActivateDelay = _factorActivateDelay; uniswapV3RouterAddress = _uniswapV3RouterAddress; sushiswapRouterAddress = _sushiswapRouterAddress; feeCollector = _feeCollector; @@ -139,11 +146,25 @@ contract LimitOrder is ILimitOrder, BaseLibEIP712, SignatureValidator, Reentranc require(_takerFeeFactor <= LibConstant.BPS_MAX, "LimitOrder: Invalid taker fee factor"); require(_profitFeeFactor <= LibConstant.BPS_MAX, "LimitOrder: Invalid profit fee factor"); - makerFeeFactor = _makerFeeFactor; - takerFeeFactor = _takerFeeFactor; - profitFeeFactor = _profitFeeFactor; + pendingMakerFeeFactor = _makerFeeFactor; + pendingTakerFeeFactor = _takerFeeFactor; + pendingProfitFeeFactor = _profitFeeFactor; - emit FactorsUpdated(_makerFeeFactor, _takerFeeFactor, _profitFeeFactor); + factorsTimeLock = block.timestamp + factorActivateDelay; + } + + function activateFactors() external onlyOperator { + require(factorsTimeLock != 0, "LimitOrder: no pending fee factors"); + require(block.timestamp >= factorsTimeLock, "LimitOrder: fee factors timelocked"); + factorsTimeLock = 0; + makerFeeFactor = pendingMakerFeeFactor; + takerFeeFactor = pendingTakerFeeFactor; + profitFeeFactor = pendingProfitFeeFactor; + pendingMakerFeeFactor = 0; + pendingTakerFeeFactor = 0; + pendingProfitFeeFactor = 0; + + emit FactorsUpdated(makerFeeFactor, takerFeeFactor, profitFeeFactor); } /** diff --git a/contracts/test/forkMainnet/LimitOrder.t.sol b/contracts/test/forkMainnet/LimitOrder.t.sol index cd6d5c89..a36b0df8 100644 --- a/contracts/test/forkMainnet/LimitOrder.t.sol +++ b/contracts/test/forkMainnet/LimitOrder.t.sol @@ -71,6 +71,7 @@ contract LimitOrderTest is StrategySharedSetup { LimitOrder limitOrder; uint64 DEADLINE = uint64(block.timestamp + 2 days); + uint256 FACTORSDEALY = 12 hours; // effectively a "beforeEach" block function setUp() public { @@ -149,6 +150,7 @@ contract LimitOrderTest is StrategySharedSetup { ISpender(address(spender)), IPermanentStorage(address(permanentStorage)), IWETH(address(weth)), + FACTORSDEALY, UNISWAP_V3_ADDRESS, SUSHISWAP_ADDRESS, feeCollector @@ -303,6 +305,14 @@ contract LimitOrderTest is StrategySharedSetup { function testSetFactors() public { limitOrder.setFactors(1, 2, 3); + // fee factors should stay same before new ones activate + assertEq(uint256(limitOrder.makerFeeFactor()), 0); + assertEq(uint256(limitOrder.takerFeeFactor()), 0); + assertEq(uint256(limitOrder.profitFeeFactor()), 0); + vm.warp(block.timestamp + limitOrder.factorActivateDelay()); + + // fee factors should be updated now + limitOrder.activateFactors(); assertEq(uint256(limitOrder.makerFeeFactor()), 1); assertEq(uint256(limitOrder.takerFeeFactor()), 2); assertEq(uint256(limitOrder.profitFeeFactor()), 3); @@ -593,6 +603,8 @@ contract LimitOrderTest is StrategySharedSetup { // makerFeeFactor/takerFeeFactor : 10% // profitFeeFactor : 20% limitOrder.setFactors(1000, 1000, 2000); + vm.warp(block.timestamp + limitOrder.factorActivateDelay()); + limitOrder.activateFactors(); bytes memory payload = _genFillByTraderPayload(DEFAULT_ORDER, DEFAULT_ORDER_MAKER_SIG, DEFAULT_TRADER_PARAMS, DEFAULT_CRD_PARAMS); vm.expectEmit(true, true, true, true); @@ -993,6 +1005,8 @@ contract LimitOrderTest is StrategySharedSetup { // makerFeeFactor/takerFeeFactor : 10% // profitFeeFactor : 20% limitOrder.setFactors(1000, 1000, 2000); + vm.warp(block.timestamp + limitOrder.factorActivateDelay()); + limitOrder.activateFactors(); // get quote from AMM uint256 ammTakerTokenOut = quoteUniswapV3ExactInput(DEFAULT_PROTOCOL_PARAMS.data, DEFAULT_ORDER.makerTokenAmount); From a7ba228f3c3c7141bf5596caba21ae36226355a4 Mon Sep 17 00:00:00 2001 From: Charles Jhong Date: Tue, 17 Jan 2023 16:40:34 +0800 Subject: [PATCH 2/3] rename setFactors and check factorActivateDelay in test --- contracts/LimitOrder.sol | 2 +- contracts/test/forkMainnet/LimitOrder.t.sol | 15 ++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/contracts/LimitOrder.sol b/contracts/LimitOrder.sol index 064bb144..9d786d79 100644 --- a/contracts/LimitOrder.sol +++ b/contracts/LimitOrder.sol @@ -137,7 +137,7 @@ contract LimitOrder is ILimitOrder, BaseLibEIP712, SignatureValidator, Reentranc } } - function setFactors( + function proposeFactors( uint16 _makerFeeFactor, uint16 _takerFeeFactor, uint16 _profitFeeFactor diff --git a/contracts/test/forkMainnet/LimitOrder.t.sol b/contracts/test/forkMainnet/LimitOrder.t.sol index a36b0df8..6de0934f 100644 --- a/contracts/test/forkMainnet/LimitOrder.t.sol +++ b/contracts/test/forkMainnet/LimitOrder.t.sol @@ -179,6 +179,7 @@ contract LimitOrderTest is StrategySharedSetup { assertEq(limitOrder.uniswapV3RouterAddress(), UNISWAP_V3_ADDRESS); assertEq(limitOrder.sushiswapRouterAddress(), SUSHISWAP_ADDRESS); + assertEq(limitOrder.factorActivateDelay(), FACTORSDEALY); assertEq(uint256(limitOrder.makerFeeFactor()), 0); assertEq(uint256(limitOrder.takerFeeFactor()), 0); assertEq(uint256(limitOrder.profitFeeFactor()), 0); @@ -291,20 +292,20 @@ contract LimitOrderTest is StrategySharedSetup { } /********************************* - * Test: setFactors * + * Test: proposeFactors * *********************************/ function testCannotSetFactorsIfLargerThanBpsMax() public { vm.expectRevert("LimitOrder: Invalid maker fee factor"); - limitOrder.setFactors(LibConstant.BPS_MAX + 1, 1, 1); + limitOrder.proposeFactors(LibConstant.BPS_MAX + 1, 1, 1); vm.expectRevert("LimitOrder: Invalid taker fee factor"); - limitOrder.setFactors(1, LibConstant.BPS_MAX + 1, 1); + limitOrder.proposeFactors(1, LibConstant.BPS_MAX + 1, 1); vm.expectRevert("LimitOrder: Invalid profit fee factor"); - limitOrder.setFactors(1, 1, LibConstant.BPS_MAX + 1); + limitOrder.proposeFactors(1, 1, LibConstant.BPS_MAX + 1); } function testSetFactors() public { - limitOrder.setFactors(1, 2, 3); + limitOrder.proposeFactors(1, 2, 3); // fee factors should stay same before new ones activate assertEq(uint256(limitOrder.makerFeeFactor()), 0); assertEq(uint256(limitOrder.takerFeeFactor()), 0); @@ -602,7 +603,7 @@ contract LimitOrderTest is StrategySharedSetup { // makerFeeFactor/takerFeeFactor : 10% // profitFeeFactor : 20% - limitOrder.setFactors(1000, 1000, 2000); + limitOrder.proposeFactors(1000, 1000, 2000); vm.warp(block.timestamp + limitOrder.factorActivateDelay()); limitOrder.activateFactors(); @@ -1004,7 +1005,7 @@ contract LimitOrderTest is StrategySharedSetup { // makerFeeFactor/takerFeeFactor : 10% // profitFeeFactor : 20% - limitOrder.setFactors(1000, 1000, 2000); + limitOrder.proposeFactors(1000, 1000, 2000); vm.warp(block.timestamp + limitOrder.factorActivateDelay()); limitOrder.activateFactors(); From a23af06cc687118c8153b570c1653c969018ec79 Mon Sep 17 00:00:00 2001 From: Charles Jhong Date: Tue, 17 Jan 2023 16:43:09 +0800 Subject: [PATCH 3/3] allow activateFactors called by anyone --- contracts/LimitOrder.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/LimitOrder.sol b/contracts/LimitOrder.sol index 9d786d79..e951533f 100644 --- a/contracts/LimitOrder.sol +++ b/contracts/LimitOrder.sol @@ -153,7 +153,7 @@ contract LimitOrder is ILimitOrder, BaseLibEIP712, SignatureValidator, Reentranc factorsTimeLock = block.timestamp + factorActivateDelay; } - function activateFactors() external onlyOperator { + function activateFactors() external { require(factorsTimeLock != 0, "LimitOrder: no pending fee factors"); require(block.timestamp >= factorsTimeLock, "LimitOrder: fee factors timelocked"); factorsTimeLock = 0;