diff --git a/packages/perennial-account/contracts/Controller.sol b/packages/perennial-account/contracts/Controller.sol index 461f30099..d70bf45e1 100644 --- a/packages/perennial-account/contracts/Controller.sol +++ b/packages/perennial-account/contracts/Controller.sol @@ -103,10 +103,17 @@ contract Controller is Factory, IController { IMarket market = groupToMarkets[owner][group][i]; RebalanceConfig memory marketRebalanceConfig = _rebalanceConfigs[owner][group][address(market)]; (bool canMarketRebalance, Fixed6 imbalance) = - RebalanceLib.checkMarket(marketRebalanceConfig, groupCollateral, actualCollateral[i]); + RebalanceLib.checkMarket( + marketRebalanceConfig, + groupToMaxRebalanceFee[owner][group], + groupCollateral, + actualCollateral[i] + ); imbalances[i] = imbalance; canRebalance = canRebalance || canMarketRebalance; } + + // if group does not exist or was deleted, arrays will be empty and function will return (0, false, 0) } /// @inheritdoc IController diff --git a/packages/perennial-account/contracts/Controller_Arbitrum.sol b/packages/perennial-account/contracts/Controller_Arbitrum.sol index 99f3d1fea..e5cf46064 100644 --- a/packages/perennial-account/contracts/Controller_Arbitrum.sol +++ b/packages/perennial-account/contracts/Controller_Arbitrum.sol @@ -13,9 +13,11 @@ contract Controller_Arbitrum is Controller_Incentivized, Kept_Arbitrum { /// @dev Creates instance of Controller which compensates keepers /// @param implementation Pristine collateral account contract /// @param marketFactory Market Factory contract - /// @param nonceManager Nonce manager contract for relayed messages - constructor(address implementation, IMarketFactory marketFactory, IVerifierBase nonceManager) - Controller_Incentivized(implementation, marketFactory, nonceManager) {} + /// @param nonceManager Verifier contract to which nonce and group cancellations are relayed + constructor( + address implementation, IMarketFactory marketFactory, + IVerifierBase nonceManager + ) Controller_Incentivized(implementation, marketFactory, nonceManager) {} /// @dev Use the Kept_Arbitrum implementation for calculating the dynamic fee function _calldataFee( diff --git a/packages/perennial-account/contracts/Controller_Incentivized.sol b/packages/perennial-account/contracts/Controller_Incentivized.sol index 1db9d89f7..308f64e17 100644 --- a/packages/perennial-account/contracts/Controller_Incentivized.sol +++ b/packages/perennial-account/contracts/Controller_Incentivized.sol @@ -34,78 +34,115 @@ import { Withdrawal } from "./types/Withdrawal.sol"; abstract contract Controller_Incentivized is Controller, IRelayer, Kept { /// @dev Handles relayed messages for nonce cancellation IVerifierBase public immutable nonceManager; - + /// @dev Configuration used to calculate keeper compensation KeepConfig public keepConfig; + /// @dev Configuration used to calculate keeper compensation with buffered gas + KeepConfig public keepConfigBuffered; + + /// @dev Configuration used to calculate keeper compensation to withdraw from a collateral account + KeepConfig public keepConfigWithdrawal; + /// @dev Creates instance of Controller which compensates keepers /// @param implementation_ Pristine collateral account contract /// @param marketFactory_ Market factory contract - /// @param nonceManager_ Nonce manager contract for relayed messages - constructor(address implementation_, IMarketFactory marketFactory_, IVerifierBase nonceManager_) - Controller(implementation_, marketFactory_) { + /// @param nonceManager_ Verifier contract to which nonce and group cancellations are relayed + constructor( + address implementation_, IMarketFactory marketFactory_, + IVerifierBase nonceManager_ + ) Controller(implementation_, marketFactory_) { nonceManager = nonceManager_; } /// @notice Configures message verification and keeper compensation /// @param verifier_ Contract used to validate collateral account message signatures /// @param chainlinkFeed_ ETH-USD price feed used for calculating keeper compensation - /// @param keepConfig_ Configuration used to compensate keepers + /// @param keepConfig_ Configuration used for unbuffered keeper compensation + /// @param keepConfigBuffered_ Configuration used for buffered keeper compensation + /// @param keepConfigWithdrawal_ Configuration used to compensate keepers for withdrawals function initialize( IAccountVerifier verifier_, AggregatorV3Interface chainlinkFeed_, - KeepConfig memory keepConfig_ + KeepConfig memory keepConfig_, + KeepConfig memory keepConfigBuffered_, + KeepConfig memory keepConfigWithdrawal_ ) external initializer(1) { __Factory__initialize(); __Kept__initialize(chainlinkFeed_, DSU); verifier = verifier_; keepConfig = keepConfig_; + keepConfigBuffered = keepConfigBuffered_; + keepConfigWithdrawal = keepConfigWithdrawal_; } /// @inheritdoc IController function changeRebalanceConfigWithSignature( RebalanceConfigChange calldata configChange, bytes calldata signature - ) override external { + ) + external + override + keepCollateralAccount( + configChange.action.common.account, + abi.encode(configChange, signature), + configChange.action.maxFee, + 0 + ) + { _changeRebalanceConfigWithSignature(configChange, signature); - _compensateKeeper(configChange.action); } /// @inheritdoc IController function deployAccountWithSignature( DeployAccount calldata deployAccount_, bytes calldata signature - ) override external { - IAccount account = _deployAccountWithSignature(deployAccount_, signature); - bytes memory data = abi.encode(address(account), deployAccount_.action.maxFee); - _handleKeeperFee(keepConfig, 0, msg.data[0:0], 0, data); + ) + external + override + keepCollateralAccount( + deployAccount_.action.common.account, + abi.encode(deployAccount_, signature), + deployAccount_.action.maxFee, + 0 + ) + { + _deployAccountWithSignature(deployAccount_, signature); } /// @inheritdoc IController function marketTransferWithSignature( MarketTransfer calldata marketTransfer, bytes calldata signature - ) override external { + ) + external + override + keepCollateralAccount( + marketTransfer.action.common.account, + abi.encode(marketTransfer, signature), + marketTransfer.action.maxFee, + 1 + ) + { IAccount account = IAccount(getAccountAddress(marketTransfer.action.common.account)); - bytes memory data = abi.encode(account, marketTransfer.action.maxFee); - - // if we're depositing collateral to the market, pay the keeper before transferring funds - if (marketTransfer.amount.gte(Fixed6Lib.ZERO)) { - _handleKeeperFee(keepConfig, 0, msg.data[0:0], 0, data); - _marketTransferWithSignature(account, marketTransfer, signature); - // otherwise handle the keeper fee normally, after withdrawing to the collateral account - } else { - _marketTransferWithSignature(account, marketTransfer, signature); - _handleKeeperFee(keepConfig, 0, msg.data[0:0], 0, data); - } + _marketTransferWithSignature(account, marketTransfer, signature); } /// @inheritdoc IController - function rebalanceGroup(address owner, uint256 group) override external { + function rebalanceGroup( + address owner, + uint256 group + ) + external + override + keepCollateralAccount( + owner, + abi.encode(owner, group), + groupToMaxRebalanceFee[owner][group], + groupToMarkets[owner][group].length + ) + { _rebalanceGroup(owner, group); - address account = getAccountAddress(owner); - bytes memory data = abi.encode(account, groupToMaxRebalanceFee[owner][group]); - _handleKeeperFee(keepConfig, 0, msg.data[0:0], 0, data); } /// @inheritdoc IController @@ -116,7 +153,13 @@ abstract contract Controller_Incentivized is Controller, IRelayer, Kept { address account = getAccountAddress(withdrawal.action.common.account); // levy fee prior to withdrawal bytes memory data = abi.encode(account, withdrawal.action.maxFee); - _handleKeeperFee(keepConfig, 0, msg.data[0:0], 0, data); + _handleKeeperFee( + keepConfigWithdrawal, + 0, // no way to calculate applicable gas prior to invocation + abi.encode(withdrawal, signature), + 0, + data + ); _withdrawWithSignature(IAccount(account), withdrawal, signature); } @@ -125,12 +168,19 @@ abstract contract Controller_Incentivized is Controller, IRelayer, Kept { RelayedNonceCancellation calldata message, bytes calldata outerSignature, bytes calldata innerSignature - ) override external { + ) + external + override + keepCollateralAccount( + message.action.common.account, + abi.encode(message, outerSignature, innerSignature), + message.action.maxFee, + 0 + ) + { // ensure the message was signed by the owner or a delegated signer verifier.verifyRelayedNonceCancellation(message, outerSignature); - _compensateKeeper(message.action); - // relay the message to Verifier nonceManager.cancelNonceWithSignature(message.nonceCancellation, innerSignature); } @@ -140,12 +190,19 @@ abstract contract Controller_Incentivized is Controller, IRelayer, Kept { RelayedGroupCancellation calldata message, bytes calldata outerSignature, bytes calldata innerSignature - ) override external { + ) + external + override + keepCollateralAccount( + message.action.common.account, + abi.encode(message, outerSignature, innerSignature), + message.action.maxFee, + 0 + ) + { // ensure the message was signed by the owner or a delegated signer verifier.verifyRelayedGroupCancellation(message, outerSignature); - _compensateKeeper(message.action); - // relay the message to Verifier nonceManager.cancelGroupWithSignature(message.groupCancellation, innerSignature); } @@ -155,12 +212,19 @@ abstract contract Controller_Incentivized is Controller, IRelayer, Kept { RelayedOperatorUpdate calldata message, bytes calldata outerSignature, bytes calldata innerSignature - ) override external { + ) + external + override + keepCollateralAccount( + message.action.common.account, + abi.encode(message, outerSignature, innerSignature), + message.action.maxFee, + 0 + ) + { // ensure the message was signed by the owner or a delegated signer verifier.verifyRelayedOperatorUpdate(message, outerSignature); - _compensateKeeper(message.action); - // relay the message to MarketFactory marketFactory.updateOperatorWithSignature(message.operatorUpdate, innerSignature); } @@ -170,12 +234,19 @@ abstract contract Controller_Incentivized is Controller, IRelayer, Kept { RelayedSignerUpdate calldata message, bytes calldata outerSignature, bytes calldata innerSignature - ) override external { + ) + external + override + keepCollateralAccount( + message.action.common.account, + abi.encode(message, outerSignature, innerSignature), + message.action.maxFee, + 0 + ) + { // ensure the message was signed by the owner or a delegated signer verifier.verifyRelayedSignerUpdate(message, outerSignature); - _compensateKeeper(message.action); - // relay the message to MarketFactory marketFactory.updateSignerWithSignature(message.signerUpdate, innerSignature); } @@ -185,21 +256,23 @@ abstract contract Controller_Incentivized is Controller, IRelayer, Kept { RelayedAccessUpdateBatch calldata message, bytes calldata outerSignature, bytes calldata innerSignature - ) override external { + ) + external + override + keepCollateralAccount( + message.action.common.account, + abi.encode(message, outerSignature, innerSignature), + message.action.maxFee, + 0 + ) + { // ensure the message was signed by the owner or a delegated signer verifier.verifyRelayedAccessUpdateBatch(message, outerSignature); - _compensateKeeper(message.action); - // relay the message to MarketFactory marketFactory.updateAccessBatchWithSignature(message.accessUpdateBatch, innerSignature); } - function _compensateKeeper(Action calldata action) internal virtual { - bytes memory data = abi.encode(getAccountAddress(action.common.account), action.maxFee); - _handleKeeperFee(keepConfig, 0, msg.data[0:0], 0, data); - } - /// @dev Transfers funds from collateral account to controller, and limits compensation /// to the user-defined maxFee in the Action message /// @param amount Calculated keeper fee @@ -218,4 +291,33 @@ abstract contract Controller_Incentivized is Controller, IRelayer, Kept { // transfer DSU to the Controller, such that Kept can transfer to keeper DSU.pull(account, raisedKeeperFee); } -} + + modifier keepCollateralAccount( + address account, + bytes memory applicableCalldata, + UFixed6 maxFee, + uint256 bufferMultiplier + ) { + bytes memory data = abi.encode(getAccountAddress(account), maxFee); + uint256 startGas = gasleft(); + + _; + + uint256 applicableGas = startGas - gasleft(); + + _handleKeeperFee( + bufferMultiplier > 0 + ? KeepConfig( + keepConfigBuffered.multiplierBase, + keepConfigBuffered.bufferBase * (bufferMultiplier), + keepConfigBuffered.multiplierCalldata, + keepConfigBuffered.bufferCalldata * (bufferMultiplier) + ) + : keepConfig, + applicableGas, + applicableCalldata, + 0, + data + ); + } +} \ No newline at end of file diff --git a/packages/perennial-account/contracts/libs/RebalanceLib.sol b/packages/perennial-account/contracts/libs/RebalanceLib.sol index 30528579a..40c500312 100644 --- a/packages/perennial-account/contracts/libs/RebalanceLib.sol +++ b/packages/perennial-account/contracts/libs/RebalanceLib.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.13; import { Fixed6, Fixed6Lib } from "@equilibria/root/number/types/Fixed6.sol"; -import { UFixed6 } from "@equilibria/root/number/types/UFixed6.sol"; +import { UFixed6, UFixed6Lib } from "@equilibria/root/number/types/UFixed6.sol"; import { IController } from "../interfaces/IController.sol"; import { RebalanceConfig } from "../types/RebalanceConfig.sol"; @@ -11,26 +11,36 @@ import { RebalanceConfig } from "../types/RebalanceConfig.sol"; library RebalanceLib { /// @dev Compares actual market collateral for owner with their account's target /// @param marketConfig Rebalance group configuration for this market + /// @param maxFee Maximum fee paid to keeper for rebalancing the group /// @param groupCollateral Owner's collateral across all markets in the group /// @param marketCollateral Owner's actual amount of collateral in this market /// @return canRebalance True if actual collateral in this market is outside of configured threshold /// @return imbalance Amount which needs to be transferred to balance the market function checkMarket( RebalanceConfig memory marketConfig, + UFixed6 maxFee, Fixed6 groupCollateral, Fixed6 marketCollateral ) internal pure returns (bool canRebalance, Fixed6 imbalance) { // determine how much collateral the market should have Fixed6 targetCollateral = groupCollateral.mul(Fixed6Lib.from(marketConfig.target)); - // if market is empty, prevent divide-by-zero condition - if (marketCollateral.eq(Fixed6Lib.ZERO)) return (false, targetCollateral); - // calculate percentage difference between target and actual collateral - Fixed6 pctFromTarget = Fixed6Lib.ONE.sub(targetCollateral.div(marketCollateral)); - // if this percentage exceeds the configured threshold, the market may be rebelanced - canRebalance = pctFromTarget.abs().gt(marketConfig.threshold); + // if target is zero, prevent divide-by-zero condition + if (targetCollateral.eq(Fixed6Lib.ZERO)) { + imbalance = marketCollateral.mul(Fixed6Lib.NEG_ONE); + // can rebalance if market is not empty and imbalance exceeds max fee paid to keeper + canRebalance = !marketCollateral.eq(Fixed6Lib.ZERO) && imbalance.abs().gt(maxFee); + return (canRebalance, imbalance); + } + // calculate percentage difference between actual and target collateral + Fixed6 pctFromTarget = Fixed6Lib.ONE.sub(marketCollateral.div(targetCollateral)); // return negative number for surplus, positive number for deficit imbalance = targetCollateral.sub(marketCollateral); + + // if this percentage exceeds the configured threshold, + // and the amount to rebalance exceeds max fee paid to keeper, the market may be rebalanced + canRebalance = pctFromTarget.abs().gt(marketConfig.threshold) + && imbalance.abs().gt(maxFee); } } \ No newline at end of file diff --git a/packages/perennial-account/contracts/types/RebalanceConfig.sol b/packages/perennial-account/contracts/types/RebalanceConfig.sol index 55d594132..b19968524 100644 --- a/packages/perennial-account/contracts/types/RebalanceConfig.sol +++ b/packages/perennial-account/contracts/types/RebalanceConfig.sol @@ -7,7 +7,7 @@ import { UFixed6, UFixed6Lib } from "@equilibria/root/number/types/UFixed6.sol"; struct RebalanceConfig { /// @dev Percentage of collateral from the group to deposit into the market UFixed6 target; - /// @dev Percentage away from the target at which keepers may rebalance + /// @dev Ratio of market collateral to target at which keepers may rebalance UFixed6 threshold; } diff --git a/packages/perennial-account/contracts/types/RebalanceConfigChange.sol b/packages/perennial-account/contracts/types/RebalanceConfigChange.sol index 642d2eb45..0e4655278 100644 --- a/packages/perennial-account/contracts/types/RebalanceConfigChange.sol +++ b/packages/perennial-account/contracts/types/RebalanceConfigChange.sol @@ -15,7 +15,8 @@ struct RebalanceConfigChange { address[] markets; /// @dev Target allocation for markets in the aforementioned array RebalanceConfig[] configs; - /// @dev Largest amount to compensate a relayer/keeper for rebalancing the group in DSU + /// @dev Largest amount to compensate a relayer/keeper for rebalancing the group in DSU. + /// This amount also prevents keepers from rebalancing imbalances smaller than the keeper fee. UFixed6 maxFee; /// @dev Common information for collateral account actions Action action; @@ -35,7 +36,6 @@ library RebalanceConfigChangeLib { /// @dev Used to create a signed message function hash(RebalanceConfigChange memory self) internal pure returns (bytes32) { - bytes32[] memory encodedAddresses = new bytes32[](self.markets.length); bytes32[] memory encodedConfigs = new bytes32[](self.configs.length); // ensure consistent error for length mismatch @@ -43,7 +43,6 @@ library RebalanceConfigChangeLib { revert IController.ControllerInvalidRebalanceConfigError(); for (uint256 i = 0; i < self.markets.length; ++i) { - encodedAddresses[i] = keccak256(abi.encode(self.markets[i])); encodedConfigs[i] = RebalanceConfigLib.hash(self.configs[i]); } return keccak256(abi.encode( diff --git a/packages/perennial-account/test/helpers/arbitrumHelpers.ts b/packages/perennial-account/test/helpers/arbitrumHelpers.ts index befc3edf0..12c05f735 100644 --- a/packages/perennial-account/test/helpers/arbitrumHelpers.ts +++ b/packages/perennial-account/test/helpers/arbitrumHelpers.ts @@ -25,7 +25,6 @@ import { IOracleProvider, GasOracle__factory, } from '../../types/generated' -import { IKept } from '../../types/generated/contracts/Controller_Arbitrum' import { impersonate } from '../../../common/testutil' import { IVerifier } from '@equilibria/perennial-v2/types/generated' diff --git a/packages/perennial-account/test/helpers/setupHelpers.ts b/packages/perennial-account/test/helpers/setupHelpers.ts index 5ebc70a18..23d3ee31f 100644 --- a/packages/perennial-account/test/helpers/setupHelpers.ts +++ b/packages/perennial-account/test/helpers/setupHelpers.ts @@ -267,6 +267,7 @@ async function deployMarketFactory( minEfficiency: parse6decimal('0.1'), referralFee: 0, minScale: parse6decimal('0.001'), + maxStaleAfter: 7200, }) return marketFactory diff --git a/packages/perennial-account/test/integration/Controller.ts b/packages/perennial-account/test/integration/Controller.ts index 6b3c7bd27..ebfe0cb2a 100644 --- a/packages/perennial-account/test/integration/Controller.ts +++ b/packages/perennial-account/test/integration/Controller.ts @@ -295,6 +295,10 @@ describe('ControllerBase', () => { }) it('handles groups with no collateral', async () => { + const [groupCollateral, canRebalance] = await controller.callStatic.checkGroup(userA.address, 1) + expect(groupCollateral).to.equal(0) + expect(canRebalance).to.be.false + await expect(controller.rebalanceGroup(userA.address, 1, TX_OVERRIDES)).to.be.revertedWithCustomError( controller, 'ControllerGroupBalancedError', @@ -318,6 +322,96 @@ describe('ControllerBase', () => { expect(groupCollateral).to.equal(parse6decimal('15000')) expect(canRebalance).to.be.false }) + + it('rebalances markets with no collateral when others are within threshold', async () => { + // reconfigure group such that ETH market has threshold higher than it's imbalance + const message = { + group: 1, + markets: [ethMarket.address, btcMarket.address], + configs: [ + { target: parse6decimal('0.9'), threshold: parse6decimal('0.15') }, + { target: parse6decimal('0.1'), threshold: parse6decimal('0.03') }, + ], + maxFee: constants.Zero, + ...(await createAction(userA.address)), + } + const signature = await signRebalanceConfigChange(userA, verifier, message) + await expect(controller.connect(keeper).changeRebalanceConfigWithSignature(message, signature)).to.not.be.reverted + + // transfer funds only to the ETH market + await transfer(parse6decimal('10000'), userA, ethMarket) + + await expect(controller.rebalanceGroup(userA.address, 1, TX_OVERRIDES)) + .to.emit(dsu, 'Transfer') + .withArgs(ethMarket.address, accountA.address, utils.parseEther('1000')) + .to.emit(dsu, 'Transfer') + .withArgs(accountA.address, btcMarket.address, utils.parseEther('1000')) + .to.emit(controller, 'GroupRebalanced') + .withArgs(userA.address, 1) + + // ensure group collateral unchanged and cannot rebalance + const [groupCollateral, canRebalance] = await controller.callStatic.checkGroup(userA.address, 1) + expect(groupCollateral).to.equal(parse6decimal('10000')) + expect(canRebalance).to.be.false + }) + + it('should not rebalance empty market configured to be empty', async () => { + // reconfigure group such that BTC market is empty + const message = { + group: 1, + markets: [ethMarket.address, btcMarket.address], + configs: [ + { target: parse6decimal('1'), threshold: parse6decimal('0.05') }, + { target: parse6decimal('0'), threshold: parse6decimal('0.05') }, + ], + maxFee: constants.Zero, + ...(await createAction(userA.address)), + } + const signature = await signRebalanceConfigChange(userA, verifier, message) + await expect(controller.connect(keeper).changeRebalanceConfigWithSignature(message, signature)).to.not.be.reverted + + // transfer funds to the ETH market + await transfer(parse6decimal('2500'), userA, ethMarket) + + // ensure group balanced + await expect(controller.rebalanceGroup(userA.address, 1, TX_OVERRIDES)).to.be.revertedWithCustomError( + controller, + 'ControllerGroupBalancedError', + ) + }) + + it('should rebalance non-empty market configured to be empty', async () => { + // reconfigure group such that BTC market is empty + const message = { + group: 1, + markets: [ethMarket.address, btcMarket.address], + configs: [ + { target: parse6decimal('1'), threshold: parse6decimal('0.05') }, + { target: parse6decimal('0'), threshold: parse6decimal('0.05') }, + ], + maxFee: constants.Zero, + ...(await createAction(userA.address)), + } + const signature = await signRebalanceConfigChange(userA, verifier, message) + await expect(controller.connect(keeper).changeRebalanceConfigWithSignature(message, signature)).to.not.be.reverted + + // transfer funds to both markets + await transfer(parse6decimal('2500'), userA, ethMarket) + await transfer(parse6decimal('2500'), userA, btcMarket) + + await expect(controller.rebalanceGroup(userA.address, 1, TX_OVERRIDES)) + .to.emit(dsu, 'Transfer') + .withArgs(btcMarket.address, accountA.address, utils.parseEther('2500')) + .to.emit(dsu, 'Transfer') + .withArgs(accountA.address, ethMarket.address, utils.parseEther('2500')) + .to.emit(controller, 'GroupRebalanced') + .withArgs(userA.address, 1) + + // ensure group collateral unchanged and cannot rebalance + const [groupCollateral, canRebalance] = await controller.callStatic.checkGroup(userA.address, 1) + expect(groupCollateral).to.equal(parse6decimal('5000')) + expect(canRebalance).to.be.false + }) }) describe('#transfer', () => { diff --git a/packages/perennial-account/test/integration/Controller_Arbitrum.ts b/packages/perennial-account/test/integration/Controller_Arbitrum.ts index 452fa0f04..57cb9534d 100644 --- a/packages/perennial-account/test/integration/Controller_Arbitrum.ts +++ b/packages/perennial-account/test/integration/Controller_Arbitrum.ts @@ -58,10 +58,10 @@ import { IKeeperOracle, IOracleFactory, PythFactory } from '@equilibria/perennia const { ethers } = HRE const CHAINLINK_ETH_USD_FEED = '0x639Fe6ab55C921f74e7fac1ee960C0B6293ba612' // price feed used for keeper compensation -const DEFAULT_MAX_FEE = utils.parseEther('0.5') +const DEFAULT_MAX_FEE = parse6decimal('0.5') // hack around issues estimating gas for instrumented contracts when running tests under coverage -const TX_OVERRIDES = { gasLimit: 3_000_000, maxFeePerGas: 200_000_000 } +const TX_OVERRIDES = { gasLimit: 3_000_000, maxPriorityFeePerGas: 0, maxFeePerGas: 100_000_000 } describe('Controller_Arbitrum', () => { let dsu: IERC20Metadata @@ -83,6 +83,8 @@ describe('Controller_Arbitrum', () => { let receiver: SignerWithAddress let lastNonce = 0 let currentTime: BigNumber + let keeperBalanceBefore: BigNumber + let keeperEthBalanceBefore: BigNumber // create a default action for the specified user with reasonable fee and expiry function createAction( @@ -117,6 +119,7 @@ describe('Controller_Arbitrum', () => { const tx = await controller .connect(keeper) .deployAccountWithSignature(deployAccountMessage, signatureCreate, TX_OVERRIDES) + // verify the address from event arguments const creationArgs = await getEventArguments(tx, 'AccountDeployed') expect(creationArgs.account).to.equal(accountAddress) @@ -132,6 +135,19 @@ describe('Controller_Arbitrum', () => { await fundWalletUSDC(wallet, parse6decimal('50000'), { maxFeePerGas: 100000000 }) } + async function checkCompensation(priceCommitments = 0) { + const keeperFeesPaid = (await dsu.balanceOf(keeper.address)).sub(keeperBalanceBefore) + let keeperEthSpentOnGas = keeperEthBalanceBefore.sub(await keeper.getBalance()) + + // if TXes in test required outside price commitments, compensate the keeper for them + keeperEthSpentOnGas = keeperEthSpentOnGas.add(utils.parseEther('0.0000644306').mul(priceCommitments)) + + // cost of transaction + const keeperGasCostInUSD = keeperEthSpentOnGas.mul(3413) + // keeper should be compensated between 100-125% of actual gas cost + expect(keeperFeesPaid).to.be.within(keeperGasCostInUSD, keeperGasCostInUSD.mul(125).div(100)) + } + // create a serial nonce for testing purposes; real users may choose a nonce however they please function nextNonce(): BigNumber { lastNonce += 1 @@ -179,10 +195,22 @@ describe('Controller_Arbitrum', () => { // set up users and deploy artifacts const keepConfig = { - multiplierBase: 0, - bufferBase: 1_000_000, - multiplierCalldata: 0, - bufferCalldata: 500_000, + multiplierBase: ethers.utils.parseEther('1'), + bufferBase: 275_000, // buffer for handling the keeper fee + multiplierCalldata: ethers.utils.parseEther('1'), + bufferCalldata: 0, + } + const keepConfigBuffered = { + multiplierBase: ethers.utils.parseEther('1.08'), + bufferBase: 1_500_000, // for price commitment + multiplierCalldata: ethers.utils.parseEther('1.08'), + bufferCalldata: 35_200, + } + const keepConfigWithdrawal = { + multiplierBase: ethers.utils.parseEther('1.05'), + bufferBase: 1_500_000, + multiplierCalldata: ethers.utils.parseEther('1.05'), + bufferCalldata: 35_200, } const marketVerifier = IVerifier__factory.connect(await marketFactory.verifier(), owner) controller = await deployControllerArbitrum(owner, marketFactory, marketVerifier, { @@ -192,10 +220,13 @@ describe('Controller_Arbitrum', () => { maxFeePerGas: 100000000, }) // chainlink feed is used by Kept for keeper compensation - await controller['initialize(address,address,(uint256,uint256,uint256,uint256))']( + const KeepConfig = '(uint256,uint256,uint256,uint256)' + await controller[`initialize(address,address,${KeepConfig},${KeepConfig},${KeepConfig})`]( accountVerifier.address, CHAINLINK_ETH_USD_FEED, keepConfig, + keepConfigBuffered, + keepConfigWithdrawal, ) // fund userA await fundWallet(userA) @@ -213,12 +244,13 @@ describe('Controller_Arbitrum', () => { beforeEach(async () => { // update the timestamp used for calculating expiry and adjusting oracle price currentTime = BigNumber.from(await currentBlockTimestamp()) - await loadFixture(fixture) // set a realistic base gas fee await HRE.ethers.provider.send('hardhat_setNextBlockBaseFeePerGas', ['0x5F5E100']) // 0.1 gwei + keeperBalanceBefore = await dsu.balanceOf(keeper.address) + keeperEthBalanceBefore = await keeper.getBalance() currentTime = BigNumber.from(await currentBlockTimestamp()) }) @@ -240,7 +272,7 @@ describe('Controller_Arbitrum', () => { it('can create an account', async () => { // pre-fund the address where the account will be deployed - await usdc.connect(userA).transfer(accountAddressA, parse6decimal('15000'), { maxFeePerGas: 100000000 }) + await usdc.connect(userA).transfer(accountAddressA, parse6decimal('15000'), TX_OVERRIDES) // sign a message to deploy the account const deployAccountMessage = { @@ -249,37 +281,26 @@ describe('Controller_Arbitrum', () => { const signature = await signDeployAccount(userA, accountVerifier, deployAccountMessage) // keeper executes deployment of the account and is compensated - const keeperBalanceBefore = await dsu.balanceOf(keeper.address) - await expect( - controller - .connect(keeper) - .deployAccountWithSignature(deployAccountMessage, signature, { maxFeePerGas: 100000000 }), - ) + await expect(controller.connect(keeper).deployAccountWithSignature(deployAccountMessage, signature, TX_OVERRIDES)) .to.emit(controller, 'AccountDeployed') .withArgs(userA.address, accountAddressA) - const keeperFeePaid = (await dsu.balanceOf(keeper.address)).sub(keeperBalanceBefore) - expect(keeperFeePaid).to.be.within(utils.parseEther('0.001'), DEFAULT_MAX_FEE) + await checkCompensation() }) it('keeper fee is limited by maxFee', async () => { // pre-fund the address where the account will be deployed - await usdc.connect(userA).transfer(accountAddressA, parse6decimal('15000'), { maxFeePerGas: 100000000 }) + await usdc.connect(userA).transfer(accountAddressA, parse6decimal('15000'), TX_OVERRIDES) // sign a message with maxFee smaller than the calculated keeper fee (~0.0033215) - const maxFee = parse6decimal('0.000789') + const maxFee = parse6decimal('0.0789') const deployAccountMessage = { ...createAction(userA.address, userA.address, maxFee), } const signature = await signDeployAccount(userA, accountVerifier, deployAccountMessage) // keeper executes deployment of the account and is compensated - const keeperBalanceBefore = await dsu.balanceOf(keeper.address) - await expect( - controller - .connect(keeper) - .deployAccountWithSignature(deployAccountMessage, signature, { maxFeePerGas: 100000000 }), - ) + await expect(controller.connect(keeper).deployAccountWithSignature(deployAccountMessage, signature, TX_OVERRIDES)) .to.emit(controller, 'AccountDeployed') .withArgs(userA.address, accountAddressA) @@ -310,18 +331,10 @@ describe('Controller_Arbitrum', () => { describe('#transfer', async () => { const INITIAL_DEPOSIT_6 = parse6decimal('13000') let accountA: Account - let keeperBalanceBefore: BigNumber beforeEach(async () => { // deploy collateral account for userA accountA = await createCollateralAccount(userA, INITIAL_DEPOSIT_6) - keeperBalanceBefore = await dsu.balanceOf(keeper.address) - }) - - afterEach(async () => { - // confirm keeper earned their fee - const keeperFeePaid = (await dsu.balanceOf(keeper.address)).sub(keeperBalanceBefore) - expect(keeperFeePaid).to.be.within(utils.parseEther('0.001'), DEFAULT_MAX_FEE) }) it('collects fee for depositing some funds to market', async () => { @@ -352,6 +365,8 @@ describe('Controller_Arbitrum', () => { .to.emit(controller, 'KeeperCall') .withArgs(keeper.address, anyValue, 0, anyValue, anyValue, anyValue) expect((await market.locals(userA.address)).collateral).to.equal(transferAmount) + + await checkCompensation(1) }) it('collects fee for withdrawing some funds from market', async () => { @@ -386,6 +401,8 @@ describe('Controller_Arbitrum', () => { .to.emit(controller, 'KeeperCall') .withArgs(keeper.address, anyValue, 0, anyValue, anyValue, anyValue) expect((await market.locals(userA.address)).collateral).to.equal(parse6decimal('10000')) // 12k-2k + + await checkCompensation(2) }) it('collects fee for withdrawing native deposit from market', async () => { @@ -431,6 +448,8 @@ describe('Controller_Arbitrum', () => { .to.emit(controller, 'KeeperCall') .withArgs(keeper.address, anyValue, 0, anyValue, anyValue, anyValue) expect((await market.locals(userA.address)).collateral).to.equal(0) + + await checkCompensation(1) }) it('collects fee for withdrawing funds into empty collateral account', async () => { @@ -470,6 +489,8 @@ describe('Controller_Arbitrum', () => { parse6decimal('9999'), parse6decimal('10000'), ) // 12k-2k + + await checkCompensation(2) }) }) @@ -481,8 +502,6 @@ describe('Controller_Arbitrum', () => { }) it('collects fee for changing rebalance configuration', async () => { - const keeperBalanceBefore = await dsu.balanceOf(keeper.address) - // sign message to create a new group const message = { group: 5, @@ -501,8 +520,7 @@ describe('Controller_Arbitrum', () => { .withArgs(userA.address, message.group, 1) // ensure keeper was compensated - const keeperFeePaid = (await dsu.balanceOf(keeper.address)).sub(keeperBalanceBefore) - expect(keeperFeePaid).to.be.within(utils.parseEther('0.01'), DEFAULT_MAX_FEE) + await checkCompensation() }) it('collects fee for rebalancing a group', async () => { @@ -528,9 +546,6 @@ describe('Controller_Arbitrum', () => { expect((await ethMarket.locals(userA.address)).collateral).to.equal(parse6decimal('10000')) expect((await btcMarket.locals(userA.address)).collateral).to.equal(0) - // now that test setup is complete, record keeper balance - const keeperBalanceBefore = await dsu.balanceOf(keeper.address) - // rebalance the group await expect(controller.connect(keeper).rebalanceGroup(userA.address, 4, TX_OVERRIDES)) .to.emit(dsu, 'Transfer') @@ -543,8 +558,7 @@ describe('Controller_Arbitrum', () => { .withArgs(keeper.address, anyValue, 0, anyValue, anyValue, anyValue) // confirm keeper earned their fee - const keeperFeePaid = (await dsu.balanceOf(keeper.address)).sub(keeperBalanceBefore) - expect(keeperFeePaid).to.be.within(utils.parseEther('0.01'), DEFAULT_MAX_FEE) + await checkCompensation(2) }) it('honors max rebalance fee when rebalancing a group', async () => { @@ -565,7 +579,7 @@ describe('Controller_Arbitrum', () => { await expect(controller.connect(keeper).changeRebalanceConfigWithSignature(message, signature, TX_OVERRIDES)).to .not.be.reverted - // transfer some collateral to ethMarket and record keeper balance + // transfer some collateral to ethMarket and record keeper balance after account creation await deposit(parse6decimal('5000'), accountA) const keeperBalanceBefore = await dsu.balanceOf(keeper.address) @@ -584,24 +598,83 @@ describe('Controller_Arbitrum', () => { const keeperFeePaid = (await dsu.balanceOf(keeper.address)).sub(keeperBalanceBefore) expect(keeperFeePaid).to.equal(utils.parseEther('0.00923')) }) + + it('cannot award more keeper fees than collateral rebalanced', async () => { + const ethMarket = market + + // create a new group with two markets + const message = { + group: 4, + markets: [ethMarket.address, btcMarket.address], + configs: [ + { target: parse6decimal('0.5'), threshold: parse6decimal('0.05') }, + { target: parse6decimal('0.5'), threshold: parse6decimal('0.05') }, + ], + maxFee: DEFAULT_MAX_FEE, + ...(await createAction(userA.address)), + } + const signature = await signRebalanceConfigChange(userA, accountVerifier, message) + await expect(controller.connect(keeper).changeRebalanceConfigWithSignature(message, signature, TX_OVERRIDES)).to + .not.be.reverted + + let dustAmount = parse6decimal('0.000001') + await dsu.connect(keeper).approve(ethMarket.address, dustAmount.mul(1e12), TX_OVERRIDES) + + // keeper dusts one of the markets + await ethMarket + .connect(keeper) + ['update(address,uint256,uint256,uint256,int256,bool)']( + userA.address, + constants.MaxUint256, + constants.MaxUint256, + constants.MaxUint256, + dustAmount, + false, + { maxFeePerGas: 150000000 }, + ) + expect((await ethMarket.locals(userA.address)).collateral).to.equal(dustAmount) + + // keeper cannot rebalance because dust did not exceed maxFee + await expect( + controller.connect(keeper).rebalanceGroup(userA.address, 4, TX_OVERRIDES), + ).to.be.revertedWithCustomError(controller, 'ControllerGroupBalancedError') + + // keeper dusts the other market, such that target is nonzero, and percentage exceeded + dustAmount = parse6decimal('0.000003') + await dsu.connect(keeper).approve(btcMarket.address, dustAmount.mul(1e12), TX_OVERRIDES) + await btcMarket + .connect(keeper) + ['update(address,uint256,uint256,uint256,int256,bool)']( + userA.address, + constants.MaxUint256, + constants.MaxUint256, + constants.MaxUint256, + dustAmount, + false, + { maxFeePerGas: 150000000 }, + ) + expect((await btcMarket.locals(userA.address)).collateral).to.equal(dustAmount) + + // keeper still cannot rebalance because dust did not exceed maxFee + await expect( + controller.connect(keeper).rebalanceGroup(userA.address, 4, TX_OVERRIDES), + ).to.be.revertedWithCustomError(controller, 'ControllerGroupBalancedError') + }) }) describe('#withdrawal', async () => { let accountA: Account let userBalanceBefore: BigNumber - let keeperBalanceBefore: BigNumber beforeEach(async () => { // deploy collateral account for userA accountA = await createCollateralAccount(userA, parse6decimal('17000')) userBalanceBefore = await usdc.balanceOf(userA.address) - keeperBalanceBefore = await dsu.balanceOf(keeper.address) }) afterEach(async () => { // confirm keeper earned their fee - const keeperFeePaid = (await dsu.balanceOf(keeper.address)).sub(keeperBalanceBefore) - expect(keeperFeePaid).to.be.within(utils.parseEther('0.001'), DEFAULT_MAX_FEE) + await checkCompensation(1) }) it('collects fee for partial withdrawal from a delegated signer', async () => { @@ -656,7 +729,6 @@ describe('Controller_Arbitrum', () => { describe('#relay', async () => { let downstreamVerifier: Verifier - let keeperBalanceBefore: BigNumber function createCommon(domain: Address) { return { @@ -680,8 +752,7 @@ describe('Controller_Arbitrum', () => { afterEach(async () => { // confirm keeper earned their fee - const keeperFeePaid = (await dsu.balanceOf(keeper.address)).sub(keeperBalanceBefore) - expect(keeperFeePaid).to.be.within(utils.parseEther('0'), DEFAULT_MAX_FEE) + await checkCompensation() }) it('relays nonce cancellation messages', async () => { diff --git a/packages/perennial-account/test/unit/Controller.ts b/packages/perennial-account/test/unit/Controller.ts index 57f061d93..6084ecfa8 100644 --- a/packages/perennial-account/test/unit/Controller.ts +++ b/packages/perennial-account/test/unit/Controller.ts @@ -695,6 +695,12 @@ describe('Controller', () => { .withArgs(userA.address, group, 0) await verifyConfigAgainstMessage(userA, message, group) + + // cannot rebalance a deleted group + await expect(controller.connect(keeper).rebalanceGroup(userA.address, group)).to.be.revertedWithCustomError( + controller, + 'ControllerGroupBalancedError', + ) }) }) }) diff --git a/packages/perennial-deploy/tasks/multisig_ops/constants.ts b/packages/perennial-deploy/tasks/multisig_ops/constants.ts index 759d46b93..22e96f217 100644 --- a/packages/perennial-deploy/tasks/multisig_ops/constants.ts +++ b/packages/perennial-deploy/tasks/multisig_ops/constants.ts @@ -576,4 +576,5 @@ export const NewProtocolParameter: ProtocolParameterStruct = { minMaintenance: 4000, minEfficiency: 250000, referralFee: 0, + maxStaleAfter: 172800, // 2 days } diff --git a/packages/perennial-deploy/util/constants.ts b/packages/perennial-deploy/util/constants.ts index 0cfc37dda..723efadb1 100644 --- a/packages/perennial-deploy/util/constants.ts +++ b/packages/perennial-deploy/util/constants.ts @@ -12,6 +12,7 @@ export const DEFAULT_PROTOCOL_PARAMETER = { minMaintenance: utils.parseUnits('0.004', 6), // 0.4% minEfficiency: utils.parseUnits('0.25', 6), // 25% referralFee: 0, + maxStaleAfter: 7200, // 2 hours } export const DEFAULT_MARKET_PARAMETER = { diff --git a/packages/perennial-extensions/test/integration/helpers/setupHelpers.ts b/packages/perennial-extensions/test/integration/helpers/setupHelpers.ts index 3ce516d2b..e7f8d3fc9 100644 --- a/packages/perennial-extensions/test/integration/helpers/setupHelpers.ts +++ b/packages/perennial-extensions/test/integration/helpers/setupHelpers.ts @@ -202,6 +202,7 @@ export async function deployProtocol(chainlinkContext?: ChainlinkContext): Promi minEfficiency: parse6decimal('0.1'), referralFee: 0, minScale: parse6decimal('0.001'), + maxStaleAfter: 7200, }) await oracleFactory.connect(owner).register(chainlink.oracleFactory.address) const oracle = IOracle__factory.connect( diff --git a/packages/perennial-oracle/contracts/keeper/KeeperOracle.sol b/packages/perennial-oracle/contracts/keeper/KeeperOracle.sol index a6b6b6269..44f9f3fb9 100644 --- a/packages/perennial-oracle/contracts/keeper/KeeperOracle.sol +++ b/packages/perennial-oracle/contracts/keeper/KeeperOracle.sol @@ -66,21 +66,21 @@ contract KeeperOracle is IKeeperOracle, Instance { /// @notice Returns the local oracle callback set for a version and market /// @param version The version to lookup /// @return The local oracle callback set for the version and market - function localCallbacks(uint256 version) external view returns (address[] memory) { + function localCallbacks(uint256 version) external view virtual returns (address[] memory) { return _localCallbacks[version].values(); } /// @notice Returns the next requested oracle version /// @dev Returns 0 if no next version is requested /// @return The next requested oracle version - function next() public view returns (uint256) { + function next() public view virtual returns (uint256) { return requests[_global.latestIndex + 1]; } /// @notice Returns the response for a given oracle version /// @param timestamp The timestamp of the oracle version /// @return The response for the given oracle version - function responses(uint256 timestamp) external view returns (PriceResponse memory) { + function responses(uint256 timestamp) external view virtual returns (PriceResponse memory) { return _responses[timestamp].read(); } @@ -88,7 +88,7 @@ contract KeeperOracle is IKeeperOracle, Instance { /// @dev - If no request has been made this version, a price request will be created /// - If a request has been made this version, no action will be taken /// @param account The account to callback to - function request(IMarket, address account) external onlyOracle { + function request(IMarket, address account) external virtual onlyOracle { uint256 currentTimestamp = current(); _localCallbacks[currentTimestamp].add(account); @@ -103,19 +103,19 @@ contract KeeperOracle is IKeeperOracle, Instance { /// @notice Returns the latest synced oracle version and the current oracle version /// @return The latest synced oracle version /// @return The current oracle version collecting new orders - function status() external view returns (OracleVersion memory, uint256) { + function status() external view virtual returns (OracleVersion memory, uint256) { return (latest(), current()); } /// @notice Returns the latest synced oracle version /// @return latestVersion Latest oracle version - function latest() public view returns (OracleVersion memory latestVersion) { + function latest() public view virtual returns (OracleVersion memory latestVersion) { (latestVersion, ) = at(_global.latestVersion); } /// @notice Returns the current oracle version accepting new orders /// @return Current oracle version - function current() public view returns (uint256) { + function current() public view virtual returns (uint256) { return IKeeperFactory(address(factory())).current(); } @@ -123,7 +123,7 @@ contract KeeperOracle is IKeeperOracle, Instance { /// @param timestamp The timestamp of which to lookup /// @return Oracle version at version `version` /// @return Oracle receipt at version `version` - function at(uint256 timestamp) public view returns (OracleVersion memory, OracleReceipt memory) { + function at(uint256 timestamp) public view virtual returns (OracleVersion memory, OracleReceipt memory) { return ( _responses[timestamp].read().toOracleVersion(timestamp), _responses[timestamp].read().toOracleReceipt(_localCallbacks[timestamp].length()) @@ -135,7 +135,7 @@ contract KeeperOracle is IKeeperOracle, Instance { /// @param version The oracle version to commit /// @param receiver The receiver of the settlement fee /// @param value The value charged to commit the price in ether - function commit(OracleVersion memory version, address receiver, uint256 value) external onlyFactory { + function commit(OracleVersion memory version, address receiver, uint256 value) external virtual onlyFactory { if (version.timestamp == 0) revert KeeperOracleVersionOutsideRangeError(); PriceResponse memory priceResponse = (version.timestamp == next()) ? _commitRequested(version, value) : @@ -144,17 +144,20 @@ contract KeeperOracle is IKeeperOracle, Instance { emit OracleProviderVersionFulfilled(version); - IMarket market = oracle.market(); - market.settle(address(0)); - oracle.claimFee(priceResponse.toOracleReceipt(_localCallbacks[version.timestamp].length()).settlementFee); - market.token().push(receiver, UFixed18Lib.from(priceResponse.syncFee)); + try oracle.market() returns (IMarket market) { // v2.3 migration -- don't callback if Oracle is still on its v2.2 implementation + market.settle(address(0)); + oracle.claimFee(priceResponse.toOracleReceipt(_localCallbacks[version.timestamp].length()).settlementFee); + market.token().push(receiver, UFixed18Lib.from(priceResponse.syncFee)); + } catch { + return; + } } /// @notice Performs an asynchronous local settlement callback /// @dev Distribution of keeper incentive is consolidated in the oracle's factory /// @param version The version to settle /// @param maxCount The maximum number of settlement callbacks to perform before exiting - function settle(uint256 version, uint256 maxCount) external onlyFactory { + function settle(uint256 version, uint256 maxCount) external virtual onlyFactory { EnumerableSet.AddressSet storage callbacks = _localCallbacks[version]; if (_global.latestVersion < version) revert KeeperOracleVersionOutsideRangeError(); diff --git a/packages/perennial-oracle/contracts/keeper/KeeperOracle_Migration.sol b/packages/perennial-oracle/contracts/keeper/KeeperOracle_Migration.sol new file mode 100644 index 000000000..fb364b612 --- /dev/null +++ b/packages/perennial-oracle/contracts/keeper/KeeperOracle_Migration.sol @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity 0.8.24; + +import {IMarket, OracleVersion, OracleReceipt } from "../interfaces/IKeeperFactory.sol"; +import { PriceResponse } from "./types/PriceResponse.sol"; +import { KeeperOracle } from "./KeeperOracle.sol"; + +/// @title KeeperOracle_Migration +/// @notice Stub implementation to upgrade the prior KeeperOracle to during the v2.3 migration for compatibility. +contract KeeperOracle_Migration is KeeperOracle { + // sig: 0xd41c17e7 + error NotImplementedError(); + + constructor(uint256 timeout_) KeeperOracle(timeout_) { } + + /// @notice Returns an empty response of the correct v2.3 format for forwards compatibility of the previous sub-oracle + /// @dev Empty responses, as long as they are of the correct format, will be overridden by the Global.latestPrice + /// @return oracleVersion The empty oracle version + /// @return oracleReceipt The empty oracle receipt + function at(uint256 timestamp) public pure override returns ( + OracleVersion memory oracleVersion, + OracleReceipt memory oracleReceipt + ) { + oracleVersion.timestamp = timestamp; + return (oracleVersion, oracleReceipt); + } + + /* Do not allow any unintented calls to the previous sub-oracle after migration */ + + function localCallbacks(uint256) external pure override returns (address[] memory) { revert NotImplementedError(); } + function next() public pure override returns (uint256) { revert NotImplementedError(); } + function responses(uint256) external pure override returns (PriceResponse memory) { revert NotImplementedError(); } + function request(IMarket, address) external pure override { revert NotImplementedError(); } + function status() external pure override returns (OracleVersion memory, uint256) { revert NotImplementedError(); } + function latest() public pure override returns (OracleVersion memory) { revert NotImplementedError(); } + function current() public pure override returns (uint256) { revert NotImplementedError(); } + function commit(OracleVersion memory, address, uint256) external pure override { revert NotImplementedError(); } + function settle(uint256, uint256) external pure override { revert NotImplementedError(); } +} diff --git a/packages/perennial-oracle/test/integration/metaquants/MetaQuantsOracleFactory.test.ts b/packages/perennial-oracle/test/integration/metaquants/MetaQuantsOracleFactory.test.ts index 012543e2f..e10aea9ff 100644 --- a/packages/perennial-oracle/test/integration/metaquants/MetaQuantsOracleFactory.test.ts +++ b/packages/perennial-oracle/test/integration/metaquants/MetaQuantsOracleFactory.test.ts @@ -354,6 +354,7 @@ testOracles.forEach(testOracle => { minEfficiency: parse6decimal('0.1'), referralFee: 0, minScale: parse6decimal('0.001'), + maxStaleAfter: 7200, }) const riskParameter = { diff --git a/packages/perennial-oracle/test/integration/pyth/PythOracleFactory.test.ts b/packages/perennial-oracle/test/integration/pyth/PythOracleFactory.test.ts index 2360df01e..7b9d2a4f5 100644 --- a/packages/perennial-oracle/test/integration/pyth/PythOracleFactory.test.ts +++ b/packages/perennial-oracle/test/integration/pyth/PythOracleFactory.test.ts @@ -310,6 +310,7 @@ testOracles.forEach(testOracle => { minEfficiency: parse6decimal('0.1'), referralFee: 0, minScale: parse6decimal('0.001'), + maxStaleAfter: 7200, }) const riskParameter = { diff --git a/packages/perennial-oracle/test/integrationSepolia/chainlink/ChainlinkOracleFactory.test.ts b/packages/perennial-oracle/test/integrationSepolia/chainlink/ChainlinkOracleFactory.test.ts index 21c777886..adc9f2b88 100644 --- a/packages/perennial-oracle/test/integrationSepolia/chainlink/ChainlinkOracleFactory.test.ts +++ b/packages/perennial-oracle/test/integrationSepolia/chainlink/ChainlinkOracleFactory.test.ts @@ -322,6 +322,7 @@ testOracles.forEach(testOracle => { minEfficiency: parse6decimal('0.1'), referralFee: 0, minScale: parse6decimal('0.001'), + maxStaleAfter: 7200, }) const riskParameter = { diff --git a/packages/perennial-order/contracts/Manager.sol b/packages/perennial-order/contracts/Manager.sol index d4d2cd721..aa998df65 100644 --- a/packages/perennial-order/contracts/Manager.sol +++ b/packages/perennial-order/contracts/Manager.sol @@ -35,6 +35,9 @@ abstract contract Manager is IManager, Kept { /// @dev Configuration used for keeper compensation KeepConfig public keepConfig; + /// @dev Configuration used to compensate keepers for price commitments + KeepConfig public keepConfigBuffered; + /// @dev Stores trigger orders while awaiting their conditions to become true /// Market => Account => Nonce => Order mapping(IMarket => mapping(address => mapping(uint256 => TriggerOrderStorage))) private _orders; @@ -56,12 +59,15 @@ abstract contract Manager is IManager, Kept { /// @notice Initialize the contract /// @param ethOracle_ Chainlink ETH/USD oracle used for keeper compensation /// @param keepConfig_ Keeper compensation configuration + /// @param keepConfigBuffered_ Configuration used for price commitments function initialize( AggregatorV3Interface ethOracle_, - KeepConfig memory keepConfig_ + KeepConfig memory keepConfig_, + KeepConfig memory keepConfigBuffered_ ) external initializer(1) { __Kept__initialize(ethOracle_, DSU); keepConfig = keepConfig_; + keepConfigBuffered = keepConfigBuffered_; // allows DSU to unwrap to USDC DSU.approve(address(reserve)); } @@ -72,11 +78,13 @@ abstract contract Manager is IManager, Kept { } /// @inheritdoc IManager - function placeOrderWithSignature(PlaceOrderAction calldata request, bytes calldata signature) external { + function placeOrderWithSignature(PlaceOrderAction calldata request, bytes calldata signature) + external + keepAction(request.action, abi.encode(request, signature)) + { // ensure the message was signed by the owner or a delegated signer verifier.verifyPlaceOrder(request, signature); - _compensateKeeperAction(request.action); _placeOrder(request.action.market, request.action.common.account, request.action.orderId, request.order); } @@ -86,11 +94,13 @@ abstract contract Manager is IManager, Kept { } /// @inheritdoc IManager - function cancelOrderWithSignature(CancelOrderAction calldata request, bytes calldata signature) external { + function cancelOrderWithSignature(CancelOrderAction calldata request, bytes calldata signature) + external + keepAction(request.action, abi.encode(request, signature)) + { // ensure the message was signed by the owner or a delegated signer verifier.verifyCancelOrder(request, signature); - _compensateKeeperAction(request.action); _cancelOrder(request.action.market, request.action.common.account, request.action.orderId); } @@ -112,12 +122,17 @@ abstract contract Manager is IManager, Kept { } /// @inheritdoc IManager - function executeOrder(IMarket market, address account, uint256 orderId) external { + function executeOrder(IMarket market, address account, uint256 orderId) external + { + // Using a modifier to measure gas would require us reading order from storage twice. + // Instead, measure gas within the method itself. + uint256 startGas = gasleft(); + // check conditions to ensure order is executable (TriggerOrder memory order, bool canExecute) = checkOrder(market, account, orderId); + if (!canExecute) revert ManagerCannotExecuteError(); - _compensateKeeper(market, account, order.maxFee); order.execute(market, account); bool interfaceFeeCharged = _chargeInterfaceFee(market, account, order); @@ -127,17 +142,11 @@ abstract contract Manager is IManager, Kept { emit TriggerOrderExecuted(market, account, order, orderId); if (interfaceFeeCharged) emit TriggerOrderInterfaceFeeCharged(account, market, order.interfaceFee); - } - - /// @notice reads keeper compensation parameters from an action message - function _compensateKeeperAction(Action calldata action) internal { - _compensateKeeper(action.market, action.common.account, action.maxFee); - } - /// @notice encodes data needed to pull DSU from market to pay keeper for fulfilling requests - function _compensateKeeper(IMarket market, address account, UFixed6 maxFee) internal { - bytes memory data = abi.encode(market, account, maxFee); - _handleKeeperFee(keepConfig, 0, msg.data[0:0], 0, data); + // compensate keeper + uint256 applicableGas = startGas - gasleft(); + bytes memory data = abi.encode(market, account, order.maxFee); + _handleKeeperFee(keepConfigBuffered, applicableGas, abi.encode(market, account, orderId), 0, data); } /// @notice Transfers DSU from market to manager to compensate keeper @@ -180,7 +189,7 @@ abstract contract Manager is IManager, Kept { _marketWithdraw(market, account, feeAmount); - if (order.interfaceFee.unwrap) _unwrapAndWithdaw(order.interfaceFee.receiver, UFixed18Lib.from(feeAmount)); + if (order.interfaceFee.unwrap) _unwrapAndWithdraw(order.interfaceFee.receiver, UFixed18Lib.from(feeAmount)); else DSU.push(order.interfaceFee.receiver, UFixed18Lib.from(feeAmount)); return true; @@ -195,14 +204,26 @@ abstract contract Manager is IManager, Kept { // prevent user from reusing an order identifier TriggerOrder memory old = _orders[market][account][orderId].read(); if (old.isSpent) revert ManagerInvalidOrderNonceError(); + // prevent user from frontrunning keeper compensation + if (!old.isEmpty() && old.maxFee.gt(order.maxFee)) revert ManagerCannotReduceMaxFee(); _orders[market][account][orderId].store(order); emit TriggerOrderPlaced(market, account, order, orderId); } /// @notice Unwraps DSU to USDC and pushes to interface fee receiver - function _unwrapAndWithdaw(address receiver, UFixed18 amount) private { + function _unwrapAndWithdraw(address receiver, UFixed18 amount) private { reserve.redeem(amount); USDC.push(receiver, UFixed6Lib.from(amount)); } + + modifier keepAction(Action calldata action, bytes memory applicableCalldata) { + bytes memory data = abi.encode(action.market, action.common.account, action.maxFee); + + uint256 startGas = gasleft(); + _; + uint256 applicableGas = startGas - gasleft(); + + _handleKeeperFee(keepConfig, applicableGas, applicableCalldata, 0, data); + } } diff --git a/packages/perennial-order/contracts/interfaces/IManager.sol b/packages/perennial-order/contracts/interfaces/IManager.sol index 20a02755b..85e68a112 100644 --- a/packages/perennial-order/contracts/interfaces/IManager.sol +++ b/packages/perennial-order/contracts/interfaces/IManager.sol @@ -48,6 +48,10 @@ interface IManager { /// @custom:error Conditions required for order execution are not currently met error ManagerCannotExecuteError(); + // sig: 0x170dda16 + /// @custom:error Replacement order may not reduce maxFee; must cancel and resubmit with new orderId + error ManagerCannotReduceMaxFee(); + // sig: 0xd0cfc108 /// @custom:error Order nonce has already been used error ManagerInvalidOrderNonceError(); diff --git a/packages/perennial-order/test/helpers/arbitrumHelpers.ts b/packages/perennial-order/test/helpers/arbitrumHelpers.ts index 66e38fd7f..de89d2915 100644 --- a/packages/perennial-order/test/helpers/arbitrumHelpers.ts +++ b/packages/perennial-order/test/helpers/arbitrumHelpers.ts @@ -77,6 +77,7 @@ async function deployMarketFactory( minEfficiency: parse6decimal('0.1'), referralFee: 0, minScale: parse6decimal('0.001'), + maxStaleAfter: 7200, }) return marketFactory diff --git a/packages/perennial-order/test/integration/Manager_Arbitrum.test.ts b/packages/perennial-order/test/integration/Manager_Arbitrum.test.ts index a8691ee53..25008d632 100644 --- a/packages/perennial-order/test/integration/Manager_Arbitrum.test.ts +++ b/packages/perennial-order/test/integration/Manager_Arbitrum.test.ts @@ -46,13 +46,6 @@ const CHAINLINK_ETH_USD_FEED = '0x639Fe6ab55C921f74e7fac1ee960C0B6293ba612' // p const DSU_RESERVE = '0x0d49c416103Cbd276d9c3cd96710dB264e3A0c27' const USDC_ADDRESS = '0xaf88d065e77c8cC2239327C5EDb3A432268e5831' // Arbitrum native USDC, a 6-decimal token -const KEEP_CONFIG = { - multiplierBase: 0, - bufferBase: 1_000_000, - multiplierCalldata: 0, - bufferCalldata: 500_000, -} - const MAX_FEE = utils.parseEther('0.88') const NO_INTERFACE_FEE = { @@ -65,7 +58,7 @@ const NO_INTERFACE_FEE = { } // because we called hardhat_setNextBlockBaseFeePerGas, need this when running tests under coverage -const TX_OVERRIDES = { maxFeePerGas: 150_000_000 } +const TX_OVERRIDES = { maxPriorityFeePerGas: 0, maxFeePerGas: 150_000_000 } describe('Manager_Arbitrum', () => { let dsu: IERC20Metadata @@ -84,9 +77,9 @@ describe('Manager_Arbitrum', () => { let userD: SignerWithAddress let keeper: SignerWithAddress let oracleFeeReceiver: SignerWithAddress - let checkKeeperCompensation: boolean let currentTime: BigNumber let keeperBalanceBefore: BigNumber + let keeperEthBalanceBefore: BigNumber let lastMessageNonce = 0 let lastPriceCommitted: BigNumber const nextOrderId: { [key: string]: BigNumber } = {} @@ -113,7 +106,20 @@ describe('Manager_Arbitrum', () => { DSU_RESERVE, verifier.address, ) - await manager.initialize(CHAINLINK_ETH_USD_FEED, KEEP_CONFIG) + + const keepConfig = { + multiplierBase: ethers.utils.parseEther('1'), + bufferBase: 1_000_000, // buffer for withdrawing keeper fee from market + multiplierCalldata: 0, + bufferCalldata: 0, + } + const keepConfigBuffered = { + multiplierBase: ethers.utils.parseEther('1.05'), + bufferBase: 1_500_000, // for price commitment + multiplierCalldata: ethers.utils.parseEther('1.05'), + bufferCalldata: 35_200, + } + await manager.initialize(CHAINLINK_ETH_USD_FEED, keepConfig, keepConfigBuffered) // commit a start price await commitPrice(parse6decimal('4444')) @@ -137,6 +143,20 @@ describe('Manager_Arbitrum', () => { await marketFactory.connect(user).updateOperator(manager.address, true) } + async function checkCompensation(priceCommitments = 0) { + const keeperFeesPaid = (await dsu.balanceOf(keeper.address)).sub(keeperBalanceBefore) + let keeperEthSpentOnGas = keeperEthBalanceBefore.sub(await keeper.getBalance()) + + // If TXes in test required outside price commitments, compensate the keeper for them. + // Note that calls to `commitPrice` in this module do not consume keeper gas. + keeperEthSpentOnGas = keeperEthSpentOnGas.add(utils.parseEther('0.0000644306').mul(priceCommitments)) + + // cost of transaction + const keeperGasCostInUSD = keeperEthSpentOnGas.mul(2603) + // keeper should be compensated between 100-200% of actual gas cost + expect(keeperFeesPaid).to.be.within(keeperGasCostInUSD, keeperGasCostInUSD.mul(2)) + } + // commits an oracle version and advances time 10 seconds async function commitPrice( price = lastPriceCommitted, @@ -353,17 +373,11 @@ describe('Manager_Arbitrum', () => { beforeEach(async () => { await setNextBlockBaseFee() currentTime = BigNumber.from(await currentBlockTimestamp()) - checkKeeperCompensation = false keeperBalanceBefore = await dsu.balanceOf(keeper.address) + keeperEthBalanceBefore = await keeper.getBalance() }) afterEach(async () => { - // ensure keeper was paid for their transaction - if (checkKeeperCompensation) { - const keeperBalanceAfter = await dsu.balanceOf(keeper.address) - const keeperFeePaid = keeperBalanceAfter.sub(keeperBalanceBefore) - expect(keeperFeePaid).to.be.within(utils.parseEther('0.001'), MAX_FEE) - } // ensure manager has no funds at rest expect(await dsu.balanceOf(manager.address)).to.equal(constants.Zero) }) @@ -443,7 +457,7 @@ describe('Manager_Arbitrum', () => { expect(await getPendingPosition(userB, Side.LONG)).to.equal(parse6decimal('2.5')) await market.connect(userA).settle(userA.address, TX_OVERRIDES) - checkKeeperCompensation = true + await checkCompensation(3) }) it('user can place an order using a signed message', async () => { @@ -455,13 +469,12 @@ describe('Manager_Arbitrum', () => { parse6decimal('-10'), ) expect(orderId).to.equal(BigNumber.from(503)) - await executeOrder(userA, 503) + await checkCompensation(0) + await executeOrder(userA, 503) expect(await getPendingPosition(userA, Side.MAKER)).to.equal(parse6decimal('90')) expect(await getPendingPosition(userB, Side.LONG)).to.equal(parse6decimal('2.5')) await commitPrice(parse6decimal('2801')) - - checkKeeperCompensation = true }) it('user can cancel an order', async () => { @@ -496,13 +509,13 @@ describe('Manager_Arbitrum', () => { await expect(manager.connect(keeper).cancelOrderWithSignature(message, signature, TX_OVERRIDES)) .to.emit(manager, 'TriggerOrderCancelled') .withArgs(market.address, userA.address, orderId) + await checkCompensation(0) const storedOrder = await manager.orders(market.address, userA.address, orderId) expect(storedOrder.isSpent).to.be.true expect(await getPendingPosition(userA, Side.MAKER)).to.equal(parse6decimal('90')) expect(await getPendingPosition(userB, Side.LONG)).to.equal(parse6decimal('2.5')) - checkKeeperCompensation = true }) it('non-delegated signer cannot interact', async () => { @@ -553,7 +566,7 @@ describe('Manager_Arbitrum', () => { // order was not executed, so no change in position expect(await getPendingPosition(userA, Side.MAKER)).to.equal(parse6decimal('90')) expect(await getPendingPosition(userB, Side.LONG)).to.equal(parse6decimal('2.5')) - checkKeeperCompensation = true + await checkCompensation(0) }) it('charges flat interface fee upon execution', async () => { @@ -595,7 +608,7 @@ describe('Manager_Arbitrum', () => { // ensure fees were paid expect(await dsu.balanceOf(userC.address)).to.equal(interfaceBalanceBefore.add(feeAmount.mul(1e12))) - checkKeeperCompensation = true + await checkCompensation(1) }) it('unwraps flat interface fee upon execution', async () => { @@ -631,7 +644,7 @@ describe('Manager_Arbitrum', () => { // ensure fees were paid expect(await usdc.balanceOf(userC.address)).to.equal(feeAmount) - checkKeeperCompensation = true + await checkCompensation(1) }) it('unwraps notional interface fee upon execution', async () => { @@ -670,7 +683,7 @@ describe('Manager_Arbitrum', () => { // ensure fees were paid expect(await usdc.balanceOf(userC.address)).to.equal(interfaceBalanceBefore.add(expectedInterfaceFee)) - checkKeeperCompensation = true + await checkCompensation(1) }) it('users can close positions', async () => { @@ -740,6 +753,10 @@ describe('Manager_Arbitrum', () => { nextOrderId[userD.address] = BigNumber.from(600) }) + afterEach(async () => { + await checkCompensation(1) + }) + it('can execute an order with pending position before oracle request fulfilled', async () => { // userB has an unsettled long 1.2 position await changePosition(userB, 0, parse6decimal('1.2'), 0) @@ -763,8 +780,6 @@ describe('Manager_Arbitrum', () => { // settle userB and check position await market.settle(userB.address, TX_OVERRIDES) expect((await market.positions(userB.address)).long).to.equal(parse6decimal('2.0')) - - checkKeeperCompensation = true }) it('can execute an order with pending position after oracle request fulfilled', async () => { @@ -791,8 +806,6 @@ describe('Manager_Arbitrum', () => { // after settling userC, they should be short 2.5 await market.settle(userC.address, TX_OVERRIDES) expect((await market.positions(userC.address)).short).to.equal(parse6decimal('2.5')) - - checkKeeperCompensation = true }) it('can execute an order once market conditions allow', async () => { @@ -847,8 +860,6 @@ describe('Manager_Arbitrum', () => { await market.settle(userD.address, TX_OVERRIDES) expect((await market.positions(userC.address)).short).to.equal(parse6decimal('2.26')) expect((await market.positions(userD.address)).long).to.equal(parse6decimal('3')) - - checkKeeperCompensation = true }) it('market reverts when attempting to close an unsettled position', async () => { @@ -880,11 +891,9 @@ describe('Manager_Arbitrum', () => { await commitPrice(parse6decimal('2000.2'), longOrderTimestamp) await market.settle(userD.address, TX_OVERRIDES) expect((await market.positions(userD.address)).long).to.equal(parse6decimal('4.5')) - - checkKeeperCompensation = true }) - it('charges notional interface on whole position when closing', async () => { + it('charges notional interface fee on whole position when closing', async () => { const interfaceBalanceBefore = await dsu.balanceOf(userB.address) // userD closes their long position @@ -914,7 +923,6 @@ describe('Manager_Arbitrum', () => { // ensure fees were paid expect(await dsu.balanceOf(userB.address)).to.equal(interfaceBalanceBefore.add(expectedInterfaceFee.mul(1e12))) - checkKeeperCompensation = true }) }) }) diff --git a/packages/perennial-order/test/unit/Manager.test.ts b/packages/perennial-order/test/unit/Manager.test.ts index 5ed4dfb88..f980a48b5 100644 --- a/packages/perennial-order/test/unit/Manager.test.ts +++ b/packages/perennial-order/test/unit/Manager.test.ts @@ -30,9 +30,9 @@ const MAX_FEE = utils.parseEther('3.8') const KEEP_CONFIG = { multiplierBase: 0, - bufferBase: 1_000_000, + bufferBase: 0, multiplierCalldata: 0, - bufferCalldata: 500_000, + bufferCalldata: 0, } const MAKER_ORDER = { @@ -109,7 +109,8 @@ describe('Manager', () => { updatedAt: 0, answeredInRound: 0, }) - await manager.initialize(ethOracle.address, KEEP_CONFIG) + // no need for meaningful keep configs, as keeper compensation is not tested here + await manager.initialize(ethOracle.address, KEEP_CONFIG, KEEP_CONFIG) } before(async () => { @@ -165,6 +166,24 @@ describe('Manager', () => { compareOrders(order, replacement) }) + it('prevents user from reducing maxFee', async () => { + // submit the original order + await manager.connect(userA).placeOrder(market.address, nextOrderId, MAKER_ORDER) + + // user cannot reduce maxFee + const replacement = { ...MAKER_ORDER } + replacement.maxFee = MAKER_ORDER.maxFee.sub(1) + await expect( + manager.connect(userA).placeOrder(market.address, nextOrderId, replacement), + ).to.be.revertedWithCustomError(manager, 'ManagerCannotReduceMaxFee') + + // user cannot zero maxFee + replacement.maxFee = constants.Zero + await expect( + manager.connect(userA).placeOrder(market.address, nextOrderId, replacement), + ).to.be.revertedWithCustomError(manager, 'ManagerCannotReduceMaxFee') + }) + it('keeper can execute orders', async () => { // place a maker and long order const nonce1 = advanceOrderId() diff --git a/packages/perennial-vault/contracts/Vault.sol b/packages/perennial-vault/contracts/Vault.sol index f2cb5e954..6a374e73b 100644 --- a/packages/perennial-vault/contracts/Vault.sol +++ b/packages/perennial-vault/contracts/Vault.sol @@ -387,7 +387,7 @@ contract Vault is IVault, Instance { .allocate( deposit, withdrawal, - _ineligible(context, withdrawal) + _ineligible(context, deposit, withdrawal) ); for (uint256 marketId; marketId < context.registrations.length; marketId++) @@ -400,15 +400,16 @@ contract Vault is IVault, Instance { /// @notice Returns the amount of collateral is ineligible for allocation /// @param context The context to use + /// @param deposit The amount of assets that are being deposited into the vault /// @param withdrawal The amount of assets that need to be withdrawn from the markets into the vault /// @return The amount of assets that are ineligible from being allocated - function _ineligible(Context memory context, UFixed6 withdrawal) private pure returns (UFixed6) { + function _ineligible(Context memory context, UFixed6 deposit, UFixed6 withdrawal) private pure returns (UFixed6) { // assets eligible for redemption UFixed6 redemptionEligible = UFixed6Lib.unsafeFrom(context.totalCollateral) // assets pending claim (use latest global assets before withdrawal for redeemability) .unsafeSub(context.global.assets.add(withdrawal)) // assets pending deposit - .unsafeSub(context.global.deposit); + .unsafeSub(context.global.deposit.sub(deposit)); return redemptionEligible // approximate assets up for redemption diff --git a/packages/perennial-vault/contracts/types/Account.sol b/packages/perennial-vault/contracts/types/Account.sol index 63085df50..d5fef1ac5 100644 --- a/packages/perennial-vault/contracts/types/Account.sol +++ b/packages/perennial-vault/contracts/types/Account.sol @@ -12,10 +12,10 @@ struct Account { /// @dev The latest position id uint256 latest; - /// @dev The total shares + /// @dev Measures ownership of the vault, convertable to assets UFixed6 shares; - /// @dev The total assets + /// @dev Redeemed shares converted to funds set aside to be claimed UFixed6 assets; /// @dev The amount of pending deposits diff --git a/packages/perennial-vault/test/integration/vault/Vault.test.ts b/packages/perennial-vault/test/integration/vault/Vault.test.ts index acee9f29f..295e85b11 100644 --- a/packages/perennial-vault/test/integration/vault/Vault.test.ts +++ b/packages/perennial-vault/test/integration/vault/Vault.test.ts @@ -1409,6 +1409,36 @@ describe('Vault', () => { await vault.rebalance(user.address) }) + it('deposit correctly calculates funds eligible for allocation', async () => { + const ONE = parse6decimal('1') + // two users deposit funds, vault is balanced + await vault.connect(user).update(user.address, parse6decimal('50000'), 0, 0) + await vault.connect(user2).update(user2.address, parse6decimal('25000'), 0, 0) + await updateOracle() + await vault.rebalance(user.address) + await vault.settle(user.address) + await vault.settle(user2.address) + const btcCollateral = await btcCollateralInVault() + expect(await collateralInVault()).to.be.closeTo(btcCollateral.mul(4), ONE) + + // price drops from 38838 to 25535 while vault's maker position has short exposure + await updateOracle(undefined, parse6decimal('25535')) + + // user2 converts all shares to assets + await vault.connect(user2).update(user2.address, 0, ethers.constants.MaxUint256, 0) + + // in same version, user makes another deposit + await vault.connect(user).update(user.address, parse6decimal('50000'), 0, 0) + + // settle and confirm positions did not exclude assets from the second deposit + await updateOracle() + await vault.settle(user.address) + // position = assets * leverage / price = 85088.172487 * 4 / 2620.237388 + expect(await position()).to.be.closeTo(parse6decimal('129.893837'), ONE) + // position = assets * leverage / price = 21272.043121 * 4 / 25535 + expect(await btcPosition()).to.be.closeTo(parse6decimal('3.332217'), ONE) + }) + it('product closing closes all positions', async () => { const largeDeposit = parse6decimal('10000') await vault.connect(user).update(user.address, largeDeposit, 0, 0) diff --git a/packages/perennial/contracts/MarketFactory.sol b/packages/perennial/contracts/MarketFactory.sol index 3fb63aed2..f851fc0a0 100644 --- a/packages/perennial/contracts/MarketFactory.sol +++ b/packages/perennial/contracts/MarketFactory.sol @@ -18,9 +18,6 @@ contract MarketFactory is IMarketFactory, Factory { /// @dev The global protocol parameters ProtocolParameterStorage private _parameter; - /// @dev Mapping of allowed protocol-wide operators - mapping(address => bool) public extensions; - /// @dev Mapping of allowed operators per account mapping(address => mapping(address => bool)) public operators; @@ -34,6 +31,9 @@ contract MarketFactory is IMarketFactory, Factory { /// @dev Mapping of allowed signers for each account mapping(address => mapping(address => bool)) public signers; + /// @dev Mapping of allowed protocol-wide operators + mapping(address => bool) public extensions; + /// @notice Constructs the contract /// @param oracleFactory_ The oracle factory /// @param verifier_ The verifier contract diff --git a/packages/perennial/contracts/types/MarketParameter.sol b/packages/perennial/contracts/types/MarketParameter.sol index 54bb6ad63..e78e2ba1d 100644 --- a/packages/perennial/contracts/types/MarketParameter.sol +++ b/packages/perennial/contracts/types/MarketParameter.sol @@ -46,7 +46,6 @@ using MarketParameterStorageLib for MarketParameterStorage global; /// uint24 interestFee; // <= 1677% /// uint24 makerFee; // <= 1677% /// uint24 takerFee; // <= 1677% -/// uint24 __unallocated__; // <= 1677% /// uint24 riskFee; // <= 1677% /// uint16 maxPendingGlobal; // <= 65k /// uint16 maxPendingLocal; // <= 65k @@ -61,7 +60,7 @@ library MarketParameterStorageLib { function read(MarketParameterStorage storage self) internal view returns (MarketParameter memory) { uint256 slot0 = self.slot0; - uint256 flags = uint256(slot0 << (256 - 24 - 24 - 24 - 24 - 24 - 24 - 16 - 16 - 48 - 8)) >> (256 - 8); + uint256 flags = uint256(slot0 << (256 - 24 - 24 - 24 - 24 - 24 - 16 - 16 - 48 - 8)) >> (256 - 8); (bool closed, bool settle) = (flags & 0x04 == 0x04, flags & 0x08 == 0x08); @@ -70,9 +69,9 @@ library MarketParameterStorageLib { UFixed6.wrap(uint256(slot0 << (256 - 24 - 24)) >> (256 - 24)), UFixed6.wrap(uint256(slot0 << (256 - 24 - 24 - 24)) >> (256 - 24)), UFixed6.wrap(uint256(slot0 << (256 - 24 - 24 - 24 - 24)) >> (256 - 24)), - UFixed6.wrap(uint256(slot0 << (256 - 24 - 24 - 24 - 24 - 24 - 24)) >> (256 - 24)), - uint256(slot0 << (256 - 24 - 24 - 24 - 24 - 24 - 24 - 16)) >> (256 - 16), - uint256(slot0 << (256 - 24 - 24 - 24 - 24 - 24 - 24 - 16 - 16)) >> (256 - 16), + UFixed6.wrap(uint256(slot0 << (256 - 24 - 24 - 24 - 24 - 24)) >> (256 - 24)), + uint256(slot0 << (256 - 24 - 24 - 24 - 24 - 24 - 16)) >> (256 - 16), + uint256(slot0 << (256 - 24 - 24 - 24 - 24 - 24 - 16 - 16)) >> (256 - 16), closed, settle ); @@ -108,10 +107,10 @@ library MarketParameterStorageLib { uint256(UFixed6.unwrap(newValue.interestFee) << (256 - 24)) >> (256 - 24 - 24) | uint256(UFixed6.unwrap(newValue.makerFee) << (256 - 24)) >> (256 - 24 - 24 - 24) | uint256(UFixed6.unwrap(newValue.takerFee) << (256 - 24)) >> (256 - 24 - 24 - 24 - 24) | - uint256(UFixed6.unwrap(newValue.riskFee) << (256 - 24)) >> (256 - 24 - 24 - 24 - 24 - 24 - 24) | - uint256(newValue.maxPendingGlobal << (256 - 16)) >> (256 - 24 - 24 - 24 - 24 - 24 - 24 - 16) | - uint256(newValue.maxPendingLocal << (256 - 16)) >> (256 - 24 - 24 - 24 - 24 - 24 - 24 - 16 - 16) | - uint256(flags << (256 - 8)) >> (256 - 24 - 24 - 24 - 24 - 24 - 24 - 16 - 16 - 48 - 8); + uint256(UFixed6.unwrap(newValue.riskFee) << (256 - 24)) >> (256 - 24 - 24 - 24 - 24 - 24) | + uint256(newValue.maxPendingGlobal << (256 - 16)) >> (256 - 24 - 24 - 24 - 24 - 24 - 16) | + uint256(newValue.maxPendingLocal << (256 - 16)) >> (256 - 24 - 24 - 24 - 24 - 24 - 16 - 16) | + uint256(flags << (256 - 8)) >> (256 - 24 - 24 - 24 - 24 - 24 - 16 - 16 - 48 - 8); assembly { sstore(self.slot, encoded0) diff --git a/packages/perennial/contracts/types/Position.sol b/packages/perennial/contracts/types/Position.sol index 45898b833..0cb0e6363 100644 --- a/packages/perennial/contracts/types/Position.sol +++ b/packages/perennial/contracts/types/Position.sol @@ -319,12 +319,12 @@ library PositionLib { /// library PositionStorageGlobalLib { function read(PositionStorageGlobal storage self) internal view returns (Position memory) { - (uint256 slot0, uint256 slot1) = (self.slot0, self.slot1); + uint256 slot0 = self.slot0; return Position( uint256(slot0 << (256 - 32)) >> (256 - 32), - UFixed6.wrap(uint256(slot1 << (256 - 64)) >> (256 - 64)), - UFixed6.wrap(uint256(slot0 << (256 - 32 - 48 - 48 - 64)) >> (256 - 64)), - UFixed6.wrap(uint256(slot0 << (256 - 32 - 48 - 48 - 64 - 64)) >> (256 - 64)) + UFixed6.wrap(uint256(slot0 << (256 - 32 - 32 - 64)) >> (256 - 64)), + UFixed6.wrap(uint256(slot0 << (256 - 32 - 32 - 64 - 64)) >> (256 - 64)), + UFixed6.wrap(uint256(slot0 << (256 - 32 - 32 - 64 - 64 - 64)) >> (256 - 64)) ); } @@ -337,14 +337,12 @@ library PositionStorageGlobalLib { uint256 encoded0 = uint256(newValue.timestamp << (256 - 32)) >> (256 - 32) | - uint256(UFixed6.unwrap(newValue.long) << (256 - 64)) >> (256 - 32 - 48 - 48 - 64) | - uint256(UFixed6.unwrap(newValue.short) << (256 - 64)) >> (256 - 32 - 48 - 48 - 64 - 64); - uint256 encoded1 = - uint256(UFixed6.unwrap(newValue.maker) << (256 - 64)) >> (256 - 64); + uint256(UFixed6.unwrap(newValue.maker) << (256 - 64)) >> (256 - 32 - 32 - 64) | + uint256(UFixed6.unwrap(newValue.long) << (256 - 64)) >> (256 - 32 - 32 - 64 - 64) | + uint256(UFixed6.unwrap(newValue.short) << (256 - 64)) >> (256 - 32 - 32 - 64 - 64 - 64); assembly { sstore(self.slot, encoded0) - sstore(add(self.slot, 1), encoded1) } } @@ -359,6 +357,9 @@ library PositionStorageGlobalLib { position.maker = deprecatedMaker; store(self, position); + assembly { + sstore(add(self.slot, 1), 0) // Part of the v2.3 migration. Can be removed once migration is complete. + } } } @@ -425,6 +426,7 @@ library PositionStorageLocalLib { assembly { sstore(self.slot, encoded0) + sstore(add(self.slot, 1), 0) // Part of the v2.3 migration. Can be removed once migration is complete. } } } diff --git a/packages/perennial/contracts/types/ProtocolParameter.sol b/packages/perennial/contracts/types/ProtocolParameter.sol index c4c4177e0..dbe4f9803 100644 --- a/packages/perennial/contracts/types/ProtocolParameter.sol +++ b/packages/perennial/contracts/types/ProtocolParameter.sol @@ -28,9 +28,12 @@ struct ProtocolParameter { /// @dev The minimum ratio between scale vs makerLimit / efficiencyLimit UFixed6 minScale; + + /// @dev The maximum for parameter restricting maximum time between oracle version and update + uint256 maxStaleAfter; } struct StoredProtocolParameter { - /* slot 0 (28) */ + /* slot 0 (31) */ uint24 maxFee; // <= 1677% uint48 maxFeeAbsolute; // <= 281m uint24 maxCut; // <= 1677% @@ -39,6 +42,7 @@ struct StoredProtocolParameter { uint24 minEfficiency; // <= 1677% uint24 referralFee; // <= 1677% uint24 minScale; // <= 1677% + uint24 maxStaleAfter; // <= 4660 hours } struct ProtocolParameterStorage { StoredProtocolParameter value; } // SECURITY: must remain at (1) slots using ProtocolParameterStorageLib for ProtocolParameterStorage global; @@ -58,7 +62,8 @@ library ProtocolParameterStorageLib { UFixed6.wrap(uint256(value.minMaintenance)), UFixed6.wrap(uint256(value.minEfficiency)), UFixed6.wrap(uint256(value.referralFee)), - UFixed6.wrap(uint256(value.minScale)) + UFixed6.wrap(uint256(value.minScale)), + uint24(value.maxStaleAfter) ); } @@ -76,6 +81,7 @@ library ProtocolParameterStorageLib { if (newValue.maxRate.gt(UFixed6.wrap(type(uint32).max / 2))) revert ProtocolParameterStorageInvalidError(); if (newValue.minMaintenance.gt(UFixed6.wrap(type(uint24).max))) revert ProtocolParameterStorageInvalidError(); if (newValue.minEfficiency.gt(UFixed6.wrap(type(uint24).max))) revert ProtocolParameterStorageInvalidError(); + if (newValue.maxStaleAfter > uint256(type(uint24).max)) revert ProtocolParameterStorageInvalidError(); self.value = StoredProtocolParameter( uint24(UFixed6.unwrap(newValue.maxFee)), @@ -85,7 +91,8 @@ library ProtocolParameterStorageLib { uint24(UFixed6.unwrap(newValue.minMaintenance)), uint24(UFixed6.unwrap(newValue.minEfficiency)), uint24(UFixed6.unwrap(newValue.referralFee)), - uint24(UFixed6.unwrap(newValue.minScale)) + uint24(UFixed6.unwrap(newValue.minScale)), + uint24(newValue.maxStaleAfter) ); } } \ No newline at end of file diff --git a/packages/perennial/contracts/types/RiskParameter.sol b/packages/perennial/contracts/types/RiskParameter.sol index 0b161b2cb..30d621c10 100644 --- a/packages/perennial/contracts/types/RiskParameter.sol +++ b/packages/perennial/contracts/types/RiskParameter.sol @@ -144,6 +144,8 @@ library RiskParameterStorageLib { .gt(protocolParameter.maxRate) ) revert RiskParameterStorageInvalidError(); + if (self.staleAfter > protocolParameter.maxStaleAfter) revert RiskParameterStorageInvalidError(); + if (self.maintenance.lt(protocolParameter.minMaintenance)) revert RiskParameterStorageInvalidError(); if (self.margin.lt(self.maintenance)) revert RiskParameterStorageInvalidError(); diff --git a/packages/perennial/test/integration/core/happyPath.test.ts b/packages/perennial/test/integration/core/happyPath.test.ts index 8cbb63261..a528e6dff 100644 --- a/packages/perennial/test/integration/core/happyPath.test.ts +++ b/packages/perennial/test/integration/core/happyPath.test.ts @@ -110,13 +110,11 @@ describe('Happy Path', () => { const parameter = { fundingFee: parse6decimal('0.1'), interestFee: parse6decimal('0.1'), - oracleFee: 0, riskFee: 0, makerFee: 0, takerFee: 0, maxPendingGlobal: 8, maxPendingLocal: 8, - settlementFee: 0, closed: false, settle: false, } @@ -1287,9 +1285,7 @@ describe('Happy Path', () => { const parameter = { fundingFee: parse6decimal('0.1'), interestFee: parse6decimal('0.1'), - oracleFee: 0, riskFee: 0, - settlementFee: 0, maxPendingGlobal: 8, maxPendingLocal: 8, makerFee: positionFeesOn ? parse6decimal('0.2') : 0, @@ -1452,9 +1448,7 @@ describe('Happy Path', () => { const parameter = { fundingFee: parse6decimal('0.1'), interestFee: parse6decimal('0.1'), - oracleFee: 0, riskFee: 0, - settlementFee: 0, maxPendingGlobal: 8, maxPendingLocal: 8, makerFee: parse6decimal('0.2'), @@ -2517,9 +2511,7 @@ describe('Happy Path', () => { const parameter = { fundingFee: parse6decimal('0.1'), interestFee: parse6decimal('0.1'), - oracleFee: 0, riskFee: 0, - settlementFee: 0, maxPendingGlobal: 8, maxPendingLocal: 8, makerFee: positionFeesOn ? parse6decimal('0.2') : 0, diff --git a/packages/perennial/test/integration/helpers/setupHelpers.ts b/packages/perennial/test/integration/helpers/setupHelpers.ts index 648945d00..2254c9003 100644 --- a/packages/perennial/test/integration/helpers/setupHelpers.ts +++ b/packages/perennial/test/integration/helpers/setupHelpers.ts @@ -162,6 +162,7 @@ export async function deployProtocol(chainlinkContext?: ChainlinkContext): Promi minEfficiency: parse6decimal('0.1'), referralFee: 0, minScale: parse6decimal('0.001'), + maxStaleAfter: 172800, // 2 days }) await oracleFactory.connect(owner).register(chainlink.oracleFactory.address) await oracleFactory.connect(owner).updateParameter({ @@ -265,13 +266,11 @@ export async function createMarket( const marketParameter = { fundingFee: parse6decimal('0.1'), interestFee: parse6decimal('0.1'), - oracleFee: 0, riskFee: 0, makerFee: 0, takerFee: 0, maxPendingGlobal: 8, maxPendingLocal: 8, - settlementFee: 0, closed: false, settle: false, ...marketParamOverrides, diff --git a/packages/perennial/test/unit/market/Market.test.ts b/packages/perennial/test/unit/market/Market.test.ts index 23b304ddd..1370eaa09 100644 --- a/packages/perennial/test/unit/market/Market.test.ts +++ b/packages/perennial/test/unit/market/Market.test.ts @@ -462,6 +462,7 @@ describe('Market', () => { minEfficiency: parse6decimal('0.1'), referralFee: 0, minScale: parse6decimal('0.001'), + maxStaleAfter: 14400, }) factory.oracleFactory.returns(oracleFactorySigner.address) @@ -16465,6 +16466,7 @@ describe('Market', () => { minEfficiency: parse6decimal('0.1'), referralFee: parse6decimal('0.20'), minScale: parse6decimal('0.001'), + maxStaleAfter: 14400, }) oracle.at.whenCalledWith(ORACLE_VERSION_2.timestamp).returns([ORACLE_VERSION_2, INITIALIZED_ORACLE_RECEIPT]) @@ -16526,6 +16528,7 @@ describe('Market', () => { minEfficiency: parse6decimal('0.1'), referralFee: parse6decimal('0.20'), minScale: parse6decimal('0.001'), + maxStaleAfter: 14400, }) oracle.at.whenCalledWith(ORACLE_VERSION_2.timestamp).returns([ORACLE_VERSION_2, INITIALIZED_ORACLE_RECEIPT]) @@ -18092,6 +18095,7 @@ describe('Market', () => { minEfficiency: parse6decimal('0.1'), referralFee: parse6decimal('0.20'), minScale: parse6decimal('0.001'), + maxStaleAfter: 14400, }) const marketParameter = { ...(await market.parameter()) } @@ -20042,6 +20046,7 @@ describe('Market', () => { minEfficiency: parse6decimal('0.1'), referralFee: parse6decimal('0.20'), minScale: parse6decimal('0.001'), + maxStaleAfter: 14400, }) const riskParameter = { ...(await market.riskParameter()) } @@ -20166,6 +20171,7 @@ describe('Market', () => { minEfficiency: parse6decimal('0.1'), referralFee: parse6decimal('0.20'), minScale: parse6decimal('0.001'), + maxStaleAfter: 14400, }) const riskParameter = { ...(await market.riskParameter()) } @@ -20330,6 +20336,7 @@ describe('Market', () => { minEfficiency: parse6decimal('0.1'), referralFee: parse6decimal('0.20'), minScale: parse6decimal('0.001'), + maxStaleAfter: 14400, }) const riskParameter = { ...(await market.riskParameter()) } @@ -20501,6 +20508,7 @@ describe('Market', () => { minEfficiency: parse6decimal('0.1'), referralFee: parse6decimal('0.20'), minScale: parse6decimal('0.001'), + maxStaleAfter: 14400, }) const marketParameter = { ...(await market.parameter()) } @@ -20746,6 +20754,7 @@ describe('Market', () => { minEfficiency: parse6decimal('0.1'), referralFee: parse6decimal('0.20'), minScale: parse6decimal('0.001'), + maxStaleAfter: 14400, }) const marketParameter = { ...(await market.parameter()) } @@ -20992,6 +21001,7 @@ describe('Market', () => { minEfficiency: parse6decimal('0.1'), referralFee: parse6decimal('0.20'), minScale: parse6decimal('0.001'), + maxStaleAfter: 14400, }) const marketParameter = { ...(await market.parameter()) } @@ -21238,6 +21248,7 @@ describe('Market', () => { minEfficiency: parse6decimal('0.1'), referralFee: parse6decimal('0.20'), minScale: parse6decimal('0.001'), + maxStaleAfter: 14400, }) const marketParameter = { ...(await market.parameter()) } @@ -21483,6 +21494,7 @@ describe('Market', () => { minEfficiency: parse6decimal('0.1'), referralFee: parse6decimal('0.20'), minScale: parse6decimal('0.001'), + maxStaleAfter: 14400, }) const marketParameter = { ...(await market.parameter()) } @@ -21828,6 +21840,7 @@ describe('Market', () => { minEfficiency: parse6decimal('0.1'), referralFee: parse6decimal('0.20'), minScale: parse6decimal('0.001'), + maxStaleAfter: 14400, }) const marketParameter = { ...(await market.parameter()) } @@ -22079,6 +22092,7 @@ describe('Market', () => { minEfficiency: parse6decimal('0.1'), referralFee: parse6decimal('0.20'), minScale: parse6decimal('0.001'), + maxStaleAfter: 14400, }) const marketParameter = { ...(await market.parameter()) } @@ -22340,6 +22354,7 @@ describe('Market', () => { minEfficiency: parse6decimal('0.1'), referralFee: parse6decimal('0.20'), minScale: parse6decimal('0.001'), + maxStaleAfter: 14400, }) const marketParameter = { ...(await market.parameter()) } @@ -22601,6 +22616,7 @@ describe('Market', () => { minEfficiency: parse6decimal('0.1'), referralFee: parse6decimal('0.20'), minScale: parse6decimal('0.001'), + maxStaleAfter: 14400, }) const marketParameter = { ...(await market.parameter()) } @@ -22863,6 +22879,7 @@ describe('Market', () => { minEfficiency: parse6decimal('0.1'), referralFee: parse6decimal('0.20'), minScale: parse6decimal('0.001'), + maxStaleAfter: 14400, }) const marketParameter = { ...(await market.parameter()) } @@ -23125,6 +23142,7 @@ describe('Market', () => { minEfficiency: parse6decimal('0.1'), referralFee: parse6decimal('0.20'), minScale: parse6decimal('0.001'), + maxStaleAfter: 14400, }) const marketParameter = { ...(await market.parameter()) } @@ -23393,6 +23411,7 @@ describe('Market', () => { minEfficiency: parse6decimal('0.1'), referralFee: parse6decimal('0.20'), minScale: parse6decimal('0.001'), + maxStaleAfter: 14400, }) const marketParameter = { ...(await market.parameter()) } @@ -23662,6 +23681,7 @@ describe('Market', () => { minEfficiency: parse6decimal('0.1'), referralFee: parse6decimal('0.20'), minScale: parse6decimal('0.001'), + maxStaleAfter: 14400, }) const marketParameter = { ...(await market.parameter()) } @@ -23719,6 +23739,7 @@ describe('Market', () => { minEfficiency: parse6decimal('0.1'), referralFee: parse6decimal('0.20'), minScale: parse6decimal('0.001'), + maxStaleAfter: 14400, }) const marketParameter = { ...(await market.parameter()) } @@ -23776,6 +23797,7 @@ describe('Market', () => { minEfficiency: parse6decimal('0.1'), referralFee: parse6decimal('0.20'), minScale: parse6decimal('0.001'), + maxStaleAfter: 14400, }) const marketParameter = { ...(await market.parameter()) } @@ -23850,6 +23872,7 @@ describe('Market', () => { minEfficiency: parse6decimal('0.1'), referralFee: parse6decimal('0.20'), minScale: parse6decimal('0.001'), + maxStaleAfter: 14400, }) const riskParameter = { ...(await market.riskParameter()) } diff --git a/packages/perennial/test/unit/marketfactory/MarketFactory.test.ts b/packages/perennial/test/unit/marketfactory/MarketFactory.test.ts index 31666d936..5ab7730be 100644 --- a/packages/perennial/test/unit/marketfactory/MarketFactory.test.ts +++ b/packages/perennial/test/unit/marketfactory/MarketFactory.test.ts @@ -339,6 +339,7 @@ describe('MarketFactory', () => { minEfficiency: parse6decimal('0.1'), referralFee: parse6decimal('0.2'), minScale: parse6decimal('0.001'), + maxStaleAfter: 3600, } it('updates the parameters', async () => { @@ -353,6 +354,7 @@ describe('MarketFactory', () => { expect(parameter.minEfficiency).to.equal(newParameter.minEfficiency) expect(parameter.referralFee).to.equal(newParameter.referralFee) expect(parameter.minScale).to.equal(newParameter.minScale) + expect(parameter.maxStaleAfter).to.equal(newParameter.maxStaleAfter) }) it('reverts if not owner', async () => { diff --git a/packages/perennial/test/unit/types/Global.test.ts b/packages/perennial/test/unit/types/Global.test.ts index 9e7b9c581..152a82522 100644 --- a/packages/perennial/test/unit/types/Global.test.ts +++ b/packages/perennial/test/unit/types/Global.test.ts @@ -62,6 +62,7 @@ function generateProtocolParameter(protocolFee: BigNumberish): ProtocolParameter minEfficiency: 0, referralFee: 0, minScale: 0, + maxStaleAfter: 172800, // 2 days } } diff --git a/packages/perennial/test/unit/types/MarketParameter.test.ts b/packages/perennial/test/unit/types/MarketParameter.test.ts index e577a1822..60809bf7d 100644 --- a/packages/perennial/test/unit/types/MarketParameter.test.ts +++ b/packages/perennial/test/unit/types/MarketParameter.test.ts @@ -39,6 +39,7 @@ const PROTOCOL_PARAMETER: ProtocolParameterStruct = { minEfficiency: 0, referralFee: 0, minScale: parse6decimal('0.1'), + maxStaleAfter: 172800, // 2 days } describe('MarketParameter', () => { diff --git a/packages/perennial/test/unit/types/ProtocolParameter.test.ts b/packages/perennial/test/unit/types/ProtocolParameter.test.ts index cdaf4dd44..3416ce97e 100644 --- a/packages/perennial/test/unit/types/ProtocolParameter.test.ts +++ b/packages/perennial/test/unit/types/ProtocolParameter.test.ts @@ -20,6 +20,7 @@ export const VALID_PROTOCOL_PARAMETER: ProtocolParameterStruct = { minEfficiency: 8, referralFee: 9, minScale: 10, + maxStaleAfter: 11, } describe('ProtocolParameter', () => { @@ -46,6 +47,7 @@ describe('ProtocolParameter', () => { expect(value.minEfficiency).to.equal(8) expect(value.referralFee).to.equal(9) expect(value.minScale).to.equal(10) + expect(value.maxStaleAfter).to.equal(11) }) context('.maxFee', async () => { @@ -212,5 +214,26 @@ describe('ProtocolParameter', () => { ).to.be.revertedWithCustomError(protocolParameter, 'ProtocolParameterStorageInvalidError') }) }) + + context('.maxStaleAfter', async () => { + const STORAGE_SIZE = 24 + it('saves if in range', async () => { + await protocolParameter.validateAndStore({ + ...VALID_PROTOCOL_PARAMETER, + maxStaleAfter: BigNumber.from(2).pow(STORAGE_SIZE).sub(1), + }) + const value = await protocolParameter.read() + expect(value.maxStaleAfter).to.equal(BigNumber.from(2).pow(STORAGE_SIZE).sub(1)) + }) + + it('reverts if out of range', async () => { + await expect( + protocolParameter.validateAndStore({ + ...VALID_PROTOCOL_PARAMETER, + maxStaleAfter: BigNumber.from(2).pow(STORAGE_SIZE), + }), + ).to.be.revertedWithCustomError(protocolParameter, 'ProtocolParameterStorageInvalidError') + }) + }) }) }) diff --git a/packages/perennial/test/unit/types/RiskParameter.test.ts b/packages/perennial/test/unit/types/RiskParameter.test.ts index 8bbb4b318..f50d823f2 100644 --- a/packages/perennial/test/unit/types/RiskParameter.test.ts +++ b/packages/perennial/test/unit/types/RiskParameter.test.ts @@ -61,6 +61,7 @@ const PROTOCOL_PARAMETER: ProtocolParameterStruct = { minEfficiency: parse6decimal('0.5'), referralFee: 0, minScale: parse6decimal('0.10'), + maxStaleAfter: 3600, } describe('RiskParameter', () => { @@ -808,12 +809,12 @@ describe('RiskParameter', () => { await riskParameter.validateAndStore( { ...VALID_RISK_PARAMETER, - staleAfter: BigNumber.from(2).pow(STORAGE_SIZE).sub(1), + staleAfter: BigNumber.from(1800), }, PROTOCOL_PARAMETER, ) const value = await riskParameter.read() - expect(value.staleAfter).to.equal(BigNumber.from(2).pow(STORAGE_SIZE).sub(1)) + expect(value.staleAfter).to.equal(BigNumber.from(1800)) }) it('reverts if out of range', async () => { @@ -827,6 +828,18 @@ describe('RiskParameter', () => { ), ).to.be.revertedWithCustomError(riskParameterStorage, 'RiskParameterStorageInvalidError') }) + + it('reverts if invalid', async () => { + await expect( + riskParameter.validateAndStore( + { + ...VALID_RISK_PARAMETER, + staleAfter: BigNumber.from(PROTOCOL_PARAMETER.maxStaleAfter).add(1), + }, + PROTOCOL_PARAMETER, + ), + ).to.be.revertedWithCustomError(riskParameterStorage, 'RiskParameterStorageInvalidError') + }) }) describe('.liquidationFee', () => {