From 398717d2f8b961be443c8c5efbfd84c8c1193acb Mon Sep 17 00:00:00 2001 From: Brechtpd Date: Mon, 31 May 2021 23:54:46 +0200 Subject: [PATCH 1/4] [AMM] Fix pools with active asset manager --- packages/loopring_v3.js/src/exchange_v3.ts | 2 +- .../contracts/amm/IAssetManager.sol | 8 ++ .../contracts/amm/LoopringAmmPool.sol | 43 +++--- .../amm/libamm/AmmAssetManagement.sol | 52 ++++++++ .../contracts/amm/libamm/AmmData.sol | 5 +- .../amm/libamm/AmmDepositProcess.sol | 74 +++++++++++ .../amm/libamm/AmmTransactionReceiver.sol | 125 ++++-------------- .../contracts/amm/libamm/AmmUpdateProcess.sol | 16 ++- .../amm/libamm/AmmWithdrawProcess.sol | 82 ++++++++++++ .../aux/access/LoopringIOExchangeOwner.sol | 14 +- .../contracts/test/LoopringAmmPoolCopy.sol | 6 +- .../contracts/test/TestAssetManager.sol | 20 ++- .../loopring_v3/migrations/7_deploy_amm.js | 4 + packages/loopring_v3/test/ammUtils.ts | 36 ++--- packages/loopring_v3/test/testAMMPool.ts | 19 ++- packages/loopring_v3/test/testExchangeUtil.ts | 4 +- 16 files changed, 352 insertions(+), 158 deletions(-) create mode 100644 packages/loopring_v3/contracts/amm/IAssetManager.sol create mode 100644 packages/loopring_v3/contracts/amm/libamm/AmmAssetManagement.sol create mode 100644 packages/loopring_v3/contracts/amm/libamm/AmmDepositProcess.sol create mode 100644 packages/loopring_v3/contracts/amm/libamm/AmmWithdrawProcess.sol diff --git a/packages/loopring_v3.js/src/exchange_v3.ts b/packages/loopring_v3.js/src/exchange_v3.ts index c277153a6..79c7b8e96 100644 --- a/packages/loopring_v3.js/src/exchange_v3.ts +++ b/packages/loopring_v3.js/src/exchange_v3.ts @@ -626,7 +626,7 @@ export class ExchangeV3 { // Get the block data from the transaction data //const submitBlocksFunctionSignature = "0x8dadd3af"; // submitBlocks - const submitBlocksFunctionSignature = "0xc39ce618"; // submitBlocksWithCallbacks + const submitBlocksFunctionSignature = "0xfbb404ab"; // submitBlocksWithCallbacks const transaction = await this.web3.eth.getTransaction( event.transactionHash diff --git a/packages/loopring_v3/contracts/amm/IAssetManager.sol b/packages/loopring_v3/contracts/amm/IAssetManager.sol new file mode 100644 index 000000000..3fe23e360 --- /dev/null +++ b/packages/loopring_v3/contracts/amm/IAssetManager.sol @@ -0,0 +1,8 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2017 Loopring Technology Limited. +pragma solidity ^0.7.0; + + +/// @author Brecht Devos - +interface IAssetManager +{} diff --git a/packages/loopring_v3/contracts/amm/LoopringAmmPool.sol b/packages/loopring_v3/contracts/amm/LoopringAmmPool.sol index 553933644..2dd4a8f72 100644 --- a/packages/loopring_v3/contracts/amm/LoopringAmmPool.sol +++ b/packages/loopring_v3/contracts/amm/LoopringAmmPool.sol @@ -7,12 +7,13 @@ import "../aux/access/ITransactionReceiver.sol"; import "../core/iface/IAgentRegistry.sol"; import "../lib/ReentrancyGuard.sol"; import "../lib/TransferUtil.sol"; -import "./libamm/AmmTransactionReceiver.sol"; +import "./libamm/AmmAssetManagement.sol"; import "./libamm/AmmData.sol"; import "./libamm/AmmExitRequest.sol"; import "./libamm/AmmJoinRequest.sol"; import "./libamm/AmmPoolToken.sol"; import "./libamm/AmmStatus.sol"; +import "./libamm/AmmTransactionReceiver.sol"; import "./libamm/AmmWithdrawal.sol"; import "./PoolToken.sol"; @@ -24,11 +25,12 @@ contract LoopringAmmPool is ITransactionReceiver, ReentrancyGuard { - using AmmTransactionReceiver for AmmData.State; + using AmmAssetManagement for AmmData.State; using AmmJoinRequest for AmmData.State; using AmmExitRequest for AmmData.State; using AmmPoolToken for AmmData.State; using AmmStatus for AmmData.State; + using AmmTransactionReceiver for AmmData.State; using AmmWithdrawal for AmmData.State; using TransferUtil for address; @@ -38,7 +40,7 @@ contract LoopringAmmPool is event Shutdown(uint timestamp); IAmmController public immutable controller; - address public immutable assetManager; + IAssetManager public immutable assetManager; bool public immutable joinsDisabled; modifier onlyFromExchangeOwner() @@ -49,7 +51,7 @@ contract LoopringAmmPool is modifier onlyFromAssetManager() { - require(msg.sender == assetManager, "UNAUTHORIZED"); + require(msg.sender == address(assetManager), "UNAUTHORIZED"); _; } @@ -73,7 +75,7 @@ contract LoopringAmmPool is constructor( IAmmController _controller, - address _assetManager, + IAssetManager _assetManager, bool _joinsDisabled ) { @@ -183,29 +185,36 @@ contract LoopringAmmPool is state.withdrawWhenOffline(); } - function depositAssetsToL2( - uint96[] memory amounts + function transferOut( + address to, + address token, + uint amount ) external nonReentrant - onlyWhenOnline - onlyFromExchangeOwner + onlyFromAssetManager { - for(uint i = 0; i < state.tokens.length; i++) { - require(amounts[i] != 0, "INVALID_DEPOSIT_AMOUNT"); - state.deposit(state.tokens[i].addr, amounts[i]); - } + state.transferOut(to, token, amount); } - function transferOut( - address to, + function setBalanceL1( address token, - uint amount + uint96 balance ) external nonReentrant onlyFromAssetManager { - token.transferOut(to, amount); + state.balancesL1[token] = balance; + } + + function getBalanceL1( + address token + ) + external + view + returns (uint96) + { + return state.balancesL1[token]; } } diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmAssetManagement.sol b/packages/loopring_v3/contracts/amm/libamm/AmmAssetManagement.sol new file mode 100644 index 000000000..7af90a742 --- /dev/null +++ b/packages/loopring_v3/contracts/amm/libamm/AmmAssetManagement.sol @@ -0,0 +1,52 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2017 Loopring Technology Limited. +pragma solidity ^0.7.0; +pragma experimental ABIEncoderV2; + +import "../../lib/ERC20.sol"; +import "../../lib/MathUint.sol"; +import "../../lib/TransferUtil.sol"; +import "./AmmData.sol"; + + +/// @title AmmAssetManagement +library AmmAssetManagement +{ + using TransferUtil for address; + + function deposit( + AmmData.State storage S, + address token, + uint96 amount + ) + public + { + if (amount == 0) { + return; + } + uint ethValue = 0; + if (token == address(0)) { + ethValue = amount; + } else { + ERC20(token).approve(address(S.exchange.getDepositContract()), amount); + } + S.exchange.deposit{value: ethValue}( + address(this), + address(this), + token, + amount, + new bytes(0) + ); + } + + function transferOut( + AmmData.State storage /*S*/, + address to, + address token, + uint amount + ) + public + { + token.transferOut(to, amount); + } +} diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmData.sol b/packages/loopring_v3/contracts/amm/libamm/AmmData.sol index 2e2d16adb..eac2a605c 100644 --- a/packages/loopring_v3/contracts/amm/libamm/AmmData.sol +++ b/packages/loopring_v3/contracts/amm/libamm/AmmData.sol @@ -6,6 +6,7 @@ pragma experimental ABIEncoderV2; import "../../core/iface/ExchangeData.sol"; import "../../core/iface/IExchangeV3.sol"; import "../IAmmController.sol"; +import "../IAssetManager.sol"; import "./IAmmSharedConfig.sol"; @@ -83,7 +84,7 @@ library AmmData struct Settings { IAmmController controller; - address assetManager; + IAssetManager assetManager; bool joinsDisabled; } @@ -139,5 +140,7 @@ library AmmData // A map from a user to the forced exit. mapping (address => PoolExit) forcedExit; mapping (bytes32 => bool) approvedTx; + + mapping (address => uint96) balancesL1; } } diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmDepositProcess.sol b/packages/loopring_v3/contracts/amm/libamm/AmmDepositProcess.sol new file mode 100644 index 000000000..ea6b07f9b --- /dev/null +++ b/packages/loopring_v3/contracts/amm/libamm/AmmDepositProcess.sol @@ -0,0 +1,74 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2017 Loopring Technology Limited. +pragma solidity ^0.7.0; +pragma experimental ABIEncoderV2; + +import "../../lib/ERC20.sol"; +import "../../lib/MathUint96.sol"; +import "../../lib/TransferUtil.sol"; +import "./AmmAssetManagement.sol"; +import "./AmmData.sol"; +import "./AmmPoolToken.sol"; +import "./AmmStatus.sol"; +import "./AmmUtil.sol"; +import "./AmmWithdrawal.sol"; + + +/// @title AmmDepositProcess +library AmmDepositProcess +{ + using AmmAssetManagement for AmmData.State; + using MathUint96 for uint96; + using TransferUtil for address; + + function processDeposit( + AmmData.State storage S, + AmmData.Context memory ctx, + AmmData.PoolDeposit memory poolDeposit + ) + internal + { + require(poolDeposit.amounts.length == ctx.tokens.length, "INVALID_DEPOSIT_DATA"); + for (uint i = 0; i < ctx.tokens.length; i++) { + verifyDepositTx(ctx, ctx.tokens[i].tokenID, poolDeposit.amounts[i]); + S.deposit(ctx.tokens[i].addr, poolDeposit.amounts[i]); + S.balancesL1[ctx.tokens[i].addr] = S.balancesL1[ctx.tokens[i].addr].sub(poolDeposit.amounts[i]); + } + } + + function verifyDepositTx( + AmmData.Context memory ctx, + uint tokenID, + uint96 amount + ) + internal + view + { + if (amount == 0) { + return; + } + + // Verify deposit data + // Start by reading the first 27 bytes into packedData + uint txsDataPtr = ctx.txsDataPtr + 27; + // packedData: txType (1) | owner (20) | accountID (4) | tokenID (2) + uint packedData; + uint96 txAmount; + assembly { + packedData := calldataload(txsDataPtr) + txAmount := calldataload(add(txsDataPtr, 12)) + } + + require( + // txType == ExchangeData.TransactionType.DEPOSIT && + // owner == address(this) && + // accountID == crx.accountID && + // tokenID == tokenID && + packedData & 0xffffffffffffffffffffffffffffffffffffffffffffffffffffff == (uint(ExchangeData.TransactionType.DEPOSIT) << 208) | (uint(address(this)) << 48) | (uint(ctx.accountID) << 16) | uint(tokenID) && + amount == txAmount, + "INVALID_DEPOSIT_TX_DATA" + ); + + ctx.txsDataPtr += ExchangeData.TX_DATA_AVAILABILITY_SIZE; + } +} \ No newline at end of file diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmTransactionReceiver.sol b/packages/loopring_v3/contracts/amm/libamm/AmmTransactionReceiver.sol index 0e5bcde48..28113d557 100644 --- a/packages/loopring_v3/contracts/amm/libamm/AmmTransactionReceiver.sol +++ b/packages/loopring_v3/contracts/amm/libamm/AmmTransactionReceiver.sol @@ -6,25 +6,29 @@ pragma experimental ABIEncoderV2; import "../../core/impl/libtransactions/BlockReader.sol"; import "../../lib/MathUint.sol"; import "../../thirdparty/SafeCast.sol"; +import "./AmmDepositProcess.sol"; import "./AmmData.sol"; import "./AmmExitProcess.sol"; import "./AmmJoinProcess.sol"; import "./AmmPoolToken.sol"; import "./AmmUpdateProcess.sol"; +import "./AmmWithdrawProcess.sol"; /// @title AmmTransactionReceiver library AmmTransactionReceiver { - using AmmExitProcess for AmmData.State; - using AmmJoinProcess for AmmData.State; - using AmmPoolToken for AmmData.State; - using AmmUtil for AmmData.State; - using AmmUpdateProcess for AmmData.Context; - using BlockReader for bytes; - using MathUint for uint; - using MathUint96 for uint96; - using SafeCast for uint; + using AmmDepositProcess for AmmData.State; + using AmmExitProcess for AmmData.State; + using AmmJoinProcess for AmmData.State; + using AmmPoolToken for AmmData.State; + using AmmUtil for AmmData.State; + using AmmUpdateProcess for AmmData.State; + using AmmWithdrawProcess for AmmData.State; + using BlockReader for bytes; + using MathUint for uint; + using MathUint96 for uint96; + using SafeCast for uint; function onReceiveTransactions( AmmData.State storage S, @@ -101,110 +105,37 @@ library AmmTransactionReceiver signature.offset := add(signature.offset, 0x20) } if (txType == AmmData.PoolTxType.JOIN) { - ctx.approveAmmUpdates(true); + S.approveAmmUpdates(ctx, true); S.processJoin( ctx, abi.decode(data, (AmmData.PoolJoin)), signature ); - ctx.approveAmmUpdates(false); + S.approveAmmUpdates(ctx, false); } else if (txType == AmmData.PoolTxType.EXIT) { - ctx.approveAmmUpdates(true); + S.approveAmmUpdates(ctx, true); S.processExit( ctx, abi.decode(data, (AmmData.PoolExit)), signature ); - ctx.approveAmmUpdates(false); + S.approveAmmUpdates(ctx, false); } else if (txType == AmmData.PoolTxType.SET_VIRTUAL_BALANCES) { - ctx.approveAmmUpdates(true); + S.approveAmmUpdates(ctx, true); ctx.vTokenBalancesL2 = ctx.settings.controller.getVirtualBalances(ctx.tokenBalancesL2, ctx.vTokenBalancesL2); - ctx.approveAmmUpdates(false); + S.approveAmmUpdates(ctx, false); } else if (txType == AmmData.PoolTxType.DEPOSIT) { - AmmData.PoolDeposit memory poolDeposit = abi.decode(data, (AmmData.PoolDeposit)); - require(poolDeposit.amounts.length == ctx.tokens.length, "INVALID_DEPOSIT_AMOUNTS"); - for (uint i = 0; i < ctx.tokens.length; i++) { - deposit(S, ctx.tokens[i].addr, poolDeposit.amounts[i]); - } - } else if (txType == AmmData.PoolTxType.WITHDRAW) { - AmmData.PoolWithdrawal memory poolWithdrawal = abi.decode(data, (AmmData.PoolWithdrawal)); - require(poolWithdrawal.amounts.length == ctx.tokens.length, "INVALID_WITHDRAWAL_AMOUNTS"); - for (uint i = 0; i < ctx.tokens.length; i++) { - withdraw(ctx, ctx.tokens[i].tokenID, poolWithdrawal.amounts[i]); - } + S.processDeposit( + ctx, + abi.decode(data, (AmmData.PoolDeposit)) + ); + } else if (txType == AmmData.PoolTxType.WITHDRAW) { + S.processWithdrawal( + ctx, + abi.decode(data, (AmmData.PoolWithdrawal)) + ); } else { revert("INVALID_POOL_TX_TYPE"); } } - - function deposit( - AmmData.State storage S, - address token, - uint96 amount - ) - internal - { - if (amount == 0) { - return; - } - uint ethValue = 0; - if (token == address(0)) { - ethValue = amount; - } else { - ERC20(token).approve(address(S.exchange.getDepositContract()), amount); - } - S.exchange.deposit{value: ethValue}( - address(this), - address(this), - token, - amount, - new bytes(0) - ); - } - - function withdraw( - AmmData.Context memory ctx, - uint tokenID, - uint96 amount - ) - internal - view - { - if (amount == 0) { - return; - } - - bytes20 onchainDataHash = WithdrawTransaction.hashOnchainData( - 0, // Withdrawal needs to succeed no matter the gas coast - address(this), // Withdraw to this contract first - new bytes(0) - ); - - // Verify withdrawal data - // Start by reading the first 2 bytes into header - uint txsDataPtr = ctx.txsDataPtr + 2; - // header: txType (1) | type (1) - uint header; - // packedData: tokenID (2) | amount (12) | feeTokenID (2) | fee (2) - uint packedData; - bytes20 dataHash; - assembly { - header := calldataload( txsDataPtr ) - packedData := calldataload(add(txsDataPtr, 42)) - dataHash := and(calldataload(add(txsDataPtr, 78)), 0xffffffffffffffffffffffffffffffffffffffff000000000000000000000000) - } - require( - // txType == ExchangeData.TransactionType.WITHDRAWAL && - // withdrawal.type == 1 && - header & 0xffff == (uint(ExchangeData.TransactionType.WITHDRAWAL) << 8) | 1 && - // withdrawal.tokenID == token.tokenID && - // withdrawal.amount == token.amount && - // withdrawal.fee == 0, - packedData & 0xffffffffffffffffffffffffffff0000ffff == (uint(tokenID) << 128) | (uint(amount) << 32) && - onchainDataHash == dataHash, - "INVALID_WITHDRAWAL_TX_DATA" - ); - - ctx.txsDataPtr += ExchangeData.TX_DATA_AVAILABILITY_SIZE; - } } diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmUpdateProcess.sol b/packages/loopring_v3/contracts/amm/libamm/AmmUpdateProcess.sol index d51009e3e..a2f5dd286 100644 --- a/packages/loopring_v3/contracts/amm/libamm/AmmUpdateProcess.sol +++ b/packages/loopring_v3/contracts/amm/libamm/AmmUpdateProcess.sol @@ -7,16 +7,19 @@ import "../../aux/transactions/TransactionReader.sol"; import "../../core/impl/libtransactions/AmmUpdateTransaction.sol"; import "./AmmData.sol"; import "./AmmUtil.sol"; +import "../../lib/MathUint96.sol"; /// @title AmmUpdateProcess library AmmUpdateProcess { + using MathUint96 for uint96; using TransactionReader for ExchangeData.Block; function approveAmmUpdates( - AmmData.Context memory ctx, - bool start + AmmData.State storage S, + AmmData.Context memory ctx, + bool start ) internal view @@ -60,9 +63,11 @@ library AmmUpdateProcess ); if (start) { + // Bring actual balances and virtual balances from L2 to L1 ctx.tokenBalancesL2[i] = /*balance*/uint96(packedDataB & 0xffffffffffffffffffffffff); ctx.vTokenBalancesL2[i] = /*vbalance*/uint96((packedDataB >> 128) & 0xffffffffffffffffffffffff); } else { + // Verify new virtual balances are the same value as on L2. require( ctx.vTokenBalancesL2[i] == /*vbalance*/uint96((packedDataB >> 128) & 0xffffffffffffffffffffffff), "INVALID_VIRTUAL_BALANCE_UPDATE" @@ -72,6 +77,13 @@ library AmmUpdateProcess txsDataPtr += ExchangeData.TX_DATA_AVAILABILITY_SIZE; } + if (start && ctx.settings.assetManager != IAssetManager(0)) { + // Add the L1 balances on top of the balances on L2 + for (uint i = 0; i < ctx.tokens.length; i++) { + ctx.tokenBalancesL2[i] = ctx.tokenBalancesL2[i].add(S.balancesL1[ctx.tokens[i].addr]); + } + } + ctx.txsDataPtr += ExchangeData.TX_DATA_AVAILABILITY_SIZE * ctx.tokens.length; } } diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmWithdrawProcess.sol b/packages/loopring_v3/contracts/amm/libamm/AmmWithdrawProcess.sol new file mode 100644 index 000000000..9778dcef6 --- /dev/null +++ b/packages/loopring_v3/contracts/amm/libamm/AmmWithdrawProcess.sol @@ -0,0 +1,82 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2017 Loopring Technology Limited. +pragma solidity ^0.7.0; +pragma experimental ABIEncoderV2; + +import "../../lib/ERC20.sol"; +import "../../lib/MathUint96.sol"; +import "../../lib/TransferUtil.sol"; +import "./AmmData.sol"; +import "./AmmPoolToken.sol"; +import "./AmmStatus.sol"; +import "./AmmUtil.sol"; + + +/// @title AmmWithdrawProcess +library AmmWithdrawProcess +{ + using MathUint96 for uint96; + using TransferUtil for address; + + function processWithdrawal( + AmmData.State storage S, + AmmData.Context memory ctx, + AmmData.PoolWithdrawal memory poolWithdrawal + ) + internal + { + require(ctx.settings.assetManager != IAssetManager(0), "CANNOT_WITHDRAW_FROM_POOL"); + require(poolWithdrawal.amounts.length == ctx.tokens.length, "INVALID_WITHDRAWAL_AMOUNTS"); + for (uint i = 0; i < ctx.tokens.length; i++) { + verifyWithdrawalTx(ctx, ctx.tokens[i].tokenID, poolWithdrawal.amounts[i]); + S.balancesL1[ctx.tokens[i].addr] = S.balancesL1[ctx.tokens[i].addr].add(poolWithdrawal.amounts[i]); + } + } + + function verifyWithdrawalTx( + AmmData.Context memory ctx, + uint tokenID, + uint96 amount + ) + internal + view + { + if (amount == 0) { + return; + } + + bytes20 onchainDataHash = WithdrawTransaction.hashOnchainData( + 0, // Withdrawal needs to succeed no matter the gas coast + address(this), // Withdraw to this contract first + new bytes(0) + ); + + // Verify withdrawal data + // Start by reading the first 2 bytes into header + uint txsDataPtr = ctx.txsDataPtr + 2; + // header: txType (1) | type (1) + uint header; + // packedData: tokenID (2) | amount (12) | feeTokenID (2) | fee (2) + uint packedData; + bytes20 dataHash; + assembly { + header := calldataload( txsDataPtr ) + packedData := calldataload(add(txsDataPtr, 42)) + dataHash := and(calldataload(add(txsDataPtr, 78)), 0xffffffffffffffffffffffffffffffffffffffff000000000000000000000000) + } + + require( + // txType == ExchangeData.TransactionType.WITHDRAWAL && + // withdrawal.type == 1 && + header & 0xffff == (uint(ExchangeData.TransactionType.WITHDRAWAL) << 8) | 1 && + // withdrawal.tokenID == token.tokenID && + // withdrawal.amount == token.amount && + // withdrawal.fee == 0, + packedData & 0xffffffffffffffffffffffffffff0000ffff == (uint(tokenID) << 128) | (uint(amount) << 32) && + onchainDataHash == dataHash, + "INVALID_WITHDRAWAL_TX_DATA" + ); + + ctx.txsDataPtr += ExchangeData.TX_DATA_AVAILABILITY_SIZE; + } +} \ No newline at end of file diff --git a/packages/loopring_v3/contracts/aux/access/LoopringIOExchangeOwner.sol b/packages/loopring_v3/contracts/aux/access/LoopringIOExchangeOwner.sol index fc08a7d65..530433528 100644 --- a/packages/loopring_v3/contracts/aux/access/LoopringIOExchangeOwner.sol +++ b/packages/loopring_v3/contracts/aux/access/LoopringIOExchangeOwner.sol @@ -48,6 +48,7 @@ contract LoopringIOExchangeOwner is SelectorBasedAccessManager, ERC1271, Drainab { TransactionReceiverCallback[] callbacks; address[] receivers; + bool before; } struct SubmitBlocksCallback @@ -143,11 +144,18 @@ contract LoopringIOExchangeOwner is SelectorBasedAccessManager, ERC1271, Drainab IExchangeV3(target).flashDeposit(flashDeposits); } + if (txReceiverCallbacks.before) { + // Do transaction verifying blocks callbacks + _verifyTransactions(blocks, txReceiverCallbacks); + } + // Submit blocks target.fastCallAndVerify(gasleft(), 0, decompressed); - // Do transaction verifying blocks callbacks - _verifyTransactions(blocks, txReceiverCallbacks); + if (!txReceiverCallbacks.before) { + // Do transaction verifying blocks callbacks + _verifyTransactions(blocks, txReceiverCallbacks); + } // Do post blocks callbacks _processCallbacks(submitBlocksCallbacks, false); @@ -202,7 +210,7 @@ contract LoopringIOExchangeOwner is SelectorBasedAccessManager, ERC1271, Drainab uint txIdx; bool approved; uint auxOffset; - for(uint j = 0; j < auxiliaryData.length; j++) { + for (uint j = 0; j < auxiliaryData.length; j++) { // Load the data from auxiliaryData, which is still encoded as calldata assembly { // Offset to auxiliaryData[j] diff --git a/packages/loopring_v3/contracts/test/LoopringAmmPoolCopy.sol b/packages/loopring_v3/contracts/test/LoopringAmmPoolCopy.sol index 37a85a4ff..b9545ebde 100644 --- a/packages/loopring_v3/contracts/test/LoopringAmmPoolCopy.sol +++ b/packages/loopring_v3/contracts/test/LoopringAmmPoolCopy.sol @@ -8,9 +8,9 @@ import "../amm/LoopringAmmPool.sol"; /// @title LoopringAmmPool contract LoopringAmmPoolCopy is LoopringAmmPool { constructor( - IAmmController _defaultController, - address _defaultAssetManager, + IAmmController _controller, + IAssetManager _assetManager, bool _joinsDisabled - ) LoopringAmmPool(_defaultController, _defaultAssetManager, _joinsDisabled) + ) LoopringAmmPool(_controller, _assetManager, _joinsDisabled) {} } diff --git a/packages/loopring_v3/contracts/test/TestAssetManager.sol b/packages/loopring_v3/contracts/test/TestAssetManager.sol index 8b2bb1b34..d452fd34c 100644 --- a/packages/loopring_v3/contracts/test/TestAssetManager.sol +++ b/packages/loopring_v3/contracts/test/TestAssetManager.sol @@ -6,15 +6,31 @@ import "../lib/Claimable.sol"; import "../lib/MathUint.sol"; import "../lib/TransferUtil.sol"; import "../amm/LoopringAmmPool.sol"; +import "../amm/IAssetManager.sol"; /// @author Brecht Devos - -contract TestAssetManager is Claimable +contract TestAssetManager is IAssetManager, Claimable { using MathUint for uint; using TransferUtil for address; mapping(address => mapping(address => uint)) public poolBalances; + function getBalances( + address pool, + address[] memory tokens + ) + external + view + returns (uint[] memory) + { + uint[] memory balances = new uint[](tokens.length); + for (uint i = 0; i < tokens.length; i++) { + balances[i] = poolBalances[address(pool)][tokens[i]]; + } + return balances; + } + function withdraw( LoopringAmmPool pool, address token, @@ -55,7 +71,7 @@ contract TestAssetManager is Claimable require(!pool.isOnline(), "POOL_NOT_OFFLINE"); uint amount = poolBalances[address(pool)][token]; - poolBalances[address(pool)][token] = 0; + delete poolBalances[address(pool)][token]; token.transferOut(address(pool), amount); } diff --git a/packages/loopring_v3/migrations/7_deploy_amm.js b/packages/loopring_v3/migrations/7_deploy_amm.js index 92953b36e..a9e3b1a40 100644 --- a/packages/loopring_v3/migrations/7_deploy_amm.js +++ b/packages/loopring_v3/migrations/7_deploy_amm.js @@ -7,6 +7,7 @@ const AmmJoinRequest = artifacts.require("AmmJoinRequest"); const AmmExitRequest = artifacts.require("AmmExitRequest"); const AmmStatus = artifacts.require("AmmStatus"); const AmmWithdrawal = artifacts.require("AmmWithdrawal"); +const AmmAssetManagement = artifacts.require("AmmAssetManagement"); const AmplifiedAmmController = artifacts.require("AmplifiedAmmController"); module.exports = function(deployer, network, accounts) { @@ -14,10 +15,12 @@ module.exports = function(deployer, network, accounts) { deployer.then(async () => { await deployer.deploy(AmplifiedAmmController); + await deployer.deploy(AmmAssetManagement); await deployer.deploy(AmmJoinRequest); await deployer.deploy(AmmExitRequest); await deployer.deploy(AmmStatus); await deployer.deploy(AmmWithdrawal); + await deployer.link(AmmAssetManagement, LoopringAmmPool); await deployer.link(AmmJoinRequest, LoopringAmmPool); await deployer.link(AmmExitRequest, LoopringAmmPool); await deployer.link(AmmStatus, LoopringAmmPool); @@ -29,6 +32,7 @@ module.exports = function(deployer, network, accounts) { false ); + await deployer.link(AmmAssetManagement, LoopringAmmPoolCopy); await deployer.link(AmmJoinRequest, LoopringAmmPoolCopy); await deployer.link(AmmExitRequest, LoopringAmmPoolCopy); await deployer.link(AmmStatus, LoopringAmmPoolCopy); diff --git a/packages/loopring_v3/test/ammUtils.ts b/packages/loopring_v3/test/ammUtils.ts index c09603703..477b54e40 100644 --- a/packages/loopring_v3/test/ammUtils.ts +++ b/packages/loopring_v3/test/ammUtils.ts @@ -292,7 +292,8 @@ export class AmmPool { vTokenBalancesL2: BN[], feeBips: number, amplificationFactor: BN, - controller?: any + controllerAddress?: string, + assetManagerAddress?: string ) { this.sharedConfig = sharedConfig; this.feeBips = feeBips; @@ -302,14 +303,16 @@ export class AmmPool { this.totalSupply = new BN(0); - const controllerAddress = controller - ? controller.address + controllerAddress = controllerAddress + ? controllerAddress : Constants.zeroAddress; + console.log("controllerAddress: " + controllerAddress); + const AmmPool = artifacts.require("LoopringAmmPool"); this.contract = await AmmPool.new( controllerAddress, - "0x" + "00".repeat(20), + assetManagerAddress, false ); @@ -520,7 +523,7 @@ export class AmmPool { } public async deposit(amounts: BN[]) { - /*const deposit: PoolDeposit = { + const deposit: PoolDeposit = { txType: "Deposit", poolAddress: this.contract.address, amounts: amounts, @@ -530,28 +533,7 @@ export class AmmPool { await this.process(deposit, undefined); - return deposit;*/ - - const _amounts: string[] = []; - for (let i = 0; i < amounts.length; i++) { - _amounts[i] = amounts[i].toString(10); - } - - await this.ctx.addCallback( - this.contract.address, - this.contract.contract.methods.depositAssetsToL2(_amounts).encodeABI(), - true - ); - - for (let i = 0; i < this.tokens.length; i++) { - if (!amounts[i].isZero()) { - await this.ctx.requestDeposit( - this.contract.address, - this.tokens[i], - amounts[i] - ); - } - } + return deposit; } public async withdraw(amounts: BN[]) { diff --git a/packages/loopring_v3/test/testAMMPool.ts b/packages/loopring_v3/test/testAMMPool.ts index 4cf7f0a94..3e79c5159 100644 --- a/packages/loopring_v3/test/testAMMPool.ts +++ b/packages/loopring_v3/test/testAMMPool.ts @@ -12,7 +12,8 @@ contract("LoopringAmmPool", (accounts: string[]) => { let sharedConfig: any; let agentRegistry: any; let registryOwner: string; - let ammController: string; + let ammController: any; + let assetManager: any; let ownerA: string; let ownerB: string; @@ -36,9 +37,11 @@ contract("LoopringAmmPool", (accounts: string[]) => { const setupDefaultPool = async ( amplificationFactor = new BN(web3.utils.toWei("1", "ether")), - controller?: any + controller?: any, + assetManager?: any ) => { controller = controller ? controller : ammController; + const assetManagerAddress = assetManager ? assetManager.address : Constants.zeroAddress; const feeBipsAMM = 30; const tokens = ["WETH", "GTO"]; @@ -64,7 +67,8 @@ contract("LoopringAmmPool", (accounts: string[]) => { weights, feeBipsAMM, amplificationFactor, - controller + controller.address, + assetManagerAddress ); if (controller) { @@ -109,6 +113,9 @@ contract("LoopringAmmPool", (accounts: string[]) => { const amplifiedAmmController = artifacts.require("AmplifiedAmmController"); ammController = await amplifiedAmmController.new(); + + const TestAssetManager = artifacts.require("TestAssetManager"); + assetManager = await TestAssetManager.new(); }); after(async () => { @@ -587,7 +594,11 @@ contract("LoopringAmmPool", (accounts: string[]) => { }); it("Manage pool assets", async () => { - const pool = await setupDefaultPool(); + const pool = await setupDefaultPool( + new BN(web3.utils.toWei("1", "ether")), + undefined, + assetManager + ); const amounts = [ new BN(web3.utils.toWei("10000", "ether")), diff --git a/packages/loopring_v3/test/testExchangeUtil.ts b/packages/loopring_v3/test/testExchangeUtil.ts index 6ced98c8c..61b3125d5 100644 --- a/packages/loopring_v3/test/testExchangeUtil.ts +++ b/packages/loopring_v3/test/testExchangeUtil.ts @@ -1867,11 +1867,13 @@ export class ExchangeTestUtil { interface TransactionReceiverCallbacks { callbacks: OnchainBlockCallback[]; receivers: string[]; + before: boolean; } const transactionReceiverCallbacks: TransactionReceiverCallbacks = { callbacks: [], - receivers: [] + receivers: [], + before: true }; //console.log("Block callbacks: "); From fbe83ae1987609e9603104f82137551ffd4df11d Mon Sep 17 00:00:00 2001 From: Brechtpd Date: Tue, 1 Jun 2021 17:16:04 +0200 Subject: [PATCH 2/4] Feedback --- packages/loopring_v3/contracts/amm/LoopringAmmPool.sol | 2 +- .../contracts/amm/libamm/AmmDepositProcess.sol | 10 ++++++---- .../contracts/amm/libamm/AmmWithdrawProcess.sol | 6 ++++-- .../contracts/aux/access/LoopringIOExchangeOwner.sol | 6 +++--- .../loopring_v3/contracts/test/TestAssetManager.sol | 2 +- packages/loopring_v3/test/testExchangeUtil.ts | 4 ++-- 6 files changed, 17 insertions(+), 13 deletions(-) diff --git a/packages/loopring_v3/contracts/amm/LoopringAmmPool.sol b/packages/loopring_v3/contracts/amm/LoopringAmmPool.sol index 2dd4a8f72..fcbe7bd7f 100644 --- a/packages/loopring_v3/contracts/amm/LoopringAmmPool.sol +++ b/packages/loopring_v3/contracts/amm/LoopringAmmPool.sol @@ -211,7 +211,7 @@ contract LoopringAmmPool is function getBalanceL1( address token ) - external + public view returns (uint96) { diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmDepositProcess.sol b/packages/loopring_v3/contracts/amm/libamm/AmmDepositProcess.sol index ea6b07f9b..591f4edf0 100644 --- a/packages/loopring_v3/contracts/amm/libamm/AmmDepositProcess.sol +++ b/packages/loopring_v3/contracts/amm/libamm/AmmDepositProcess.sol @@ -30,9 +30,11 @@ library AmmDepositProcess { require(poolDeposit.amounts.length == ctx.tokens.length, "INVALID_DEPOSIT_DATA"); for (uint i = 0; i < ctx.tokens.length; i++) { - verifyDepositTx(ctx, ctx.tokens[i].tokenID, poolDeposit.amounts[i]); - S.deposit(ctx.tokens[i].addr, poolDeposit.amounts[i]); - S.balancesL1[ctx.tokens[i].addr] = S.balancesL1[ctx.tokens[i].addr].sub(poolDeposit.amounts[i]); + address token = ctx.tokens[i].addr; + uint96 amount = poolDeposit.amounts[i]; + verifyDepositTx(ctx, ctx.tokens[i].tokenID, amount); + S.deposit(token, amount); + S.balancesL1[token] = S.balancesL1[token].sub(amount); } } @@ -62,7 +64,7 @@ library AmmDepositProcess require( // txType == ExchangeData.TransactionType.DEPOSIT && // owner == address(this) && - // accountID == crx.accountID && + // accountID == ctx.accountID && // tokenID == tokenID && packedData & 0xffffffffffffffffffffffffffffffffffffffffffffffffffffff == (uint(ExchangeData.TransactionType.DEPOSIT) << 208) | (uint(address(this)) << 48) | (uint(ctx.accountID) << 16) | uint(tokenID) && amount == txAmount, diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmWithdrawProcess.sol b/packages/loopring_v3/contracts/amm/libamm/AmmWithdrawProcess.sol index 9778dcef6..61f59eb1c 100644 --- a/packages/loopring_v3/contracts/amm/libamm/AmmWithdrawProcess.sol +++ b/packages/loopring_v3/contracts/amm/libamm/AmmWithdrawProcess.sol @@ -28,8 +28,10 @@ library AmmWithdrawProcess require(ctx.settings.assetManager != IAssetManager(0), "CANNOT_WITHDRAW_FROM_POOL"); require(poolWithdrawal.amounts.length == ctx.tokens.length, "INVALID_WITHDRAWAL_AMOUNTS"); for (uint i = 0; i < ctx.tokens.length; i++) { - verifyWithdrawalTx(ctx, ctx.tokens[i].tokenID, poolWithdrawal.amounts[i]); - S.balancesL1[ctx.tokens[i].addr] = S.balancesL1[ctx.tokens[i].addr].add(poolWithdrawal.amounts[i]); + address token = ctx.tokens[i].addr; + uint96 amount = poolWithdrawal.amounts[i]; + verifyWithdrawalTx(ctx, ctx.tokens[i].tokenID, amount); + S.balancesL1[token] = S.balancesL1[token].add(amount); } } diff --git a/packages/loopring_v3/contracts/aux/access/LoopringIOExchangeOwner.sol b/packages/loopring_v3/contracts/aux/access/LoopringIOExchangeOwner.sol index 530433528..d58aac3ee 100644 --- a/packages/loopring_v3/contracts/aux/access/LoopringIOExchangeOwner.sol +++ b/packages/loopring_v3/contracts/aux/access/LoopringIOExchangeOwner.sol @@ -48,7 +48,7 @@ contract LoopringIOExchangeOwner is SelectorBasedAccessManager, ERC1271, Drainab { TransactionReceiverCallback[] callbacks; address[] receivers; - bool before; + bool beforeBlockSubmission; } struct SubmitBlocksCallback @@ -144,7 +144,7 @@ contract LoopringIOExchangeOwner is SelectorBasedAccessManager, ERC1271, Drainab IExchangeV3(target).flashDeposit(flashDeposits); } - if (txReceiverCallbacks.before) { + if (txReceiverCallbacks.beforeBlockSubmission) { // Do transaction verifying blocks callbacks _verifyTransactions(blocks, txReceiverCallbacks); } @@ -152,7 +152,7 @@ contract LoopringIOExchangeOwner is SelectorBasedAccessManager, ERC1271, Drainab // Submit blocks target.fastCallAndVerify(gasleft(), 0, decompressed); - if (!txReceiverCallbacks.before) { + if (!txReceiverCallbacks.beforeBlockSubmission) { // Do transaction verifying blocks callbacks _verifyTransactions(blocks, txReceiverCallbacks); } diff --git a/packages/loopring_v3/contracts/test/TestAssetManager.sol b/packages/loopring_v3/contracts/test/TestAssetManager.sol index d452fd34c..29db6825c 100644 --- a/packages/loopring_v3/contracts/test/TestAssetManager.sol +++ b/packages/loopring_v3/contracts/test/TestAssetManager.sol @@ -20,7 +20,7 @@ contract TestAssetManager is IAssetManager, Claimable address pool, address[] memory tokens ) - external + public view returns (uint[] memory) { diff --git a/packages/loopring_v3/test/testExchangeUtil.ts b/packages/loopring_v3/test/testExchangeUtil.ts index 61b3125d5..adfc92bf8 100644 --- a/packages/loopring_v3/test/testExchangeUtil.ts +++ b/packages/loopring_v3/test/testExchangeUtil.ts @@ -1867,13 +1867,13 @@ export class ExchangeTestUtil { interface TransactionReceiverCallbacks { callbacks: OnchainBlockCallback[]; receivers: string[]; - before: boolean; + beforeBlockSubmission: boolean; } const transactionReceiverCallbacks: TransactionReceiverCallbacks = { callbacks: [], receivers: [], - before: true + beforeBlockSubmission: true }; //console.log("Block callbacks: "); From 68c26f3f4a4ce5387d14c6af8afa4a9291de9e98 Mon Sep 17 00:00:00 2001 From: Brecht Devos Date: Sat, 5 Jun 2021 17:29:31 +0200 Subject: [PATCH 3/4] Amm v2 - Exit mode and pool reuse (#2450) --- .../contracts/amm/AmplifiedAmmController.sol | 91 +++++++++++++++---- .../contracts/amm/IAmmController.sol | 19 ++-- .../contracts/amm/LoopringAmmPool.sol | 10 +- .../contracts/amm/libamm/AmmData.sol | 10 +- .../amm/libamm/AmmDepositProcess.sol | 12 ++- .../contracts/amm/libamm/AmmExitProcess.sol | 24 +++-- .../contracts/amm/libamm/AmmJoinProcess.sol | 4 +- .../contracts/amm/libamm/AmmPoolToken.sol | 2 +- .../contracts/amm/libamm/AmmSignature.sol | 25 +++++ .../contracts/amm/libamm/AmmStatus.sol | 56 ++++++------ .../amm/libamm/AmmTransactionReceiver.sol | 24 ++++- .../amm/libamm/AmmWithdrawProcess.sol | 10 +- .../loopring_v3/migrations/7_deploy_amm.js | 8 ++ packages/loopring_v3/test/ammUtils.ts | 32 +++++-- packages/loopring_v3/test/testAMMPool.ts | 13 ++- 15 files changed, 250 insertions(+), 90 deletions(-) create mode 100644 packages/loopring_v3/contracts/amm/libamm/AmmSignature.sol diff --git a/packages/loopring_v3/contracts/amm/AmplifiedAmmController.sol b/packages/loopring_v3/contracts/amm/AmplifiedAmmController.sol index 59764a9d7..1ef44f4ce 100644 --- a/packages/loopring_v3/contracts/amm/AmplifiedAmmController.sol +++ b/packages/loopring_v3/contracts/amm/AmplifiedAmmController.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright 2017 Loopring Technology Limited. pragma solidity ^0.7.0; +pragma experimental ABIEncoderV2; import "./IAmmController.sol"; import "../amm/LoopringAmmPool.sol"; @@ -19,8 +20,14 @@ contract AmplifiedAmmController is IAmmController, Claimable uint public constant AMPLIFICATION_FACTOR_BASE = (10 ** 18); + uint public constant MIN_CURVE_CHANGE_DELAY = 7 days; + uint public constant CURVE_CHANGE_AUTH_WINDOW = 7 days; + mapping(address => uint) public amplificationFactors; + mapping(address => uint) public curveChangeAuthorization; + + function getInitialVirtualBalances( uint96[] memory joinAmounts ) @@ -37,44 +44,96 @@ contract AmplifiedAmmController is IAmmController, Claimable return vTokenBalancesL2; } - function getVirtualBalances( - uint96[] memory tokenBalancesL2, - uint96[] memory /*vTokenBalancesL2*/ + function authorizeVirtualBalances( + uint96[] memory balances, + uint96[] memory /*vBalancesOld*/, + uint96[] memory vBalancesNew, + bytes memory /*data*/ ) external - view override - returns (uint96[] memory) + returns (bool) { - // Only allow updating the virtual balances if the AF = 1 - require(getAmplificationFactor(msg.sender) == AMPLIFICATION_FACTOR_BASE, "INVALID_OPERATION"); + address pool = msg.sender; - // Just set the virtual balances to the actual balances - uint96[] memory vTokenBalancesL2 = new uint96[](tokenBalancesL2.length); - for (uint i = 0; i < tokenBalancesL2.length; i++) { - vTokenBalancesL2[i] = tokenBalancesL2[i]; + // Check if a curve change was explicitly authorized + if (consumeCurveChangeAuthorized(pool)) { + return true; } - return vTokenBalancesL2; + + // Special case: Always allow updating the virtual balances if the AF = 1 + if (getAmplificationFactor(pool) == AMPLIFICATION_FACTOR_BASE) { + for (uint i = 0; i < balances.length; i++) { + if(vBalancesNew[i] != balances[i]) { + return false; + } + } + return true; + } + + return false; + } + + function authorizeCurveChange(address pool) + external + onlyOwner + { + curveChangeAuthorization[pool] = block.timestamp + MIN_CURVE_CHANGE_DELAY; } function setAmplificationFactor( - address amm, + address pool, uint amplificationFactor ) external onlyOwner { - amplificationFactors[amm] = amplificationFactor; + amplificationFactors[pool] = amplificationFactor; } - function getAmplificationFactor(address amm) + function getAmplificationFactor(address pool) public view returns (uint amplificationFactor) { - amplificationFactor = amplificationFactors[amm]; + amplificationFactor = amplificationFactors[pool]; if (amplificationFactor == 0) { amplificationFactor = AMPLIFICATION_FACTOR_BASE; } } + + function setupPool( + LoopringAmmPool pool, + AmmData.PoolConfig calldata config + ) + external + onlyOwner + { + pool.setupPool(config); + } + + function enterExitMode( + LoopringAmmPool pool, + bool enabled + ) + external + onlyOwner + { + pool.enterExitMode(enabled); + } + + // == Internal Functions == + + function consumeCurveChangeAuthorized(address pool) + internal + returns (bool) + { + uint timestamp = curveChangeAuthorization[pool]; + bool authorized = (timestamp <= block.timestamp) && (block.timestamp <= timestamp + MIN_CURVE_CHANGE_DELAY); + + // Remove authorization + delete curveChangeAuthorization[pool]; + + return authorized; + } } diff --git a/packages/loopring_v3/contracts/amm/IAmmController.sol b/packages/loopring_v3/contracts/amm/IAmmController.sol index f668eb416..afe148885 100644 --- a/packages/loopring_v3/contracts/amm/IAmmController.sol +++ b/packages/loopring_v3/contracts/amm/IAmmController.sol @@ -18,14 +18,17 @@ interface IAmmController /// @dev Called by the pool contract when a SET_VIRTUAL_BALANCES operation is done /// on the pool. - /// @param tokenBalancesL2 The balances in the pool - /// @param vTokenBalancesL2 The current virtual balances in the pool - /// @return The new virtual balances in the pool - function getVirtualBalances( - uint96[] memory tokenBalancesL2, - uint96[] memory vTokenBalancesL2 + /// @param balances The balances in the pool + /// @param vBalancesOld The current virtual balances in the pool + /// @param vBalancesNew The new virtual balances in the pool + /// @param data Custom data + /// @return True if vBalancesNew can be used, else false + function authorizeVirtualBalances( + uint96[] memory balances, + uint96[] memory vBalancesOld, + uint96[] memory vBalancesNew, + bytes memory data ) external - view - returns (uint96[] memory); + returns (bool); } diff --git a/packages/loopring_v3/contracts/amm/LoopringAmmPool.sol b/packages/loopring_v3/contracts/amm/LoopringAmmPool.sol index fcbe7bd7f..7913a1771 100644 --- a/packages/loopring_v3/contracts/amm/LoopringAmmPool.sol +++ b/packages/loopring_v3/contracts/amm/LoopringAmmPool.sol @@ -79,6 +79,7 @@ contract LoopringAmmPool is bool _joinsDisabled ) { + require(_controller != IAmmController(0), "ZERO_ADDRESS"); controller = _controller; assetManager = _assetManager; joinsDisabled = _joinsDisabled; @@ -98,9 +99,17 @@ contract LoopringAmmPool is external nonReentrant { + require(state.accountID == 0 || msg.sender == address(controller), "UNAUTHORIZED"); state.setupPool(config); } + function enterExitMode(bool enabled) + external + onlyFromController + { + state.exitMode = enabled; + } + // Anyone is able to shut down the pool when requests aren't being processed any more. function shutdown(address exitOwner) external @@ -113,7 +122,6 @@ contract LoopringAmmPool is function shutdownByController() external - payable onlyWhenOnline nonReentrant onlyFromController diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmData.sol b/packages/loopring_v3/contracts/amm/libamm/AmmData.sol index eac2a605c..ede3051fa 100644 --- a/packages/loopring_v3/contracts/amm/libamm/AmmData.sol +++ b/packages/loopring_v3/contracts/amm/libamm/AmmData.sol @@ -58,6 +58,12 @@ library AmmData uint32 validUntil; } + struct PoolVirtualBalances + { + uint96[] vBalancesNew; + bytes data; + } + struct PoolDeposit { uint96[] amounts; @@ -82,7 +88,8 @@ library AmmData uint16 tokenID; } - struct Settings { + struct Settings + { IAmmController controller; IAssetManager assetManager; bool joinsDisabled; @@ -142,5 +149,6 @@ library AmmData mapping (bytes32 => bool) approvedTx; mapping (address => uint96) balancesL1; + bool exitMode; } } diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmDepositProcess.sol b/packages/loopring_v3/contracts/amm/libamm/AmmDepositProcess.sol index 591f4edf0..c4d3f7cbe 100644 --- a/packages/loopring_v3/contracts/amm/libamm/AmmDepositProcess.sol +++ b/packages/loopring_v3/contracts/amm/libamm/AmmDepositProcess.sol @@ -30,11 +30,13 @@ library AmmDepositProcess { require(poolDeposit.amounts.length == ctx.tokens.length, "INVALID_DEPOSIT_DATA"); for (uint i = 0; i < ctx.tokens.length; i++) { - address token = ctx.tokens[i].addr; uint96 amount = poolDeposit.amounts[i]; - verifyDepositTx(ctx, ctx.tokens[i].tokenID, amount); - S.deposit(token, amount); - S.balancesL1[token] = S.balancesL1[token].sub(amount); + if (amount > 0) { + address token = ctx.tokens[i].addr; + verifyDepositTx(ctx, ctx.tokens[i].tokenID, amount); + S.deposit(token, amount); + S.balancesL1[token] = S.balancesL1[token].sub(amount); + } } } @@ -68,7 +70,7 @@ library AmmDepositProcess // tokenID == tokenID && packedData & 0xffffffffffffffffffffffffffffffffffffffffffffffffffffff == (uint(ExchangeData.TransactionType.DEPOSIT) << 208) | (uint(address(this)) << 48) | (uint(ctx.accountID) << 16) | uint(tokenID) && amount == txAmount, - "INVALID_DEPOSIT_TX_DATA" + "INVALID_AMM_DEPOSIT_TX_DATA" ); ctx.txsDataPtr += ExchangeData.TX_DATA_AVAILABILITY_SIZE; diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmExitProcess.sol b/packages/loopring_v3/contracts/amm/libamm/AmmExitProcess.sol index 6403e8d60..2371b3b8d 100644 --- a/packages/loopring_v3/contracts/amm/libamm/AmmExitProcess.sol +++ b/packages/loopring_v3/contracts/amm/libamm/AmmExitProcess.sol @@ -9,26 +9,26 @@ import "../../lib/EIP712.sol"; import "../../lib/ERC20SafeTransfer.sol"; import "../../lib/MathUint.sol"; import "../../lib/MathUint96.sol"; -import "../../lib/SignatureUtil.sol"; import "../../lib/TransferUtil.sol"; import "../../thirdparty/SafeCast.sol"; -import "./AmmUtil.sol"; import "./AmmData.sol"; import "./AmmExitRequest.sol"; import "./AmmPoolToken.sol"; +import "./AmmSignature.sol"; +import "./AmmUtil.sol"; /// @title AmmExitProcess library AmmExitProcess { using AmmPoolToken for AmmData.State; + using AmmSignature for bytes32; using AmmUtil for AmmData.Context; using AmmUtil for uint96; using ERC20SafeTransfer for address; using MathUint for uint; using MathUint96 for uint96; using SafeCast for uint; - using SignatureUtil for bytes32; using TransactionReader for ExchangeData.Block; using TransferUtil for address; @@ -49,14 +49,18 @@ library AmmExitProcess bool isForcedExit = false; if (signature.length == 0) { - bytes32 forcedExitHash = AmmExitRequest.hash(ctx.domainSeparator, S.forcedExit[exit.owner]); - if (txHash == forcedExitHash) { - delete S.forcedExit[exit.owner]; - S.forcedExitCount--; - isForcedExit = true; + if (S.exitMode) { + require(exit.fee == 0, "INVALID_FEE"); } else { - require(S.approvedTx[txHash], "INVALID_ONCHAIN_APPROVAL"); - delete S.approvedTx[txHash]; + bytes32 forcedExitHash = AmmExitRequest.hash(ctx.domainSeparator, S.forcedExit[exit.owner]); + if (txHash == forcedExitHash) { + delete S.forcedExit[exit.owner]; + S.forcedExitCount--; + isForcedExit = true; + } else { + require(S.approvedTx[txHash], "INVALID_ONCHAIN_APPROVAL"); + delete S.approvedTx[txHash]; + } } } else if (signature.length == 1) { ctx.verifySignatureL2(exit.owner, txHash, signature); diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmJoinProcess.sol b/packages/loopring_v3/contracts/amm/libamm/AmmJoinProcess.sol index 29eb5fe95..047f69a3d 100644 --- a/packages/loopring_v3/contracts/amm/libamm/AmmJoinProcess.sol +++ b/packages/loopring_v3/contracts/amm/libamm/AmmJoinProcess.sol @@ -8,11 +8,11 @@ import "../../core/impl/libtransactions/TransferTransaction.sol"; import "../../lib/EIP712.sol"; import "../../lib/MathUint.sol"; import "../../lib/MathUint96.sol"; -import "../../lib/SignatureUtil.sol"; import "../../thirdparty/SafeCast.sol"; import "./AmmData.sol"; import "./AmmJoinRequest.sol"; import "./AmmPoolToken.sol"; +import "./AmmSignature.sol"; import "./AmmUtil.sol"; @@ -20,13 +20,13 @@ import "./AmmUtil.sol"; library AmmJoinProcess { using AmmPoolToken for AmmData.State; + using AmmSignature for bytes32; using AmmUtil for AmmData.State; using AmmUtil for AmmData.Context; using AmmUtil for uint96; using MathUint for uint; using MathUint96 for uint96; using SafeCast for uint; - using SignatureUtil for bytes32; using TransactionReader for ExchangeData.Block; // event JoinProcessed(address owner, uint96 mintAmount, uint96[] amounts); diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmPoolToken.sol b/packages/loopring_v3/contracts/amm/libamm/AmmPoolToken.sol index 91b5ca760..522bd09d6 100644 --- a/packages/loopring_v3/contracts/amm/libamm/AmmPoolToken.sol +++ b/packages/loopring_v3/contracts/amm/libamm/AmmPoolToken.sol @@ -80,7 +80,7 @@ library AmmPoolToken uint256 deadline, bytes calldata signature ) - internal + public { require(deadline >= block.timestamp, 'EXPIRED'); diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmSignature.sol b/packages/loopring_v3/contracts/amm/libamm/AmmSignature.sol new file mode 100644 index 000000000..ffbefae4f --- /dev/null +++ b/packages/loopring_v3/contracts/amm/libamm/AmmSignature.sol @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2017 Loopring Technology Limited. +pragma solidity ^0.7.0; +pragma experimental ABIEncoderV2; + +import "../../lib/SignatureUtil.sol"; + + +/// @title AmmSignature +library AmmSignature +{ + using SignatureUtil for bytes32; + + function verifySignature( + bytes32 signHash, + address signer, + bytes memory signature + ) + public + view + returns (bool) + { + return signHash.verifySignature(signer, signature); + } +} \ No newline at end of file diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmStatus.sol b/packages/loopring_v3/contracts/amm/libamm/AmmStatus.sol index 6b19cbcd8..300db1744 100644 --- a/packages/loopring_v3/contracts/amm/libamm/AmmStatus.sol +++ b/packages/loopring_v3/contracts/amm/libamm/AmmStatus.sol @@ -5,23 +5,13 @@ pragma experimental ABIEncoderV2; import "../../core/iface/IExchangeV3.sol"; import "../../lib/EIP712.sol"; -import "../../lib/ERC20.sol"; -import "../../lib/MathUint.sol"; -import "../../lib/MathUint96.sol"; -import "../../lib/SignatureUtil.sol"; import "./AmmData.sol"; -import "./AmmPoolToken.sol"; import "./IAmmSharedConfig.sol"; /// @title AmmStatus library AmmStatus { - using AmmPoolToken for AmmData.State; - using MathUint for uint; - using MathUint96 for uint96; - using SignatureUtil for bytes32; - event Shutdown(uint timestamp); function isOnline(AmmData.State storage S) @@ -42,26 +32,26 @@ library AmmStatus bytes(config.poolName).length > 0 && bytes(config.tokenSymbol).length > 0, "INVALID_NAME_OR_SYMBOL" ); + require(config.sharedConfig != address(0), "INVALID_SHARED_CONFIG"); require(config.tokens.length == config.weights.length, "INVALID_DATA"); require(config.tokens.length >= 2, "INVALID_DATA"); require(config.exchange != address(0), "INVALID_EXCHANGE"); require(config.accountID != 0, "INVALID_ACCOUNT_ID"); - require(S.tokens.length == 0, "ALREADY_INITIALIZED"); - S.sharedConfig = IAmmSharedConfig(config.sharedConfig); - IExchangeV3 exchange = IExchangeV3(config.exchange); - S.exchange = exchange; - S.exchangeOwner = exchange.owner(); - S.exchangeDomainSeparator = exchange.getDomainSeparator(); - S.accountID = config.accountID; - S.poolTokenID = exchange.getTokenID(address(this)); + require(S._totalSupply == 0, "POOL_IN_USE"); + S.feeBips = config.feeBips; S.domainSeparator = EIP712.hash(EIP712.Domain(config.poolName, "1.0.0", address(this))); S.poolName = config.poolName; S.symbol = config.tokenSymbol; + S.shutdownTimestamp = 0; + S.exitMode = false; + IExchangeV3 exchange = IExchangeV3(config.exchange); + + delete S.tokens; for (uint i = 0; i < config.tokens.length; i++) { address token = config.tokens[i]; S.tokens.push(AmmData.Token({ @@ -71,16 +61,26 @@ library AmmStatus })); } - // Mint all liquidity tokens to the pool account on L2 - S.balanceOf[address(this)] = AmmData.POOL_TOKEN_MINTED_SUPPLY; - S.allowance[address(this)][address(exchange.getDepositContract())] = type(uint256).max; - exchange.deposit( - address(this), // from - address(this), // to - address(this), // token - uint96(AmmData.POOL_TOKEN_MINTED_SUPPLY), - new bytes(0) - ); + bool newPool = (S.accountID == 0); + if (newPool) { + S.sharedConfig = IAmmSharedConfig(config.sharedConfig); + S.exchange = exchange; + S.exchangeOwner = exchange.owner(); + S.exchangeDomainSeparator = exchange.getDomainSeparator(); + S.accountID = config.accountID; + S.poolTokenID = exchange.getTokenID(address(this)); + + // Mint all liquidity tokens to the pool account on L2 + S.balanceOf[address(this)] = AmmData.POOL_TOKEN_MINTED_SUPPLY; + S.allowance[address(this)][address(exchange.getDepositContract())] = type(uint256).max; + exchange.deposit( + address(this), // from + address(this), // to + address(this), // token + uint96(AmmData.POOL_TOKEN_MINTED_SUPPLY), + new bytes(0) + ); + } } // Anyone is able to shut down the pool when requests aren't being processed any more. diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmTransactionReceiver.sol b/packages/loopring_v3/contracts/amm/libamm/AmmTransactionReceiver.sol index 28113d557..2b49156ee 100644 --- a/packages/loopring_v3/contracts/amm/libamm/AmmTransactionReceiver.sol +++ b/packages/loopring_v3/contracts/amm/libamm/AmmTransactionReceiver.sol @@ -122,7 +122,10 @@ library AmmTransactionReceiver S.approveAmmUpdates(ctx, false); } else if (txType == AmmData.PoolTxType.SET_VIRTUAL_BALANCES) { S.approveAmmUpdates(ctx, true); - ctx.vTokenBalancesL2 = ctx.settings.controller.getVirtualBalances(ctx.tokenBalancesL2, ctx.vTokenBalancesL2); + processSetVirtualBalances( + ctx, + abi.decode(data, (AmmData.PoolVirtualBalances)) + ); S.approveAmmUpdates(ctx, false); } else if (txType == AmmData.PoolTxType.DEPOSIT) { S.processDeposit( @@ -138,4 +141,23 @@ library AmmTransactionReceiver revert("INVALID_POOL_TX_TYPE"); } } + + function processSetVirtualBalances( + AmmData.Context memory ctx, + AmmData.PoolVirtualBalances memory poolVirtualBalances + ) + internal + { + require(poolVirtualBalances.vBalancesNew.length == ctx.tokens.length, "INVALID_DATA"); + require( + ctx.settings.controller.authorizeVirtualBalances( + ctx.tokenBalancesL2, + ctx.vTokenBalancesL2, + poolVirtualBalances.vBalancesNew, + poolVirtualBalances.data + ), + "NEW_VIRTUAL_BALANCES_NOT_AUTHORIZED" + ); + ctx.vTokenBalancesL2 = poolVirtualBalances.vBalancesNew; + } } diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmWithdrawProcess.sol b/packages/loopring_v3/contracts/amm/libamm/AmmWithdrawProcess.sol index 61f59eb1c..244061bef 100644 --- a/packages/loopring_v3/contracts/amm/libamm/AmmWithdrawProcess.sol +++ b/packages/loopring_v3/contracts/amm/libamm/AmmWithdrawProcess.sol @@ -28,10 +28,12 @@ library AmmWithdrawProcess require(ctx.settings.assetManager != IAssetManager(0), "CANNOT_WITHDRAW_FROM_POOL"); require(poolWithdrawal.amounts.length == ctx.tokens.length, "INVALID_WITHDRAWAL_AMOUNTS"); for (uint i = 0; i < ctx.tokens.length; i++) { - address token = ctx.tokens[i].addr; uint96 amount = poolWithdrawal.amounts[i]; - verifyWithdrawalTx(ctx, ctx.tokens[i].tokenID, amount); - S.balancesL1[token] = S.balancesL1[token].add(amount); + if (amount > 0) { + address token = ctx.tokens[i].addr; + verifyWithdrawalTx(ctx, ctx.tokens[i].tokenID, amount); + S.balancesL1[token] = S.balancesL1[token].add(amount); + } } } @@ -76,7 +78,7 @@ library AmmWithdrawProcess // withdrawal.fee == 0, packedData & 0xffffffffffffffffffffffffffff0000ffff == (uint(tokenID) << 128) | (uint(amount) << 32) && onchainDataHash == dataHash, - "INVALID_WITHDRAWAL_TX_DATA" + "INVALID_AMM_WITHDRAWAL_TX_DATA" ); ctx.txsDataPtr += ExchangeData.TX_DATA_AVAILABILITY_SIZE; diff --git a/packages/loopring_v3/migrations/7_deploy_amm.js b/packages/loopring_v3/migrations/7_deploy_amm.js index a9e3b1a40..de7edb0e8 100644 --- a/packages/loopring_v3/migrations/7_deploy_amm.js +++ b/packages/loopring_v3/migrations/7_deploy_amm.js @@ -8,6 +8,8 @@ const AmmExitRequest = artifacts.require("AmmExitRequest"); const AmmStatus = artifacts.require("AmmStatus"); const AmmWithdrawal = artifacts.require("AmmWithdrawal"); const AmmAssetManagement = artifacts.require("AmmAssetManagement"); +const AmmPoolToken = artifacts.require("AmmPoolToken"); +const AmmSignature = artifacts.require("AmmSignature"); const AmplifiedAmmController = artifacts.require("AmplifiedAmmController"); module.exports = function(deployer, network, accounts) { @@ -15,11 +17,15 @@ module.exports = function(deployer, network, accounts) { deployer.then(async () => { await deployer.deploy(AmplifiedAmmController); + await deployer.deploy(AmmSignature); + await deployer.deploy(AmmPoolToken); await deployer.deploy(AmmAssetManagement); await deployer.deploy(AmmJoinRequest); await deployer.deploy(AmmExitRequest); await deployer.deploy(AmmStatus); await deployer.deploy(AmmWithdrawal); + await deployer.link(AmmSignature, LoopringAmmPool); + await deployer.link(AmmPoolToken, LoopringAmmPool); await deployer.link(AmmAssetManagement, LoopringAmmPool); await deployer.link(AmmJoinRequest, LoopringAmmPool); await deployer.link(AmmExitRequest, LoopringAmmPool); @@ -32,6 +38,8 @@ module.exports = function(deployer, network, accounts) { false ); + await deployer.link(AmmSignature, LoopringAmmPoolCopy); + await deployer.link(AmmPoolToken, LoopringAmmPoolCopy); await deployer.link(AmmAssetManagement, LoopringAmmPoolCopy); await deployer.link(AmmJoinRequest, LoopringAmmPoolCopy); await deployer.link(AmmExitRequest, LoopringAmmPoolCopy); diff --git a/packages/loopring_v3/test/ammUtils.ts b/packages/loopring_v3/test/ammUtils.ts index 477b54e40..327201044 100644 --- a/packages/loopring_v3/test/ammUtils.ts +++ b/packages/loopring_v3/test/ammUtils.ts @@ -53,6 +53,8 @@ export interface PoolExit { export interface PoolVirtualBalances { txType?: "SetVirtualBalances"; poolAddress: string; + vBalances: BN[]; + data: string; owner?: string; signature?: string; @@ -509,12 +511,14 @@ export class AmmPool { return exit; } - public async setVirtualBalances() { + public async setVirtualBalances(vBalances: BN[], data: string = "0x") { const vb: PoolVirtualBalances = { txType: "SetVirtualBalances", poolAddress: this.contract.address, signature: "0x00", - owner: Constants.zeroAddress + owner: Constants.zeroAddress, + vBalances, + data }; await this.process(vb, undefined); @@ -822,11 +826,10 @@ export class AmmPool { ) ); } else if (transaction.txType === "SetVirtualBalances") { - // Set virtual balances - for (let i = 0; i < this.tokens.length; i++) { - this.vTokenBalancesL2[i] = this.tokenBalancesL2[i] - .mul(this.amplificationFactor) - .div(this.AMPLIFICATION_FACTOR_BASE); + const poolVirtualBalances = transaction; + this.vTokenBalancesL2 = poolVirtualBalances.vBalances; + for (const vBalance of poolVirtualBalances.vBalances) { + logDebug("setVirtualBalance: " + vBalance.toString(10)); } } else if (transaction.txType === "Deposit") { const deposit = transaction; @@ -918,6 +921,19 @@ export class AmmPool { ); } + public static getPoolSetVirtualBalancesAuxData( + poolVirtualBalances: PoolVirtualBalances + ) { + const vBalances: string[] = []; + for (const vBalance of poolVirtualBalances.vBalances) { + vBalances.push(vBalance.toString(10)); + } + return web3.eth.abi.encodeParameter("tuple(uint96[],bytes)", [ + vBalances, + poolVirtualBalances.data + ]); + } + public static getPoolDepositAuxData(deposit: PoolDeposit) { const amounts: string[] = []; for (const amount of deposit.amounts) { @@ -955,7 +971,7 @@ export class AmmPool { } else if (transaction.txType === "SetVirtualBalances") { poolTx = { txType: PoolTransactionType.SET_VIRTUAL_BALANCES, - data: "0x", + data: this.getPoolSetVirtualBalancesAuxData(transaction), signature: transaction.signature }; } else if (transaction.txType === "Deposit") { diff --git a/packages/loopring_v3/test/testAMMPool.ts b/packages/loopring_v3/test/testAMMPool.ts index 3e79c5159..d9880f616 100644 --- a/packages/loopring_v3/test/testAMMPool.ts +++ b/packages/loopring_v3/test/testAMMPool.ts @@ -41,7 +41,9 @@ contract("LoopringAmmPool", (accounts: string[]) => { assetManager?: any ) => { controller = controller ? controller : ammController; - const assetManagerAddress = assetManager ? assetManager.address : Constants.zeroAddress; + const assetManagerAddress = assetManager + ? assetManager.address + : Constants.zeroAddress; const feeBipsAMM = 30; const tokens = ["WETH", "GTO"]; @@ -586,7 +588,10 @@ contract("LoopringAmmPool", (accounts: string[]) => { await ctx.submitTransactions(16); await pool.prePoolTransactions(); - await pool.setVirtualBalances(); + + // Set virtual balances to the actual balances + const balances = await pool.getBalancesL2(); + await pool.setVirtualBalances(balances); await ctx.submitTransactions(16); await ctx.submitPendingBlocks(); @@ -1076,9 +1081,7 @@ contract("LoopringAmmPool", (accounts: string[]) => { "INVALID_CHALLENGE" ); - const maxForcedExitAge = ( - await sharedConfig.maxForcedExitAge() - ).toNumber(); + const maxForcedExitAge = (await sharedConfig.maxForcedExitAge()).toNumber(); // Wait await ctx.advanceBlockTimestamp(maxForcedExitAge - 100); From 994e698449abd60b713baff77e3650ac5b5376cf Mon Sep 17 00:00:00 2001 From: Daniel Wang Date: Sun, 6 Jun 2021 00:39:17 +0800 Subject: [PATCH 4/4] AMM v2 - minor improvement over brecht's code (#2458) --- .../circuit/Circuits/SpotTradeCircuit.h | 28 +++++++++++-- .../circuit/Gadgets/AccountGadgets.h | 1 - .../contracts/amm/AmplifiedAmmController.sol | 40 +++++++++---------- .../contracts/amm/LoopringAmmPool.sol | 1 + .../contracts/amm/libamm/AmmData.sol | 9 +++-- .../contracts/amm/libamm/AmmStatus.sol | 21 +++++----- .../aux/access/DelayedTransaction.sol | 4 +- .../aux/access/SelectorBasedAccessManager.sol | 2 +- .../contracts/aux/bridge/BatchDepositor.sol | 8 ++-- .../contracts/core/iface/ExchangeData.sol | 2 +- .../loopring_v3/contracts/lib/LPToken.sol | 8 ++-- .../loopring_v3/contracts/test/LRCToken.sol | 2 +- .../contracts/test/TestAssetManager.sol | 3 +- .../test/TestMigrationBridgeConnector.sol | 2 +- .../test/TestSwappperBridgeConnector.sol | 2 +- .../contracts/thirdparty/MockContract.sol | 28 ++++++------- packages/loopring_v3/ethsnarks | 2 +- 17 files changed, 92 insertions(+), 71 deletions(-) diff --git a/packages/loopring_v3/circuit/Circuits/SpotTradeCircuit.h b/packages/loopring_v3/circuit/Circuits/SpotTradeCircuit.h index f9b062e44..6d8c4e367 100644 --- a/packages/loopring_v3/circuit/Circuits/SpotTradeCircuit.h +++ b/packages/loopring_v3/circuit/Circuits/SpotTradeCircuit.h @@ -201,10 +201,30 @@ class SpotTradeCircuit : public BaseTransactionCircuit vfills_B_A(pb, orderA.amm.packed, fillS_B.value(), state.constants._0, FMT(prefix, ".vfills_B_A")), vfills_S_B(pb, orderB.amm.packed, fillS_B.value(), state.constants._0, FMT(prefix, ".vfills_S_B")), vfills_B_B(pb, orderB.amm.packed, fillS_A.value(), state.constants._0, FMT(prefix, ".vfills_B_B")), - update_vbalanceS_A(pb, state.accountA.balanceS.weightAMM, vfills_S_A.result(), NUM_BITS_AMOUNT, FMT(prefix, ".update_vbalanceS_A")), - update_vbalanceB_A(pb, state.accountA.balanceB.weightAMM, vfills_B_A.result(), NUM_BITS_AMOUNT, FMT(prefix, ".update_vbalanceB_A")), - update_vbalanceS_B(pb, state.accountB.balanceS.weightAMM, vfills_S_B.result(), NUM_BITS_AMOUNT, FMT(prefix, ".update_vbalanceS_B")), - update_vbalanceB_B(pb, state.accountB.balanceB.weightAMM, vfills_B_B.result(), NUM_BITS_AMOUNT, FMT(prefix, ".update_vbalanceB_B")), + update_vbalanceS_A( + pb, + state.accountA.balanceS.weightAMM, + vfills_S_A.result(), + NUM_BITS_AMOUNT, + FMT(prefix, ".update_vbalanceS_A")), + update_vbalanceB_A( + pb, + state.accountA.balanceB.weightAMM, + vfills_B_A.result(), + NUM_BITS_AMOUNT, + FMT(prefix, ".update_vbalanceB_A")), + update_vbalanceS_B( + pb, + state.accountB.balanceS.weightAMM, + vfills_S_B.result(), + NUM_BITS_AMOUNT, + FMT(prefix, ".update_vbalanceS_B")), + update_vbalanceB_B( + pb, + state.accountB.balanceB.weightAMM, + vfills_B_B.result(), + NUM_BITS_AMOUNT, + FMT(prefix, ".update_vbalanceB_B")), validateAMM( pb, diff --git a/packages/loopring_v3/circuit/Gadgets/AccountGadgets.h b/packages/loopring_v3/circuit/Gadgets/AccountGadgets.h index a8d4e98e4..9c579b4bc 100644 --- a/packages/loopring_v3/circuit/Gadgets/AccountGadgets.h +++ b/packages/loopring_v3/circuit/Gadgets/AccountGadgets.h @@ -335,7 +335,6 @@ class DynamicBalanceGadget : public DynamicVariableGadget } }; - } // namespace Loopring #endif diff --git a/packages/loopring_v3/contracts/amm/AmplifiedAmmController.sol b/packages/loopring_v3/contracts/amm/AmplifiedAmmController.sol index 1ef44f4ce..f9cf38eea 100644 --- a/packages/loopring_v3/contracts/amm/AmplifiedAmmController.sol +++ b/packages/loopring_v3/contracts/amm/AmplifiedAmmController.sol @@ -20,13 +20,11 @@ contract AmplifiedAmmController is IAmmController, Claimable uint public constant AMPLIFICATION_FACTOR_BASE = (10 ** 18); - uint public constant MIN_CURVE_CHANGE_DELAY = 7 days; + uint public constant CURVE_CHANGE_MIN_DELAY = 7 days; uint public constant CURVE_CHANGE_AUTH_WINDOW = 7 days; - mapping(address => uint) public amplificationFactors; - - mapping(address => uint) public curveChangeAuthorization; - + mapping (address => uint) public amplificationFactors; + mapping (address => uint) public curveChangeAuthorization; function getInitialVirtualBalances( uint96[] memory joinAmounts @@ -61,24 +59,25 @@ contract AmplifiedAmmController is IAmmController, Claimable return true; } + if (getAmplificationFactor(pool) != AMPLIFICATION_FACTOR_BASE) { + return false; + } + // Special case: Always allow updating the virtual balances if the AF = 1 - if (getAmplificationFactor(pool) == AMPLIFICATION_FACTOR_BASE) { - for (uint i = 0; i < balances.length; i++) { - if(vBalancesNew[i] != balances[i]) { - return false; - } + for (uint i = 0; i < balances.length; i++) { + if (vBalancesNew[i] != balances[i]) { + return false; } - return true; } - return false; + return true; } function authorizeCurveChange(address pool) external onlyOwner { - curveChangeAuthorization[pool] = block.timestamp + MIN_CURVE_CHANGE_DELAY; + curveChangeAuthorization[pool] = block.timestamp + CURVE_CHANGE_MIN_DELAY; } function setAmplificationFactor( @@ -103,7 +102,7 @@ contract AmplifiedAmmController is IAmmController, Claimable } function setupPool( - LoopringAmmPool pool, + LoopringAmmPool pool, AmmData.PoolConfig calldata config ) external @@ -114,7 +113,7 @@ contract AmplifiedAmmController is IAmmController, Claimable function enterExitMode( LoopringAmmPool pool, - bool enabled + bool enabled ) external onlyOwner @@ -126,14 +125,15 @@ contract AmplifiedAmmController is IAmmController, Claimable function consumeCurveChangeAuthorized(address pool) internal - returns (bool) + returns (bool authorized) { uint timestamp = curveChangeAuthorization[pool]; - bool authorized = (timestamp <= block.timestamp) && (block.timestamp <= timestamp + MIN_CURVE_CHANGE_DELAY); + authorized = (timestamp <= block.timestamp) && + (block.timestamp <= timestamp + CURVE_CHANGE_AUTH_WINDOW); // Remove authorization - delete curveChangeAuthorization[pool]; - - return authorized; + if (timestamp > 0) { + delete curveChangeAuthorization[pool]; + } } } diff --git a/packages/loopring_v3/contracts/amm/LoopringAmmPool.sol b/packages/loopring_v3/contracts/amm/LoopringAmmPool.sol index 7913a1771..f0a12d801 100644 --- a/packages/loopring_v3/contracts/amm/LoopringAmmPool.sol +++ b/packages/loopring_v3/contracts/amm/LoopringAmmPool.sol @@ -107,6 +107,7 @@ contract LoopringAmmPool is external onlyFromController { + require(state.exitMode != enabled, "INVALID_STATE"); state.exitMode = enabled; } diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmData.sol b/packages/loopring_v3/contracts/amm/libamm/AmmData.sol index ede3051fa..2212498c6 100644 --- a/packages/loopring_v3/contracts/amm/libamm/AmmData.sol +++ b/packages/loopring_v3/contracts/amm/libamm/AmmData.sol @@ -122,9 +122,9 @@ library AmmData string symbol; uint _totalSupply; - mapping(address => uint) balanceOf; - mapping(address => mapping(address => uint)) allowance; - mapping(address => uint) nonces; + mapping (address => uint) balanceOf; + mapping (address => mapping (address => uint)) allowance; + mapping (address => uint) nonces; // AMM pool state variables IAmmSharedConfig sharedConfig; @@ -149,6 +149,7 @@ library AmmData mapping (bytes32 => bool) approvedTx; mapping (address => uint96) balancesL1; - bool exitMode; + + bool exitMode; } } diff --git a/packages/loopring_v3/contracts/amm/libamm/AmmStatus.sol b/packages/loopring_v3/contracts/amm/libamm/AmmStatus.sol index 300db1744..faee45143 100644 --- a/packages/loopring_v3/contracts/amm/libamm/AmmStatus.sol +++ b/packages/loopring_v3/contracts/amm/libamm/AmmStatus.sol @@ -37,20 +37,21 @@ library AmmStatus require(config.tokens.length == config.weights.length, "INVALID_DATA"); require(config.tokens.length >= 2, "INVALID_DATA"); require(config.exchange != address(0), "INVALID_EXCHANGE"); - require(config.accountID != 0, "INVALID_ACCOUNT_ID"); - require(S._totalSupply == 0, "POOL_IN_USE"); + IExchangeV3 exchange = IExchangeV3(config.exchange); + S.feeBips = config.feeBips; S.domainSeparator = EIP712.hash(EIP712.Domain(config.poolName, "1.0.0", address(this))); - S.poolName = config.poolName; S.symbol = config.tokenSymbol; + S.sharedConfig = IAmmSharedConfig(config.sharedConfig); + S.exchange = exchange; + S.exchangeOwner = exchange.owner(); + S.exchangeDomainSeparator = exchange.getDomainSeparator(); S.shutdownTimestamp = 0; S.exitMode = false; - IExchangeV3 exchange = IExchangeV3(config.exchange); - delete S.tokens; for (uint i = 0; i < config.tokens.length; i++) { address token = config.tokens[i]; @@ -61,12 +62,8 @@ library AmmStatus })); } - bool newPool = (S.accountID == 0); - if (newPool) { - S.sharedConfig = IAmmSharedConfig(config.sharedConfig); - S.exchange = exchange; - S.exchangeOwner = exchange.owner(); - S.exchangeDomainSeparator = exchange.getDomainSeparator(); + if (S.accountID == 0) { // new pool + require(config.accountID != 0, "INVALID_ACCOUNT_ID"); S.accountID = config.accountID; S.poolTokenID = exchange.getTokenID(address(this)); @@ -80,6 +77,8 @@ library AmmStatus uint96(AmmData.POOL_TOKEN_MINTED_SUPPLY), new bytes(0) ); + } else {// reused pool + require(config.accountID == 0, "INCONSISTENT_ACCOUNT_ID"); } } diff --git a/packages/loopring_v3/contracts/aux/access/DelayedTransaction.sol b/packages/loopring_v3/contracts/aux/access/DelayedTransaction.sol index bd19dac61..59aef5b91 100644 --- a/packages/loopring_v3/contracts/aux/access/DelayedTransaction.sol +++ b/packages/loopring_v3/contracts/aux/access/DelayedTransaction.sol @@ -110,11 +110,11 @@ abstract contract DelayedTransaction is IDelayedTransaction, ReentrancyGuard { // First cache all transactions ids of the transactions we will remove uint[] memory transactionIds = new uint[](pendingTransactions.length); - for(uint i = 0; i < pendingTransactions.length; i++) { + for (uint i = 0; i < pendingTransactions.length; i++) { transactionIds[i] = pendingTransactions[i].id; } // Now remove all delayed transactions - for(uint i = 0; i < transactionIds.length; i++) { + for (uint i = 0; i < transactionIds.length; i++) { cancelTransactionInternal(transactionIds[i]); } } diff --git a/packages/loopring_v3/contracts/aux/access/SelectorBasedAccessManager.sol b/packages/loopring_v3/contracts/aux/access/SelectorBasedAccessManager.sol index 0fc245268..c51006ba3 100644 --- a/packages/loopring_v3/contracts/aux/access/SelectorBasedAccessManager.sol +++ b/packages/loopring_v3/contracts/aux/access/SelectorBasedAccessManager.sol @@ -20,7 +20,7 @@ contract SelectorBasedAccessManager is Claimable ); address public immutable target; - mapping(address => mapping(bytes4 => bool)) public permissions; + mapping (address => mapping (bytes4 => bool)) public permissions; modifier withAccess(bytes4 selector) { diff --git a/packages/loopring_v3/contracts/aux/bridge/BatchDepositor.sol b/packages/loopring_v3/contracts/aux/bridge/BatchDepositor.sol index b7f342614..3ad74f553 100644 --- a/packages/loopring_v3/contracts/aux/bridge/BatchDepositor.sol +++ b/packages/loopring_v3/contracts/aux/bridge/BatchDepositor.sol @@ -56,7 +56,7 @@ abstract contract BatchDepositor is IBatchDepositor, ReentrancyGuard IDepositContract public immutable depositContract; mapping (uint => mapping (bytes32 => uint)) public pendingDeposits; - mapping (uint => mapping(uint => bool)) public withdrawn; + mapping (uint => mapping (uint => bool)) public withdrawn; // token -> tokenID mapping (address => uint16) public cachedTokenIDs; uint public batchIDGenerator; @@ -193,10 +193,10 @@ abstract contract BatchDepositor is IBatchDepositor, ReentrancyGuard IBatchDepositor.Deposit[] memory _deposits = depositsList[i]; for (uint j = 0; j < _deposits.length; j++) { deposit = _deposits[j]; - if(token != deposit.token) { + if (token != deposit.token) { token = deposit.token; tokenIdx = 0; - while(tokenIdx < numDistinctTokens && tokens[tokenIdx].token != token) { + while (tokenIdx < numDistinctTokens && tokens[tokenIdx].token != token) { tokenIdx++; } if (tokenIdx == numDistinctTokens) { @@ -223,7 +223,7 @@ abstract contract BatchDepositor is IBatchDepositor, ReentrancyGuard } // Do a normal deposit per token - for(uint i = 0; i < numDistinctTokens; i++) { + for (uint i = 0; i < numDistinctTokens; i++) { if (tokens[i].token == address(0)) { require(tokens[i].amount == msg.value || from == address(this), "INVALID_ETH_DEPOSIT"); } diff --git a/packages/loopring_v3/contracts/core/iface/ExchangeData.sol b/packages/loopring_v3/contracts/core/iface/ExchangeData.sol index 7f020c705..a64c2ca04 100644 --- a/packages/loopring_v3/contracts/core/iface/ExchangeData.sol +++ b/packages/loopring_v3/contracts/core/iface/ExchangeData.sol @@ -188,7 +188,7 @@ library ExchangeData bytes32 merkleRoot; // List of all blocks - mapping(uint => BlockInfo) blocks; + mapping (uint => BlockInfo) blocks; uint numBlocks; // List of all tokens diff --git a/packages/loopring_v3/contracts/lib/LPToken.sol b/packages/loopring_v3/contracts/lib/LPToken.sol index ec9d80c9e..4e21a9baf 100644 --- a/packages/loopring_v3/contracts/lib/LPToken.sol +++ b/packages/loopring_v3/contracts/lib/LPToken.sol @@ -18,10 +18,10 @@ contract LPToken is ERC20 string public symbol; uint8 public decimals; - uint public override totalSupply; - mapping(address => uint) public override balanceOf; - mapping(address => mapping(address => uint)) public override allowance; - mapping(address => uint) public nonces; + uint public override totalSupply; + mapping (address => uint) public override balanceOf; + mapping (address => mapping (address => uint)) public override allowance; + mapping (address => uint) public nonces; event Approval(address indexed owner, address indexed spender, uint value); event Transfer(address indexed from, address indexed to, uint value); diff --git a/packages/loopring_v3/contracts/test/LRCToken.sol b/packages/loopring_v3/contracts/test/LRCToken.sol index 2c3c348ff..12b86354a 100644 --- a/packages/loopring_v3/contracts/test/LRCToken.sol +++ b/packages/loopring_v3/contracts/test/LRCToken.sol @@ -60,7 +60,7 @@ library SafeMath { */ contract BasicToken is ERC20Basic { using SafeMath for uint; - mapping(address => uint) balances; + mapping (address => uint) balances; uint totalSupply_; /** * @dev total number of tokens in existence diff --git a/packages/loopring_v3/contracts/test/TestAssetManager.sol b/packages/loopring_v3/contracts/test/TestAssetManager.sol index 29db6825c..e321fcb30 100644 --- a/packages/loopring_v3/contracts/test/TestAssetManager.sol +++ b/packages/loopring_v3/contracts/test/TestAssetManager.sol @@ -8,13 +8,14 @@ import "../lib/TransferUtil.sol"; import "../amm/LoopringAmmPool.sol"; import "../amm/IAssetManager.sol"; + /// @author Brecht Devos - contract TestAssetManager is IAssetManager, Claimable { using MathUint for uint; using TransferUtil for address; - mapping(address => mapping(address => uint)) public poolBalances; + mapping (address => mapping (address => uint)) public poolBalances; function getBalances( address pool, diff --git a/packages/loopring_v3/contracts/test/TestMigrationBridgeConnector.sol b/packages/loopring_v3/contracts/test/TestMigrationBridgeConnector.sol index 33b7fbf9b..950389e29 100644 --- a/packages/loopring_v3/contracts/test/TestMigrationBridgeConnector.sol +++ b/packages/loopring_v3/contracts/test/TestMigrationBridgeConnector.sol @@ -76,7 +76,7 @@ contract TestMigrationBridgeConnector is IBridgeConnector require(txs[i].token == settings.token, "WRONG_TOKEN_IN_GROUP"); address to = bridgeCall.owner; - if(bridgeCall.userData.length == 32) { + if (bridgeCall.userData.length == 32) { UserSettings memory userSettings = abi.decode(bridgeCall.userData, (UserSettings)); to = userSettings.to; } diff --git a/packages/loopring_v3/contracts/test/TestSwappperBridgeConnector.sol b/packages/loopring_v3/contracts/test/TestSwappperBridgeConnector.sol index cc77548cf..d4d51a8e0 100644 --- a/packages/loopring_v3/contracts/test/TestSwappperBridgeConnector.sol +++ b/packages/loopring_v3/contracts/test/TestSwappperBridgeConnector.sol @@ -82,7 +82,7 @@ contract TestSwappperBridgeConnector is IBridgeConnector uint ammountInInvalid = 0; for (uint i = 0; i < txs.length; i++) { bridgeTx = txs[i]; - if(valid[i] && bridgeTx.userData.length == 32) { + if (valid[i] && bridgeTx.userData.length == 32) { UserSettings memory userSettings = abi.decode(bridgeTx.userData, (UserSettings)); uint userAmountOut = uint(bridgeTx.amount).mul(amountOut) / amountInExpected; if (userAmountOut < userSettings.minAmountOut) { diff --git a/packages/loopring_v3/contracts/thirdparty/MockContract.sol b/packages/loopring_v3/contracts/thirdparty/MockContract.sol index 4237e2fbb..992f08bcc 100644 --- a/packages/loopring_v3/contracts/thirdparty/MockContract.sol +++ b/packages/loopring_v3/contracts/thirdparty/MockContract.sol @@ -86,17 +86,17 @@ contract MockContract is MockInterface { bytes4 public constant SENTINEL_ANY_MOCKS = hex"01"; // A linked list allows easy iteration and inclusion checks - mapping(bytes32 => bytes) calldataMocks; - mapping(bytes => MockType) calldataMockTypes; - mapping(bytes => bytes) calldataExpectations; - mapping(bytes => string) calldataRevertMessage; - mapping(bytes32 => uint) calldataInvocations; - - mapping(bytes4 => bytes4) methodIdMocks; - mapping(bytes4 => MockType) methodIdMockTypes; - mapping(bytes4 => bytes) methodIdExpectations; - mapping(bytes4 => string) methodIdRevertMessages; - mapping(bytes32 => uint) methodIdInvocations; + mapping (bytes32 => bytes) calldataMocks; + mapping (bytes => MockType) calldataMockTypes; + mapping (bytes => bytes) calldataExpectations; + mapping (bytes => string) calldataRevertMessage; + mapping (bytes32 => uint) calldataInvocations; + + mapping (bytes4 => bytes4) methodIdMocks; + mapping (bytes4 => MockType) methodIdMockTypes; + mapping (bytes4 => bytes) methodIdExpectations; + mapping (bytes4 => string) methodIdRevertMessages; + mapping (bytes32 => uint) methodIdInvocations; MockType fallbackMockType; bytes fallbackExpectation; @@ -261,7 +261,7 @@ contract MockContract is MockInterface { bytes memory nextMock = calldataMocks[MOCKS_LIST_START]; bytes32 mockHash = keccak256(nextMock); // We cannot compary bytes - while(mockHash != MOCKS_LIST_END_HASH) { + while (mockHash != MOCKS_LIST_END_HASH) { // Reset all mock maps calldataMockTypes[nextMock] = MockType.Return; calldataExpectations[nextMock] = hex""; @@ -278,7 +278,7 @@ contract MockContract is MockInterface { // Reset all any calldataMocks bytes4 nextAnyMock = methodIdMocks[SENTINEL_ANY_MOCKS]; - while(nextAnyMock != SENTINEL_ANY_MOCKS) { + while (nextAnyMock != SENTINEL_ANY_MOCKS) { bytes4 currentAnyMock = nextAnyMock; methodIdMockTypes[currentAnyMock] = MockType.Return; methodIdExpectations[currentAnyMock] = hex""; @@ -297,7 +297,7 @@ contract MockContract is MockInterface { } function useAllGas() private { - while(true) { + while (true) { bool s; assembly { //expensive call to EC multiply contract diff --git a/packages/loopring_v3/ethsnarks b/packages/loopring_v3/ethsnarks index 842f6fec6..8ae1cb5b9 160000 --- a/packages/loopring_v3/ethsnarks +++ b/packages/loopring_v3/ethsnarks @@ -1 +1 @@ -Subproject commit 842f6fec638616a938b5361a5b603912ef5315ef +Subproject commit 8ae1cb5b9cfd25a0b37be7c34afcb34463d93381