From c03edfbdbc93203cb44544c5baade7831a509a86 Mon Sep 17 00:00:00 2001 From: gerg Date: Tue, 23 Jul 2024 13:47:04 -0400 Subject: [PATCH 01/54] add ,g to sed command --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index e9d581ac5..7c3aaaf7e 100644 --- a/README.md +++ b/README.md @@ -51,7 +51,7 @@ You will also need to configure your environment variables to point to RPC endpo Write your preferred RPC URL to `.env`, and source it. For example: ```bash -$ sed 's,YOUR_MAINNET_RPC_URL,' .env.example > .env +$ sed 's,YOUR_MAINNET_RPC_URL,,g' .env.example > .env $ source .env ``` From ca249b368407b2438ebc1257e148c50edb3f8f8a Mon Sep 17 00:00:00 2001 From: gerg Date: Tue, 23 Jul 2024 15:46:51 -0400 Subject: [PATCH 02/54] add beginning of LBPool and gradual value change --- pkg/pool-weighted/contracts/LBPool.sol | 79 ++++++++++++++++ .../contracts/lib/GradualValueChange.sol | 90 +++++++++++++++++++ 2 files changed, 169 insertions(+) create mode 100644 pkg/pool-weighted/contracts/LBPool.sol create mode 100644 pkg/pool-weighted/contracts/lib/GradualValueChange.sol diff --git a/pkg/pool-weighted/contracts/LBPool.sol b/pkg/pool-weighted/contracts/LBPool.sol new file mode 100644 index 000000000..995df71d4 --- /dev/null +++ b/pkg/pool-weighted/contracts/LBPool.sol @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +pragma solidity ^0.8.24; + +import { WeightedPool } from "./WeightedPool.sol"; +import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; +import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol"; +import { InputHelpers } from "@balancer-labs/v3-solidity-utils/contracts/helpers/InputHelpers.sol"; +import { GradualValueChange } from "../lib/GradualValueChange.sol"; + +/// @notice Basic Weighted Pool with immutable weights. +contract LBPool is WeightedPool, Ownable { + + uint256 private constant _NUM_TOKENS = 2; + + // Since we have max 2 tokens and the weights must sum to 1, we only need to track one weight + struct PoolState { + uint60 startTime; + uint60 endTime; + uint64 startWeight0; + uint64 endWeight0; + bool isPaused; + } + PoolState private _poolState; + + /// @dev Indicates end time before start time. + error EndTimeBeforeStartTime(); + + constructor(NewPoolParams memory params, IVault vault, bool startPaused, address owner) WeightedPool(params, vault) Ownable(owner) { + _isPaused = startPaused; + + InputHelpers.ensureInputLengthMatch(_NUM_TOKENS, params.numTokens); + // WeightedPool takes care of numTokens == normalizedWeights.length + + uint256 currentTime = block.timestamp; + _startGradualWeightChange(currentTime, currentTime, normalizedWeights, normalizedWeights); + + } + + function updateWeightsGradually( + uint256 startTime, + uint256 endTime, + uint256[] memory endWeights + ) external view onlyOwner { + InputHelpers.ensureInputLengthMatch(_NUM_TOKENS, endWeights.length); + + startTime = GradualValueChange.resolveStartTime(startTime, endTime); + _startGradualWeightChange(startTime, endTime, _getNormalizedWeights(), endWeights); + } + + function _getNormalizedWeights() internal view override returns (uint256[] memory) { + uint256[] memory normalizedWeights = new uint256[](_NUM_TOKENS); + + normalizedWeights[0] = _getNormalizedWeight0(); + normalizedWeights[1] = FixedPoint.ONE - normalizedWeights[0]; + return normalizedWeights; + } + + function _getNormalizedWeight(uint256 tokenIndex) internal view virtual override returns (uint256) { + uint256 normalizedWeight0 = _getNormalizedWeight0() + + // prettier-ignore + if (tokenIndex == 0) { return normalizedWeight0; } + else if (tokenIndex == 1) { FixedPoint.ONE - normalizedWeight0; } + else { + revert IVaultErrors.InvalidToken(); + } + } + + function _getNormalizedWeight0() internal view virtual returns (uint256) { + PoolState poolState = _poolState; + uint256 pctProgress = _getWeightChangeProgress(poolState); + return GradualValueChange.interpolateValue(poolState.startWeight0, poolState.endWeight0, pctProgress); + } + + function _getWeightChangeProgress(PoolState poolState) internal view returns (uint256) { + return GradualValueChange.calculateValueChangeProgress(poolState.startTime, poolState.endTime); + } +} diff --git a/pkg/pool-weighted/contracts/lib/GradualValueChange.sol b/pkg/pool-weighted/contracts/lib/GradualValueChange.sol new file mode 100644 index 000000000..06259accc --- /dev/null +++ b/pkg/pool-weighted/contracts/lib/GradualValueChange.sol @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +import { FixedPoint } from "@balancer-labs/v3-solidity-utils/contracts/math/FixedPoint.sol"; + +pragma solidity ^0.8.24; + +// solhint-disable not-rely-on-time + +library GradualValueChange { + using FixedPoint for uint256; + + function getInterpolatedValue( + uint256 startValue, + uint256 endValue, + uint256 startTime, + uint256 endTime + ) internal view returns (uint256) { + uint256 pctProgress = calculateValueChangeProgress(startTime, endTime); + + return interpolateValue(startValue, endValue, pctProgress); + } + + function resolveStartTime(uint256 startTime, uint256 endTime) internal view returns (uint256 resolvedStartTime) { + // If the start time is in the past, "fast forward" to start now + // This avoids discontinuities in the value curve. Otherwise, if you set the start/end times with + // only 10% of the period in the future, the value would immediately jump 90% + resolvedStartTime = _max(block.timestamp, startTime); + + _require(resolvedStartTime <= endTime, Errors.GRADUAL_UPDATE_TIME_TRAVEL); + } + + function interpolateValue( + uint256 startValue, + uint256 endValue, + uint256 pctProgress + ) internal pure returns (uint256) { + if (pctProgress >= FixedPoint.ONE || startValue == endValue) return endValue; + if (pctProgress == 0) return startValue; + + if (startValue > endValue) { + uint256 delta = pctProgress.mulDown(startValue - endValue); + return startValue - delta; + } else { + uint256 delta = pctProgress.mulDown(endValue - startValue); + return startValue + delta; + } + } + + /** + * @dev Returns a fixed-point number representing how far along the current value change is, where 0 means the + * change has not yet started, and FixedPoint.ONE means it has fully completed. + */ + function calculateValueChangeProgress(uint256 startTime, uint256 endTime) internal view returns (uint256) { + if (block.timestamp >= endTime) { + return FixedPoint.ONE; + } else if (block.timestamp <= startTime) { + return 0; + } + + // No need for SafeMath as it was checked right above: endTime > block.timestamp > startTime + uint256 totalSeconds = endTime - startTime; + uint256 secondsElapsed = block.timestamp - startTime; + + // We don't need to consider zero division here as this is covered above. + return secondsElapsed.divDown(totalSeconds); + } + + /** + * @dev Returns the larger of two uint256s. + */ + function _max(uint256 a, uint256 b) internal pure returns (uint256 result) { + // Equivalent to: + // result = (a < b) ? b : a; + assembly { + result := sub(a, mul(sub(a, b), lt(a, b))) + } + } +} From 151248750ecf38f0307184258a011e47707d022f Mon Sep 17 00:00:00 2001 From: gerg Date: Thu, 25 Jul 2024 13:13:04 -0400 Subject: [PATCH 03/54] move towards libraries for common functions; add max to FP library; optimize weight validation for 2 weights --- pkg/pool-weighted/contracts/LBPool.sol | 6 +- .../contracts/lib/GradualValueChange.sol | 12 +-- .../contracts/lib/WeightValidation.sol | 73 +++++++++++++++++++ .../contracts/math/FixedPoint.sol | 11 +++ 4 files changed, 89 insertions(+), 13 deletions(-) create mode 100644 pkg/pool-weighted/contracts/lib/WeightValidation.sol diff --git a/pkg/pool-weighted/contracts/LBPool.sol b/pkg/pool-weighted/contracts/LBPool.sol index 995df71d4..327676480 100644 --- a/pkg/pool-weighted/contracts/LBPool.sol +++ b/pkg/pool-weighted/contracts/LBPool.sol @@ -6,7 +6,8 @@ import { WeightedPool } from "./WeightedPool.sol"; import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol"; import { InputHelpers } from "@balancer-labs/v3-solidity-utils/contracts/helpers/InputHelpers.sol"; -import { GradualValueChange } from "../lib/GradualValueChange.sol"; +import { GradualValueChange } from "./lib/GradualValueChange.sol"; +import { WeightValidation } from "./lib/WeightValidation.sol"; /// @notice Basic Weighted Pool with immutable weights. contract LBPool is WeightedPool, Ownable { @@ -30,7 +31,7 @@ contract LBPool is WeightedPool, Ownable { _isPaused = startPaused; InputHelpers.ensureInputLengthMatch(_NUM_TOKENS, params.numTokens); - // WeightedPool takes care of numTokens == normalizedWeights.length + // WeightedPool takes care of numTokens == normalizedWeights.length // TODO: do this when no longer inheriting from WP uint256 currentTime = block.timestamp; _startGradualWeightChange(currentTime, currentTime, normalizedWeights, normalizedWeights); @@ -43,6 +44,7 @@ contract LBPool is WeightedPool, Ownable { uint256[] memory endWeights ) external view onlyOwner { InputHelpers.ensureInputLengthMatch(_NUM_TOKENS, endWeights.length); + WeightValidation.validateTwoWeights(endWeights[0], endWeights[1]); startTime = GradualValueChange.resolveStartTime(startTime, endTime); _startGradualWeightChange(startTime, endTime, _getNormalizedWeights(), endWeights); diff --git a/pkg/pool-weighted/contracts/lib/GradualValueChange.sol b/pkg/pool-weighted/contracts/lib/GradualValueChange.sol index 06259accc..ce53f1c69 100644 --- a/pkg/pool-weighted/contracts/lib/GradualValueChange.sol +++ b/pkg/pool-weighted/contracts/lib/GradualValueChange.sol @@ -36,7 +36,7 @@ library GradualValueChange { // If the start time is in the past, "fast forward" to start now // This avoids discontinuities in the value curve. Otherwise, if you set the start/end times with // only 10% of the period in the future, the value would immediately jump 90% - resolvedStartTime = _max(block.timestamp, startTime); + resolvedStartTime = FixedPoint.max(block.timestamp, startTime); _require(resolvedStartTime <= endTime, Errors.GRADUAL_UPDATE_TIME_TRAVEL); } @@ -77,14 +77,4 @@ library GradualValueChange { return secondsElapsed.divDown(totalSeconds); } - /** - * @dev Returns the larger of two uint256s. - */ - function _max(uint256 a, uint256 b) internal pure returns (uint256 result) { - // Equivalent to: - // result = (a < b) ? b : a; - assembly { - result := sub(a, mul(sub(a, b), lt(a, b))) - } - } } diff --git a/pkg/pool-weighted/contracts/lib/WeightValidation.sol b/pkg/pool-weighted/contracts/lib/WeightValidation.sol new file mode 100644 index 000000000..07670bb5c --- /dev/null +++ b/pkg/pool-weighted/contracts/lib/WeightValidation.sol @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +import { FixedPoint } from "@balancer-labs/v3-solidity-utils/contracts/math/FixedPoint.sol"; +import { WeightedMath } from "@balancer-labs/v3-solidity-utils/contracts/math/WeightedMath.sol"; + +pragma solidity ^0.8.24; + +library WeightValidation { + using FixedPoint for uint256; + + /// @dev Indicates that one of the pool tokens' weight is below the minimum allowed. + error MinWeight(); + + /// @dev Indicates that the sum of the pool tokens' weights is not FP 1. + error NormalizedWeightInvariant(); + + /** + * @dev Returns a fixed-point number representing how far along the current value change is, where 0 means the + * change has not yet started, and FixedPoint.ONE means it has fully completed. + */ + function validateWeights( + uint256[] memory normalizedWeights, + uint256 numTokens + ) internal pure { + // Ensure each normalized weight is above the minimum + uint256 normalizedSum = 0; + for (uint8 i = 0; i < numTokens; ++i) { + uint256 normalizedWeight = normalizedWeights[i]; + + if (normalizedWeight < WeightedMath._MIN_WEIGHT) { + revert MinWeight(); + } + normalizedSum += normalizedWeight; + } + // Ensure that the normalized weights sum to ONE + if (normalizedSum != FixedPoint.ONE) { + revert NormalizedWeightInvariant(); + } + } + + /** + * @dev Returns a fixed-point number representing how far along the current value change is, where 0 means the + * change has not yet started, and FixedPoint.ONE means it has fully completed. + */ + function validateTwoWeights( + uint256 memory normalizedWeight0, + uint256 memory normalizedWeight1, + ) internal pure { + + // Ensure each normalized weight is above the minimum + if (normalizedWeight0 < WeightedMath._MIN_WEIGHT || normalizedWeight1 < WeightedMath._MIN_WEIGHT) { + revert MinWeight(); + } + + // Ensure that the normalized weights sum to ONE + if (normalizedWeight0 + normalizedWeight1 != FixedPoint.ONE) { + revert NormalizedWeightInvariant(); + } + + } +} diff --git a/pkg/solidity-utils/contracts/math/FixedPoint.sol b/pkg/solidity-utils/contracts/math/FixedPoint.sol index 78897787c..244868374 100644 --- a/pkg/solidity-utils/contracts/math/FixedPoint.sol +++ b/pkg/solidity-utils/contracts/math/FixedPoint.sol @@ -152,4 +152,15 @@ library FixedPoint { result := mul(lt(x, ONE), sub(ONE, x)) } } + + /** + * @dev Returns the larger of two uint256s. + */ + function max(uint256 a, uint256 b) internal pure returns (uint256 result) { + // Equivalent to: + // result = (a < b) ? b : a; + assembly { + result := sub(a, mul(sub(a, b), lt(a, b))) + } + } } From 410a9c45f2905631c1f773302b209600c3de2a96 Mon Sep 17 00:00:00 2001 From: gerg Date: Fri, 23 Aug 2024 10:32:16 -0400 Subject: [PATCH 04/54] do all imports; track swap fee percentage; enforce 2 tokens; track max time to avoid time overflow attack; add misc events and errors; change paused vars to be swapEnabled; add setters and getters for swap enabled/swap fee/gradual update params; add 3 hooks for dynamic swap fee percentage, paused blocking swaps, and only owner can join; build out GradualValueChange library --- pkg/pool-weighted/contracts/LBPool.sol | 240 +++++++++++++++--- .../contracts/lib/GradualValueChange.sol | 16 +- .../contracts/lib/WeightValidation.sol | 4 +- 3 files changed, 226 insertions(+), 34 deletions(-) diff --git a/pkg/pool-weighted/contracts/LBPool.sol b/pkg/pool-weighted/contracts/LBPool.sol index 327676480..4ad051964 100644 --- a/pkg/pool-weighted/contracts/LBPool.sol +++ b/pkg/pool-weighted/contracts/LBPool.sol @@ -5,77 +5,255 @@ pragma solidity ^0.8.24; import { WeightedPool } from "./WeightedPool.sol"; import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol"; +import { IRouterCommon } from "@balancer-labs/v3-interfaces/contracts/vault/IRouterCommon.sol"; +import { AddLiquidityKind } from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; + import { InputHelpers } from "@balancer-labs/v3-solidity-utils/contracts/helpers/InputHelpers.sol"; +import { FixedPoint } from "@balancer-labs/v3-solidity-utils/contracts/math/FixedPoint.sol"; +import { IVaultErrors } from "@balancer-labs/v3-interfaces/contracts/vault/IVaultErrors.sol"; import { GradualValueChange } from "./lib/GradualValueChange.sol"; import { WeightValidation } from "./lib/WeightValidation.sol"; - -/// @notice Basic Weighted Pool with immutable weights. +/// @notice Inheriting from WeightedPool is only slightly wasteful (setting 2 immutable weights +/// that will not be used later), and it is tremendously helpful for pool validation and +/// any potential future parent class changes. contract LBPool is WeightedPool, Ownable { - uint256 private constant _NUM_TOKENS = 2; - - // Since we have max 2 tokens and the weights must sum to 1, we only need to track one weight + // Since we have max 2 tokens and the weights must sum to 1, we only need to store one weight struct PoolState { - uint60 startTime; - uint60 endTime; + uint56 startTime; + uint56 endTime; uint64 startWeight0; uint64 endWeight0; - bool isPaused; + bool swapEnabled; } PoolState private _poolState; + uint256 private _swapFeePercentage; - /// @dev Indicates end time before start time. - error EndTimeBeforeStartTime(); + uint256 private constant _NUM_TOKENS = 2; + + // `{start,end}Time` are `uint56`s. Ensure that no input time (passed as `uint256`) will overflow. + uint256 private constant _MAX_TIME = type(uint56).max; - constructor(NewPoolParams memory params, IVault vault, bool startPaused, address owner) WeightedPool(params, vault) Ownable(owner) { - _isPaused = startPaused; + event SwapFeePercentageChanged(uint256 swapFeePercentage); + event SwapEnabledSet(bool swapEnabled); + event GradualWeightUpdateScheduled( + uint256 startTime, + uint256 endTime, + uint256[] startWeights, + uint256[] endWeights + ); + /// @dev Indicates that the swap fee is below the minimum allowable swap fee. + error MinSwapFee(); + + /// @dev Indicates that the swap fee is above the maximum allowable swap fee. + error MaxSwapFee(); + + constructor(NewPoolParams memory params, IVault vault, address owner, bool swapEnabledOnStart) + WeightedPool(params, vault) + Ownable(owner) + { InputHelpers.ensureInputLengthMatch(_NUM_TOKENS, params.numTokens); - // WeightedPool takes care of numTokens == normalizedWeights.length // TODO: do this when no longer inheriting from WP + // WeightedPool validates `numTokens == normalizedWeights.length` + // _startGradualWeightChange validates weights uint256 currentTime = block.timestamp; - _startGradualWeightChange(currentTime, currentTime, normalizedWeights, normalizedWeights); - + _startGradualWeightChange(uint56(currentTime), uint56(currentTime), params.normalizedWeights, params.normalizedWeights, true, swapEnabledOnStart); } function updateWeightsGradually( uint256 startTime, uint256 endTime, uint256[] memory endWeights - ) external view onlyOwner { + ) external onlyOwner { InputHelpers.ensureInputLengthMatch(_NUM_TOKENS, endWeights.length); WeightValidation.validateTwoWeights(endWeights[0], endWeights[1]); + // Ensure startTime >= now startTime = GradualValueChange.resolveStartTime(startTime, endTime); - _startGradualWeightChange(startTime, endTime, _getNormalizedWeights(), endWeights); + + // Ensure time will not overflow in storage; only check end (startTime <= endTime) + GradualValueChange.ensureNoTimeOverflow(endTime, _MAX_TIME); + + _startGradualWeightChange(uint56(startTime), uint56(endTime), _getNormalizedWeights(), endWeights, false, true); } + /** + * @dev Return start time, end time, and endWeights as an array. + * Current weights should be retrieved via `getNormalizedWeights()`. + */ + function getGradualWeightUpdateParams() + external + view + returns ( + uint256 startTime, + uint256 endTime, + uint256[] memory endWeights + ) + { + PoolState memory poolState = _poolState; + + startTime = poolState.startTime; + endTime = poolState.endTime; + + endWeights = new uint256[](_NUM_TOKENS); + endWeights[0] = poolState.endWeight0; + endWeights[1] = FixedPoint.ONE - poolState.endWeight0; + } + + /** + * @notice Set the swap fee percentage. + * @dev This is a permissioned function. The swap fee must be within the bounds set by + * MIN_SWAP_FEE_PERCENTAGE/MAX_SWAP_FEE_PERCENTAGE. Emits the SwapFeePercentageChanged event. + */ + function setSwapFeePercentage(uint256 swapFeePercentage) public virtual onlyOwner { + _setSwapFeePercentage(swapFeePercentage); + } + + /** + * @notice Return the current value of the swap fee percentage. + */ + function getSwapFeePercentage() public view virtual returns (uint256) { + return _swapFeePercentage; + } + + /** + * @notice Pause/unpause trading. + */ + function setSwapEnabled(bool swapEnabled) external onlyOwner { + _poolState.swapEnabled = !swapEnabled; + } + + /** + * @notice Return whether swaps are enabled or not for the given pool. + */ + function getSwapEnabled() external view returns (bool) { + return _getPoolSwapEnabledState(); + } + + /* ========================================= + * ========================================= + * ============HOOK FUNCTIONS=============== + * ========================================= + * ========================================= + */ + + /** + * @notice Check that the caller who initiated the add liquidity operation is the owner. + * @param initiator The address (usually a router contract) that initiated a swap operation on the Vault + */ + function onBeforeAddLiquidity( + address initiator, + address, + AddLiquidityKind, + uint256[] memory, + uint256, + uint256[] memory, + bytes memory + ) external view onlyVault returns (bool success) { + // Check `initiator == owner` first to avoid calling `getSender()` on a potentially non-router contract/address + success = (initiator == owner()) || (IRouterCommon(initiator).getSender() == owner()); + } + + /** + * @notice Called before a swap to give the Pool block swap in paused pool. + * @return success True if the pool is not paused. + */ + function onBeforeSwap(PoolSwapParams calldata, address) public onlyVault virtual returns (bool) { + return _getPoolSwapEnabledState(); + } + + /** + * @notice Called after `onBeforeSwap` and before the main swap operation, if the pool has dynamic fees. + * @return success True if the pool wishes to proceed with settlement + * @return dynamicSwapFeePercentage Value of the swap fee percentage, as an 18-decimal FP value + */ + function onComputeDynamicSwapFeePercentage( + PoolSwapParams calldata, + address, + uint256 + ) external onlyVault view returns (bool, uint256) { + return (true, getSwapFeePercentage()); + } + + /* ========================================= + * ========================================= + * ==========INTERNAL FUNCTIONS============= + * ========================================= + * ========================================= + */ + + function _getNormalizedWeight0() internal view virtual returns (uint256) { + PoolState memory poolState = _poolState; + uint256 pctProgress = _getWeightChangeProgress(poolState); + return GradualValueChange.interpolateValue(poolState.startWeight0, poolState.endWeight0, pctProgress); + } + + function _getNormalizedWeight(uint256 tokenIndex) internal view virtual override returns (uint256) { + uint256 normalizedWeight0 = _getNormalizedWeight0(); + if (tokenIndex == 0) { + return normalizedWeight0; + } else if (tokenIndex == 1) { + return FixedPoint.ONE - normalizedWeight0; + } + revert IVaultErrors.InvalidToken(); + } + function _getNormalizedWeights() internal view override returns (uint256[] memory) { uint256[] memory normalizedWeights = new uint256[](_NUM_TOKENS); - normalizedWeights[0] = _getNormalizedWeight0(); normalizedWeights[1] = FixedPoint.ONE - normalizedWeights[0]; return normalizedWeights; } - function _getNormalizedWeight(uint256 tokenIndex) internal view virtual override returns (uint256) { - uint256 normalizedWeight0 = _getNormalizedWeight0() + function _getWeightChangeProgress(PoolState memory poolState) internal view returns (uint256) { + return GradualValueChange.calculateValueChangeProgress(poolState.startTime, poolState.endTime); + } - // prettier-ignore - if (tokenIndex == 0) { return normalizedWeight0; } - else if (tokenIndex == 1) { FixedPoint.ONE - normalizedWeight0; } - else { - revert IVaultErrors.InvalidToken(); + /** + * @dev When calling updateWeightsGradually again during an update, reset the start weights to the current weights, + * if necessary. + */ + function _startGradualWeightChange( + uint56 startTime, + uint56 endTime, + uint256[] memory startWeights, + uint256[] memory endWeights, + bool modifySwapEnabledStatus, + bool newSwapEnabled + ) internal virtual { + WeightValidation.validateTwoWeights(endWeights[0], endWeights[1]); + + PoolState memory poolState = _poolState; + poolState.startTime = startTime; + poolState.endTime = endTime; + poolState.startWeight0 = uint64(startWeights[0]); + poolState.endWeight0 = uint64(endWeights[0]); + + if (modifySwapEnabledStatus) { + poolState.swapEnabled = newSwapEnabled; + emit SwapEnabledSet(newSwapEnabled); } + + _poolState = poolState; + emit GradualWeightUpdateScheduled(startTime, endTime, startWeights, endWeights); } - function _getNormalizedWeight0() internal view virtual returns (uint256) { - PoolState poolState = _poolState; - uint256 pctProgress = _getWeightChangeProgress(poolState); - return GradualValueChange.interpolateValue(poolState.startWeight0, poolState.endWeight0, pctProgress); + function _setSwapFeePercentage(uint256 swapFeePercentage) internal virtual { + // TODO: can we get min/max swap fee as internal fns in the WP base class? External call is wasteful. + if (swapFeePercentage < this.getMinimumSwapFeePercentage()) { + revert MinSwapFee(); + } + // TODO: can we get min/max swap fee as internal fns in the WP base class? External call is wasteful. + if (swapFeePercentage > this.getMaximumSwapFeePercentage()) { + revert MaxSwapFee(); + } + + _swapFeePercentage = swapFeePercentage; + emit SwapFeePercentageChanged(swapFeePercentage); } - function _getWeightChangeProgress(PoolState poolState) internal view returns (uint256) { - return GradualValueChange.calculateValueChangeProgress(poolState.startTime, poolState.endTime); + function _getPoolSwapEnabledState() internal view returns (bool) { + return _poolState.swapEnabled; } } diff --git a/pkg/pool-weighted/contracts/lib/GradualValueChange.sol b/pkg/pool-weighted/contracts/lib/GradualValueChange.sol index ce53f1c69..bf3f488c1 100644 --- a/pkg/pool-weighted/contracts/lib/GradualValueChange.sol +++ b/pkg/pool-weighted/contracts/lib/GradualValueChange.sol @@ -19,6 +19,12 @@ pragma solidity ^0.8.24; // solhint-disable not-rely-on-time library GradualValueChange { + /// @dev Indicates that the start time is after the end time + error GradualUpdateTimeTravel(); + + /// @dev Indicates that an input time is larger than the maximum storage value. + error TimeTruncatedInStorage(); + using FixedPoint for uint256; function getInterpolatedValue( @@ -38,7 +44,15 @@ library GradualValueChange { // only 10% of the period in the future, the value would immediately jump 90% resolvedStartTime = FixedPoint.max(block.timestamp, startTime); - _require(resolvedStartTime <= endTime, Errors.GRADUAL_UPDATE_TIME_TRAVEL); + if (resolvedStartTime > endTime) { + revert GradualUpdateTimeTravel(); + } + } + + function ensureNoTimeOverflow(uint256 time, uint256 maxTime) internal pure { + if (time > maxTime) { + revert TimeTruncatedInStorage(); + } } function interpolateValue( diff --git a/pkg/pool-weighted/contracts/lib/WeightValidation.sol b/pkg/pool-weighted/contracts/lib/WeightValidation.sol index 07670bb5c..43ba1a3ba 100644 --- a/pkg/pool-weighted/contracts/lib/WeightValidation.sol +++ b/pkg/pool-weighted/contracts/lib/WeightValidation.sol @@ -55,8 +55,8 @@ library WeightValidation { * change has not yet started, and FixedPoint.ONE means it has fully completed. */ function validateTwoWeights( - uint256 memory normalizedWeight0, - uint256 memory normalizedWeight1, + uint256 normalizedWeight0, + uint256 normalizedWeight1 ) internal pure { // Ensure each normalized weight is above the minimum From 1aa5e659eeae9cde20eeea728f8dbea143f91cc9 Mon Sep 17 00:00:00 2001 From: gerg Date: Fri, 23 Aug 2024 10:47:59 -0400 Subject: [PATCH 05/54] add LBPoolFactory; lint --- pkg/pool-weighted/contracts/LBPool.sol | 39 ++++---- pkg/pool-weighted/contracts/LBPoolFactory.sol | 90 +++++++++++++++++++ .../contracts/lib/GradualValueChange.sol | 1 - .../contracts/lib/WeightValidation.sol | 12 +-- 4 files changed, 114 insertions(+), 28 deletions(-) create mode 100644 pkg/pool-weighted/contracts/LBPoolFactory.sol diff --git a/pkg/pool-weighted/contracts/LBPool.sol b/pkg/pool-weighted/contracts/LBPool.sol index 4ad051964..53b731791 100644 --- a/pkg/pool-weighted/contracts/LBPool.sol +++ b/pkg/pool-weighted/contracts/LBPool.sol @@ -13,11 +13,11 @@ import { FixedPoint } from "@balancer-labs/v3-solidity-utils/contracts/math/Fixe import { IVaultErrors } from "@balancer-labs/v3-interfaces/contracts/vault/IVaultErrors.sol"; import { GradualValueChange } from "./lib/GradualValueChange.sol"; import { WeightValidation } from "./lib/WeightValidation.sol"; + /// @notice Inheriting from WeightedPool is only slightly wasteful (setting 2 immutable weights /// that will not be used later), and it is tremendously helpful for pool validation and /// any potential future parent class changes. contract LBPool is WeightedPool, Ownable { - // Since we have max 2 tokens and the weights must sum to 1, we only need to store one weight struct PoolState { uint56 startTime; @@ -49,16 +49,25 @@ contract LBPool is WeightedPool, Ownable { /// @dev Indicates that the swap fee is above the maximum allowable swap fee. error MaxSwapFee(); - constructor(NewPoolParams memory params, IVault vault, address owner, bool swapEnabledOnStart) - WeightedPool(params, vault) - Ownable(owner) - { + constructor( + NewPoolParams memory params, + IVault vault, + address owner, + bool swapEnabledOnStart + ) WeightedPool(params, vault) Ownable(owner) { InputHelpers.ensureInputLengthMatch(_NUM_TOKENS, params.numTokens); // WeightedPool validates `numTokens == normalizedWeights.length` // _startGradualWeightChange validates weights uint256 currentTime = block.timestamp; - _startGradualWeightChange(uint56(currentTime), uint56(currentTime), params.normalizedWeights, params.normalizedWeights, true, swapEnabledOnStart); + _startGradualWeightChange( + uint56(currentTime), + uint56(currentTime), + params.normalizedWeights, + params.normalizedWeights, + true, + swapEnabledOnStart + ); } function updateWeightsGradually( @@ -77,7 +86,7 @@ contract LBPool is WeightedPool, Ownable { _startGradualWeightChange(uint56(startTime), uint56(endTime), _getNormalizedWeights(), endWeights, false, true); } - + /** * @dev Return start time, end time, and endWeights as an array. * Current weights should be retrieved via `getNormalizedWeights()`. @@ -85,11 +94,7 @@ contract LBPool is WeightedPool, Ownable { function getGradualWeightUpdateParams() external view - returns ( - uint256 startTime, - uint256 endTime, - uint256[] memory endWeights - ) + returns (uint256 startTime, uint256 endTime, uint256[] memory endWeights) { PoolState memory poolState = _poolState; @@ -103,7 +108,7 @@ contract LBPool is WeightedPool, Ownable { /** * @notice Set the swap fee percentage. - * @dev This is a permissioned function. The swap fee must be within the bounds set by + * @dev This is a permissioned function. The swap fee must be within the bounds set by * MIN_SWAP_FEE_PERCENTAGE/MAX_SWAP_FEE_PERCENTAGE. Emits the SwapFeePercentageChanged event. */ function setSwapFeePercentage(uint256 swapFeePercentage) public virtual onlyOwner { @@ -136,7 +141,7 @@ contract LBPool is WeightedPool, Ownable { * ============HOOK FUNCTIONS=============== * ========================================= * ========================================= - */ + */ /** * @notice Check that the caller who initiated the add liquidity operation is the owner. @@ -159,7 +164,7 @@ contract LBPool is WeightedPool, Ownable { * @notice Called before a swap to give the Pool block swap in paused pool. * @return success True if the pool is not paused. */ - function onBeforeSwap(PoolSwapParams calldata, address) public onlyVault virtual returns (bool) { + function onBeforeSwap(PoolSwapParams calldata, address) public virtual onlyVault returns (bool) { return _getPoolSwapEnabledState(); } @@ -172,7 +177,7 @@ contract LBPool is WeightedPool, Ownable { PoolSwapParams calldata, address, uint256 - ) external onlyVault view returns (bool, uint256) { + ) external view onlyVault returns (bool, uint256) { return (true, getSwapFeePercentage()); } @@ -181,7 +186,7 @@ contract LBPool is WeightedPool, Ownable { * ==========INTERNAL FUNCTIONS============= * ========================================= * ========================================= - */ + */ function _getNormalizedWeight0() internal view virtual returns (uint256) { PoolState memory poolState = _poolState; diff --git a/pkg/pool-weighted/contracts/LBPoolFactory.sol b/pkg/pool-weighted/contracts/LBPoolFactory.sol new file mode 100644 index 000000000..5db253849 --- /dev/null +++ b/pkg/pool-weighted/contracts/LBPoolFactory.sol @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +pragma solidity ^0.8.24; + +import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; + +import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol"; +import { IRateProvider } from "@balancer-labs/v3-interfaces/contracts/vault/IRateProvider.sol"; +import { IPoolVersion } from "@balancer-labs/v3-interfaces/contracts/solidity-utils/helpers/IPoolVersion.sol"; +import "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; + +import { BasePoolFactory } from "@balancer-labs/v3-pool-utils/contracts/BasePoolFactory.sol"; +import { Version } from "@balancer-labs/v3-solidity-utils/contracts/helpers/Version.sol"; + +import { LBPool } from "./LBPool.sol"; +import { WeightedPool } from "./WeightedPool.sol"; + +/** + * @notice LBPool Factory + * @dev This is a factory specific to LBPools, allowing only 2 tokens + */ +contract LBPoolFactory is IPoolVersion, BasePoolFactory, Version { + // solhint-disable not-rely-on-time + + string private _poolVersion; + + constructor( + IVault vault, + uint32 pauseWindowDuration, + string memory factoryVersion, + string memory poolVersion + ) BasePoolFactory(vault, pauseWindowDuration, type(LBPool).creationCode) Version(factoryVersion) { + _poolVersion = poolVersion; + } + + /// @inheritdoc IPoolVersion + function getPoolVersion() external view returns (string memory) { + return _poolVersion; + } + + /** + * @notice Deploys a new `LBPool`. + * @dev Tokens must be sorted for pool registration. + * @param name The name of the pool + * @param symbol The symbol of the pool + * @param tokens An array of descriptors for the tokens the pool will manage + * @param normalizedWeights The pool weights (must add to FixedPoint.ONE) + * @param roleAccounts Addresses the Vault will allow to change certain pool settings + * @param swapFeePercentage Initial swap fee percentage + * @param owner The owner address for pool; the sole LP with swapEnable/swapFee change permissions + * @param salt The salt value that will be passed to create3 deployment + */ + function create( + string memory name, + string memory symbol, + TokenConfig[] memory tokens, + uint256[] memory normalizedWeights, + PoolRoleAccounts memory roleAccounts, + uint256 swapFeePercentage, + address owner, + bool swapEnabledOnStart, + bytes32 salt + ) external returns (address pool) { + if (roleAccounts.poolCreator != address(0)) { + revert StandardPoolWithCreator(); + } + + LiquidityManagement memory liquidityManagement = getDefaultLiquidityManagement(); + liquidityManagement.enableDonation = false; + liquidityManagement.disableUnbalancedLiquidity = false; + + pool = _create( + abi.encode( + WeightedPool.NewPoolParams({ + name: name, + symbol: symbol, + numTokens: tokens.length, + normalizedWeights: normalizedWeights, + version: _poolVersion + }), + getVault(), + owner, + swapEnabledOnStart + ), + salt + ); + + _registerPoolWithVault(pool, tokens, swapFeePercentage, true, roleAccounts, pool, liquidityManagement); + } +} diff --git a/pkg/pool-weighted/contracts/lib/GradualValueChange.sol b/pkg/pool-weighted/contracts/lib/GradualValueChange.sol index bf3f488c1..0e243b858 100644 --- a/pkg/pool-weighted/contracts/lib/GradualValueChange.sol +++ b/pkg/pool-weighted/contracts/lib/GradualValueChange.sol @@ -90,5 +90,4 @@ library GradualValueChange { // We don't need to consider zero division here as this is covered above. return secondsElapsed.divDown(totalSeconds); } - } diff --git a/pkg/pool-weighted/contracts/lib/WeightValidation.sol b/pkg/pool-weighted/contracts/lib/WeightValidation.sol index 43ba1a3ba..891d9f4df 100644 --- a/pkg/pool-weighted/contracts/lib/WeightValidation.sol +++ b/pkg/pool-weighted/contracts/lib/WeightValidation.sol @@ -30,10 +30,7 @@ library WeightValidation { * @dev Returns a fixed-point number representing how far along the current value change is, where 0 means the * change has not yet started, and FixedPoint.ONE means it has fully completed. */ - function validateWeights( - uint256[] memory normalizedWeights, - uint256 numTokens - ) internal pure { + function validateWeights(uint256[] memory normalizedWeights, uint256 numTokens) internal pure { // Ensure each normalized weight is above the minimum uint256 normalizedSum = 0; for (uint8 i = 0; i < numTokens; ++i) { @@ -54,11 +51,7 @@ library WeightValidation { * @dev Returns a fixed-point number representing how far along the current value change is, where 0 means the * change has not yet started, and FixedPoint.ONE means it has fully completed. */ - function validateTwoWeights( - uint256 normalizedWeight0, - uint256 normalizedWeight1 - ) internal pure { - + function validateTwoWeights(uint256 normalizedWeight0, uint256 normalizedWeight1) internal pure { // Ensure each normalized weight is above the minimum if (normalizedWeight0 < WeightedMath._MIN_WEIGHT || normalizedWeight1 < WeightedMath._MIN_WEIGHT) { revert MinWeight(); @@ -68,6 +61,5 @@ library WeightValidation { if (normalizedWeight0 + normalizedWeight1 != FixedPoint.ONE) { revert NormalizedWeightInvariant(); } - } } From 4c1253d06406de3171ef4eeca0e08c774bef50b2 Mon Sep 17 00:00:00 2001 From: gerg Date: Fri, 23 Aug 2024 10:52:59 -0400 Subject: [PATCH 06/54] replace references to paused pool with swap enabled; fix logical inversion bug/typo --- pkg/pool-weighted/contracts/LBPool.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/pool-weighted/contracts/LBPool.sol b/pkg/pool-weighted/contracts/LBPool.sol index 53b731791..0aa3354a9 100644 --- a/pkg/pool-weighted/contracts/LBPool.sol +++ b/pkg/pool-weighted/contracts/LBPool.sol @@ -123,10 +123,10 @@ contract LBPool is WeightedPool, Ownable { } /** - * @notice Pause/unpause trading. + * @notice Enable/disable trading. */ function setSwapEnabled(bool swapEnabled) external onlyOwner { - _poolState.swapEnabled = !swapEnabled; + _poolState.swapEnabled = swapEnabled; } /** @@ -161,8 +161,8 @@ contract LBPool is WeightedPool, Ownable { } /** - * @notice Called before a swap to give the Pool block swap in paused pool. - * @return success True if the pool is not paused. + * @notice Called before a swap to let pool block swaps if not enabled. + * @return success True if the pool has swaps enabled. */ function onBeforeSwap(PoolSwapParams calldata, address) public virtual onlyVault returns (bool) { return _getPoolSwapEnabledState(); From e3fb44ae2e8a87f6db3799ea10ee7b72461417cd Mon Sep 17 00:00:00 2001 From: gerg Date: Fri, 23 Aug 2024 10:55:07 -0400 Subject: [PATCH 07/54] directly use default liquidity management params --- pkg/pool-weighted/contracts/LBPoolFactory.sol | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/pkg/pool-weighted/contracts/LBPoolFactory.sol b/pkg/pool-weighted/contracts/LBPoolFactory.sol index 5db253849..9d9e1ab72 100644 --- a/pkg/pool-weighted/contracts/LBPoolFactory.sol +++ b/pkg/pool-weighted/contracts/LBPoolFactory.sol @@ -65,10 +65,6 @@ contract LBPoolFactory is IPoolVersion, BasePoolFactory, Version { revert StandardPoolWithCreator(); } - LiquidityManagement memory liquidityManagement = getDefaultLiquidityManagement(); - liquidityManagement.enableDonation = false; - liquidityManagement.disableUnbalancedLiquidity = false; - pool = _create( abi.encode( WeightedPool.NewPoolParams({ @@ -85,6 +81,6 @@ contract LBPoolFactory is IPoolVersion, BasePoolFactory, Version { salt ); - _registerPoolWithVault(pool, tokens, swapFeePercentage, true, roleAccounts, pool, liquidityManagement); + _registerPoolWithVault(pool, tokens, swapFeePercentage, true, roleAccounts, pool, getDefaultLiquidityManagement()); } } From 3433090f6a4e605a1bd5ce7561b6ec518f8ab9c1 Mon Sep 17 00:00:00 2001 From: gerg Date: Fri, 23 Aug 2024 11:07:14 -0400 Subject: [PATCH 08/54] let vault handle static swap fees via PoolRoleAccounts.swapFeeManager --- pkg/pool-weighted/contracts/LBPool.sol | 51 ------------------- pkg/pool-weighted/contracts/LBPoolFactory.sol | 19 ++++--- 2 files changed, 12 insertions(+), 58 deletions(-) diff --git a/pkg/pool-weighted/contracts/LBPool.sol b/pkg/pool-weighted/contracts/LBPool.sol index 0aa3354a9..c98945ec9 100644 --- a/pkg/pool-weighted/contracts/LBPool.sol +++ b/pkg/pool-weighted/contracts/LBPool.sol @@ -27,14 +27,12 @@ contract LBPool is WeightedPool, Ownable { bool swapEnabled; } PoolState private _poolState; - uint256 private _swapFeePercentage; uint256 private constant _NUM_TOKENS = 2; // `{start,end}Time` are `uint56`s. Ensure that no input time (passed as `uint256`) will overflow. uint256 private constant _MAX_TIME = type(uint56).max; - event SwapFeePercentageChanged(uint256 swapFeePercentage); event SwapEnabledSet(bool swapEnabled); event GradualWeightUpdateScheduled( uint256 startTime, @@ -43,12 +41,6 @@ contract LBPool is WeightedPool, Ownable { uint256[] endWeights ); - /// @dev Indicates that the swap fee is below the minimum allowable swap fee. - error MinSwapFee(); - - /// @dev Indicates that the swap fee is above the maximum allowable swap fee. - error MaxSwapFee(); - constructor( NewPoolParams memory params, IVault vault, @@ -106,22 +98,6 @@ contract LBPool is WeightedPool, Ownable { endWeights[1] = FixedPoint.ONE - poolState.endWeight0; } - /** - * @notice Set the swap fee percentage. - * @dev This is a permissioned function. The swap fee must be within the bounds set by - * MIN_SWAP_FEE_PERCENTAGE/MAX_SWAP_FEE_PERCENTAGE. Emits the SwapFeePercentageChanged event. - */ - function setSwapFeePercentage(uint256 swapFeePercentage) public virtual onlyOwner { - _setSwapFeePercentage(swapFeePercentage); - } - - /** - * @notice Return the current value of the swap fee percentage. - */ - function getSwapFeePercentage() public view virtual returns (uint256) { - return _swapFeePercentage; - } - /** * @notice Enable/disable trading. */ @@ -168,19 +144,6 @@ contract LBPool is WeightedPool, Ownable { return _getPoolSwapEnabledState(); } - /** - * @notice Called after `onBeforeSwap` and before the main swap operation, if the pool has dynamic fees. - * @return success True if the pool wishes to proceed with settlement - * @return dynamicSwapFeePercentage Value of the swap fee percentage, as an 18-decimal FP value - */ - function onComputeDynamicSwapFeePercentage( - PoolSwapParams calldata, - address, - uint256 - ) external view onlyVault returns (bool, uint256) { - return (true, getSwapFeePercentage()); - } - /* ========================================= * ========================================= * ==========INTERNAL FUNCTIONS============= @@ -244,20 +207,6 @@ contract LBPool is WeightedPool, Ownable { emit GradualWeightUpdateScheduled(startTime, endTime, startWeights, endWeights); } - function _setSwapFeePercentage(uint256 swapFeePercentage) internal virtual { - // TODO: can we get min/max swap fee as internal fns in the WP base class? External call is wasteful. - if (swapFeePercentage < this.getMinimumSwapFeePercentage()) { - revert MinSwapFee(); - } - // TODO: can we get min/max swap fee as internal fns in the WP base class? External call is wasteful. - if (swapFeePercentage > this.getMaximumSwapFeePercentage()) { - revert MaxSwapFee(); - } - - _swapFeePercentage = swapFeePercentage; - emit SwapFeePercentageChanged(swapFeePercentage); - } - function _getPoolSwapEnabledState() internal view returns (bool) { return _poolState.swapEnabled; } diff --git a/pkg/pool-weighted/contracts/LBPoolFactory.sol b/pkg/pool-weighted/contracts/LBPoolFactory.sol index 9d9e1ab72..e9366b775 100644 --- a/pkg/pool-weighted/contracts/LBPoolFactory.sol +++ b/pkg/pool-weighted/contracts/LBPoolFactory.sol @@ -45,9 +45,8 @@ contract LBPoolFactory is IPoolVersion, BasePoolFactory, Version { * @param symbol The symbol of the pool * @param tokens An array of descriptors for the tokens the pool will manage * @param normalizedWeights The pool weights (must add to FixedPoint.ONE) - * @param roleAccounts Addresses the Vault will allow to change certain pool settings * @param swapFeePercentage Initial swap fee percentage - * @param owner The owner address for pool; the sole LP with swapEnable/swapFee change permissions + * @param owner The owner address for pool; sole LP with swapEnable/swapFee change permissions * @param salt The salt value that will be passed to create3 deployment */ function create( @@ -55,15 +54,13 @@ contract LBPoolFactory is IPoolVersion, BasePoolFactory, Version { string memory symbol, TokenConfig[] memory tokens, uint256[] memory normalizedWeights, - PoolRoleAccounts memory roleAccounts, uint256 swapFeePercentage, address owner, bool swapEnabledOnStart, bytes32 salt ) external returns (address pool) { - if (roleAccounts.poolCreator != address(0)) { - revert StandardPoolWithCreator(); - } + PoolRoleAccounts memory roleAccounts; + roleAccounts.swapFeeManager = owner; pool = _create( abi.encode( @@ -81,6 +78,14 @@ contract LBPoolFactory is IPoolVersion, BasePoolFactory, Version { salt ); - _registerPoolWithVault(pool, tokens, swapFeePercentage, true, roleAccounts, pool, getDefaultLiquidityManagement()); + _registerPoolWithVault( + pool, + tokens, + swapFeePercentage, + true, + roleAccounts, + pool, + getDefaultLiquidityManagement() + ); } } From 174aa679f57c5ebb81f5b4987f86566ef74fdc6b Mon Sep 17 00:00:00 2001 From: gerg Date: Thu, 29 Aug 2024 10:16:25 -0400 Subject: [PATCH 09/54] implement getHookFlags and add a TODO to use TrustedRoutersProvider whenever it's developed --- pkg/pool-weighted/contracts/LBPool.sol | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/pkg/pool-weighted/contracts/LBPool.sol b/pkg/pool-weighted/contracts/LBPool.sol index c98945ec9..8467971a7 100644 --- a/pkg/pool-weighted/contracts/LBPool.sol +++ b/pkg/pool-weighted/contracts/LBPool.sol @@ -5,6 +5,8 @@ pragma solidity ^0.8.24; import { WeightedPool } from "./WeightedPool.sol"; import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol"; +import { HookFlags } from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; + import { IRouterCommon } from "@balancer-labs/v3-interfaces/contracts/vault/IRouterCommon.sol"; import { AddLiquidityKind } from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; @@ -119,6 +121,13 @@ contract LBPool is WeightedPool, Ownable { * ========================================= */ + // Return HookFlags struct that indicates which hooks this contract supports + function getHookFlags() public pure returns (HookFlags memory hookFlags) { + // Support hooks before swap/join for swapEnabled/onlyOwner LP + hookFlags.shouldCallBeforeSwap = true; + hookFlags.shouldCallBeforeAddLiquidity = true; + } + /** * @notice Check that the caller who initiated the add liquidity operation is the owner. * @param initiator The address (usually a router contract) that initiated a swap operation on the Vault @@ -132,6 +141,9 @@ contract LBPool is WeightedPool, Ownable { uint256[] memory, bytes memory ) external view onlyVault returns (bool success) { + + // TODO use TrustedRoutersProvider + // Check `initiator == owner` first to avoid calling `getSender()` on a potentially non-router contract/address success = (initiator == owner()) || (IRouterCommon(initiator).getSender() == owner()); } From eee1f28590fc526374fde485006e5d121d239b0b Mon Sep 17 00:00:00 2001 From: gerg Date: Thu, 29 Aug 2024 10:28:32 -0400 Subject: [PATCH 10/54] set up (but don't yet use) the TrustedRoutersProvider logic for the LBP's add liquidity hook; change terminology back to what it originally was in the interface --- pkg/pool-weighted/contracts/LBPool.sol | 19 +++++++++++++------ pkg/pool-weighted/contracts/LBPoolFactory.sol | 10 ++++++++-- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/pkg/pool-weighted/contracts/LBPool.sol b/pkg/pool-weighted/contracts/LBPool.sol index 8467971a7..cfed3a2e8 100644 --- a/pkg/pool-weighted/contracts/LBPool.sol +++ b/pkg/pool-weighted/contracts/LBPool.sol @@ -35,6 +35,8 @@ contract LBPool is WeightedPool, Ownable { // `{start,end}Time` are `uint56`s. Ensure that no input time (passed as `uint256`) will overflow. uint256 private constant _MAX_TIME = type(uint56).max; + address internal immutable TRUSTED_ROUTERS_PROVIDER; + event SwapEnabledSet(bool swapEnabled); event GradualWeightUpdateScheduled( uint256 startTime, @@ -47,12 +49,16 @@ contract LBPool is WeightedPool, Ownable { NewPoolParams memory params, IVault vault, address owner, - bool swapEnabledOnStart + bool swapEnabledOnStart, + address trustedRoutersProvider ) WeightedPool(params, vault) Ownable(owner) { InputHelpers.ensureInputLengthMatch(_NUM_TOKENS, params.numTokens); // WeightedPool validates `numTokens == normalizedWeights.length` // _startGradualWeightChange validates weights + // Provider address validation performed at the factory level + TRUSTED_ROUTERS_PROVIDER = trustedRoutersProvider; + uint256 currentTime = block.timestamp; _startGradualWeightChange( uint56(currentTime), @@ -130,10 +136,10 @@ contract LBPool is WeightedPool, Ownable { /** * @notice Check that the caller who initiated the add liquidity operation is the owner. - * @param initiator The address (usually a router contract) that initiated a swap operation on the Vault + * @param router The address (usually a router contract) that initiated add liquidity operation on the Vault */ function onBeforeAddLiquidity( - address initiator, + address router, address, AddLiquidityKind, uint256[] memory, @@ -142,10 +148,11 @@ contract LBPool is WeightedPool, Ownable { bytes memory ) external view onlyVault returns (bool success) { - // TODO use TrustedRoutersProvider + // TODO use TrustedRoutersProvider. Presumably something like this: + // success = ITrustedRoutersProvider(TRUSTED_ROUTERS_PROVIDER).isTrusted(router); - // Check `initiator == owner` first to avoid calling `getSender()` on a potentially non-router contract/address - success = (initiator == owner()) || (IRouterCommon(initiator).getSender() == owner()); + // Check `router == owner` first to avoid calling `getSender()` on a potentially non-router contract/address + success = (router == owner()) || (IRouterCommon(router).getSender() == owner()); } /** diff --git a/pkg/pool-weighted/contracts/LBPoolFactory.sol b/pkg/pool-weighted/contracts/LBPoolFactory.sol index e9366b775..b43004999 100644 --- a/pkg/pool-weighted/contracts/LBPoolFactory.sol +++ b/pkg/pool-weighted/contracts/LBPoolFactory.sol @@ -23,14 +23,19 @@ contract LBPoolFactory is IPoolVersion, BasePoolFactory, Version { // solhint-disable not-rely-on-time string private _poolVersion; + address internal immutable TRUSTED_ROUTERS_PROVIDER; constructor( IVault vault, uint32 pauseWindowDuration, string memory factoryVersion, - string memory poolVersion + string memory poolVersion, + address trustedRoutersProvider ) BasePoolFactory(vault, pauseWindowDuration, type(LBPool).creationCode) Version(factoryVersion) { _poolVersion = poolVersion; + + // TODO: validate input address before storing + TRUSTED_ROUTERS_PROVIDER = trustedRoutersProvider; } /// @inheritdoc IPoolVersion @@ -73,7 +78,8 @@ contract LBPoolFactory is IPoolVersion, BasePoolFactory, Version { }), getVault(), owner, - swapEnabledOnStart + swapEnabledOnStart, + TRUSTED_ROUTERS_PROVIDER ), salt ); From b9bbbaf9f80ac423f982cdd7af2743cf0f59cb57 Mon Sep 17 00:00:00 2001 From: gerg Date: Thu, 29 Aug 2024 10:34:06 -0400 Subject: [PATCH 11/54] reorganize comments --- pkg/pool-weighted/contracts/LBPool.sol | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/pool-weighted/contracts/LBPool.sol b/pkg/pool-weighted/contracts/LBPool.sol index cfed3a2e8..9954e7366 100644 --- a/pkg/pool-weighted/contracts/LBPool.sol +++ b/pkg/pool-weighted/contracts/LBPool.sol @@ -17,8 +17,8 @@ import { GradualValueChange } from "./lib/GradualValueChange.sol"; import { WeightValidation } from "./lib/WeightValidation.sol"; /// @notice Inheriting from WeightedPool is only slightly wasteful (setting 2 immutable weights -/// that will not be used later), and it is tremendously helpful for pool validation and -/// any potential future parent class changes. +/// and _totalTokens, which will not be used later), and it is tremendously helpful for pool +/// validation and any potential future parent class changes. contract LBPool is WeightedPool, Ownable { // Since we have max 2 tokens and the weights must sum to 1, we only need to store one weight struct PoolState { @@ -52,8 +52,11 @@ contract LBPool is WeightedPool, Ownable { bool swapEnabledOnStart, address trustedRoutersProvider ) WeightedPool(params, vault) Ownable(owner) { - InputHelpers.ensureInputLengthMatch(_NUM_TOKENS, params.numTokens); + + // _NUM_TOKENS == 2 == params.normalizedWeights.length == params.numTokens // WeightedPool validates `numTokens == normalizedWeights.length` + InputHelpers.ensureInputLengthMatch(_NUM_TOKENS, params.numTokens); + // _startGradualWeightChange validates weights // Provider address validation performed at the factory level From 652e4b5ae23b1ffa16f72c8c35cfbee363598700 Mon Sep 17 00:00:00 2001 From: gerg Date: Thu, 29 Aug 2024 10:37:38 -0400 Subject: [PATCH 12/54] fill out sample router allowlist code --- pkg/pool-weighted/contracts/LBPool.sol | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/pool-weighted/contracts/LBPool.sol b/pkg/pool-weighted/contracts/LBPool.sol index 9954e7366..5817cb78f 100644 --- a/pkg/pool-weighted/contracts/LBPool.sol +++ b/pkg/pool-weighted/contracts/LBPool.sol @@ -152,10 +152,11 @@ contract LBPool is WeightedPool, Ownable { ) external view onlyVault returns (bool success) { // TODO use TrustedRoutersProvider. Presumably something like this: - // success = ITrustedRoutersProvider(TRUSTED_ROUTERS_PROVIDER).isTrusted(router); - - // Check `router == owner` first to avoid calling `getSender()` on a potentially non-router contract/address - success = (router == owner()) || (IRouterCommon(router).getSender() == owner()); + // + // if (ITrustedRoutersProvider(TRUSTED_ROUTERS_PROVIDER).isTrusted(router)) { + // success = IRouterCommon(router).getSender() == owner(); + // } + success = IRouterCommon(router).getSender() == owner(); } /** From 92dda530cc9748c1984da1f556bff0f3a7e1123f Mon Sep 17 00:00:00 2001 From: gerg Date: Thu, 29 Aug 2024 11:51:13 -0400 Subject: [PATCH 13/54] add temporary lazy trustedRouter as a placeholder until the trusted router provider exists --- pkg/pool-weighted/contracts/LBPool.sol | 18 ++++++++++++------ pkg/pool-weighted/contracts/LBPoolFactory.sol | 8 ++++++-- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/pkg/pool-weighted/contracts/LBPool.sol b/pkg/pool-weighted/contracts/LBPool.sol index 5817cb78f..ae0d5d15c 100644 --- a/pkg/pool-weighted/contracts/LBPool.sol +++ b/pkg/pool-weighted/contracts/LBPool.sol @@ -36,6 +36,7 @@ contract LBPool is WeightedPool, Ownable { uint256 private constant _MAX_TIME = type(uint56).max; address internal immutable TRUSTED_ROUTERS_PROVIDER; + address internal immutable TRUSTED_ROUTER_TODO_DELETE_ME; event SwapEnabledSet(bool swapEnabled); event GradualWeightUpdateScheduled( @@ -45,12 +46,16 @@ contract LBPool is WeightedPool, Ownable { uint256[] endWeights ); + /// @dev Indicates that the router that called the Vault is not trusted and should be ignored. + error RouterNotTrusted(); + constructor( NewPoolParams memory params, IVault vault, address owner, bool swapEnabledOnStart, - address trustedRoutersProvider + address trustedRoutersProvider, + address trustedRouterTodoDeleteMe ) WeightedPool(params, vault) Ownable(owner) { // _NUM_TOKENS == 2 == params.normalizedWeights.length == params.numTokens @@ -61,6 +66,7 @@ contract LBPool is WeightedPool, Ownable { // Provider address validation performed at the factory level TRUSTED_ROUTERS_PROVIDER = trustedRoutersProvider; + TRUSTED_ROUTER_TODO_DELETE_ME = trustedRouterTodoDeleteMe; uint256 currentTime = block.timestamp; _startGradualWeightChange( @@ -149,14 +155,14 @@ contract LBPool is WeightedPool, Ownable { uint256, uint256[] memory, bytes memory - ) external view onlyVault returns (bool success) { + ) external view onlyVault returns (bool) { // TODO use TrustedRoutersProvider. Presumably something like this: - // // if (ITrustedRoutersProvider(TRUSTED_ROUTERS_PROVIDER).isTrusted(router)) { - // success = IRouterCommon(router).getSender() == owner(); - // } - success = IRouterCommon(router).getSender() == owner(); + if (router == TRUSTED_ROUTER_TODO_DELETE_ME) { + return IRouterCommon(router).getSender() == owner(); + } + revert RouterNotTrusted(); //TODO: should hooks revert or just return false? } /** diff --git a/pkg/pool-weighted/contracts/LBPoolFactory.sol b/pkg/pool-weighted/contracts/LBPoolFactory.sol index b43004999..3a2331439 100644 --- a/pkg/pool-weighted/contracts/LBPoolFactory.sol +++ b/pkg/pool-weighted/contracts/LBPoolFactory.sol @@ -24,18 +24,21 @@ contract LBPoolFactory is IPoolVersion, BasePoolFactory, Version { string private _poolVersion; address internal immutable TRUSTED_ROUTERS_PROVIDER; + address internal immutable TRUSTED_ROUTER_TODO_DELETE_ME; constructor( IVault vault, uint32 pauseWindowDuration, string memory factoryVersion, string memory poolVersion, - address trustedRoutersProvider + address trustedRoutersProvider, + address trustedRouterTodoDeleteMe ) BasePoolFactory(vault, pauseWindowDuration, type(LBPool).creationCode) Version(factoryVersion) { _poolVersion = poolVersion; // TODO: validate input address before storing TRUSTED_ROUTERS_PROVIDER = trustedRoutersProvider; + TRUSTED_ROUTER_TODO_DELETE_ME = trustedRouterTodoDeleteMe; } /// @inheritdoc IPoolVersion @@ -79,7 +82,8 @@ contract LBPoolFactory is IPoolVersion, BasePoolFactory, Version { getVault(), owner, swapEnabledOnStart, - TRUSTED_ROUTERS_PROVIDER + TRUSTED_ROUTERS_PROVIDER, + TRUSTED_ROUTER_TODO_DELETE_ME ), salt ); From 40c5577bf8a76518772badd0a6c59ef207bef873 Mon Sep 17 00:00:00 2001 From: gerg Date: Wed, 4 Sep 2024 13:00:06 -0400 Subject: [PATCH 14/54] add onRegister function to get hardhat tests to work for LBPool --- pkg/pool-weighted/contracts/LBPool.sol | 27 ++- pkg/pool-weighted/contracts/LBPoolFactory.sol | 2 +- pkg/pool-weighted/test/LBPool.test.ts | 163 ++++++++++++++++++ 3 files changed, 188 insertions(+), 4 deletions(-) create mode 100644 pkg/pool-weighted/test/LBPool.test.ts diff --git a/pkg/pool-weighted/contracts/LBPool.sol b/pkg/pool-weighted/contracts/LBPool.sol index ae0d5d15c..647b0dfaf 100644 --- a/pkg/pool-weighted/contracts/LBPool.sol +++ b/pkg/pool-weighted/contracts/LBPool.sol @@ -5,14 +5,14 @@ pragma solidity ^0.8.24; import { WeightedPool } from "./WeightedPool.sol"; import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol"; -import { HookFlags } from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; +import { HookFlags, TokenConfig, AddLiquidityKind, LiquidityManagement } from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; import { IRouterCommon } from "@balancer-labs/v3-interfaces/contracts/vault/IRouterCommon.sol"; -import { AddLiquidityKind } from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; +import { IBasePoolFactory } from "@balancer-labs/v3-interfaces/contracts/vault/IBasePoolFactory.sol"; +import { IVaultErrors } from "@balancer-labs/v3-interfaces/contracts/vault/IVaultErrors.sol"; import { InputHelpers } from "@balancer-labs/v3-solidity-utils/contracts/helpers/InputHelpers.sol"; import { FixedPoint } from "@balancer-labs/v3-solidity-utils/contracts/math/FixedPoint.sol"; -import { IVaultErrors } from "@balancer-labs/v3-interfaces/contracts/vault/IVaultErrors.sol"; import { GradualValueChange } from "./lib/GradualValueChange.sol"; import { WeightValidation } from "./lib/WeightValidation.sol"; @@ -136,6 +136,27 @@ contract LBPool is WeightedPool, Ownable { * ========================================= */ + /** + * @notice Hook to be executed when pool is registered. Returns true if registration was successful, and false to + * revert the registration of the pool. Make sure this function is properly implemented (e.g. check the factory, + * and check that the given pool is from the factory). + * + * @param factory Address of the pool factory + * @param pool Address of the pool + * @return success True if the hook allowed the registration, false otherwise + */ + function onRegister( + address factory, + address pool, + TokenConfig[] memory, + LiquidityManagement calldata + ) external view onlyVault returns (bool success) { + if (pool == address(this) && IBasePoolFactory(factory).isPoolFromFactory(pool)) { + success = true; + } + } + + // Return HookFlags struct that indicates which hooks this contract supports function getHookFlags() public pure returns (HookFlags memory hookFlags) { // Support hooks before swap/join for swapEnabled/onlyOwner LP diff --git a/pkg/pool-weighted/contracts/LBPoolFactory.sol b/pkg/pool-weighted/contracts/LBPoolFactory.sol index 3a2331439..719f62f29 100644 --- a/pkg/pool-weighted/contracts/LBPoolFactory.sol +++ b/pkg/pool-weighted/contracts/LBPoolFactory.sol @@ -92,7 +92,7 @@ contract LBPoolFactory is IPoolVersion, BasePoolFactory, Version { pool, tokens, swapFeePercentage, - true, + true, //protocol fee exempt roleAccounts, pool, getDefaultLiquidityManagement() diff --git a/pkg/pool-weighted/test/LBPool.test.ts b/pkg/pool-weighted/test/LBPool.test.ts new file mode 100644 index 000000000..205d4ff8b --- /dev/null +++ b/pkg/pool-weighted/test/LBPool.test.ts @@ -0,0 +1,163 @@ +import { ethers } from 'hardhat'; +import { expect } from 'chai'; +import { deploy, deployedAt } from '@balancer-labs/v3-helpers/src/contract'; +import { sharedBeforeEach } from '@balancer-labs/v3-common/sharedBeforeEach'; +import { Router } from '@balancer-labs/v3-vault/typechain-types/contracts/Router'; +import { ERC20TestToken } from '@balancer-labs/v3-solidity-utils/typechain-types/contracts/test/ERC20TestToken'; +import { SignerWithAddress } from '@nomicfoundation/hardhat-ethers/dist/src/signer-with-address'; +import { FP_ZERO, fp } from '@balancer-labs/v3-helpers/src/numbers'; +import { + MAX_UINT256, + MAX_UINT160, + MAX_UINT48, + ZERO_BYTES32, + ZERO_ADDRESS, +} from '@balancer-labs/v3-helpers/src/constants'; +import * as VaultDeployer from '@balancer-labs/v3-helpers/src/models/vault/VaultDeployer'; +import { IVaultMock } from '@balancer-labs/v3-interfaces/typechain-types'; +import TypesConverter from '@balancer-labs/v3-helpers/src/models/types/TypesConverter'; +import { buildTokenConfig } from '@balancer-labs/v3-helpers/src/models/tokens/tokenConfig'; +import { LBPool, LBPoolFactory } from '../typechain-types'; +import { actionId } from '@balancer-labs/v3-helpers/src/models/misc/actions'; +import { MONTH } from '@balancer-labs/v3-helpers/src/time'; +import * as expectEvent from '@balancer-labs/v3-helpers/src/test/expectEvent'; +import { sortAddresses } from '@balancer-labs/v3-helpers/src/models/tokens/sortingHelper'; +import { deployPermit2 } from '@balancer-labs/v3-vault/test/Permit2Deployer'; +import { IPermit2 } from '@balancer-labs/v3-vault/typechain-types/permit2/src/interfaces/IPermit2'; +import { PoolConfigStructOutput } from '@balancer-labs/v3-solidity-utils/typechain-types/@balancer-labs/v3-interfaces/contracts/vault/IVault'; +import { TokenConfigStruct } from '../typechain-types/@balancer-labs/v3-interfaces/contracts/vault/IVault'; + +describe('LBPool', function () { + const POOL_SWAP_FEE = fp(0.01); + + const TOKEN_AMOUNT = fp(100); + + let permit2: IPermit2; + let vault: IVaultMock; + let factory: LBPoolFactory; + let pool: LBPool; + let router: Router; + let alice: SignerWithAddress; + let bob: SignerWithAddress; + let tokenA: ERC20TestToken; + let tokenB: ERC20TestToken; + let poolTokens: string[]; + + let tokenAAddress: string; + let tokenBAddress: string; + + const FACTORY_VERSION = 'LBPool Factory v1'; + const POOL_VERSION = 'LBPool v1'; + + const WEIGHTS = [fp(0.5), fp(0.5)]; + const INITIAL_BALANCES = [TOKEN_AMOUNT, TOKEN_AMOUNT]; + const SWAP_AMOUNT = fp(20); + + const SWAP_FEE = fp(0.01); + + before('setup signers', async () => { + [, alice, bob] = await ethers.getSigners(); + }); + + sharedBeforeEach('deploy vault, router, tokens, and pool', async function () { + vault = await TypesConverter.toIVaultMock(await VaultDeployer.deployMock()); + + const WETH = await deploy('v3-solidity-utils/WETHTestToken'); + permit2 = await deployPermit2(); + router = await deploy('v3-vault/Router', { args: [vault, WETH, permit2] }); + + tokenA = await deploy('v3-solidity-utils/ERC20TestToken', { args: ['Token A', 'TKNA', 18] }); + tokenB = await deploy('v3-solidity-utils/ERC20TestToken', { args: ['Token B', 'TKNB', 6] }); + + tokenAAddress = await tokenA.getAddress(); + tokenBAddress = await tokenB.getAddress(); + }); + + sharedBeforeEach('create and initialize pool', async () => { + + const tempRouter = bob.address + const tempRouterProvider = bob.address + factory = await deploy('LBPoolFactory', { + args: [await vault.getAddress(), MONTH * 12, FACTORY_VERSION, POOL_VERSION, tempRouter, tempRouterProvider], + }); + poolTokens = sortAddresses([tokenAAddress, tokenBAddress]); + + const tokenConfig: TokenConfigStruct[] = buildTokenConfig(poolTokens); + + const tx = await factory.create( + 'LBPool', + 'Test', + tokenConfig, + WEIGHTS, + SWAP_FEE, + bob.address, //owner + true, // swapEnabledOnStart + ZERO_BYTES32 + ); + const receipt = await tx.wait(); + + const event = expectEvent.inReceipt(receipt, 'PoolCreated'); + + pool = (await deployedAt('LBPool', event.args.pool)) as unknown as LBPool; + + await tokenA.mint(bob, TOKEN_AMOUNT + SWAP_AMOUNT); + await tokenB.mint(bob, TOKEN_AMOUNT); + + await pool.connect(bob).approve(router, MAX_UINT256); + for (const token of [tokenA, tokenB]) { + await token.connect(bob).approve(permit2, MAX_UINT256); + await permit2.connect(bob).approve(token, router, MAX_UINT160, MAX_UINT48); + } + + await expect(await router.connect(bob).initialize(pool, poolTokens, INITIAL_BALANCES, FP_ZERO, false, '0x')) + .to.emit(vault, 'PoolInitialized') + .withArgs(pool); + }); + + sharedBeforeEach('grant permission', async () => { + const setPoolSwapFeeAction = await actionId(vault, 'setStaticSwapFeePercentage'); + + const authorizerAddress = await vault.getAuthorizer(); + const authorizer = await deployedAt('v3-solidity-utils/BasicAuthorizerMock', authorizerAddress); + + await authorizer.grantRole(setPoolSwapFeeAction, bob.address); + + await vault.connect(bob).setStaticSwapFeePercentage(pool, POOL_SWAP_FEE); + }); + + it('should have correct versions', async () => { + expect(await factory.version()).to.eq(FACTORY_VERSION); + expect(await factory.getPoolVersion()).to.eq(POOL_VERSION); + expect(await pool.version()).to.eq(POOL_VERSION); + }); + + it('pool and protocol fee preconditions', async () => { + const poolConfig: PoolConfigStructOutput = await vault.getPoolConfig(pool); + + expect(poolConfig.isPoolRegistered).to.be.true; + expect(poolConfig.isPoolInitialized).to.be.true; + + expect(await vault.getStaticSwapFeePercentage(pool)).to.eq(POOL_SWAP_FEE); + }); + + it('has the correct pool tokens and balances', async () => { + const tokensFromPool = await pool.getTokens(); + expect(tokensFromPool).to.deep.equal(poolTokens); + + const [tokensFromVault, , balancesFromVault] = await vault.getPoolTokenInfo(pool); + + expect(tokensFromVault).to.deep.equal(tokensFromPool); + expect(balancesFromVault).to.deep.equal(INITIAL_BALANCES); + }); + + it('cannot be initialized twice', async () => { + await expect(router.connect(alice).initialize(pool, poolTokens, INITIAL_BALANCES, FP_ZERO, false, '0x')) + .to.be.revertedWithCustomError(vault, 'PoolAlreadyInitialized') + .withArgs(await pool.getAddress()); + }); + + it('returns weights', async () => { + const weights = await pool.getNormalizedWeights(); + expect(weights).to.be.deep.eq(WEIGHTS); + }); +}); From 336f899f15ece2b6300cd95ec45df5524f1a32e8 Mon Sep 17 00:00:00 2001 From: gerg Date: Wed, 4 Sep 2024 21:44:05 -0400 Subject: [PATCH 15/54] cleanup/tweak misc things post merge from main --- pkg/pool-weighted/contracts/LBPool.sol | 2 +- pkg/pool-weighted/contracts/LBPoolFactory.sol | 1 - pkg/pool-weighted/contracts/lib/WeightValidation.sol | 8 ++++---- pkg/pool-weighted/test/LBPool.test.ts | 3 +-- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/pkg/pool-weighted/contracts/LBPool.sol b/pkg/pool-weighted/contracts/LBPool.sol index 647b0dfaf..e5a49692d 100644 --- a/pkg/pool-weighted/contracts/LBPool.sol +++ b/pkg/pool-weighted/contracts/LBPool.sol @@ -5,7 +5,7 @@ pragma solidity ^0.8.24; import { WeightedPool } from "./WeightedPool.sol"; import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol"; -import { HookFlags, TokenConfig, AddLiquidityKind, LiquidityManagement } from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; +import { HookFlags, TokenConfig, AddLiquidityKind, LiquidityManagement, PoolSwapParams } from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; import { IRouterCommon } from "@balancer-labs/v3-interfaces/contracts/vault/IRouterCommon.sol"; import { IBasePoolFactory } from "@balancer-labs/v3-interfaces/contracts/vault/IBasePoolFactory.sol"; diff --git a/pkg/pool-weighted/contracts/LBPoolFactory.sol b/pkg/pool-weighted/contracts/LBPoolFactory.sol index 719f62f29..b7d8c302a 100644 --- a/pkg/pool-weighted/contracts/LBPoolFactory.sol +++ b/pkg/pool-weighted/contracts/LBPoolFactory.sol @@ -5,7 +5,6 @@ pragma solidity ^0.8.24; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol"; -import { IRateProvider } from "@balancer-labs/v3-interfaces/contracts/vault/IRateProvider.sol"; import { IPoolVersion } from "@balancer-labs/v3-interfaces/contracts/solidity-utils/helpers/IPoolVersion.sol"; import "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; diff --git a/pkg/pool-weighted/contracts/lib/WeightValidation.sol b/pkg/pool-weighted/contracts/lib/WeightValidation.sol index 891d9f4df..2219b2f29 100644 --- a/pkg/pool-weighted/contracts/lib/WeightValidation.sol +++ b/pkg/pool-weighted/contracts/lib/WeightValidation.sol @@ -13,12 +13,10 @@ // along with this program. If not, see . import { FixedPoint } from "@balancer-labs/v3-solidity-utils/contracts/math/FixedPoint.sol"; -import { WeightedMath } from "@balancer-labs/v3-solidity-utils/contracts/math/WeightedMath.sol"; pragma solidity ^0.8.24; library WeightValidation { - using FixedPoint for uint256; /// @dev Indicates that one of the pool tokens' weight is below the minimum allowed. error MinWeight(); @@ -26,6 +24,8 @@ library WeightValidation { /// @dev Indicates that the sum of the pool tokens' weights is not FP 1. error NormalizedWeightInvariant(); + uint256 internal constant _MIN_WEIGHT = 1e16; //1% + /** * @dev Returns a fixed-point number representing how far along the current value change is, where 0 means the * change has not yet started, and FixedPoint.ONE means it has fully completed. @@ -36,7 +36,7 @@ library WeightValidation { for (uint8 i = 0; i < numTokens; ++i) { uint256 normalizedWeight = normalizedWeights[i]; - if (normalizedWeight < WeightedMath._MIN_WEIGHT) { + if (normalizedWeight < _MIN_WEIGHT) { revert MinWeight(); } normalizedSum += normalizedWeight; @@ -53,7 +53,7 @@ library WeightValidation { */ function validateTwoWeights(uint256 normalizedWeight0, uint256 normalizedWeight1) internal pure { // Ensure each normalized weight is above the minimum - if (normalizedWeight0 < WeightedMath._MIN_WEIGHT || normalizedWeight1 < WeightedMath._MIN_WEIGHT) { + if (normalizedWeight0 < _MIN_WEIGHT || normalizedWeight1 < _MIN_WEIGHT) { revert MinWeight(); } diff --git a/pkg/pool-weighted/test/LBPool.test.ts b/pkg/pool-weighted/test/LBPool.test.ts index 205d4ff8b..fb264a3ba 100644 --- a/pkg/pool-weighted/test/LBPool.test.ts +++ b/pkg/pool-weighted/test/LBPool.test.ts @@ -95,7 +95,6 @@ describe('LBPool', function () { ZERO_BYTES32 ); const receipt = await tx.wait(); - const event = expectEvent.inReceipt(receipt, 'PoolCreated'); pool = (await deployedAt('LBPool', event.args.pool)) as unknown as LBPool; @@ -118,7 +117,7 @@ describe('LBPool', function () { const setPoolSwapFeeAction = await actionId(vault, 'setStaticSwapFeePercentage'); const authorizerAddress = await vault.getAuthorizer(); - const authorizer = await deployedAt('v3-solidity-utils/BasicAuthorizerMock', authorizerAddress); + const authorizer = await deployedAt('v3-vault/BasicAuthorizerMock', authorizerAddress); await authorizer.grantRole(setPoolSwapFeeAction, bob.address); From 0bdb84ba7b867614b5ce487e7ae480bda3e29350 Mon Sep 17 00:00:00 2001 From: gerg Date: Wed, 4 Sep 2024 21:44:55 -0400 Subject: [PATCH 16/54] lint --- pkg/pool-weighted/contracts/LBPool.sol | 11 +++++++---- pkg/pool-weighted/contracts/lib/WeightValidation.sol | 1 - 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/pkg/pool-weighted/contracts/LBPool.sol b/pkg/pool-weighted/contracts/LBPool.sol index e5a49692d..e158936f1 100644 --- a/pkg/pool-weighted/contracts/LBPool.sol +++ b/pkg/pool-weighted/contracts/LBPool.sol @@ -5,7 +5,13 @@ pragma solidity ^0.8.24; import { WeightedPool } from "./WeightedPool.sol"; import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol"; -import { HookFlags, TokenConfig, AddLiquidityKind, LiquidityManagement, PoolSwapParams } from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; +import { + HookFlags, + TokenConfig, + AddLiquidityKind, + LiquidityManagement, + PoolSwapParams +} from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; import { IRouterCommon } from "@balancer-labs/v3-interfaces/contracts/vault/IRouterCommon.sol"; import { IBasePoolFactory } from "@balancer-labs/v3-interfaces/contracts/vault/IBasePoolFactory.sol"; @@ -57,7 +63,6 @@ contract LBPool is WeightedPool, Ownable { address trustedRoutersProvider, address trustedRouterTodoDeleteMe ) WeightedPool(params, vault) Ownable(owner) { - // _NUM_TOKENS == 2 == params.normalizedWeights.length == params.numTokens // WeightedPool validates `numTokens == normalizedWeights.length` InputHelpers.ensureInputLengthMatch(_NUM_TOKENS, params.numTokens); @@ -156,7 +161,6 @@ contract LBPool is WeightedPool, Ownable { } } - // Return HookFlags struct that indicates which hooks this contract supports function getHookFlags() public pure returns (HookFlags memory hookFlags) { // Support hooks before swap/join for swapEnabled/onlyOwner LP @@ -177,7 +181,6 @@ contract LBPool is WeightedPool, Ownable { uint256[] memory, bytes memory ) external view onlyVault returns (bool) { - // TODO use TrustedRoutersProvider. Presumably something like this: // if (ITrustedRoutersProvider(TRUSTED_ROUTERS_PROVIDER).isTrusted(router)) { if (router == TRUSTED_ROUTER_TODO_DELETE_ME) { diff --git a/pkg/pool-weighted/contracts/lib/WeightValidation.sol b/pkg/pool-weighted/contracts/lib/WeightValidation.sol index 2219b2f29..7cc68ddea 100644 --- a/pkg/pool-weighted/contracts/lib/WeightValidation.sol +++ b/pkg/pool-weighted/contracts/lib/WeightValidation.sol @@ -17,7 +17,6 @@ import { FixedPoint } from "@balancer-labs/v3-solidity-utils/contracts/math/Fixe pragma solidity ^0.8.24; library WeightValidation { - /// @dev Indicates that one of the pool tokens' weight is below the minimum allowed. error MinWeight(); From 3a8b00911b417e2a4159e49f31375a680fe124fb Mon Sep 17 00:00:00 2001 From: gerg Date: Sun, 8 Sep 2024 21:42:18 -0400 Subject: [PATCH 17/54] add tests --- pkg/pool-weighted/contracts/LBPool.sol | 1 + pkg/pool-weighted/test/LBPool.test.ts | 199 ++++++++++++++++++ .../test/foundry/GradualValueChange.t.sol | 193 +++++++++++++++++ .../test/foundry/WeightValidationTest.t.sol | 109 ++++++++++ 4 files changed, 502 insertions(+) create mode 100644 pkg/pool-weighted/test/foundry/GradualValueChange.t.sol create mode 100644 pkg/pool-weighted/test/foundry/WeightValidationTest.t.sol diff --git a/pkg/pool-weighted/contracts/LBPool.sol b/pkg/pool-weighted/contracts/LBPool.sol index e158936f1..b84e13de3 100644 --- a/pkg/pool-weighted/contracts/LBPool.sol +++ b/pkg/pool-weighted/contracts/LBPool.sol @@ -125,6 +125,7 @@ contract LBPool is WeightedPool, Ownable { */ function setSwapEnabled(bool swapEnabled) external onlyOwner { _poolState.swapEnabled = swapEnabled; + emit SwapEnabledSet(swapEnabled); } /** diff --git a/pkg/pool-weighted/test/LBPool.test.ts b/pkg/pool-weighted/test/LBPool.test.ts index fb264a3ba..f76a4abbf 100644 --- a/pkg/pool-weighted/test/LBPool.test.ts +++ b/pkg/pool-weighted/test/LBPool.test.ts @@ -26,6 +26,7 @@ import { deployPermit2 } from '@balancer-labs/v3-vault/test/Permit2Deployer'; import { IPermit2 } from '@balancer-labs/v3-vault/typechain-types/permit2/src/interfaces/IPermit2'; import { PoolConfigStructOutput } from '@balancer-labs/v3-solidity-utils/typechain-types/@balancer-labs/v3-interfaces/contracts/vault/IVault'; import { TokenConfigStruct } from '../typechain-types/@balancer-labs/v3-interfaces/contracts/vault/IVault'; +import { time } from '@nomicfoundation/hardhat-network-helpers'; describe('LBPool', function () { const POOL_SWAP_FEE = fp(0.01); @@ -159,4 +160,202 @@ describe('LBPool', function () { const weights = await pool.getNormalizedWeights(); expect(weights).to.be.deep.eq(WEIGHTS); }); + + describe('Owner operations and events', () => { + it('should emit SwapEnabledSet event when setSwapEnabled is called', async () => { + await expect(pool.connect(bob).setSwapEnabled(false)) + .to.emit(pool, 'SwapEnabledSet') + .withArgs(false); + + await expect(pool.connect(bob).setSwapEnabled(true)) + .to.emit(pool, 'SwapEnabledSet') + .withArgs(true); + }); + + it('should emit GradualWeightUpdateScheduled event when updateWeightsGradually is called', async () => { + const startTime = await time.latest(); + const endTime = startTime + MONTH; + const endWeights = [fp(0.7), fp(0.3)]; + + const tx = await pool.connect(bob).updateWeightsGradually(startTime, endTime, endWeights); + const receipt = await tx.wait(); + const actualStartTime = await time.latest(); + + await expect(tx) + .to.emit(pool, 'GradualWeightUpdateScheduled') + .withArgs(actualStartTime, endTime, WEIGHTS, endWeights); + }); + + it('should only allow owner to be the LP', async () => { + const joinKind = 0; // Assuming 0 is the correct join kind for initialization + const userData = ethers.AbiCoder.defaultAbiCoder().encode( + ['uint256', 'uint256[]'], + [joinKind, INITIAL_BALANCES] + ); + + await expect(router.connect(alice).addLiquidity({ + pool: await pool.getAddress(), + tokens: poolTokens, + lastChangeBlock: 0, + userData: userData, + from: await alice.getAddress(), + to: await alice.getAddress(), + onlyManagePoolTokens: false + })).to.be.revertedWithCustomError(pool, 'RouterNotTrusted'); + + await expect(router.connect(bob).addLiquidity({ + pool: await pool.getAddress(), + tokens: poolTokens, + lastChangeBlock: 0, + userData: userData, + from: await bob.getAddress(), + to: await bob.getAddress(), + onlyManagePoolTokens: false + })).to.not.be.reverted; + }); + + it('should only allow owner to update weights', async () => { + const startTime = await time.latest(); + const endTime = startTime + MONTH; + const endWeights = [fp(0.7), fp(0.3)]; + + await expect(pool.connect(alice).updateWeightsGradually(startTime, endTime, endWeights)) + .to.be.revertedWithCustomError(pool, 'OwnableUnauthorizedAccount'); + + await expect(pool.connect(bob).updateWeightsGradually(startTime, endTime, endWeights)) + .to.not.be.reverted; + }); + }); + + describe('Weight updates', () => { + it('should update weights gradually', async () => { + const startTime = await time.latest(); + const endTime = startTime + MONTH; + const endWeights = [fp(0.7), fp(0.3)]; + + await pool.connect(bob).updateWeightsGradually(startTime, endTime, endWeights); + + // Check weights at start + expect(await pool.getNormalizedWeights()).to.deep.equal(WEIGHTS); + + // Check weights halfway through + await time.increaseTo(startTime + MONTH / 2); + const midWeights = await pool.getNormalizedWeights(); + expect(midWeights[0]).to.be.closeTo(fp(0.6), fp(1e-6)); + expect(midWeights[1]).to.be.closeTo(fp(0.4), fp(1e-6)); + + // Check weights at end + await time.increaseTo(endTime); + expect(await pool.getNormalizedWeights()).to.deep.equal(endWeights); + }); + + it('should constrain weights to [1%, 99%]', async () => { + const startTime = await time.latest(); + const endTime = startTime + MONTH; + + // Try to set weight below 1% + await expect(pool.connect(bob).updateWeightsGradually(startTime, endTime, [fp(0.009), fp(0.991)])) + .to.be.revertedWithCustomError(pool, 'MinWeight'); + + // Try to set weight above 99% + await expect(pool.connect(bob).updateWeightsGradually(startTime, endTime, [fp(0.991), fp(0.009)])) + .to.be.revertedWithCustomError(pool, 'MinWeight'); + + // Valid weight update + await expect(pool.connect(bob).updateWeightsGradually(startTime, endTime, [fp(0.01), fp(0.99)])) + .to.not.be.reverted; + }); + +it('should always sum weights to 1', async () => { + const currentTime = await time.latest(); + const startTime = currentTime + 60; // Set startTime 60 seconds in the future + const endTime = startTime + MONTH; + const startWeights = [fp(0.5), fp(0.5)]; + const endWeights = [fp(0.7), fp(0.3)]; + + // Move time to just before startTime + await time.increaseTo(startTime - 1); + + // Set weights to 50/50 instantaneously + const tx1 = await pool.connect(bob).updateWeightsGradually(startTime, startTime, startWeights); + await tx1.wait(); + + // Schedule gradual shift to 70/30 + const tx2 = await pool.connect(bob).updateWeightsGradually(startTime, endTime, endWeights); + await tx2.wait(); + + // Check weights at various points during the transition + for (let i = 0; i <= 100; i++) { + const checkTime = startTime + (i * MONTH) / 100; + + // Only increase time if it's greater than the current time + const currentBlockTime = await time.latest(); + if (checkTime > currentBlockTime) { + await time.increaseTo(checkTime); + } + + const weights = await pool.getNormalizedWeights(); + const sum = (BigInt(weights[0].toString()) + BigInt(weights[1].toString())).toString(); + + // Assert exact equality + expect(sum).to.equal(fp(1)); + + } +}); + + + }); + + describe('Setters and Getters', () => { + it('should set and get swap enabled status', async () => { + await pool.connect(bob).setSwapEnabled(false); + expect(await pool.getSwapEnabled()).to.be.false; + + await pool.connect(bob).setSwapEnabled(true); + expect(await pool.getSwapEnabled()).to.be.true; + }); + + it('should get gradual weight update params', async () => { + const startTime = await time.latest(); + const endTime = startTime + MONTH; + const endWeights = [fp(0.7), fp(0.3)]; + + const tx = await pool.connect(bob).updateWeightsGradually(startTime, endTime, endWeights); + await tx.wait(); + const actualStartTime = await time.latest(); + + const params = await pool.getGradualWeightUpdateParams(); + expect(params.startTime).to.equal(actualStartTime); + expect(params.endTime).to.equal(endTime); + expect(params.endWeights).to.deep.equal(endWeights); + }); + }); + + describe('Swap restrictions', () => { + it('should not allow swaps when disabled', async () => { + await pool.connect(bob).setSwapEnabled(false); + + await expect(router.connect(bob).swap( + pool, + poolTokens[0], + poolTokens[1], + SWAP_AMOUNT, + 0, + false, + '0x' + )).to.be.reverted; // Exact error message depends on implementation + + await pool.connect(bob).setSwapEnabled(true); + + await expect(router.connect(bob).swap( + pool, + poolTokens[0], + poolTokens[1], + SWAP_AMOUNT, + 0, + false, + '0x' + )).to.not.be.reverted; + }); + }); }); diff --git a/pkg/pool-weighted/test/foundry/GradualValueChange.t.sol b/pkg/pool-weighted/test/foundry/GradualValueChange.t.sol new file mode 100644 index 000000000..fa6b1c786 --- /dev/null +++ b/pkg/pool-weighted/test/foundry/GradualValueChange.t.sol @@ -0,0 +1,193 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity ^0.8.24; + +import "forge-std/Test.sol"; +import "../../contracts/lib/GradualValueChange.sol"; + +// Wrapper contract to expose library functions +contract GradualValueChangeWrapper { + function getInterpolatedValue( + uint256 startValue, + uint256 endValue, + uint256 startTime, + uint256 endTime + ) public view returns (uint256) { + return GradualValueChange.getInterpolatedValue(startValue, endValue, startTime, endTime); + } + + function resolveStartTime(uint256 startTime, uint256 endTime) public view returns (uint256) { + return GradualValueChange.resolveStartTime(startTime, endTime); + } + + function ensureNoTimeOverflow(uint256 time, uint256 maxTime) public pure { + GradualValueChange.ensureNoTimeOverflow(time, maxTime); + } + + function interpolateValue( + uint256 startValue, + uint256 endValue, + uint256 pctProgress + ) public pure returns (uint256) { + return GradualValueChange.interpolateValue(startValue, endValue, pctProgress); + } + + function calculateValueChangeProgress(uint256 startTime, uint256 endTime) public view returns (uint256) { + return GradualValueChange.calculateValueChangeProgress(startTime, endTime); + } +} + +contract GradualValueChangeTest is Test { + GradualValueChangeWrapper private wrapper; + uint256 private constant FP_ONE = 1e18; + + function setUp() public { + wrapper = new GradualValueChangeWrapper(); + } + + function testGetInterpolatedValue() public { + uint256 startValue = 100e18; + uint256 endValue = 200e18; + uint256 startTime = block.timestamp; + uint256 endTime = startTime + 100; + uint256 steps = 100; + + for (uint256 i = 0; i <= steps; i++) { + uint256 currentTime = startTime + (i * (endTime - startTime) / steps); + vm.warp(currentTime); + uint256 expectedValue = startValue + (i * (endValue - startValue) / steps); + uint256 actualValue = wrapper.getInterpolatedValue(startValue, endValue, startTime, endTime); + assertEq(actualValue, expectedValue, "Interpolated value should match expected"); + } + } + + function testResolveStartTime() public { + uint256 currentTime = 1000000; + uint256 futureTime = currentTime + 100; + + vm.warp(currentTime); + assertEq(wrapper.resolveStartTime(futureTime, futureTime + 100), futureTime, "Should return future start time"); + assertEq(wrapper.resolveStartTime(currentTime - 100, futureTime), currentTime, "Should return current time for past start time"); + + vm.expectRevert(GradualValueChange.GradualUpdateTimeTravel.selector); + wrapper.resolveStartTime(futureTime + 200, futureTime + 100); + } + + function testEnsureNoTimeOverflow() public { + uint256 maxTime = 1000; + + wrapper.ensureNoTimeOverflow(maxTime, maxTime); + wrapper.ensureNoTimeOverflow(maxTime - 1, maxTime); + + vm.expectRevert(GradualValueChange.TimeTruncatedInStorage.selector); + wrapper.ensureNoTimeOverflow(maxTime + 1, maxTime); + } + + function testInterpolateValue() public view { + uint256 startValue = 100e18; + uint256 endValue = 200e18; + uint256 steps = 100; + + for (uint256 i = 0; i <= steps; i++) { + uint256 pctProgress = (i * FP_ONE) / steps; + uint256 expectedValue = startValue + (i * (endValue - startValue) / steps); + uint256 actualValue = wrapper.interpolateValue(startValue, endValue, pctProgress); + assertEq(actualValue, expectedValue, "Interpolated value should match expected"); + } + + // Test decreasing value + startValue = 200e18; + endValue = 100e18; + + for (uint256 i = 0; i <= steps; i++) { + uint256 pctProgress = (i * FP_ONE) / steps; + uint256 expectedValue = startValue - (i * (startValue - endValue) / steps); + uint256 actualValue = wrapper.interpolateValue(startValue, endValue, pctProgress); + assertEq(actualValue, expectedValue, "Interpolated value should match expected for decreasing value"); + } + } + + function testCalculateValueChangeProgress() public { + uint256 startTime = block.timestamp; + uint256 endTime = startTime + 100; + uint256 steps = 100; + + for (uint256 i = 0; i <= steps; i++) { + uint256 currentTime = startTime + (i * (endTime - startTime) / steps); + vm.warp(currentTime); + uint256 expectedProgress = (i * FP_ONE) / steps; + uint256 actualProgress = wrapper.calculateValueChangeProgress(startTime, endTime); + // Use a very tight tolerance for progress calculation + assertApproxEqAbs(actualProgress, expectedProgress, 1, "Progress should be very close to expected"); + } + + vm.warp(endTime + 50); + assertEq(wrapper.calculateValueChangeProgress(startTime, endTime), FP_ONE, "Should be complete after end time"); + } + + function testEdgeCases() public { + uint256 startTime = block.timestamp; + uint256 endTime = startTime + 100; + + uint256 startValue = 100e18; + uint256 endValue = 200e18; + + for (uint256 i = 0; i <= 100; i++) { + vm.warp(startTime + i); + assertEq(wrapper.getInterpolatedValue(startValue, startValue, startTime, endTime), startValue, "Should always return the same value"); + } + + // Test before start time + vm.warp(startTime - 1); + assertEq( + wrapper.getInterpolatedValue(startValue, endValue, startTime, startTime), + startValue, + "Should return start value before start time for zero duration" + ); + + // Test at start time + vm.warp(startTime); + assertEq( + wrapper.getInterpolatedValue(startValue, endValue, startTime, startTime), + endValue, + "Should return end value at start time for zero duration" + ); + + // Test after start time + vm.warp(startTime + 1); + assertEq( + wrapper.getInterpolatedValue(startValue, endValue, startTime, startTime), + endValue, + "Should return end value after start time for zero duration" + ); + + uint256 bigVal = 1e18 * FP_ONE; //1 quintillion w/ 18 decimals + + // Test exact endpoints + vm.warp(0); + assertEq(wrapper.getInterpolatedValue(0, bigVal, 0, bigVal), 0, "Should be 0 at start time"); + vm.warp(bigVal); + assertEq(wrapper.getInterpolatedValue(0, bigVal, 0, bigVal), bigVal, "Should be bigVal at end time"); + + // Test intermediate points + uint256 steps = 1e5; + for (uint256 i = 0; i <= steps; i++) { + uint256 currentTime = (bigVal / steps) * i; + vm.warp(currentTime); + uint256 expectedValue = (bigVal / steps) * i; + + uint256 actualValue; + try wrapper.getInterpolatedValue(0, bigVal, 0, bigVal) returns (uint256 value) { + actualValue = value; + assertApproxEqRel(actualValue, expectedValue, 1, "Should be close for large numbers"); + } catch Error(string memory reason) { + revert(string(abi.encodePacked("getInterpolatedValue reverted: ", reason))); + } catch (bytes memory /*lowLevelData*/) { + revert("getInterpolatedValue reverted unexpectedly"); + } + + // Additional check to ensure the value is within the expected range + assertGe(actualValue, 0, "Value should not be less than 0"); + assertLe(actualValue, bigVal, "Value should not exceed bigVal"); + } + } +} \ No newline at end of file diff --git a/pkg/pool-weighted/test/foundry/WeightValidationTest.t.sol b/pkg/pool-weighted/test/foundry/WeightValidationTest.t.sol new file mode 100644 index 000000000..12b97bf4b --- /dev/null +++ b/pkg/pool-weighted/test/foundry/WeightValidationTest.t.sol @@ -0,0 +1,109 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity ^0.8.24; + +import "forge-std/Test.sol"; +import "../../contracts/lib/WeightValidation.sol"; + +// Wrapper contract to expose library functions +contract WeightValidationWrapper { + function validateWeights(uint256[] memory normalizedWeights, uint256 numTokens) public pure { + WeightValidation.validateWeights(normalizedWeights, numTokens); + } + + function validateTwoWeights(uint256 normalizedWeight0, uint256 normalizedWeight1) public pure { + WeightValidation.validateTwoWeights(normalizedWeight0, normalizedWeight1); + } +} + +contract WeightValidationTest is Test { + WeightValidationWrapper private wrapper; + uint256 private constant FP_ONE = 1e18; // FixedPoint.ONE + uint256 private constant MIN_WEIGHT = 1e16; // 1% + + function setUp() public { + wrapper = new WeightValidationWrapper(); + } + + function testValidateWeightsValidRangeMultipleTokens() public view { + for (uint256 numTokens = 2; numTokens <= 8; numTokens++) { + uint256[] memory weights = new uint256[](numTokens); + uint256 remainingWeight = FP_ONE; + + for (uint256 i = 0; i < numTokens - 1; i++) { + uint256 weight = (remainingWeight / (numTokens - i)) + 1e16; // Ensure it's above MIN_WEIGHT + weights[i] = weight; + remainingWeight -= weight; + } + weights[numTokens - 1] = remainingWeight; + + wrapper.validateWeights(weights, numTokens); + } + } + + function testValidateWeightsEdgeCases() public view { + // Test with all weights at minimum except one + uint256[] memory weights = new uint256[](5); + for (uint256 i = 0; i < 4; i++) { + weights[i] = MIN_WEIGHT; + } + weights[4] = FP_ONE - (MIN_WEIGHT * 4); + wrapper.validateWeights(weights, 5); + + // Test with two weights splitting the total + uint256[] memory twoWeights = new uint256[](2); + twoWeights[0] = FP_ONE / 2; + twoWeights[1] = FP_ONE / 2; + wrapper.validateWeights(twoWeights, 2); + } + + function testValidateWeightsInvalidSum() public { + uint256[] memory weights = new uint256[](3); + weights[0] = 0.3e18; + weights[1] = 0.3e18; + weights[2] = 0.3e18; + vm.expectRevert(WeightValidation.NormalizedWeightInvariant.selector); + wrapper.validateWeights(weights, 3); + + // Test with sum slightly above FP_ONE + weights[2] = 0.400000000000000001e18; + vm.expectRevert(WeightValidation.NormalizedWeightInvariant.selector); + wrapper.validateWeights(weights, 3); + } + + function testValidateWeightsBelowMinWeight() public { + uint256[] memory weights = new uint256[](3); + weights[0] = MIN_WEIGHT - 1; + weights[1] = (FP_ONE - MIN_WEIGHT + 1) / 2; + weights[2] = (FP_ONE - MIN_WEIGHT + 1) / 2; + vm.expectRevert(WeightValidation.MinWeight.selector); + wrapper.validateWeights(weights, 3); + } + + function testValidateTwoWeightsValidRange() public view { + for (uint256 i = MIN_WEIGHT; i <= FP_ONE - MIN_WEIGHT; i += 1e16) { + wrapper.validateTwoWeights(i, FP_ONE - i); + } + } + + function testValidateTwoWeightsInvalidSum() public { + vm.expectRevert(WeightValidation.NormalizedWeightInvariant.selector); + wrapper.validateTwoWeights(0.5e18, 0.500000000000000001e18); + + vm.expectRevert(WeightValidation.NormalizedWeightInvariant.selector); + wrapper.validateTwoWeights(0.5e18, 0.499999999999999999e18); + } + + function testValidateTwoWeightsBelowMinWeight() public { + vm.expectRevert(WeightValidation.MinWeight.selector); + wrapper.validateTwoWeights(MIN_WEIGHT - 1, FP_ONE - MIN_WEIGHT + 1); + + vm.expectRevert(WeightValidation.MinWeight.selector); + wrapper.validateTwoWeights(FP_ONE - MIN_WEIGHT + 1, MIN_WEIGHT - 1); + } + + function testValidateTwoWeightsEdgeCases() public view { + wrapper.validateTwoWeights(MIN_WEIGHT, FP_ONE - MIN_WEIGHT); + wrapper.validateTwoWeights(FP_ONE - MIN_WEIGHT, MIN_WEIGHT); + wrapper.validateTwoWeights(FP_ONE / 2, FP_ONE / 2); + } +} \ No newline at end of file From 37cc15caa7e07da85732dea575f6e2dbbef5bfc2 Mon Sep 17 00:00:00 2001 From: gerg Date: Mon, 9 Sep 2024 10:42:53 -0400 Subject: [PATCH 18/54] make foundry tests work; make BasePoolTests override-able --- pkg/pool-weighted/contracts/LBPool.sol | 8 +- pkg/pool-weighted/test/foundry/LBPool.t.sol | 373 ++++++++++++++++++ pkg/vault/test/foundry/utils/BasePoolTest.sol | 16 +- 3 files changed, 384 insertions(+), 13 deletions(-) create mode 100644 pkg/pool-weighted/test/foundry/LBPool.t.sol diff --git a/pkg/pool-weighted/contracts/LBPool.sol b/pkg/pool-weighted/contracts/LBPool.sol index b84e13de3..b73018866 100644 --- a/pkg/pool-weighted/contracts/LBPool.sol +++ b/pkg/pool-weighted/contracts/LBPool.sol @@ -25,7 +25,7 @@ import { WeightValidation } from "./lib/WeightValidation.sol"; /// @notice Inheriting from WeightedPool is only slightly wasteful (setting 2 immutable weights /// and _totalTokens, which will not be used later), and it is tremendously helpful for pool /// validation and any potential future parent class changes. -contract LBPool is WeightedPool, Ownable { +contract LBPool is WeightedPool, Ownable { //TODO is BaseHooks // Since we have max 2 tokens and the weights must sum to 1, we only need to store one weight struct PoolState { uint56 startTime; @@ -156,10 +156,8 @@ contract LBPool is WeightedPool, Ownable { address pool, TokenConfig[] memory, LiquidityManagement calldata - ) external view onlyVault returns (bool success) { - if (pool == address(this) && IBasePoolFactory(factory).isPoolFromFactory(pool)) { - success = true; - } + ) external view onlyVault returns (bool) { + return (pool == address(this) && IBasePoolFactory(factory).isPoolFromFactory(pool)); } // Return HookFlags struct that indicates which hooks this contract supports diff --git a/pkg/pool-weighted/test/foundry/LBPool.t.sol b/pkg/pool-weighted/test/foundry/LBPool.t.sol new file mode 100644 index 000000000..5c3632034 --- /dev/null +++ b/pkg/pool-weighted/test/foundry/LBPool.t.sol @@ -0,0 +1,373 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +pragma solidity ^0.8.24; + +import "forge-std/Test.sol"; +import "@openzeppelin/contracts/utils/Strings.sol"; + +import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; + +import { IBasePool } from "@balancer-labs/v3-interfaces/contracts/vault/IBasePool.sol"; +import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol"; +import { TokenConfig, PoolRoleAccounts } from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; +import { IVaultErrors } from "@balancer-labs/v3-interfaces/contracts/vault/IVaultErrors.sol"; + +import { CastingHelpers } from "@balancer-labs/v3-solidity-utils/contracts/helpers/CastingHelpers.sol"; +import { ArrayHelpers } from "@balancer-labs/v3-solidity-utils/contracts/test/ArrayHelpers.sol"; +import { InputHelpers } from "@balancer-labs/v3-solidity-utils/contracts/helpers/InputHelpers.sol"; +import { PoolHooksMock } from "@balancer-labs/v3-vault/contracts/test/PoolHooksMock.sol"; +import { BasePoolTest } from "@balancer-labs/v3-vault/test/foundry/utils/BasePoolTest.sol"; +import { FixedPoint } from "@balancer-labs/v3-solidity-utils/contracts/math/FixedPoint.sol"; + +import { LBPoolFactory } from "../../contracts/LBPoolFactory.sol"; +import { LBPool } from "../../contracts/LBPool.sol"; +import "forge-std/console.sol"; + +contract LBPoolTest is BasePoolTest { + using CastingHelpers for address[]; + using ArrayHelpers for *; + using FixedPoint for uint256; + + uint256 constant DEFAULT_SWAP_FEE = 1e16; // 1% + uint256 constant TOKEN_AMOUNT = 1e3 * 1e18; + + uint256[] internal weights; + + uint256 daiIdx; + uint256 usdcIdx; + + function setUp() public virtual override { + expectedAddLiquidityBptAmountOut = TOKEN_AMOUNT; + tokenAmountIn = TOKEN_AMOUNT / 4; + isTestSwapFeeEnabled = false; + + BasePoolTest.setUp(); + + (daiIdx, usdcIdx) = getSortedIndexes(address(dai), address(usdc)); + + poolMinSwapFeePercentage = 0.001e16; // 0.001% + poolMaxSwapFeePercentage = 10e16; + } + + function createPool() internal override returns (address) { + IERC20[] memory sortedTokens = InputHelpers.sortTokens( + [address(dai), address(usdc)].toMemoryArray().asIERC20() + ); + for (uint256 i = 0; i < sortedTokens.length; i++) { + poolTokens.push(sortedTokens[i]); + tokenAmounts.push(TOKEN_AMOUNT); + } + + factory = new LBPoolFactory(IVault(address(vault)), 365 days, "Factory v1", "Pool v1", address(0), address(router)); + weights = [uint256(50e16), uint256(50e16)].toMemoryArray(); + + // Allow pools created by `factory` to use poolHooksMock hooks + PoolHooksMock(poolHooksContract).allowFactory(address(factory)); + + LBPool newPool = LBPool( + LBPoolFactory(address(factory)).create( + "LB Pool", + "LBPOOL", + vault.buildTokenConfig(sortedTokens), + weights, + DEFAULT_SWAP_FEE, + bob, + true, + ZERO_BYTES32 + ) + ); + return address(newPool); + } + + function testInitialize() public view override { + (, , uint256[] memory balances, ) = vault.getPoolTokenInfo(address(pool)); + + for (uint256 i = 0; i < poolTokens.length; ++i) { + // Tokens are transferred from bob (lp/owner) + assertEq( + defaultBalance - poolTokens[i].balanceOf(bob), + tokenAmounts[i], + string.concat("LP: Wrong balance for ", Strings.toString(i)) + ); + + // Tokens are stored in the Vault + assertEq( + poolTokens[i].balanceOf(address(vault)), + tokenAmounts[i], + string.concat("LP: Vault balance for ", Strings.toString(i)) + ); + + // Tokens are deposited to the pool + assertEq( + balances[i], + tokenAmounts[i], + string.concat("Pool: Wrong token balance for ", Strings.toString(i)) + ); + } + + // should mint correct amount of BPT poolTokens + // Account for the precision loss + assertApproxEqAbs(IERC20(pool).balanceOf(bob), bptAmountOut, DELTA, "LP: Wrong bptAmountOut"); + assertApproxEqAbs(bptAmountOut, expectedAddLiquidityBptAmountOut, DELTA, "Wrong bptAmountOut"); + } + + function initPool() internal override { + vm.startPrank(bob); + bptAmountOut = _initPool( + pool, + tokenAmounts, + // Account for the precision loss + expectedAddLiquidityBptAmountOut - DELTA + ); + vm.stopPrank(); + } + + // overriding b/c bob needs to be the LP and has contributed double the "normal" amount of tokens + function testAddLiquidity() public override { + uint256 oldBptAmount = IERC20(pool).balanceOf(bob); + vm.prank(bob); + bptAmountOut = router.addLiquidityUnbalanced(pool, tokenAmounts, tokenAmountIn - DELTA, false, bytes("")); + + (, , uint256[] memory balances, ) = vault.getPoolTokenInfo(address(pool)); + + for (uint256 i = 0; i < poolTokens.length; ++i) { + // Tokens are transferred from Bob + assertEq( + defaultBalance - poolTokens[i].balanceOf(bob), + tokenAmounts[i] * 2, // x2 because bob (as owner) did init join and subsequent join + string.concat("LP: Wrong token balance for ", Strings.toString(i)) + ); + + // Tokens are stored in the Vault + assertEq( + poolTokens[i].balanceOf(address(vault)), + tokenAmounts[i] * 2, + string.concat("Vault: Wrong token balance for ", Strings.toString(i)) + ); + + assertEq( + balances[i], + tokenAmounts[i] * 2, + string.concat("Pool: Wrong token balance for ", Strings.toString(i)) + ); + } + + uint256 newBptAmount = IERC20(pool).balanceOf(bob); + + // should mint correct amount of BPT poolTokens + assertApproxEqAbs(newBptAmount - oldBptAmount, bptAmountOut, DELTA, "LP: Wrong bptAmountOut"); + assertApproxEqAbs(bptAmountOut, expectedAddLiquidityBptAmountOut, DELTA, "Wrong bptAmountOut"); + } + + // overriding b/c bob has swap fee authority, not governance + // TODO: why does this test need to change swap fee anyway? + function testAddLiquidityUnbalanced() public override { + vm.prank(bob); + vault.setStaticSwapFeePercentage(pool, 10e16); + + uint256[] memory amountsIn = tokenAmounts; + amountsIn[0] = amountsIn[0].mulDown(IBasePool(pool).getMaximumInvariantRatio()); + vm.prank(bob); + + router.addLiquidityUnbalanced(pool, amountsIn, 0, false, bytes("")); + } + + function testRemoveLiquidity() public override { + vm.startPrank(bob); + uint256 oldBptAmount = IERC20(pool).balanceOf(bob); + router.addLiquidityUnbalanced(pool, tokenAmounts, tokenAmountIn - DELTA, false, bytes("")); + uint256 newBptAmount = IERC20(pool).balanceOf(bob); + + IERC20(pool).approve(address(vault), MAX_UINT256); + + uint256 bptAmountIn = newBptAmount - oldBptAmount; + + uint256[] memory minAmountsOut = new uint256[](poolTokens.length); + for (uint256 i = 0; i < poolTokens.length; ++i) { + minAmountsOut[i] = _less(tokenAmounts[i], 1e4); + } + + uint256[] memory amountsOut = router.removeLiquidityProportional( + pool, + bptAmountIn, + minAmountsOut, + false, + bytes("") + ); + + vm.stopPrank(); + + (, , uint256[] memory balances, ) = vault.getPoolTokenInfo(address(pool)); + + for (uint256 i = 0; i < poolTokens.length; ++i) { + // Tokens are transferred to Bob + assertApproxEqAbs( + poolTokens[i].balanceOf(bob) + TOKEN_AMOUNT, //add TOKEN_AMOUNT to account for init join + defaultBalance, + DELTA, + string.concat("LP: Wrong token balance for ", Strings.toString(i)) + ); + + // Tokens are stored in the Vault + assertApproxEqAbs( + poolTokens[i].balanceOf(address(vault)), + tokenAmounts[i], + DELTA, + string.concat("Vault: Wrong token balance for ", Strings.toString(i)) + ); + + // Tokens are deposited to the pool + assertApproxEqAbs( + balances[i], + tokenAmounts[i], + DELTA, + string.concat("Pool: Wrong token balance for ", Strings.toString(i)) + ); + + // amountsOut are correct + assertApproxEqAbs( + amountsOut[i], + tokenAmounts[i], + DELTA, + string.concat("Wrong token amountOut for ", Strings.toString(i)) + ); + } + + // should return to correct amount of BPT poolTokens + assertEq(IERC20(pool).balanceOf(bob), oldBptAmount, "LP: Wrong BPT balance"); + } + + function testSwap() public override { + if (!isTestSwapFeeEnabled) { + vault.manuallySetSwapFee(pool, 0); + } + + IERC20 tokenIn = poolTokens[tokenIndexIn]; + IERC20 tokenOut = poolTokens[tokenIndexOut]; + + uint256 bobBeforeBalanceTokenOut = tokenOut.balanceOf(bob); + uint256 bobBeforeBalanceTokenIn = tokenIn.balanceOf(bob); + + vm.prank(bob); + uint256 amountCalculated = router.swapSingleTokenExactIn( + pool, + tokenIn, + tokenOut, + tokenAmountIn, + _less(tokenAmountOut, 1e3), + MAX_UINT256, + false, + bytes("") + ); + + // Tokens are transferred from Bob + assertEq(tokenOut.balanceOf(bob), bobBeforeBalanceTokenOut + amountCalculated, "LP: Wrong tokenOut balance"); + assertEq(tokenIn.balanceOf(bob), bobBeforeBalanceTokenIn - tokenAmountIn, "LP: Wrong tokenIn balance"); + + // Tokens are stored in the Vault + assertEq( + tokenOut.balanceOf(address(vault)), + tokenAmounts[tokenIndexOut] - amountCalculated, + "Vault: Wrong tokenOut balance" + ); + assertEq( + tokenIn.balanceOf(address(vault)), + tokenAmounts[tokenIndexIn] + tokenAmountIn, + "Vault: Wrong tokenIn balance" + ); + + (, , uint256[] memory balances, ) = vault.getPoolTokenInfo(pool); + + assertEq(balances[tokenIndexIn], tokenAmounts[tokenIndexIn] + tokenAmountIn, "Pool: Wrong tokenIn balance"); + assertEq( + balances[tokenIndexOut], + tokenAmounts[tokenIndexOut] - amountCalculated, + "Pool: Wrong tokenOut balance" + ); + } + + function testSetSwapFeeTooLow() public override { + vm.prank(bob); + vm.expectRevert(IVaultErrors.SwapFeePercentageTooLow.selector); + vault.setStaticSwapFeePercentage(pool, poolMinSwapFeePercentage - 1); + } + + function testSetSwapFeeTooHigh() public override { + vm.prank(bob); + vm.expectRevert(abi.encodeWithSelector(IVaultErrors.SwapFeePercentageTooHigh.selector)); + vault.setStaticSwapFeePercentage(pool, poolMaxSwapFeePercentage + 1); + } + + function testOnlyOwnerCanBeLP() public { + uint256[] memory amounts = [TOKEN_AMOUNT, TOKEN_AMOUNT].toMemoryArray(); + + vm.startPrank(bob); + router.addLiquidityUnbalanced(address(pool), amounts, 0, false, ""); + vm.stopPrank(); + + vm.startPrank(alice); + vm.expectRevert(abi.encodeWithSelector(IVaultErrors.BeforeAddLiquidityHookFailed.selector)); + router.addLiquidityUnbalanced(address(pool), amounts, 0, false, ""); + vm.stopPrank(); + } + + function testSwapRestrictions() public { + // Ensure swaps are initially enabled + assertTrue(LBPool(address(pool)).getSwapEnabled(), "Swaps should be enabled initially"); + + // Test swap when enabled + vm.prank(alice); + router.swapSingleTokenExactIn( + address(pool), + IERC20(dai), + IERC20(usdc), + TOKEN_AMOUNT / 10, + 0, + block.timestamp + 1 hours, + false, + "" + ); + + // Disable swaps + vm.prank(bob); + LBPool(address(pool)).setSwapEnabled(false); + + // Verify swaps are disabled + assertFalse(LBPool(address(pool)).getSwapEnabled(), "Swaps should be disabled"); + + // Test swap when disabled + vm.prank(alice); + vm.expectRevert(abi.encodeWithSelector(IVaultErrors.BeforeSwapHookFailed.selector)); + router.swapSingleTokenExactIn( + address(pool), + IERC20(dai), + IERC20(usdc), + TOKEN_AMOUNT / 10, + 0, + block.timestamp + 1 hours, + false, + "" + ); + + // Re-enable swaps + vm.prank(bob); + LBPool(address(pool)).setSwapEnabled(true); + + // Verify swaps are re-enabled + assertTrue(LBPool(address(pool)).getSwapEnabled(), "Swaps should be re-enabled"); + + // Test swap after re-enabling + vm.prank(alice); + router.swapSingleTokenExactIn( + address(pool), + IERC20(dai), + IERC20(usdc), + TOKEN_AMOUNT / 10, + 0, + block.timestamp + 1 hours, + false, + "" + ); + } + +} \ No newline at end of file diff --git a/pkg/vault/test/foundry/utils/BasePoolTest.sol b/pkg/vault/test/foundry/utils/BasePoolTest.sol index d202374dd..417cf1f22 100644 --- a/pkg/vault/test/foundry/utils/BasePoolTest.sol +++ b/pkg/vault/test/foundry/utils/BasePoolTest.sol @@ -66,7 +66,7 @@ abstract contract BasePoolTest is BaseVaultTest { assertEq(pauseManager, address(0), "Pause manager should be 0"); } - function testInitialize() public view { + function testInitialize() public view virtual { (, , uint256[] memory balances, ) = vault.getPoolTokenInfo(address(pool)); for (uint256 i = 0; i < poolTokens.length; ++i) { @@ -98,7 +98,7 @@ abstract contract BasePoolTest is BaseVaultTest { assertApproxEqAbs(bptAmountOut, expectedAddLiquidityBptAmountOut, DELTA, "Wrong bptAmountOut"); } - function testAddLiquidity() public { + function testAddLiquidity() public virtual { vm.prank(bob); bptAmountOut = router.addLiquidityUnbalanced(pool, tokenAmounts, tokenAmountIn - DELTA, false, bytes("")); @@ -131,7 +131,7 @@ abstract contract BasePoolTest is BaseVaultTest { assertApproxEqAbs(bptAmountOut, expectedAddLiquidityBptAmountOut, DELTA, "Wrong bptAmountOut"); } - function testRemoveLiquidity() public { + function testRemoveLiquidity() public virtual { vm.startPrank(bob); router.addLiquidityUnbalanced(pool, tokenAmounts, tokenAmountIn - DELTA, false, bytes("")); @@ -196,7 +196,7 @@ abstract contract BasePoolTest is BaseVaultTest { assertEq(bobBptBalance, bptAmountIn, "LP: Wrong bptAmountIn"); } - function testSwap() public { + function testSwap() public virtual { if (!isTestSwapFeeEnabled) { vault.manuallySetSwapFee(pool, 0); } @@ -250,7 +250,7 @@ abstract contract BasePoolTest is BaseVaultTest { assertEq(IBasePool(pool).getMaximumSwapFeePercentage(), poolMaxSwapFeePercentage, "Maximum swap fee mismatch"); } - function testSetSwapFeeTooLow() public { + function testSetSwapFeeTooLow() public virtual { authorizer.grantRole(vault.getActionId(IVaultAdmin.setStaticSwapFeePercentage.selector), alice); vm.prank(alice); @@ -258,7 +258,7 @@ abstract contract BasePoolTest is BaseVaultTest { vault.setStaticSwapFeePercentage(pool, poolMinSwapFeePercentage - 1); } - function testSetSwapFeeTooHigh() public { + function testSetSwapFeeTooHigh() public virtual { authorizer.grantRole(vault.getActionId(IVaultAdmin.setStaticSwapFeePercentage.selector), alice); vm.prank(alice); @@ -266,7 +266,7 @@ abstract contract BasePoolTest is BaseVaultTest { vault.setStaticSwapFeePercentage(pool, poolMaxSwapFeePercentage + 1); } - function testAddLiquidityUnbalanced() public { + function testAddLiquidityUnbalanced() public virtual { authorizer.grantRole(vault.getActionId(IVaultAdmin.setStaticSwapFeePercentage.selector), alice); vm.prank(alice); vault.setStaticSwapFeePercentage(pool, 10e16); @@ -295,7 +295,7 @@ abstract contract BasePoolTest is BaseVaultTest { } // Decreases the amount value by base value. Example: base = 100, decrease by 1% / base = 1e4, 0.01% and etc. - function _less(uint256 amount, uint256 base) private pure returns (uint256) { + function _less(uint256 amount, uint256 base) internal pure returns (uint256) { return (amount * (base - 1)) / base; } } From 18871699f81a52dd85f39fe1c38b801d02b74da2 Mon Sep 17 00:00:00 2001 From: gerg Date: Tue, 10 Sep 2024 12:57:50 -0400 Subject: [PATCH 19/54] generalize the BasePoolTest tests that change the static swap fee to enable swap fee changes by the assigned admin rather than assuming that power has been delegated to governance --- pkg/pool-weighted/test/foundry/LBPool.t.sol | 12 --------- pkg/vault/test/foundry/utils/BasePoolTest.sol | 26 ++++++++++++++----- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/pkg/pool-weighted/test/foundry/LBPool.t.sol b/pkg/pool-weighted/test/foundry/LBPool.t.sol index 5c3632034..4feca4b10 100644 --- a/pkg/pool-weighted/test/foundry/LBPool.t.sol +++ b/pkg/pool-weighted/test/foundry/LBPool.t.sol @@ -286,18 +286,6 @@ contract LBPoolTest is BasePoolTest { ); } - function testSetSwapFeeTooLow() public override { - vm.prank(bob); - vm.expectRevert(IVaultErrors.SwapFeePercentageTooLow.selector); - vault.setStaticSwapFeePercentage(pool, poolMinSwapFeePercentage - 1); - } - - function testSetSwapFeeTooHigh() public override { - vm.prank(bob); - vm.expectRevert(abi.encodeWithSelector(IVaultErrors.SwapFeePercentageTooHigh.selector)); - vault.setStaticSwapFeePercentage(pool, poolMaxSwapFeePercentage + 1); - } - function testOnlyOwnerCanBeLP() public { uint256[] memory amounts = [TOKEN_AMOUNT, TOKEN_AMOUNT].toMemoryArray(); diff --git a/pkg/vault/test/foundry/utils/BasePoolTest.sol b/pkg/vault/test/foundry/utils/BasePoolTest.sol index 417cf1f22..0b2d3f56b 100644 --- a/pkg/vault/test/foundry/utils/BasePoolTest.sol +++ b/pkg/vault/test/foundry/utils/BasePoolTest.sol @@ -10,6 +10,7 @@ import { IBasePoolFactory } from "@balancer-labs/v3-interfaces/contracts/vault/I import { IVaultErrors } from "@balancer-labs/v3-interfaces/contracts/vault/IVaultErrors.sol"; import { FixedPoint } from "@balancer-labs/v3-solidity-utils/contracts/math/FixedPoint.sol"; import { IVaultAdmin } from "@balancer-labs/v3-interfaces/contracts/vault/IVaultAdmin.sol"; +import { PoolRoleAccounts } from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; import { IBasePool } from "@balancer-labs/v3-interfaces/contracts/vault/IBasePool.sol"; import { Vault } from "@balancer-labs/v3-vault/contracts/Vault.sol"; @@ -250,25 +251,36 @@ abstract contract BasePoolTest is BaseVaultTest { assertEq(IBasePool(pool).getMaximumSwapFeePercentage(), poolMaxSwapFeePercentage, "Maximum swap fee mismatch"); } - function testSetSwapFeeTooLow() public virtual { - authorizer.grantRole(vault.getActionId(IVaultAdmin.setStaticSwapFeePercentage.selector), alice); - vm.prank(alice); + function getSwapFeeAdmin() public returns (address) { + address swapFeeManager; + PoolRoleAccounts memory roleAccounts = vault.getPoolRoleAccounts(pool); + if (roleAccounts.swapFeeManager != address(0)) { + return roleAccounts.swapFeeManager; + } else { + swapFeeManager = alice; + authorizer.grantRole(vault.getActionId(IVaultAdmin.setStaticSwapFeePercentage.selector), swapFeeManager); + return swapFeeManager; + } + } + function testSetSwapFeeTooLow() public virtual { + address swapFeeManager = getSwapFeeAdmin(); + vm.prank(swapFeeManager); vm.expectRevert(IVaultErrors.SwapFeePercentageTooLow.selector); vault.setStaticSwapFeePercentage(pool, poolMinSwapFeePercentage - 1); } function testSetSwapFeeTooHigh() public virtual { - authorizer.grantRole(vault.getActionId(IVaultAdmin.setStaticSwapFeePercentage.selector), alice); - vm.prank(alice); + address swapFeeManager = getSwapFeeAdmin(); + vm.prank(swapFeeManager); vm.expectRevert(abi.encodeWithSelector(IVaultErrors.SwapFeePercentageTooHigh.selector)); vault.setStaticSwapFeePercentage(pool, poolMaxSwapFeePercentage + 1); } function testAddLiquidityUnbalanced() public virtual { - authorizer.grantRole(vault.getActionId(IVaultAdmin.setStaticSwapFeePercentage.selector), alice); - vm.prank(alice); + address swapFeeManager = getSwapFeeAdmin(); + vm.prank(swapFeeManager); vault.setStaticSwapFeePercentage(pool, 10e16); uint256[] memory amountsIn = tokenAmounts; From 2973ad10f60cd2727fe2cf93604d80fecc43b8eb Mon Sep 17 00:00:00 2001 From: gerg Date: Wed, 11 Sep 2024 09:34:21 -0400 Subject: [PATCH 20/54] inherit from BaseHooks; properly override; directly use TRUSTED_ROUTER rather than the placeholder trusted router provider; leave references in comments for future addition --- pkg/pool-weighted/contracts/LBPool.sol | 25 +++++++++---------- pkg/pool-weighted/contracts/LBPoolFactory.sol | 12 +++------ pkg/pool-weighted/test/foundry/LBPool.t.sol | 2 +- 3 files changed, 17 insertions(+), 22 deletions(-) diff --git a/pkg/pool-weighted/contracts/LBPool.sol b/pkg/pool-weighted/contracts/LBPool.sol index b73018866..3f7c77c52 100644 --- a/pkg/pool-weighted/contracts/LBPool.sol +++ b/pkg/pool-weighted/contracts/LBPool.sol @@ -16,6 +16,7 @@ import { import { IRouterCommon } from "@balancer-labs/v3-interfaces/contracts/vault/IRouterCommon.sol"; import { IBasePoolFactory } from "@balancer-labs/v3-interfaces/contracts/vault/IBasePoolFactory.sol"; import { IVaultErrors } from "@balancer-labs/v3-interfaces/contracts/vault/IVaultErrors.sol"; +import { BaseHooks } from "@balancer-labs/v3-vault/contracts/BaseHooks.sol"; import { InputHelpers } from "@balancer-labs/v3-solidity-utils/contracts/helpers/InputHelpers.sol"; import { FixedPoint } from "@balancer-labs/v3-solidity-utils/contracts/math/FixedPoint.sol"; @@ -25,7 +26,7 @@ import { WeightValidation } from "./lib/WeightValidation.sol"; /// @notice Inheriting from WeightedPool is only slightly wasteful (setting 2 immutable weights /// and _totalTokens, which will not be used later), and it is tremendously helpful for pool /// validation and any potential future parent class changes. -contract LBPool is WeightedPool, Ownable { //TODO is BaseHooks +contract LBPool is WeightedPool, Ownable, BaseHooks { // Since we have max 2 tokens and the weights must sum to 1, we only need to store one weight struct PoolState { uint56 startTime; @@ -41,8 +42,7 @@ contract LBPool is WeightedPool, Ownable { //TODO is BaseHooks // `{start,end}Time` are `uint56`s. Ensure that no input time (passed as `uint256`) will overflow. uint256 private constant _MAX_TIME = type(uint56).max; - address internal immutable TRUSTED_ROUTERS_PROVIDER; - address internal immutable TRUSTED_ROUTER_TODO_DELETE_ME; + address internal immutable TRUSTED_ROUTER; event SwapEnabledSet(bool swapEnabled); event GradualWeightUpdateScheduled( @@ -60,8 +60,7 @@ contract LBPool is WeightedPool, Ownable { //TODO is BaseHooks IVault vault, address owner, bool swapEnabledOnStart, - address trustedRoutersProvider, - address trustedRouterTodoDeleteMe + address trustedRouter ) WeightedPool(params, vault) Ownable(owner) { // _NUM_TOKENS == 2 == params.normalizedWeights.length == params.numTokens // WeightedPool validates `numTokens == normalizedWeights.length` @@ -70,8 +69,7 @@ contract LBPool is WeightedPool, Ownable { //TODO is BaseHooks // _startGradualWeightChange validates weights // Provider address validation performed at the factory level - TRUSTED_ROUTERS_PROVIDER = trustedRoutersProvider; - TRUSTED_ROUTER_TODO_DELETE_ME = trustedRouterTodoDeleteMe; + TRUSTED_ROUTER = trustedRouter; uint256 currentTime = block.timestamp; _startGradualWeightChange( @@ -156,12 +154,12 @@ contract LBPool is WeightedPool, Ownable { //TODO is BaseHooks address pool, TokenConfig[] memory, LiquidityManagement calldata - ) external view onlyVault returns (bool) { + ) public view override onlyVault returns (bool) { return (pool == address(this) && IBasePoolFactory(factory).isPoolFromFactory(pool)); } // Return HookFlags struct that indicates which hooks this contract supports - function getHookFlags() public pure returns (HookFlags memory hookFlags) { + function getHookFlags() public pure override returns (HookFlags memory hookFlags) { // Support hooks before swap/join for swapEnabled/onlyOwner LP hookFlags.shouldCallBeforeSwap = true; hookFlags.shouldCallBeforeAddLiquidity = true; @@ -179,20 +177,21 @@ contract LBPool is WeightedPool, Ownable { //TODO is BaseHooks uint256, uint256[] memory, bytes memory - ) external view onlyVault returns (bool) { + ) public view override onlyVault returns (bool) { // TODO use TrustedRoutersProvider. Presumably something like this: // if (ITrustedRoutersProvider(TRUSTED_ROUTERS_PROVIDER).isTrusted(router)) { - if (router == TRUSTED_ROUTER_TODO_DELETE_ME) { + if (router == TRUSTED_ROUTER) { + //TODO: should hooks w/ failing checks revert or just return false? return IRouterCommon(router).getSender() == owner(); } - revert RouterNotTrusted(); //TODO: should hooks revert or just return false? + revert RouterNotTrusted(); } /** * @notice Called before a swap to let pool block swaps if not enabled. * @return success True if the pool has swaps enabled. */ - function onBeforeSwap(PoolSwapParams calldata, address) public virtual onlyVault returns (bool) { + function onBeforeSwap(PoolSwapParams calldata, address) public override view onlyVault returns (bool) { return _getPoolSwapEnabledState(); } diff --git a/pkg/pool-weighted/contracts/LBPoolFactory.sol b/pkg/pool-weighted/contracts/LBPoolFactory.sol index b7d8c302a..86049347d 100644 --- a/pkg/pool-weighted/contracts/LBPoolFactory.sol +++ b/pkg/pool-weighted/contracts/LBPoolFactory.sol @@ -22,22 +22,19 @@ contract LBPoolFactory is IPoolVersion, BasePoolFactory, Version { // solhint-disable not-rely-on-time string private _poolVersion; - address internal immutable TRUSTED_ROUTERS_PROVIDER; - address internal immutable TRUSTED_ROUTER_TODO_DELETE_ME; + address internal immutable TRUSTED_ROUTER; constructor( IVault vault, uint32 pauseWindowDuration, string memory factoryVersion, string memory poolVersion, - address trustedRoutersProvider, - address trustedRouterTodoDeleteMe + address trustedRouter ) BasePoolFactory(vault, pauseWindowDuration, type(LBPool).creationCode) Version(factoryVersion) { _poolVersion = poolVersion; // TODO: validate input address before storing - TRUSTED_ROUTERS_PROVIDER = trustedRoutersProvider; - TRUSTED_ROUTER_TODO_DELETE_ME = trustedRouterTodoDeleteMe; + TRUSTED_ROUTER = trustedRouter; } /// @inheritdoc IPoolVersion @@ -81,8 +78,7 @@ contract LBPoolFactory is IPoolVersion, BasePoolFactory, Version { getVault(), owner, swapEnabledOnStart, - TRUSTED_ROUTERS_PROVIDER, - TRUSTED_ROUTER_TODO_DELETE_ME + TRUSTED_ROUTER ), salt ); diff --git a/pkg/pool-weighted/test/foundry/LBPool.t.sol b/pkg/pool-weighted/test/foundry/LBPool.t.sol index 4feca4b10..3eb95ca69 100644 --- a/pkg/pool-weighted/test/foundry/LBPool.t.sol +++ b/pkg/pool-weighted/test/foundry/LBPool.t.sol @@ -58,7 +58,7 @@ contract LBPoolTest is BasePoolTest { tokenAmounts.push(TOKEN_AMOUNT); } - factory = new LBPoolFactory(IVault(address(vault)), 365 days, "Factory v1", "Pool v1", address(0), address(router)); + factory = new LBPoolFactory(IVault(address(vault)), 365 days, "Factory v1", "Pool v1", address(router)); weights = [uint256(50e16), uint256(50e16)].toMemoryArray(); // Allow pools created by `factory` to use poolHooksMock hooks From 9348242a378baeb18e59c2276c2af7164323555f Mon Sep 17 00:00:00 2001 From: gerg Date: Wed, 11 Sep 2024 13:59:35 -0400 Subject: [PATCH 21/54] make test fns virtual; make getSwapFeeAdmin internal --- pkg/vault/test/foundry/utils/BasePoolTest.sol | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/pkg/vault/test/foundry/utils/BasePoolTest.sol b/pkg/vault/test/foundry/utils/BasePoolTest.sol index 0b2d3f56b..db4e06752 100644 --- a/pkg/vault/test/foundry/utils/BasePoolTest.sol +++ b/pkg/vault/test/foundry/utils/BasePoolTest.sol @@ -53,7 +53,7 @@ abstract contract BasePoolTest is BaseVaultTest { poolMaxSwapFeePercentage = 1e18; } - function testPoolAddress() public view { + function testPoolAddress() public view virtual { address calculatedPoolAddress = factory.getDeploymentAddress(ZERO_BYTES32); assertEq(pool, calculatedPoolAddress, "Pool address mismatch"); } @@ -251,27 +251,15 @@ abstract contract BasePoolTest is BaseVaultTest { assertEq(IBasePool(pool).getMaximumSwapFeePercentage(), poolMaxSwapFeePercentage, "Maximum swap fee mismatch"); } - function getSwapFeeAdmin() public returns (address) { - address swapFeeManager; - PoolRoleAccounts memory roleAccounts = vault.getPoolRoleAccounts(pool); - if (roleAccounts.swapFeeManager != address(0)) { - return roleAccounts.swapFeeManager; - } else { - swapFeeManager = alice; - authorizer.grantRole(vault.getActionId(IVaultAdmin.setStaticSwapFeePercentage.selector), swapFeeManager); - return swapFeeManager; - } - } - function testSetSwapFeeTooLow() public virtual { - address swapFeeManager = getSwapFeeAdmin(); + address swapFeeManager = _getSwapFeeAdmin(); vm.prank(swapFeeManager); vm.expectRevert(IVaultErrors.SwapFeePercentageTooLow.selector); vault.setStaticSwapFeePercentage(pool, poolMinSwapFeePercentage - 1); } function testSetSwapFeeTooHigh() public virtual { - address swapFeeManager = getSwapFeeAdmin(); + address swapFeeManager = _getSwapFeeAdmin(); vm.prank(swapFeeManager); vm.expectRevert(abi.encodeWithSelector(IVaultErrors.SwapFeePercentageTooHigh.selector)); @@ -279,7 +267,7 @@ abstract contract BasePoolTest is BaseVaultTest { } function testAddLiquidityUnbalanced() public virtual { - address swapFeeManager = getSwapFeeAdmin(); + address swapFeeManager = _getSwapFeeAdmin(); vm.prank(swapFeeManager); vault.setStaticSwapFeePercentage(pool, 10e16); @@ -310,4 +298,16 @@ abstract contract BasePoolTest is BaseVaultTest { function _less(uint256 amount, uint256 base) internal pure returns (uint256) { return (amount * (base - 1)) / base; } + + function _getSwapFeeAdmin() internal returns (address) { + address swapFeeManager; + PoolRoleAccounts memory roleAccounts = vault.getPoolRoleAccounts(pool); + if (roleAccounts.swapFeeManager != address(0)) { + return roleAccounts.swapFeeManager; + } else { + swapFeeManager = alice; + authorizer.grantRole(vault.getActionId(IVaultAdmin.setStaticSwapFeePercentage.selector), swapFeeManager); + return swapFeeManager; + } + } } From 5a2be27feb17117547fcae6f567ab1941751bee9 Mon Sep 17 00:00:00 2001 From: gerg Date: Thu, 12 Sep 2024 11:46:45 -0400 Subject: [PATCH 22/54] add test for before/during/after gradual weight update --- pkg/pool-weighted/test/foundry/LBPool.t.sol | 91 +++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/pkg/pool-weighted/test/foundry/LBPool.t.sol b/pkg/pool-weighted/test/foundry/LBPool.t.sol index 3eb95ca69..ff9747b8c 100644 --- a/pkg/pool-weighted/test/foundry/LBPool.t.sol +++ b/pkg/pool-weighted/test/foundry/LBPool.t.sol @@ -358,4 +358,95 @@ contract LBPoolTest is BasePoolTest { ); } + function testQuerySwapDuringWeightUpdate() public { + // Cache original time to avoid issues from `block.timestamp` during `vm.warp` + uint256 blockDotTimestampTestStart = block.timestamp; + + uint256 testDuration = 1 days; + uint256 weightUpdateStep = 1 hours; + uint256 constantWeightDuration = 6 hours; + uint256 startTime = blockDotTimestampTestStart + constantWeightDuration; + + uint256[] memory endWeights = new uint256[](2); + endWeights[0] = 0.01e18; // 1% + endWeights[1] = 0.99e18; // 99% + + uint256 amountIn = TOKEN_AMOUNT / 10; + uint256 constantWeightSteps = constantWeightDuration / weightUpdateStep; + uint256 weightUpdateSteps = testDuration / weightUpdateStep; + + // Start the gradual weight update + vm.prank(bob); + LBPool(address(pool)).updateWeightsGradually(startTime, startTime + testDuration, endWeights); + + uint256 prevAmountOut; + uint256 amountOut; + + // Perform query swaps before the weight update starts + vm.warp(blockDotTimestampTestStart); + prevAmountOut = _executeAndUndoSwap(amountIn); + for (uint256 i = 1; i < constantWeightSteps; i++) { + uint256 currTime = blockDotTimestampTestStart + i * weightUpdateStep; + vm.warp(currTime); + amountOut = _executeAndUndoSwap(amountIn); + assertEq(amountOut, prevAmountOut, "Amount out should remain constant before weight update"); + prevAmountOut = amountOut; + } + + // Perform query swaps during the weight update + vm.warp(startTime); + prevAmountOut = _executeAndUndoSwap(amountIn); + for (uint256 i = 1; i <= weightUpdateSteps; i++) { + vm.warp(startTime + i * weightUpdateStep); + amountOut = _executeAndUndoSwap(amountIn); + assertTrue(amountOut > prevAmountOut, "Amount out should increase during weight update"); + prevAmountOut = amountOut; + } + + // Perform query swaps after the weight update ends + vm.warp(startTime + testDuration); + prevAmountOut = _executeAndUndoSwap(amountIn); + for (uint256 i = 1; i < constantWeightSteps; i++) { + vm.warp(startTime + testDuration + i * weightUpdateStep); + amountOut = _executeAndUndoSwap(amountIn); + assertEq(amountOut, prevAmountOut, "Amount out should remain constant after weight update"); + prevAmountOut = amountOut; + } + } + function _executeAndUndoSwap(uint256 amountIn) internal returns (uint256) { + // Create a storage checkpoint + uint256 snapshot = vm.snapshot(); + + try this.executeSwap(amountIn) returns (uint256 amountOut) { + // Revert to the snapshot to undo the swap + vm.revertTo(snapshot); + return amountOut; + } catch Error(string memory reason) { + vm.revertTo(snapshot); + revert(reason); + } catch { + vm.revertTo(snapshot); + revert("Low level error during swap"); + } + } + + function executeSwap(uint256 amountIn) external returns (uint256) { + // Ensure this contract has enough tokens and allowance + deal(address(dai), address(bob), amountIn); + vm.prank(bob); + IERC20(dai).approve(address(router), amountIn); + + // Perform the actual swap + vm.prank(bob); + return router.swapSingleTokenExactIn( + address(pool), + IERC20(dai), + IERC20(usdc), + amountIn, + 0, // minAmountOut: Set to 0 or a minimum amount if desired + block.timestamp, // deadline = now to ensure it won't timeout + false, // wethIsEth: Set to false assuming DAI and USDC are not ETH + "" // userData: Empty bytes as no additional data is needed + ); + } } \ No newline at end of file From 3bedcd8ad17f7eefd847e134bb787b93db110d37 Mon Sep 17 00:00:00 2001 From: gerg Date: Thu, 12 Sep 2024 11:47:51 -0400 Subject: [PATCH 23/54] lint --- pkg/pool-weighted/contracts/LBPool.sol | 2 +- .../test/foundry/GradualValueChange.t.sol | 38 ++++++++++--------- pkg/pool-weighted/test/foundry/LBPool.t.sol | 26 +++++++------ .../test/foundry/WeightValidationTest.t.sol | 6 +-- 4 files changed, 39 insertions(+), 33 deletions(-) diff --git a/pkg/pool-weighted/contracts/LBPool.sol b/pkg/pool-weighted/contracts/LBPool.sol index 3f7c77c52..5e3f3e5d4 100644 --- a/pkg/pool-weighted/contracts/LBPool.sol +++ b/pkg/pool-weighted/contracts/LBPool.sol @@ -191,7 +191,7 @@ contract LBPool is WeightedPool, Ownable, BaseHooks { * @notice Called before a swap to let pool block swaps if not enabled. * @return success True if the pool has swaps enabled. */ - function onBeforeSwap(PoolSwapParams calldata, address) public override view onlyVault returns (bool) { + function onBeforeSwap(PoolSwapParams calldata, address) public view override onlyVault returns (bool) { return _getPoolSwapEnabledState(); } diff --git a/pkg/pool-weighted/test/foundry/GradualValueChange.t.sol b/pkg/pool-weighted/test/foundry/GradualValueChange.t.sol index fa6b1c786..e3f3e22a5 100644 --- a/pkg/pool-weighted/test/foundry/GradualValueChange.t.sol +++ b/pkg/pool-weighted/test/foundry/GradualValueChange.t.sol @@ -23,11 +23,7 @@ contract GradualValueChangeWrapper { GradualValueChange.ensureNoTimeOverflow(time, maxTime); } - function interpolateValue( - uint256 startValue, - uint256 endValue, - uint256 pctProgress - ) public pure returns (uint256) { + function interpolateValue(uint256 startValue, uint256 endValue, uint256 pctProgress) public pure returns (uint256) { return GradualValueChange.interpolateValue(startValue, endValue, pctProgress); } @@ -52,9 +48,9 @@ contract GradualValueChangeTest is Test { uint256 steps = 100; for (uint256 i = 0; i <= steps; i++) { - uint256 currentTime = startTime + (i * (endTime - startTime) / steps); + uint256 currentTime = startTime + ((i * (endTime - startTime)) / steps); vm.warp(currentTime); - uint256 expectedValue = startValue + (i * (endValue - startValue) / steps); + uint256 expectedValue = startValue + ((i * (endValue - startValue)) / steps); uint256 actualValue = wrapper.getInterpolatedValue(startValue, endValue, startTime, endTime); assertEq(actualValue, expectedValue, "Interpolated value should match expected"); } @@ -66,7 +62,11 @@ contract GradualValueChangeTest is Test { vm.warp(currentTime); assertEq(wrapper.resolveStartTime(futureTime, futureTime + 100), futureTime, "Should return future start time"); - assertEq(wrapper.resolveStartTime(currentTime - 100, futureTime), currentTime, "Should return current time for past start time"); + assertEq( + wrapper.resolveStartTime(currentTime - 100, futureTime), + currentTime, + "Should return current time for past start time" + ); vm.expectRevert(GradualValueChange.GradualUpdateTimeTravel.selector); wrapper.resolveStartTime(futureTime + 200, futureTime + 100); @@ -74,10 +74,10 @@ contract GradualValueChangeTest is Test { function testEnsureNoTimeOverflow() public { uint256 maxTime = 1000; - + wrapper.ensureNoTimeOverflow(maxTime, maxTime); wrapper.ensureNoTimeOverflow(maxTime - 1, maxTime); - + vm.expectRevert(GradualValueChange.TimeTruncatedInStorage.selector); wrapper.ensureNoTimeOverflow(maxTime + 1, maxTime); } @@ -89,7 +89,7 @@ contract GradualValueChangeTest is Test { for (uint256 i = 0; i <= steps; i++) { uint256 pctProgress = (i * FP_ONE) / steps; - uint256 expectedValue = startValue + (i * (endValue - startValue) / steps); + uint256 expectedValue = startValue + ((i * (endValue - startValue)) / steps); uint256 actualValue = wrapper.interpolateValue(startValue, endValue, pctProgress); assertEq(actualValue, expectedValue, "Interpolated value should match expected"); } @@ -100,7 +100,7 @@ contract GradualValueChangeTest is Test { for (uint256 i = 0; i <= steps; i++) { uint256 pctProgress = (i * FP_ONE) / steps; - uint256 expectedValue = startValue - (i * (startValue - endValue) / steps); + uint256 expectedValue = startValue - ((i * (startValue - endValue)) / steps); uint256 actualValue = wrapper.interpolateValue(startValue, endValue, pctProgress); assertEq(actualValue, expectedValue, "Interpolated value should match expected for decreasing value"); } @@ -112,7 +112,7 @@ contract GradualValueChangeTest is Test { uint256 steps = 100; for (uint256 i = 0; i <= steps; i++) { - uint256 currentTime = startTime + (i * (endTime - startTime) / steps); + uint256 currentTime = startTime + ((i * (endTime - startTime)) / steps); vm.warp(currentTime); uint256 expectedProgress = (i * FP_ONE) / steps; uint256 actualProgress = wrapper.calculateValueChangeProgress(startTime, endTime); @@ -133,7 +133,11 @@ contract GradualValueChangeTest is Test { for (uint256 i = 0; i <= 100; i++) { vm.warp(startTime + i); - assertEq(wrapper.getInterpolatedValue(startValue, startValue, startTime, endTime), startValue, "Should always return the same value"); + assertEq( + wrapper.getInterpolatedValue(startValue, startValue, startTime, endTime), + startValue, + "Should always return the same value" + ); } // Test before start time @@ -174,7 +178,7 @@ contract GradualValueChangeTest is Test { uint256 currentTime = (bigVal / steps) * i; vm.warp(currentTime); uint256 expectedValue = (bigVal / steps) * i; - + uint256 actualValue; try wrapper.getInterpolatedValue(0, bigVal, 0, bigVal) returns (uint256 value) { actualValue = value; @@ -184,10 +188,10 @@ contract GradualValueChangeTest is Test { } catch (bytes memory /*lowLevelData*/) { revert("getInterpolatedValue reverted unexpectedly"); } - + // Additional check to ensure the value is within the expected range assertGe(actualValue, 0, "Value should not be less than 0"); assertLe(actualValue, bigVal, "Value should not exceed bigVal"); } } -} \ No newline at end of file +} diff --git a/pkg/pool-weighted/test/foundry/LBPool.t.sol b/pkg/pool-weighted/test/foundry/LBPool.t.sol index ff9747b8c..1e3109bbc 100644 --- a/pkg/pool-weighted/test/foundry/LBPool.t.sol +++ b/pkg/pool-weighted/test/foundry/LBPool.t.sol @@ -358,7 +358,7 @@ contract LBPoolTest is BasePoolTest { ); } - function testQuerySwapDuringWeightUpdate() public { + function testQuerySwapDuringWeightUpdate() public { // Cache original time to avoid issues from `block.timestamp` during `vm.warp` uint256 blockDotTimestampTestStart = block.timestamp; @@ -413,6 +413,7 @@ contract LBPoolTest is BasePoolTest { prevAmountOut = amountOut; } } + function _executeAndUndoSwap(uint256 amountIn) internal returns (uint256) { // Create a storage checkpoint uint256 snapshot = vm.snapshot(); @@ -438,15 +439,16 @@ contract LBPoolTest is BasePoolTest { // Perform the actual swap vm.prank(bob); - return router.swapSingleTokenExactIn( - address(pool), - IERC20(dai), - IERC20(usdc), - amountIn, - 0, // minAmountOut: Set to 0 or a minimum amount if desired - block.timestamp, // deadline = now to ensure it won't timeout - false, // wethIsEth: Set to false assuming DAI and USDC are not ETH - "" // userData: Empty bytes as no additional data is needed - ); + return + router.swapSingleTokenExactIn( + address(pool), + IERC20(dai), + IERC20(usdc), + amountIn, + 0, // minAmountOut: Set to 0 or a minimum amount if desired + block.timestamp, // deadline = now to ensure it won't timeout + false, // wethIsEth: Set to false assuming DAI and USDC are not ETH + "" // userData: Empty bytes as no additional data is needed + ); } -} \ No newline at end of file +} diff --git a/pkg/pool-weighted/test/foundry/WeightValidationTest.t.sol b/pkg/pool-weighted/test/foundry/WeightValidationTest.t.sol index 12b97bf4b..e8592c545 100644 --- a/pkg/pool-weighted/test/foundry/WeightValidationTest.t.sol +++ b/pkg/pool-weighted/test/foundry/WeightValidationTest.t.sol @@ -28,14 +28,14 @@ contract WeightValidationTest is Test { for (uint256 numTokens = 2; numTokens <= 8; numTokens++) { uint256[] memory weights = new uint256[](numTokens); uint256 remainingWeight = FP_ONE; - + for (uint256 i = 0; i < numTokens - 1; i++) { uint256 weight = (remainingWeight / (numTokens - i)) + 1e16; // Ensure it's above MIN_WEIGHT weights[i] = weight; remainingWeight -= weight; } weights[numTokens - 1] = remainingWeight; - + wrapper.validateWeights(weights, numTokens); } } @@ -106,4 +106,4 @@ contract WeightValidationTest is Test { wrapper.validateTwoWeights(FP_ONE - MIN_WEIGHT, MIN_WEIGHT); wrapper.validateTwoWeights(FP_ONE / 2, FP_ONE / 2); } -} \ No newline at end of file +} From 366aefc4a8c08da13c3a7481577df37a8b6fd2ce Mon Sep 17 00:00:00 2001 From: gerg Date: Thu, 19 Sep 2024 12:55:08 -0400 Subject: [PATCH 24/54] apply updates from review by @EndymionJkb --- pkg/pool-weighted/contracts/LBPool.sol | 268 ++++++++++-------- pkg/pool-weighted/contracts/LBPoolFactory.sol | 30 +- pkg/pool-weighted/contracts/WeightedPool.sol | 2 +- .../contracts/WeightedPoolFactory.sol | 2 - .../contracts/lib/GradualValueChange.sol | 48 ++-- .../contracts/lib/WeightValidation.sol | 64 ----- .../contracts/test/GradualValueChangeMock.sol | 28 ++ pkg/pool-weighted/test/LBPool.test.ts | 197 ++++++------- .../test/foundry/GradualValueChange.t.sol | 77 ++--- pkg/pool-weighted/test/foundry/LBPool.t.sol | 25 +- .../test/foundry/WeightValidationTest.t.sol | 109 ------- .../contracts/math/FixedPoint.sol | 11 - .../contracts/math/WeightedMath.sol | 17 +- pkg/vault/contracts/VaultFactory.sol | 88 ++---- pkg/vault/test/foundry/VaultFactory.t.sol | 92 +----- pkg/vault/test/foundry/utils/BasePoolTest.sol | 18 +- 16 files changed, 394 insertions(+), 682 deletions(-) delete mode 100644 pkg/pool-weighted/contracts/lib/WeightValidation.sol create mode 100644 pkg/pool-weighted/contracts/test/GradualValueChangeMock.sol delete mode 100644 pkg/pool-weighted/test/foundry/WeightValidationTest.t.sol diff --git a/pkg/pool-weighted/contracts/LBPool.sol b/pkg/pool-weighted/contracts/LBPool.sol index 5e3f3e5d4..ec3780767 100644 --- a/pkg/pool-weighted/contracts/LBPool.sol +++ b/pkg/pool-weighted/contracts/LBPool.sol @@ -2,49 +2,70 @@ pragma solidity ^0.8.24; -import { WeightedPool } from "./WeightedPool.sol"; +import { SafeCast } from "@openzeppelin/contracts/utils/math/SafeCast.sol"; import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; -import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol"; -import { - HookFlags, - TokenConfig, - AddLiquidityKind, - LiquidityManagement, - PoolSwapParams -} from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; -import { IRouterCommon } from "@balancer-labs/v3-interfaces/contracts/vault/IRouterCommon.sol"; import { IBasePoolFactory } from "@balancer-labs/v3-interfaces/contracts/vault/IBasePoolFactory.sol"; +import { IRouterCommon } from "@balancer-labs/v3-interfaces/contracts/vault/IRouterCommon.sol"; import { IVaultErrors } from "@balancer-labs/v3-interfaces/contracts/vault/IVaultErrors.sol"; -import { BaseHooks } from "@balancer-labs/v3-vault/contracts/BaseHooks.sol"; +import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol"; +import "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; import { InputHelpers } from "@balancer-labs/v3-solidity-utils/contracts/helpers/InputHelpers.sol"; import { FixedPoint } from "@balancer-labs/v3-solidity-utils/contracts/math/FixedPoint.sol"; +import { BaseHooks } from "@balancer-labs/v3-vault/contracts/BaseHooks.sol"; + import { GradualValueChange } from "./lib/GradualValueChange.sol"; -import { WeightValidation } from "./lib/WeightValidation.sol"; +import { WeightedPool } from "./WeightedPool.sol"; -/// @notice Inheriting from WeightedPool is only slightly wasteful (setting 2 immutable weights -/// and _totalTokens, which will not be used later), and it is tremendously helpful for pool -/// validation and any potential future parent class changes. +/** + * @notice Weighted Pool with mutable weights, designed to support v3 Liquidity Bootstrapping. + * @dev Inheriting from WeightedPool is only slightly wasteful (setting 2 immutable weights and `_totalTokens`, + * which will not be used later), and it is tremendously helpful for pool validation and any potential future + * base contract changes. + */ contract LBPool is WeightedPool, Ownable, BaseHooks { - // Since we have max 2 tokens and the weights must sum to 1, we only need to store one weight + using SafeCast for *; + + // Since we have max 2 tokens and the weights must sum to 1, we only need to store one weight. + // Weights are 18 decimal floating point values, which fit in less than 64 bits. Store smaller numeric values + // to ensure the PoolState fits in a single slot. All timestamps in the system are uint32, enforced through + // SafeCast. struct PoolState { - uint56 startTime; - uint56 endTime; + uint32 startTime; + uint32 endTime; uint64 startWeight0; uint64 endWeight0; bool swapEnabled; } - PoolState private _poolState; + // LBPs are constrained to two tokens.auto uint256 private constant _NUM_TOKENS = 2; - // `{start,end}Time` are `uint56`s. Ensure that no input time (passed as `uint256`) will overflow. - uint256 private constant _MAX_TIME = type(uint56).max; + // LBPools are deployed with the Balancer standard router address, which we know reliably reports the true + // originating account on operations. This is important for liquidity operations, as these are permissioned + // operations that can only be performed by the owner of the pool. Without this check, a malicious router + // could spoof the address of the owner, allowing anyone to call permissioned functions. - address internal immutable TRUSTED_ROUTER; + // solhint-disable-next-line var-name-mixedcase + address private immutable _TRUSTED_ROUTER; + PoolState private _poolState; + + /** + * @notice Emitted when the owner enables or disables swaps. + * @param swapEnabled True if we are enabling swaps + */ event SwapEnabledSet(bool swapEnabled); + + /** + * @notice Emitted when the owner initiates a gradual weight change (e.g., at the start of the sale). + * @dev Also emitted on deployment, recording the initial state. + * @param startTime The starting timestamp of the update + * @param endTime The ending timestamp of the update + * @param startWeights The weights at the start of the update + * @param endWeights The final weights after the update is completed + */ event GradualWeightUpdateScheduled( uint256 startTime, uint256 endTime, @@ -52,7 +73,7 @@ contract LBPool is WeightedPool, Ownable, BaseHooks { uint256[] endWeights ); - /// @dev Indicates that the router that called the Vault is not trusted and should be ignored. + /// @dev Indicates that the router that called the Vault is not trusted, so any operations should revert. error RouterNotTrusted(); constructor( @@ -62,46 +83,25 @@ contract LBPool is WeightedPool, Ownable, BaseHooks { bool swapEnabledOnStart, address trustedRouter ) WeightedPool(params, vault) Ownable(owner) { - // _NUM_TOKENS == 2 == params.normalizedWeights.length == params.numTokens - // WeightedPool validates `numTokens == normalizedWeights.length` + // WeightedPool validates `numTokens == normalizedWeights.length`, and ensures valid weights. + // Here we additionally enforce that LBPs must be two-token pools. InputHelpers.ensureInputLengthMatch(_NUM_TOKENS, params.numTokens); - // _startGradualWeightChange validates weights - - // Provider address validation performed at the factory level - TRUSTED_ROUTER = trustedRouter; + // Set the trusted router (passed down from the factory). + _TRUSTED_ROUTER = trustedRouter; - uint256 currentTime = block.timestamp; - _startGradualWeightChange( - uint56(currentTime), - uint56(currentTime), - params.normalizedWeights, - params.normalizedWeights, - true, - swapEnabledOnStart - ); - } - - function updateWeightsGradually( - uint256 startTime, - uint256 endTime, - uint256[] memory endWeights - ) external onlyOwner { - InputHelpers.ensureInputLengthMatch(_NUM_TOKENS, endWeights.length); - WeightValidation.validateTwoWeights(endWeights[0], endWeights[1]); - - // Ensure startTime >= now - startTime = GradualValueChange.resolveStartTime(startTime, endTime); - - // Ensure time will not overflow in storage; only check end (startTime <= endTime) - GradualValueChange.ensureNoTimeOverflow(endTime, _MAX_TIME); - - _startGradualWeightChange(uint56(startTime), uint56(endTime), _getNormalizedWeights(), endWeights, false, true); + // solhint-disable-next-line not-rely-on-time + uint32 currentTime = block.timestamp.toUint32(); + _startGradualWeightChange(currentTime, currentTime, params.normalizedWeights, params.normalizedWeights); + _setSwapEnabled(swapEnabledOnStart); } /** - * @dev Return start time, end time, and endWeights as an array. - * Current weights should be retrieved via `getNormalizedWeights()`. + * @notice Return start time, end time, and endWeights as an array. + * @dev Current weights should be retrieved via `getNormalizedWeights()`. + * @return startTime The starting timestamp of any ongoing weight change + * @return endTime The ending timestamp of any ongoing weight change + * @return endWeights The "destination" weights, sorted in token registration order */ function getGradualWeightUpdateParams() external @@ -118,56 +118,93 @@ contract LBPool is WeightedPool, Ownable, BaseHooks { endWeights[1] = FixedPoint.ONE - poolState.endWeight0; } + /** + * @notice Indicate whether swaps are enabled or not for the given pool. + * @return swapEnabled True if trading is enabled + */ + function getSwapEnabled() external view returns (bool) { + return _getPoolSwapEnabled(); + } + + /******************************************************************************* + Permissioned Functions + *******************************************************************************/ + /** * @notice Enable/disable trading. + * @dev This is a permissioned function that can only be called by the owner. + * @param swapEnabled True if trading should be enabled */ function setSwapEnabled(bool swapEnabled) external onlyOwner { - _poolState.swapEnabled = swapEnabled; - emit SwapEnabledSet(swapEnabled); + _setSwapEnabled(swapEnabled); } /** - * @notice Return whether swaps are enabled or not for the given pool. + * @notice Start a gradual weight change. Weights will change smoothly from current values to `endWeights`. + * @dev This is a permissioned function that can only be called by the owner. + * If the `startTime` is in the past, the weight change will begin immediately. + * + * @param startTime The timestamp when the weight change will start + * @param endTime The timestamp when the weights will reach their final values + * @param endWeights The final values of the weights */ - function getSwapEnabled() external view returns (bool) { - return _getPoolSwapEnabledState(); + function updateWeightsGradually( + uint256 startTime, + uint256 endTime, + uint256[] memory endWeights + ) external onlyOwner { + InputHelpers.ensureInputLengthMatch(_NUM_TOKENS, endWeights.length); + + if (endWeights[0] < _MIN_WEIGHT || endWeights[1] < _MIN_WEIGHT) { + revert MinWeight(); + } + if (endWeights[0] + endWeights[1] != FixedPoint.ONE) { + revert NormalizedWeightInvariant(); + } + + // Ensure startTime >= now. + startTime = GradualValueChange.resolveStartTime(startTime, endTime); + + // The SafeCast ensures `endTime` can't overflow. + _startGradualWeightChange(startTime.toUint32(), endTime.toUint32(), _getNormalizedWeights(), endWeights); } - /* ========================================= - * ========================================= - * ============HOOK FUNCTIONS=============== - * ========================================= - * ========================================= - */ + /******************************************************************************* + Hook Functions + *******************************************************************************/ /** - * @notice Hook to be executed when pool is registered. Returns true if registration was successful, and false to - * revert the registration of the pool. Make sure this function is properly implemented (e.g. check the factory, - * and check that the given pool is from the factory). - * - * @param factory Address of the pool factory + * @notice Hook to be executed when pool is registered. + * @dev Returns true if registration was successful, and false to revert the registration of the pool. * @param pool Address of the pool * @return success True if the hook allowed the registration, false otherwise */ function onRegister( - address factory, + address, address pool, TokenConfig[] memory, LiquidityManagement calldata ) public view override onlyVault returns (bool) { - return (pool == address(this) && IBasePoolFactory(factory).isPoolFromFactory(pool)); + // Since in this case the pool is the hook, we don't need to check anything else. + // We *could* check that it's two tokens, but better to let that be caught later, as it will fail with a more + // descriptive error. + return pool == address(this); } // Return HookFlags struct that indicates which hooks this contract supports function getHookFlags() public pure override returns (HookFlags memory hookFlags) { - // Support hooks before swap/join for swapEnabled/onlyOwner LP + // Check whether swaps are enabled in `onBeforeSwap`. hookFlags.shouldCallBeforeSwap = true; + // Ensure the caller is the owner, as only the owner can add liquidity. hookFlags.shouldCallBeforeAddLiquidity = true; } /** * @notice Check that the caller who initiated the add liquidity operation is the owner. - * @param router The address (usually a router contract) that initiated add liquidity operation on the Vault + * @dev We first ensure the caller is the standard router, so that we know we can trust the value it returns + * from `getSender`. + * + * @param router The address (usually a router contract) that initiated the add liquidity operation */ function onBeforeAddLiquidity( address router, @@ -178,43 +215,31 @@ contract LBPool is WeightedPool, Ownable, BaseHooks { uint256[] memory, bytes memory ) public view override onlyVault returns (bool) { - // TODO use TrustedRoutersProvider. Presumably something like this: - // if (ITrustedRoutersProvider(TRUSTED_ROUTERS_PROVIDER).isTrusted(router)) { - if (router == TRUSTED_ROUTER) { - //TODO: should hooks w/ failing checks revert or just return false? + if (router == _TRUSTED_ROUTER) { return IRouterCommon(router).getSender() == owner(); } + revert RouterNotTrusted(); } /** - * @notice Called before a swap to let pool block swaps if not enabled. + * @notice Called before a swap to let the pool block swaps if not enabled. * @return success True if the pool has swaps enabled. */ function onBeforeSwap(PoolSwapParams calldata, address) public view override onlyVault returns (bool) { - return _getPoolSwapEnabledState(); + return _getPoolSwapEnabled(); } - /* ========================================= - * ========================================= - * ==========INTERNAL FUNCTIONS============= - * ========================================= - * ========================================= - */ - - function _getNormalizedWeight0() internal view virtual returns (uint256) { - PoolState memory poolState = _poolState; - uint256 pctProgress = _getWeightChangeProgress(poolState); - return GradualValueChange.interpolateValue(poolState.startWeight0, poolState.endWeight0, pctProgress); - } + /******************************************************************************* + Internal Functions + *******************************************************************************/ + // This is unused in this contract, but must be overridden from WeightedPool for consistency. function _getNormalizedWeight(uint256 tokenIndex) internal view virtual override returns (uint256) { - uint256 normalizedWeight0 = _getNormalizedWeight0(); - if (tokenIndex == 0) { - return normalizedWeight0; - } else if (tokenIndex == 1) { - return FixedPoint.ONE - normalizedWeight0; + if (tokenIndex < _NUM_TOKENS) { + return _getNormalizedWeights()[tokenIndex]; } + revert IVaultErrors.InvalidToken(); } @@ -222,11 +247,25 @@ contract LBPool is WeightedPool, Ownable, BaseHooks { uint256[] memory normalizedWeights = new uint256[](_NUM_TOKENS); normalizedWeights[0] = _getNormalizedWeight0(); normalizedWeights[1] = FixedPoint.ONE - normalizedWeights[0]; + return normalizedWeights; } - function _getWeightChangeProgress(PoolState memory poolState) internal view returns (uint256) { - return GradualValueChange.calculateValueChangeProgress(poolState.startTime, poolState.endTime); + function _getNormalizedWeight0() internal view virtual returns (uint256) { + PoolState memory poolState = _poolState; + uint256 pctProgress = GradualValueChange.calculateValueChangeProgress(poolState.startTime, poolState.endTime); + + return GradualValueChange.interpolateValue(poolState.startWeight0, poolState.endWeight0, pctProgress); + } + + function _getPoolSwapEnabled() private view returns (bool) { + return _poolState.swapEnabled; + } + + function _setSwapEnabled(bool swapEnabled) private { + _poolState.swapEnabled = swapEnabled; + + emit SwapEnabledSet(swapEnabled); } /** @@ -234,31 +273,22 @@ contract LBPool is WeightedPool, Ownable, BaseHooks { * if necessary. */ function _startGradualWeightChange( - uint56 startTime, - uint56 endTime, + uint32 startTime, + uint32 endTime, uint256[] memory startWeights, - uint256[] memory endWeights, - bool modifySwapEnabledStatus, - bool newSwapEnabled + uint256[] memory endWeights ) internal virtual { - WeightValidation.validateTwoWeights(endWeights[0], endWeights[1]); - PoolState memory poolState = _poolState; + poolState.startTime = startTime; poolState.endTime = endTime; - poolState.startWeight0 = uint64(startWeights[0]); - poolState.endWeight0 = uint64(endWeights[0]); - if (modifySwapEnabledStatus) { - poolState.swapEnabled = newSwapEnabled; - emit SwapEnabledSet(newSwapEnabled); - } + // These have been validated, but SafeCast anyway out of an abundance of caution. + poolState.startWeight0 = startWeights[0].toUint64(); + poolState.endWeight0 = endWeights[0].toUint64(); _poolState = poolState; - emit GradualWeightUpdateScheduled(startTime, endTime, startWeights, endWeights); - } - function _getPoolSwapEnabledState() internal view returns (bool) { - return _poolState.swapEnabled; + emit GradualWeightUpdateScheduled(startTime, endTime, startWeights, endWeights); } } diff --git a/pkg/pool-weighted/contracts/LBPoolFactory.sol b/pkg/pool-weighted/contracts/LBPoolFactory.sol index 86049347d..4faab6476 100644 --- a/pkg/pool-weighted/contracts/LBPoolFactory.sol +++ b/pkg/pool-weighted/contracts/LBPoolFactory.sol @@ -2,27 +2,25 @@ pragma solidity ^0.8.24; -import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; - -import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol"; import { IPoolVersion } from "@balancer-labs/v3-interfaces/contracts/solidity-utils/helpers/IPoolVersion.sol"; -import "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; +import { TokenConfig, PoolRoleAccounts } from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; +import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol"; import { BasePoolFactory } from "@balancer-labs/v3-pool-utils/contracts/BasePoolFactory.sol"; import { Version } from "@balancer-labs/v3-solidity-utils/contracts/helpers/Version.sol"; -import { LBPool } from "./LBPool.sol"; import { WeightedPool } from "./WeightedPool.sol"; +import { LBPool } from "./LBPool.sol"; /** - * @notice LBPool Factory - * @dev This is a factory specific to LBPools, allowing only 2 tokens + * @notice LBPool Factory. + * @dev This is a factory specific to LBPools, allowing only 2 tokens. */ contract LBPoolFactory is IPoolVersion, BasePoolFactory, Version { - // solhint-disable not-rely-on-time - string private _poolVersion; - address internal immutable TRUSTED_ROUTER; + + // solhint-disable-next-line var-name-mixedcase + address internal immutable _TRUSTED_ROUTER; constructor( IVault vault, @@ -33,8 +31,8 @@ contract LBPoolFactory is IPoolVersion, BasePoolFactory, Version { ) BasePoolFactory(vault, pauseWindowDuration, type(LBPool).creationCode) Version(factoryVersion) { _poolVersion = poolVersion; - // TODO: validate input address before storing - TRUSTED_ROUTER = trustedRouter; + // LBPools are deployed with a router known to reliably report the originating address on operations. + _TRUSTED_ROUTER = trustedRouter; } /// @inheritdoc IPoolVersion @@ -64,6 +62,8 @@ contract LBPoolFactory is IPoolVersion, BasePoolFactory, Version { bytes32 salt ) external returns (address pool) { PoolRoleAccounts memory roleAccounts; + // It's not necessary to set the pauseManager, as the owner can already effectively pause the pool + // by disabling swaps. roleAccounts.swapFeeManager = owner; pool = _create( @@ -78,7 +78,7 @@ contract LBPoolFactory is IPoolVersion, BasePoolFactory, Version { getVault(), owner, swapEnabledOnStart, - TRUSTED_ROUTER + _TRUSTED_ROUTER ), salt ); @@ -87,9 +87,9 @@ contract LBPoolFactory is IPoolVersion, BasePoolFactory, Version { pool, tokens, swapFeePercentage, - true, //protocol fee exempt + true, // protocol fee exempt roleAccounts, - pool, + pool, // register the pool itself as the hook contract getDefaultLiquidityManagement() ); } diff --git a/pkg/pool-weighted/contracts/WeightedPool.sol b/pkg/pool-weighted/contracts/WeightedPool.sol index 6e3e234bd..45bf6e48c 100644 --- a/pkg/pool-weighted/contracts/WeightedPool.sol +++ b/pkg/pool-weighted/contracts/WeightedPool.sol @@ -43,7 +43,7 @@ contract WeightedPool is IWeightedPool, BalancerPoolToken, PoolInfo, Version { // A minimum normalized weight imposes a maximum weight ratio. We need this due to limitations in the // implementation of the fixed point power function, as these ratios are often exponents. - uint256 private constant _MIN_WEIGHT = 1e16; // 1% + uint256 internal constant _MIN_WEIGHT = 1e16; // 1% uint256 private immutable _totalTokens; diff --git a/pkg/pool-weighted/contracts/WeightedPoolFactory.sol b/pkg/pool-weighted/contracts/WeightedPoolFactory.sol index 713be1454..3867e6812 100644 --- a/pkg/pool-weighted/contracts/WeightedPoolFactory.sol +++ b/pkg/pool-weighted/contracts/WeightedPoolFactory.sol @@ -20,8 +20,6 @@ import { WeightedPool } from "./WeightedPool.sol"; * @dev This is the most general factory, which allows up to eight tokens and arbitrary weights. */ contract WeightedPoolFactory is IPoolVersion, BasePoolFactory, Version { - // solhint-disable not-rely-on-time - string private _poolVersion; constructor( diff --git a/pkg/pool-weighted/contracts/lib/GradualValueChange.sol b/pkg/pool-weighted/contracts/lib/GradualValueChange.sol index 0e243b858..04b5e9228 100644 --- a/pkg/pool-weighted/contracts/lib/GradualValueChange.sol +++ b/pkg/pool-weighted/contracts/lib/GradualValueChange.sol @@ -13,6 +13,7 @@ // along with this program. If not, see . import { FixedPoint } from "@balancer-labs/v3-solidity-utils/contracts/math/FixedPoint.sol"; +import { Math } from "@openzeppelin/contracts/utils/math/Math.sol"; pragma solidity ^0.8.24; @@ -22,9 +23,6 @@ library GradualValueChange { /// @dev Indicates that the start time is after the end time error GradualUpdateTimeTravel(); - /// @dev Indicates that an input time is larger than the maximum storage value. - error TimeTruncatedInStorage(); - using FixedPoint for uint256; function getInterpolatedValue( @@ -42,33 +40,34 @@ library GradualValueChange { // If the start time is in the past, "fast forward" to start now // This avoids discontinuities in the value curve. Otherwise, if you set the start/end times with // only 10% of the period in the future, the value would immediately jump 90% - resolvedStartTime = FixedPoint.max(block.timestamp, startTime); + resolvedStartTime = Math.max(block.timestamp, startTime); if (resolvedStartTime > endTime) { revert GradualUpdateTimeTravel(); } } - function ensureNoTimeOverflow(uint256 time, uint256 maxTime) internal pure { - if (time > maxTime) { - revert TimeTruncatedInStorage(); - } - } - function interpolateValue( uint256 startValue, uint256 endValue, uint256 pctProgress ) internal pure returns (uint256) { - if (pctProgress >= FixedPoint.ONE || startValue == endValue) return endValue; - if (pctProgress == 0) return startValue; - - if (startValue > endValue) { - uint256 delta = pctProgress.mulDown(startValue - endValue); - return startValue - delta; - } else { - uint256 delta = pctProgress.mulDown(endValue - startValue); - return startValue + delta; + if (pctProgress >= FixedPoint.ONE || startValue == endValue) { + return endValue; + } + + if (pctProgress == 0) { + return startValue; + } + + unchecked { + if (startValue > endValue) { + uint256 delta = pctProgress.mulDown(startValue - endValue); + return startValue - delta; + } else { + uint256 delta = pctProgress.mulDown(endValue - startValue); + return startValue + delta; + } } } @@ -83,9 +82,14 @@ library GradualValueChange { return 0; } - // No need for SafeMath as it was checked right above: endTime > block.timestamp > startTime - uint256 totalSeconds = endTime - startTime; - uint256 secondsElapsed = block.timestamp - startTime; + // No need for checked math as the magnitudes are verified above: endTime > block.timestamp > startTime + uint256 totalSeconds; + uint256 secondsElapsed; + + unchecked { + totalSeconds = endTime - startTime; + secondsElapsed = block.timestamp - startTime; + } // We don't need to consider zero division here as this is covered above. return secondsElapsed.divDown(totalSeconds); diff --git a/pkg/pool-weighted/contracts/lib/WeightValidation.sol b/pkg/pool-weighted/contracts/lib/WeightValidation.sol deleted file mode 100644 index 7cc68ddea..000000000 --- a/pkg/pool-weighted/contracts/lib/WeightValidation.sol +++ /dev/null @@ -1,64 +0,0 @@ -// SPDX-License-Identifier: GPL-3.0-or-later -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -import { FixedPoint } from "@balancer-labs/v3-solidity-utils/contracts/math/FixedPoint.sol"; - -pragma solidity ^0.8.24; - -library WeightValidation { - /// @dev Indicates that one of the pool tokens' weight is below the minimum allowed. - error MinWeight(); - - /// @dev Indicates that the sum of the pool tokens' weights is not FP 1. - error NormalizedWeightInvariant(); - - uint256 internal constant _MIN_WEIGHT = 1e16; //1% - - /** - * @dev Returns a fixed-point number representing how far along the current value change is, where 0 means the - * change has not yet started, and FixedPoint.ONE means it has fully completed. - */ - function validateWeights(uint256[] memory normalizedWeights, uint256 numTokens) internal pure { - // Ensure each normalized weight is above the minimum - uint256 normalizedSum = 0; - for (uint8 i = 0; i < numTokens; ++i) { - uint256 normalizedWeight = normalizedWeights[i]; - - if (normalizedWeight < _MIN_WEIGHT) { - revert MinWeight(); - } - normalizedSum += normalizedWeight; - } - // Ensure that the normalized weights sum to ONE - if (normalizedSum != FixedPoint.ONE) { - revert NormalizedWeightInvariant(); - } - } - - /** - * @dev Returns a fixed-point number representing how far along the current value change is, where 0 means the - * change has not yet started, and FixedPoint.ONE means it has fully completed. - */ - function validateTwoWeights(uint256 normalizedWeight0, uint256 normalizedWeight1) internal pure { - // Ensure each normalized weight is above the minimum - if (normalizedWeight0 < _MIN_WEIGHT || normalizedWeight1 < _MIN_WEIGHT) { - revert MinWeight(); - } - - // Ensure that the normalized weights sum to ONE - if (normalizedWeight0 + normalizedWeight1 != FixedPoint.ONE) { - revert NormalizedWeightInvariant(); - } - } -} diff --git a/pkg/pool-weighted/contracts/test/GradualValueChangeMock.sol b/pkg/pool-weighted/contracts/test/GradualValueChangeMock.sol new file mode 100644 index 000000000..13f469f63 --- /dev/null +++ b/pkg/pool-weighted/contracts/test/GradualValueChangeMock.sol @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +pragma solidity ^0.8.24; + +import "../lib/GradualValueChange.sol"; + +contract GradualValueChangeMock { + function getInterpolatedValue( + uint256 startValue, + uint256 endValue, + uint256 startTime, + uint256 endTime + ) public view returns (uint256) { + return GradualValueChange.getInterpolatedValue(startValue, endValue, startTime, endTime); + } + + function resolveStartTime(uint256 startTime, uint256 endTime) public view returns (uint256) { + return GradualValueChange.resolveStartTime(startTime, endTime); + } + + function interpolateValue(uint256 startValue, uint256 endValue, uint256 pctProgress) public pure returns (uint256) { + return GradualValueChange.interpolateValue(startValue, endValue, pctProgress); + } + + function calculateValueChangeProgress(uint256 startTime, uint256 endTime) public view returns (uint256) { + return GradualValueChange.calculateValueChangeProgress(startTime, endTime); + } +} diff --git a/pkg/pool-weighted/test/LBPool.test.ts b/pkg/pool-weighted/test/LBPool.test.ts index f76a4abbf..239e66f79 100644 --- a/pkg/pool-weighted/test/LBPool.test.ts +++ b/pkg/pool-weighted/test/LBPool.test.ts @@ -6,13 +6,7 @@ import { Router } from '@balancer-labs/v3-vault/typechain-types/contracts/Router import { ERC20TestToken } from '@balancer-labs/v3-solidity-utils/typechain-types/contracts/test/ERC20TestToken'; import { SignerWithAddress } from '@nomicfoundation/hardhat-ethers/dist/src/signer-with-address'; import { FP_ZERO, fp } from '@balancer-labs/v3-helpers/src/numbers'; -import { - MAX_UINT256, - MAX_UINT160, - MAX_UINT48, - ZERO_BYTES32, - ZERO_ADDRESS, -} from '@balancer-labs/v3-helpers/src/constants'; +import { MAX_UINT256, MAX_UINT160, MAX_UINT48, ZERO_BYTES32 } from '@balancer-labs/v3-helpers/src/constants'; import * as VaultDeployer from '@balancer-labs/v3-helpers/src/models/vault/VaultDeployer'; import { IVaultMock } from '@balancer-labs/v3-interfaces/typechain-types'; import TypesConverter from '@balancer-labs/v3-helpers/src/models/types/TypesConverter'; @@ -44,6 +38,9 @@ describe('LBPool', function () { let tokenB: ERC20TestToken; let poolTokens: string[]; + let tokenAIdx: number; + let tokenBIdx: number; + let tokenAAddress: string; let tokenBAddress: string; @@ -72,14 +69,14 @@ describe('LBPool', function () { tokenAAddress = await tokenA.getAddress(); tokenBAddress = await tokenB.getAddress(); + + tokenAIdx = tokenAAddress < tokenBAddress ? 0 : 1; + tokenBIdx = tokenAAddress < tokenBAddress ? 1 : 0; }); sharedBeforeEach('create and initialize pool', async () => { - - const tempRouter = bob.address - const tempRouterProvider = bob.address factory = await deploy('LBPoolFactory', { - args: [await vault.getAddress(), MONTH * 12, FACTORY_VERSION, POOL_VERSION, tempRouter, tempRouterProvider], + args: [await vault.getAddress(), MONTH * 12, FACTORY_VERSION, POOL_VERSION, router], }); poolTokens = sortAddresses([tokenAAddress, tokenBAddress]); @@ -91,7 +88,7 @@ describe('LBPool', function () { tokenConfig, WEIGHTS, SWAP_FEE, - bob.address, //owner + bob.address, // owner true, // swapEnabledOnStart ZERO_BYTES32 ); @@ -163,13 +160,9 @@ describe('LBPool', function () { describe('Owner operations and events', () => { it('should emit SwapEnabledSet event when setSwapEnabled is called', async () => { - await expect(pool.connect(bob).setSwapEnabled(false)) - .to.emit(pool, 'SwapEnabledSet') - .withArgs(false); + await expect(pool.connect(bob).setSwapEnabled(false)).to.emit(pool, 'SwapEnabledSet').withArgs(false); - await expect(pool.connect(bob).setSwapEnabled(true)) - .to.emit(pool, 'SwapEnabledSet') - .withArgs(true); + await expect(pool.connect(bob).setSwapEnabled(true)).to.emit(pool, 'SwapEnabledSet').withArgs(true); }); it('should emit GradualWeightUpdateScheduled event when updateWeightsGradually is called', async () => { @@ -178,7 +171,6 @@ describe('LBPool', function () { const endWeights = [fp(0.7), fp(0.3)]; const tx = await pool.connect(bob).updateWeightsGradually(startTime, endTime, endWeights); - const receipt = await tx.wait(); const actualStartTime = await time.latest(); await expect(tx) @@ -187,31 +179,15 @@ describe('LBPool', function () { }); it('should only allow owner to be the LP', async () => { - const joinKind = 0; // Assuming 0 is the correct join kind for initialization - const userData = ethers.AbiCoder.defaultAbiCoder().encode( - ['uint256', 'uint256[]'], - [joinKind, INITIAL_BALANCES] + const amounts: bigint[] = [FP_ZERO, FP_ZERO]; + amounts[tokenAIdx] = SWAP_AMOUNT; + + await expect(router.addLiquidityUnbalanced(pool, amounts, FP_ZERO, false, '0x')).to.be.revertedWithCustomError( + vault, + 'BeforeAddLiquidityHookFailed' ); - await expect(router.connect(alice).addLiquidity({ - pool: await pool.getAddress(), - tokens: poolTokens, - lastChangeBlock: 0, - userData: userData, - from: await alice.getAddress(), - to: await alice.getAddress(), - onlyManagePoolTokens: false - })).to.be.revertedWithCustomError(pool, 'RouterNotTrusted'); - - await expect(router.connect(bob).addLiquidity({ - pool: await pool.getAddress(), - tokens: poolTokens, - lastChangeBlock: 0, - userData: userData, - from: await bob.getAddress(), - to: await bob.getAddress(), - onlyManagePoolTokens: false - })).to.not.be.reverted; + await router.connect(bob).addLiquidityUnbalanced(pool, amounts, FP_ZERO, false, '0x'); }); it('should only allow owner to update weights', async () => { @@ -219,11 +195,11 @@ describe('LBPool', function () { const endTime = startTime + MONTH; const endWeights = [fp(0.7), fp(0.3)]; - await expect(pool.connect(alice).updateWeightsGradually(startTime, endTime, endWeights)) - .to.be.revertedWithCustomError(pool, 'OwnableUnauthorizedAccount'); + await expect( + pool.connect(alice).updateWeightsGradually(startTime, endTime, endWeights) + ).to.be.revertedWithCustomError(pool, 'OwnableUnauthorizedAccount'); - await expect(pool.connect(bob).updateWeightsGradually(startTime, endTime, endWeights)) - .to.not.be.reverted; + await expect(pool.connect(bob).updateWeightsGradually(startTime, endTime, endWeights)).to.not.be.reverted; }); }); @@ -254,56 +230,55 @@ describe('LBPool', function () { const endTime = startTime + MONTH; // Try to set weight below 1% - await expect(pool.connect(bob).updateWeightsGradually(startTime, endTime, [fp(0.009), fp(0.991)])) - .to.be.revertedWithCustomError(pool, 'MinWeight'); + await expect( + pool.connect(bob).updateWeightsGradually(startTime, endTime, [fp(0.009), fp(0.991)]) + ).to.be.revertedWithCustomError(pool, 'MinWeight'); // Try to set weight above 99% - await expect(pool.connect(bob).updateWeightsGradually(startTime, endTime, [fp(0.991), fp(0.009)])) - .to.be.revertedWithCustomError(pool, 'MinWeight'); + await expect( + pool.connect(bob).updateWeightsGradually(startTime, endTime, [fp(0.991), fp(0.009)]) + ).to.be.revertedWithCustomError(pool, 'MinWeight'); // Valid weight update - await expect(pool.connect(bob).updateWeightsGradually(startTime, endTime, [fp(0.01), fp(0.99)])) - .to.not.be.reverted; + await expect(pool.connect(bob).updateWeightsGradually(startTime, endTime, [fp(0.01), fp(0.99)])).to.not.be + .reverted; }); -it('should always sum weights to 1', async () => { - const currentTime = await time.latest(); - const startTime = currentTime + 60; // Set startTime 60 seconds in the future - const endTime = startTime + MONTH; - const startWeights = [fp(0.5), fp(0.5)]; - const endWeights = [fp(0.7), fp(0.3)]; - - // Move time to just before startTime - await time.increaseTo(startTime - 1); + it('should always sum weights to 1', async () => { + const currentTime = await time.latest(); + const startTime = currentTime + 60; // Set startTime 60 seconds in the future + const endTime = startTime + MONTH; + const startWeights = [fp(0.5), fp(0.5)]; + const endWeights = [fp(0.7), fp(0.3)]; - // Set weights to 50/50 instantaneously - const tx1 = await pool.connect(bob).updateWeightsGradually(startTime, startTime, startWeights); - await tx1.wait(); + // Move time to just before startTime + await time.increaseTo(startTime - 1); - // Schedule gradual shift to 70/30 - const tx2 = await pool.connect(bob).updateWeightsGradually(startTime, endTime, endWeights); - await tx2.wait(); + // Set weights to 50/50 instantaneously + const tx1 = await pool.connect(bob).updateWeightsGradually(startTime, startTime, startWeights); + await tx1.wait(); - // Check weights at various points during the transition - for (let i = 0; i <= 100; i++) { - const checkTime = startTime + (i * MONTH) / 100; + // Schedule gradual shift to 70/30 + const tx2 = await pool.connect(bob).updateWeightsGradually(startTime, endTime, endWeights); + await tx2.wait(); - // Only increase time if it's greater than the current time - const currentBlockTime = await time.latest(); - if (checkTime > currentBlockTime) { - await time.increaseTo(checkTime); - } + // Check weights at various points during the transition + for (let i = 0; i <= 100; i++) { + const checkTime = startTime + (i * MONTH) / 100; - const weights = await pool.getNormalizedWeights(); - const sum = (BigInt(weights[0].toString()) + BigInt(weights[1].toString())).toString(); - - // Assert exact equality - expect(sum).to.equal(fp(1)); - - } -}); + // Only increase time if it's greater than the current time + const currentBlockTime = await time.latest(); + if (checkTime > currentBlockTime) { + await time.increaseTo(checkTime); + } + const weights = await pool.getNormalizedWeights(); + const sum = (BigInt(weights[0].toString()) + BigInt(weights[1].toString())).toString(); + // Assert exact equality + expect(sum).to.equal(fp(1)); + } + }); }); describe('Setters and Getters', () => { @@ -332,30 +307,42 @@ it('should always sum weights to 1', async () => { }); describe('Swap restrictions', () => { - it('should not allow swaps when disabled', async () => { - await pool.connect(bob).setSwapEnabled(false); - - await expect(router.connect(bob).swap( - pool, - poolTokens[0], - poolTokens[1], - SWAP_AMOUNT, - 0, - false, - '0x' - )).to.be.reverted; // Exact error message depends on implementation + it('should allow swaps when enabled', async () => { + await expect( + router + .connect(bob) + .swapSingleTokenExactIn( + pool, + poolTokens[tokenAIdx], + poolTokens[tokenBIdx], + SWAP_AMOUNT, + 0, + MAX_UINT256, + false, + '0x' + ) + ).to.not.be.reverted; + }); - await pool.connect(bob).setSwapEnabled(true); + it('should not allow swaps when disabled', async () => { + await expect(await pool.connect(bob).setSwapEnabled(false)) + .to.emit(pool, 'SwapEnabledSet') + .withArgs(false); - await expect(router.connect(bob).swap( - pool, - poolTokens[0], - poolTokens[1], - SWAP_AMOUNT, - 0, - false, - '0x' - )).to.not.be.reverted; + await expect( + router + .connect(bob) + .swapSingleTokenExactIn( + pool, + poolTokens[tokenAIdx], + poolTokens[tokenBIdx], + SWAP_AMOUNT, + 0, + MAX_UINT256, + false, + '0x' + ) + ).to.be.reverted; }); }); }); diff --git a/pkg/pool-weighted/test/foundry/GradualValueChange.t.sol b/pkg/pool-weighted/test/foundry/GradualValueChange.t.sol index e3f3e22a5..d3d7916d1 100644 --- a/pkg/pool-weighted/test/foundry/GradualValueChange.t.sol +++ b/pkg/pool-weighted/test/foundry/GradualValueChange.t.sol @@ -1,43 +1,18 @@ // SPDX-License-Identifier: GPL-3.0-or-later + pragma solidity ^0.8.24; import "forge-std/Test.sol"; -import "../../contracts/lib/GradualValueChange.sol"; - -// Wrapper contract to expose library functions -contract GradualValueChangeWrapper { - function getInterpolatedValue( - uint256 startValue, - uint256 endValue, - uint256 startTime, - uint256 endTime - ) public view returns (uint256) { - return GradualValueChange.getInterpolatedValue(startValue, endValue, startTime, endTime); - } - - function resolveStartTime(uint256 startTime, uint256 endTime) public view returns (uint256) { - return GradualValueChange.resolveStartTime(startTime, endTime); - } - function ensureNoTimeOverflow(uint256 time, uint256 maxTime) public pure { - GradualValueChange.ensureNoTimeOverflow(time, maxTime); - } - - function interpolateValue(uint256 startValue, uint256 endValue, uint256 pctProgress) public pure returns (uint256) { - return GradualValueChange.interpolateValue(startValue, endValue, pctProgress); - } - - function calculateValueChangeProgress(uint256 startTime, uint256 endTime) public view returns (uint256) { - return GradualValueChange.calculateValueChangeProgress(startTime, endTime); - } -} +import "../../contracts/test/GradualValueChangeMock.sol"; contract GradualValueChangeTest is Test { - GradualValueChangeWrapper private wrapper; uint256 private constant FP_ONE = 1e18; + GradualValueChangeMock private mock; + function setUp() public { - wrapper = new GradualValueChangeWrapper(); + mock = new GradualValueChangeMock(); } function testGetInterpolatedValue() public { @@ -51,7 +26,7 @@ contract GradualValueChangeTest is Test { uint256 currentTime = startTime + ((i * (endTime - startTime)) / steps); vm.warp(currentTime); uint256 expectedValue = startValue + ((i * (endValue - startValue)) / steps); - uint256 actualValue = wrapper.getInterpolatedValue(startValue, endValue, startTime, endTime); + uint256 actualValue = mock.getInterpolatedValue(startValue, endValue, startTime, endTime); assertEq(actualValue, expectedValue, "Interpolated value should match expected"); } } @@ -61,25 +36,15 @@ contract GradualValueChangeTest is Test { uint256 futureTime = currentTime + 100; vm.warp(currentTime); - assertEq(wrapper.resolveStartTime(futureTime, futureTime + 100), futureTime, "Should return future start time"); + assertEq(mock.resolveStartTime(futureTime, futureTime + 100), futureTime, "Should return future start time"); assertEq( - wrapper.resolveStartTime(currentTime - 100, futureTime), + mock.resolveStartTime(currentTime - 100, futureTime), currentTime, "Should return current time for past start time" ); vm.expectRevert(GradualValueChange.GradualUpdateTimeTravel.selector); - wrapper.resolveStartTime(futureTime + 200, futureTime + 100); - } - - function testEnsureNoTimeOverflow() public { - uint256 maxTime = 1000; - - wrapper.ensureNoTimeOverflow(maxTime, maxTime); - wrapper.ensureNoTimeOverflow(maxTime - 1, maxTime); - - vm.expectRevert(GradualValueChange.TimeTruncatedInStorage.selector); - wrapper.ensureNoTimeOverflow(maxTime + 1, maxTime); + mock.resolveStartTime(futureTime + 200, futureTime + 100); } function testInterpolateValue() public view { @@ -90,7 +55,7 @@ contract GradualValueChangeTest is Test { for (uint256 i = 0; i <= steps; i++) { uint256 pctProgress = (i * FP_ONE) / steps; uint256 expectedValue = startValue + ((i * (endValue - startValue)) / steps); - uint256 actualValue = wrapper.interpolateValue(startValue, endValue, pctProgress); + uint256 actualValue = mock.interpolateValue(startValue, endValue, pctProgress); assertEq(actualValue, expectedValue, "Interpolated value should match expected"); } @@ -101,7 +66,7 @@ contract GradualValueChangeTest is Test { for (uint256 i = 0; i <= steps; i++) { uint256 pctProgress = (i * FP_ONE) / steps; uint256 expectedValue = startValue - ((i * (startValue - endValue)) / steps); - uint256 actualValue = wrapper.interpolateValue(startValue, endValue, pctProgress); + uint256 actualValue = mock.interpolateValue(startValue, endValue, pctProgress); assertEq(actualValue, expectedValue, "Interpolated value should match expected for decreasing value"); } } @@ -115,13 +80,13 @@ contract GradualValueChangeTest is Test { uint256 currentTime = startTime + ((i * (endTime - startTime)) / steps); vm.warp(currentTime); uint256 expectedProgress = (i * FP_ONE) / steps; - uint256 actualProgress = wrapper.calculateValueChangeProgress(startTime, endTime); + uint256 actualProgress = mock.calculateValueChangeProgress(startTime, endTime); // Use a very tight tolerance for progress calculation assertApproxEqAbs(actualProgress, expectedProgress, 1, "Progress should be very close to expected"); } vm.warp(endTime + 50); - assertEq(wrapper.calculateValueChangeProgress(startTime, endTime), FP_ONE, "Should be complete after end time"); + assertEq(mock.calculateValueChangeProgress(startTime, endTime), FP_ONE, "Should be complete after end time"); } function testEdgeCases() public { @@ -134,7 +99,7 @@ contract GradualValueChangeTest is Test { for (uint256 i = 0; i <= 100; i++) { vm.warp(startTime + i); assertEq( - wrapper.getInterpolatedValue(startValue, startValue, startTime, endTime), + mock.getInterpolatedValue(startValue, startValue, startTime, endTime), startValue, "Should always return the same value" ); @@ -143,7 +108,7 @@ contract GradualValueChangeTest is Test { // Test before start time vm.warp(startTime - 1); assertEq( - wrapper.getInterpolatedValue(startValue, endValue, startTime, startTime), + mock.getInterpolatedValue(startValue, endValue, startTime, startTime), startValue, "Should return start value before start time for zero duration" ); @@ -151,7 +116,7 @@ contract GradualValueChangeTest is Test { // Test at start time vm.warp(startTime); assertEq( - wrapper.getInterpolatedValue(startValue, endValue, startTime, startTime), + mock.getInterpolatedValue(startValue, endValue, startTime, startTime), endValue, "Should return end value at start time for zero duration" ); @@ -159,7 +124,7 @@ contract GradualValueChangeTest is Test { // Test after start time vm.warp(startTime + 1); assertEq( - wrapper.getInterpolatedValue(startValue, endValue, startTime, startTime), + mock.getInterpolatedValue(startValue, endValue, startTime, startTime), endValue, "Should return end value after start time for zero duration" ); @@ -168,19 +133,19 @@ contract GradualValueChangeTest is Test { // Test exact endpoints vm.warp(0); - assertEq(wrapper.getInterpolatedValue(0, bigVal, 0, bigVal), 0, "Should be 0 at start time"); + assertEq(mock.getInterpolatedValue(0, bigVal, 0, bigVal), 0, "Should be 0 at start time"); vm.warp(bigVal); - assertEq(wrapper.getInterpolatedValue(0, bigVal, 0, bigVal), bigVal, "Should be bigVal at end time"); + assertEq(mock.getInterpolatedValue(0, bigVal, 0, bigVal), bigVal, "Should be bigVal at end time"); // Test intermediate points - uint256 steps = 1e5; + uint256 steps = 1e4; for (uint256 i = 0; i <= steps; i++) { uint256 currentTime = (bigVal / steps) * i; vm.warp(currentTime); uint256 expectedValue = (bigVal / steps) * i; uint256 actualValue; - try wrapper.getInterpolatedValue(0, bigVal, 0, bigVal) returns (uint256 value) { + try mock.getInterpolatedValue(0, bigVal, 0, bigVal) returns (uint256 value) { actualValue = value; assertApproxEqRel(actualValue, expectedValue, 1, "Should be close for large numbers"); } catch Error(string memory reason) { diff --git a/pkg/pool-weighted/test/foundry/LBPool.t.sol b/pkg/pool-weighted/test/foundry/LBPool.t.sol index 1e3109bbc..b51513b43 100644 --- a/pkg/pool-weighted/test/foundry/LBPool.t.sol +++ b/pkg/pool-weighted/test/foundry/LBPool.t.sol @@ -3,25 +3,25 @@ pragma solidity ^0.8.24; import "forge-std/Test.sol"; -import "@openzeppelin/contracts/utils/Strings.sol"; +import { Strings } from "@openzeppelin/contracts/utils/Strings.sol"; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { IBasePool } from "@balancer-labs/v3-interfaces/contracts/vault/IBasePool.sol"; import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol"; import { TokenConfig, PoolRoleAccounts } from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; import { IVaultErrors } from "@balancer-labs/v3-interfaces/contracts/vault/IVaultErrors.sol"; +import { IBasePoolFactory } from "@balancer-labs/v3-interfaces/contracts/vault/IBasePoolFactory.sol"; import { CastingHelpers } from "@balancer-labs/v3-solidity-utils/contracts/helpers/CastingHelpers.sol"; import { ArrayHelpers } from "@balancer-labs/v3-solidity-utils/contracts/test/ArrayHelpers.sol"; import { InputHelpers } from "@balancer-labs/v3-solidity-utils/contracts/helpers/InputHelpers.sol"; import { PoolHooksMock } from "@balancer-labs/v3-vault/contracts/test/PoolHooksMock.sol"; -import { BasePoolTest } from "@balancer-labs/v3-vault/test/foundry/utils/BasePoolTest.sol"; import { FixedPoint } from "@balancer-labs/v3-solidity-utils/contracts/math/FixedPoint.sol"; +import { BasePoolTest } from "@balancer-labs/v3-vault/test/foundry/utils/BasePoolTest.sol"; import { LBPoolFactory } from "../../contracts/LBPoolFactory.sol"; import { LBPool } from "../../contracts/LBPool.sol"; -import "forge-std/console.sol"; contract LBPoolTest is BasePoolTest { using CastingHelpers for address[]; @@ -33,8 +33,8 @@ contract LBPoolTest is BasePoolTest { uint256[] internal weights; - uint256 daiIdx; - uint256 usdcIdx; + uint256 internal daiIdx; + uint256 internal usdcIdx; function setUp() public virtual override { expectedAddLiquidityBptAmountOut = TOKEN_AMOUNT; @@ -184,7 +184,7 @@ contract LBPoolTest is BasePoolTest { uint256[] memory minAmountsOut = new uint256[](poolTokens.length); for (uint256 i = 0; i < poolTokens.length; ++i) { - minAmountsOut[i] = _less(tokenAmounts[i], 1e4); + minAmountsOut[i] = less(tokenAmounts[i], 1e4); } uint256[] memory amountsOut = router.removeLiquidityProportional( @@ -254,7 +254,7 @@ contract LBPoolTest is BasePoolTest { tokenIn, tokenOut, tokenAmountIn, - _less(tokenAmountOut, 1e3), + less(tokenAmountOut, 1e3), MAX_UINT256, false, bytes("") @@ -358,6 +358,17 @@ contract LBPoolTest is BasePoolTest { ); } + function testEnsureNoTimeOverflow() public { + uint256 blockDotTimestampTestStart = block.timestamp; + uint256[] memory endWeights = new uint256[](2); + endWeights[0] = 0.01e18; // 1% + endWeights[1] = 0.99e18; // 99% + + vm.prank(bob); + vm.expectRevert(stdError.arithmeticError); + LBPool(address(pool)).updateWeightsGradually(blockDotTimestampTestStart, type(uint32).max + 1, endWeights); + } + function testQuerySwapDuringWeightUpdate() public { // Cache original time to avoid issues from `block.timestamp` during `vm.warp` uint256 blockDotTimestampTestStart = block.timestamp; diff --git a/pkg/pool-weighted/test/foundry/WeightValidationTest.t.sol b/pkg/pool-weighted/test/foundry/WeightValidationTest.t.sol deleted file mode 100644 index e8592c545..000000000 --- a/pkg/pool-weighted/test/foundry/WeightValidationTest.t.sol +++ /dev/null @@ -1,109 +0,0 @@ -// SPDX-License-Identifier: GPL-3.0-or-later -pragma solidity ^0.8.24; - -import "forge-std/Test.sol"; -import "../../contracts/lib/WeightValidation.sol"; - -// Wrapper contract to expose library functions -contract WeightValidationWrapper { - function validateWeights(uint256[] memory normalizedWeights, uint256 numTokens) public pure { - WeightValidation.validateWeights(normalizedWeights, numTokens); - } - - function validateTwoWeights(uint256 normalizedWeight0, uint256 normalizedWeight1) public pure { - WeightValidation.validateTwoWeights(normalizedWeight0, normalizedWeight1); - } -} - -contract WeightValidationTest is Test { - WeightValidationWrapper private wrapper; - uint256 private constant FP_ONE = 1e18; // FixedPoint.ONE - uint256 private constant MIN_WEIGHT = 1e16; // 1% - - function setUp() public { - wrapper = new WeightValidationWrapper(); - } - - function testValidateWeightsValidRangeMultipleTokens() public view { - for (uint256 numTokens = 2; numTokens <= 8; numTokens++) { - uint256[] memory weights = new uint256[](numTokens); - uint256 remainingWeight = FP_ONE; - - for (uint256 i = 0; i < numTokens - 1; i++) { - uint256 weight = (remainingWeight / (numTokens - i)) + 1e16; // Ensure it's above MIN_WEIGHT - weights[i] = weight; - remainingWeight -= weight; - } - weights[numTokens - 1] = remainingWeight; - - wrapper.validateWeights(weights, numTokens); - } - } - - function testValidateWeightsEdgeCases() public view { - // Test with all weights at minimum except one - uint256[] memory weights = new uint256[](5); - for (uint256 i = 0; i < 4; i++) { - weights[i] = MIN_WEIGHT; - } - weights[4] = FP_ONE - (MIN_WEIGHT * 4); - wrapper.validateWeights(weights, 5); - - // Test with two weights splitting the total - uint256[] memory twoWeights = new uint256[](2); - twoWeights[0] = FP_ONE / 2; - twoWeights[1] = FP_ONE / 2; - wrapper.validateWeights(twoWeights, 2); - } - - function testValidateWeightsInvalidSum() public { - uint256[] memory weights = new uint256[](3); - weights[0] = 0.3e18; - weights[1] = 0.3e18; - weights[2] = 0.3e18; - vm.expectRevert(WeightValidation.NormalizedWeightInvariant.selector); - wrapper.validateWeights(weights, 3); - - // Test with sum slightly above FP_ONE - weights[2] = 0.400000000000000001e18; - vm.expectRevert(WeightValidation.NormalizedWeightInvariant.selector); - wrapper.validateWeights(weights, 3); - } - - function testValidateWeightsBelowMinWeight() public { - uint256[] memory weights = new uint256[](3); - weights[0] = MIN_WEIGHT - 1; - weights[1] = (FP_ONE - MIN_WEIGHT + 1) / 2; - weights[2] = (FP_ONE - MIN_WEIGHT + 1) / 2; - vm.expectRevert(WeightValidation.MinWeight.selector); - wrapper.validateWeights(weights, 3); - } - - function testValidateTwoWeightsValidRange() public view { - for (uint256 i = MIN_WEIGHT; i <= FP_ONE - MIN_WEIGHT; i += 1e16) { - wrapper.validateTwoWeights(i, FP_ONE - i); - } - } - - function testValidateTwoWeightsInvalidSum() public { - vm.expectRevert(WeightValidation.NormalizedWeightInvariant.selector); - wrapper.validateTwoWeights(0.5e18, 0.500000000000000001e18); - - vm.expectRevert(WeightValidation.NormalizedWeightInvariant.selector); - wrapper.validateTwoWeights(0.5e18, 0.499999999999999999e18); - } - - function testValidateTwoWeightsBelowMinWeight() public { - vm.expectRevert(WeightValidation.MinWeight.selector); - wrapper.validateTwoWeights(MIN_WEIGHT - 1, FP_ONE - MIN_WEIGHT + 1); - - vm.expectRevert(WeightValidation.MinWeight.selector); - wrapper.validateTwoWeights(FP_ONE - MIN_WEIGHT + 1, MIN_WEIGHT - 1); - } - - function testValidateTwoWeightsEdgeCases() public view { - wrapper.validateTwoWeights(MIN_WEIGHT, FP_ONE - MIN_WEIGHT); - wrapper.validateTwoWeights(FP_ONE - MIN_WEIGHT, MIN_WEIGHT); - wrapper.validateTwoWeights(FP_ONE / 2, FP_ONE / 2); - } -} diff --git a/pkg/solidity-utils/contracts/math/FixedPoint.sol b/pkg/solidity-utils/contracts/math/FixedPoint.sol index bab4fbf51..086985b11 100644 --- a/pkg/solidity-utils/contracts/math/FixedPoint.sol +++ b/pkg/solidity-utils/contracts/math/FixedPoint.sol @@ -158,15 +158,4 @@ library FixedPoint { result := mul(lt(x, ONE), sub(ONE, x)) } } - - /** - * @dev Returns the larger of two uint256s. - */ - function max(uint256 a, uint256 b) internal pure returns (uint256 result) { - // Equivalent to: - // result = (a < b) ? b : a; - assembly { - result := sub(a, mul(sub(a, b), lt(a, b))) - } - } } diff --git a/pkg/solidity-utils/contracts/math/WeightedMath.sol b/pkg/solidity-utils/contracts/math/WeightedMath.sol index 5e914abd2..ca0341bb9 100644 --- a/pkg/solidity-utils/contracts/math/WeightedMath.sol +++ b/pkg/solidity-utils/contracts/math/WeightedMath.sol @@ -112,7 +112,7 @@ library WeightedMath { uint256 currentBalance, uint256 weight, uint256 invariantRatio - ) internal pure returns (uint256 newBalance) { + ) internal pure returns (uint256 invariant) { /****************************************************************************************** // calculateBalanceGivenInvariant // // o = balanceOut // @@ -121,21 +121,10 @@ library WeightedMath { // i = invariantRatio // ******************************************************************************************/ - // Rounds result up overall, rounding up the two individual steps: - // - balanceRatio = invariantRatio ^ (1 / weight) - // - newBalance = balance * balanceRatio - // - // Regarding `balanceRatio`, the exponent is always > FP(1), but the invariant ratio can be either greater or - // lower than FP(1) depending on whether this is solving an `add` or a `remove` operation. - // - For i > 1, we need to round the exponent up, as i^x is monotonically increasing for i > 1. - // - For i < 1, we need to round the exponent down, as as i^x is monotonically decreasing for i < 1. - - function(uint256, uint256) internal pure returns (uint256) divUpOrDown = invariantRatio > 1 - ? FixedPoint.divUp - : FixedPoint.divDown; + // Rounds result up overall. // Calculate by how much the token balance has to increase to match the invariantRatio. - uint256 balanceRatio = invariantRatio.powUp(divUpOrDown(FixedPoint.ONE, weight)); + uint256 balanceRatio = invariantRatio.powUp(FixedPoint.ONE.divUp(weight)); return currentBalance.mulUp(balanceRatio); } diff --git a/pkg/vault/contracts/VaultFactory.sol b/pkg/vault/contracts/VaultFactory.sol index 5185a615b..198e56373 100644 --- a/pkg/vault/contracts/VaultFactory.sol +++ b/pkg/vault/contracts/VaultFactory.sol @@ -5,9 +5,9 @@ pragma solidity ^0.8.24; import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol"; import { IAuthorizer } from "@balancer-labs/v3-interfaces/contracts/vault/IAuthorizer.sol"; import { Authentication } from "@balancer-labs/v3-solidity-utils/contracts/helpers/Authentication.sol"; -import { Create2 } from "@openzeppelin/contracts/utils/Create2.sol"; import { CREATE3 } from "@balancer-labs/v3-solidity-utils/contracts/solmate/CREATE3.sol"; +import { Vault } from "./Vault.sol"; import { VaultAdmin } from "./VaultAdmin.sol"; import { VaultExtension } from "./VaultExtension.sol"; import { ProtocolFeeController } from "./ProtocolFeeController.sol"; @@ -26,12 +26,7 @@ contract VaultFactory is Authentication { /// @notice The given salt does not match the generated address when attempting to create the Vault. error VaultAddressMismatch(); - /// @notice The bytecode for the given contract does not match the expected bytecode. - error InvalidBytecode(string contractName); - - bytes32 public immutable vaultCreationCodeHash; - bytes32 public immutable vaultAdminCreationCodeHash; - bytes32 public immutable vaultExtensionCreationCodeHash; + bool public isDisabled; IAuthorizer private immutable _authorizer; uint32 private immutable _pauseWindowDuration; @@ -40,6 +35,8 @@ contract VaultFactory is Authentication { uint256 private immutable _minWrapAmount; address private immutable _deployer; + bytes private _creationCode; + // solhint-disable not-rely-on-time constructor( @@ -47,17 +44,10 @@ contract VaultFactory is Authentication { uint32 pauseWindowDuration, uint32 bufferPeriodDuration, uint256 minTradeAmount, - uint256 minWrapAmount, - bytes32 vaultCreationCodeHash_, - bytes32 vaultAdminCreationCodeHash_, - bytes32 vaultExtensionCreationCodeHash_ + uint256 minWrapAmount ) Authentication(bytes32(uint256(uint160(address(this))))) { _deployer = msg.sender; - - vaultCreationCodeHash = vaultCreationCodeHash_; - vaultAdminCreationCodeHash = vaultAdminCreationCodeHash_; - vaultExtensionCreationCodeHash = vaultExtensionCreationCodeHash_; - + _creationCode = type(Vault).creationCode; _authorizer = authorizer; _pauseWindowDuration = pauseWindowDuration; _bufferPeriodDuration = bufferPeriodDuration; @@ -74,65 +64,31 @@ contract VaultFactory is Authentication { * @param targetAddress Expected Vault address. The function will revert if the given salt does not deploy the * Vault to the target address. */ - function create( - bytes32 salt, - address targetAddress, - bytes calldata vaultCreationCode, - bytes calldata vaultAdminCreationCode, - bytes calldata vaultExtensionCreationCode - ) external authenticate { - if (vaultCreationCodeHash != keccak256(vaultCreationCode)) { - revert InvalidBytecode("Vault"); - } else if (vaultAdminCreationCodeHash != keccak256(vaultAdminCreationCode)) { - revert InvalidBytecode("VaultAdmin"); - } else if (vaultExtensionCreationCodeHash != keccak256(vaultExtensionCreationCode)) { - revert InvalidBytecode("VaultExtension"); + function create(bytes32 salt, address targetAddress) external authenticate { + if (isDisabled) { + revert VaultAlreadyCreated(); } + isDisabled = true; address vaultAddress = getDeploymentAddress(salt); if (targetAddress != vaultAddress) { revert VaultAddressMismatch(); } - ProtocolFeeController feeController = new ProtocolFeeController(IVault(vaultAddress)); - - VaultAdmin vaultAdmin = VaultAdmin( - payable( - Create2.deploy( - 0, - bytes32(0x00), - abi.encodePacked( - vaultAdminCreationCode, - abi.encode( - IVault(vaultAddress), - _pauseWindowDuration, - _bufferPeriodDuration, - _minTradeAmount, - _minWrapAmount - ) - ) - ) - ) - ); - - VaultExtension vaultExtension = VaultExtension( - payable( - Create2.deploy( - 0, - bytes32(uint256(0x01)), - abi.encodePacked(vaultExtensionCreationCode, abi.encode(vaultAddress, vaultAdmin)) - ) - ) + VaultAdmin vaultAdmin = new VaultAdmin( + IVault(vaultAddress), + _pauseWindowDuration, + _bufferPeriodDuration, + _minTradeAmount, + _minWrapAmount ); + VaultExtension vaultExtension = new VaultExtension(IVault(vaultAddress), vaultAdmin); + ProtocolFeeController feeController = new ProtocolFeeController(IVault(vaultAddress)); - address deployedAddress = CREATE3.deploy( - salt, - abi.encodePacked(vaultCreationCode, abi.encode(vaultExtension, _authorizer, feeController)), - 0 - ); + address deployedAddress = _create(abi.encode(vaultExtension, _authorizer, feeController), salt); // This should always be the case, but we enforce the end state to match the expected outcome anyway. - if (deployedAddress != vaultAddress) { + if (deployedAddress != targetAddress) { revert VaultAddressMismatch(); } @@ -144,6 +100,10 @@ contract VaultFactory is Authentication { return CREATE3.getDeployed(salt); } + function _create(bytes memory constructorArgs, bytes32 finalSalt) internal returns (address) { + return CREATE3.deploy(finalSalt, abi.encodePacked(_creationCode, constructorArgs), 0); + } + function _canPerform(bytes32 actionId, address user) internal view virtual override returns (bool) { return _authorizer.canPerform(actionId, user, address(this)); } diff --git a/pkg/vault/test/foundry/VaultFactory.t.sol b/pkg/vault/test/foundry/VaultFactory.t.sol index 08c5de2c6..f947971e5 100644 --- a/pkg/vault/test/foundry/VaultFactory.t.sol +++ b/pkg/vault/test/foundry/VaultFactory.t.sol @@ -9,9 +9,6 @@ import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol" import { BasicAuthorizerMock } from "../../contracts/test/BasicAuthorizerMock.sol"; import { VaultFactory } from "../../contracts/VaultFactory.sol"; -import { Vault } from "../../contracts/Vault.sol"; -import { VaultAdmin } from "../../contracts/VaultAdmin.sol"; -import { VaultExtension } from "../../contracts/VaultExtension.sol"; contract VaultFactoryTest is Test { // Should match the "PRODUCTION" limits in BaseVaultTest. @@ -25,16 +22,7 @@ contract VaultFactoryTest is Test { function setUp() public virtual { deployer = makeAddr("deployer"); authorizer = new BasicAuthorizerMock(); - factory = new VaultFactory( - authorizer, - 90 days, - 30 days, - _MIN_TRADE_AMOUNT, - _MIN_WRAP_AMOUNT, - keccak256(type(Vault).creationCode), - keccak256(type(VaultAdmin).creationCode), - keccak256(type(VaultExtension).creationCode) - ); + factory = new VaultFactory(authorizer, 90 days, 30 days, _MIN_TRADE_AMOUNT, _MIN_WRAP_AMOUNT); } /// forge-config: default.fuzz.runs = 100 @@ -43,13 +31,7 @@ contract VaultFactoryTest is Test { address vaultAddress = factory.getDeploymentAddress(salt); vm.prank(deployer); - factory.create( - salt, - vaultAddress, - type(Vault).creationCode, - type(VaultAdmin).creationCode, - type(VaultExtension).creationCode - ); + factory.create(salt, vaultAddress); // We cannot compare the deployed bytecode of the created vault against a second deployment of the vault // because the actionIdDisambiguator of the authentication contract is stored in immutable storage. @@ -66,13 +48,7 @@ contract VaultFactoryTest is Test { function testCreateNotAuthorized() public { vm.prank(deployer); vm.expectRevert(IAuthentication.SenderNotAllowed.selector); - factory.create( - bytes32(0), - address(0), - type(Vault).creationCode, - type(VaultAdmin).creationCode, - type(VaultExtension).creationCode - ); + factory.create(bytes32(0), address(0)); } function testCreateMismatch() public { @@ -82,13 +58,7 @@ contract VaultFactoryTest is Test { address vaultAddress = factory.getDeploymentAddress(salt); vm.prank(deployer); vm.expectRevert(VaultFactory.VaultAddressMismatch.selector); - factory.create( - bytes32(uint256(salt) + 1), - vaultAddress, - type(Vault).creationCode, - type(VaultAdmin).creationCode, - type(VaultExtension).creationCode - ); + factory.create(bytes32(uint256(salt) + 1), vaultAddress); } function testCreateTwice() public { @@ -97,56 +67,8 @@ contract VaultFactoryTest is Test { address vaultAddress = factory.getDeploymentAddress(salt); vm.startPrank(deployer); - factory.create( - salt, - vaultAddress, - type(Vault).creationCode, - type(VaultAdmin).creationCode, - type(VaultExtension).creationCode - ); - vm.expectRevert(); - factory.create( - salt, - vaultAddress, - type(Vault).creationCode, - type(VaultAdmin).creationCode, - type(VaultExtension).creationCode - ); - } - - function testInvalidVaultBytecode() public { - bytes32 salt = bytes32(uint256(123)); - authorizer.grantRole(factory.getActionId(VaultFactory.create.selector), deployer); - - address vaultAddress = factory.getDeploymentAddress(salt); - vm.prank(deployer); - vm.expectRevert(abi.encodeWithSelector(VaultFactory.InvalidBytecode.selector, "Vault")); - factory.create( - salt, - vaultAddress, - new bytes(0), - type(VaultAdmin).creationCode, - type(VaultExtension).creationCode - ); - } - - function testInvalidVaultAdminBytecode() public { - bytes32 salt = bytes32(uint256(123)); - authorizer.grantRole(factory.getActionId(VaultFactory.create.selector), deployer); - - address vaultAddress = factory.getDeploymentAddress(salt); - vm.prank(deployer); - vm.expectRevert(abi.encodeWithSelector(VaultFactory.InvalidBytecode.selector, "VaultAdmin")); - factory.create(salt, vaultAddress, type(Vault).creationCode, new bytes(0), type(VaultExtension).creationCode); - } - - function testInvalidVaultExtensionBytecode() public { - bytes32 salt = bytes32(uint256(123)); - authorizer.grantRole(factory.getActionId(VaultFactory.create.selector), deployer); - - address vaultAddress = factory.getDeploymentAddress(salt); - vm.prank(deployer); - vm.expectRevert(abi.encodeWithSelector(VaultFactory.InvalidBytecode.selector, "VaultExtension")); - factory.create(salt, vaultAddress, type(Vault).creationCode, type(VaultAdmin).creationCode, new bytes(0)); + factory.create(salt, vaultAddress); + vm.expectRevert(VaultFactory.VaultAlreadyCreated.selector); + factory.create(salt, vaultAddress); } } diff --git a/pkg/vault/test/foundry/utils/BasePoolTest.sol b/pkg/vault/test/foundry/utils/BasePoolTest.sol index db4e06752..7e89dfd49 100644 --- a/pkg/vault/test/foundry/utils/BasePoolTest.sol +++ b/pkg/vault/test/foundry/utils/BasePoolTest.sol @@ -7,10 +7,10 @@ import { Strings } from "@openzeppelin/contracts/utils/Strings.sol"; import { IRateProvider } from "@balancer-labs/v3-interfaces/contracts/solidity-utils/helpers/IRateProvider.sol"; import { IBasePoolFactory } from "@balancer-labs/v3-interfaces/contracts/vault/IBasePoolFactory.sol"; +import { PoolRoleAccounts } from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; import { IVaultErrors } from "@balancer-labs/v3-interfaces/contracts/vault/IVaultErrors.sol"; import { FixedPoint } from "@balancer-labs/v3-solidity-utils/contracts/math/FixedPoint.sol"; import { IVaultAdmin } from "@balancer-labs/v3-interfaces/contracts/vault/IVaultAdmin.sol"; -import { PoolRoleAccounts } from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; import { IBasePool } from "@balancer-labs/v3-interfaces/contracts/vault/IBasePool.sol"; import { Vault } from "@balancer-labs/v3-vault/contracts/Vault.sol"; @@ -58,7 +58,7 @@ abstract contract BasePoolTest is BaseVaultTest { assertEq(pool, calculatedPoolAddress, "Pool address mismatch"); } - function testPoolPausedState() public view { + function testPoolPausedState() public view virtual { (bool paused, uint256 pauseWindow, uint256 bufferPeriod, address pauseManager) = vault.getPoolPausedState(pool); assertFalse(paused, "Vault should not be paused initially"); @@ -251,14 +251,15 @@ abstract contract BasePoolTest is BaseVaultTest { assertEq(IBasePool(pool).getMaximumSwapFeePercentage(), poolMaxSwapFeePercentage, "Maximum swap fee mismatch"); } - function testSetSwapFeeTooLow() public virtual { + function testSetSwapFeeTooLow() public { address swapFeeManager = _getSwapFeeAdmin(); vm.prank(swapFeeManager); + vm.expectRevert(IVaultErrors.SwapFeePercentageTooLow.selector); vault.setStaticSwapFeePercentage(pool, poolMinSwapFeePercentage - 1); } - function testSetSwapFeeTooHigh() public virtual { + function testSetSwapFeeTooHigh() public { address swapFeeManager = _getSwapFeeAdmin(); vm.prank(swapFeeManager); @@ -267,8 +268,8 @@ abstract contract BasePoolTest is BaseVaultTest { } function testAddLiquidityUnbalanced() public virtual { - address swapFeeManager = _getSwapFeeAdmin(); - vm.prank(swapFeeManager); + authorizer.grantRole(vault.getActionId(IVaultAdmin.setStaticSwapFeePercentage.selector), alice); + vm.prank(alice); vault.setStaticSwapFeePercentage(pool, 10e16); uint256[] memory amountsIn = tokenAmounts; @@ -295,13 +296,14 @@ abstract contract BasePoolTest is BaseVaultTest { } // Decreases the amount value by base value. Example: base = 100, decrease by 1% / base = 1e4, 0.01% and etc. - function _less(uint256 amount, uint256 base) internal pure returns (uint256) { + function _less(uint256 amount, uint256 base) private pure returns (uint256) { return (amount * (base - 1)) / base; } function _getSwapFeeAdmin() internal returns (address) { - address swapFeeManager; PoolRoleAccounts memory roleAccounts = vault.getPoolRoleAccounts(pool); + address swapFeeManager; + if (roleAccounts.swapFeeManager != address(0)) { return roleAccounts.swapFeeManager; } else { From 78574f1f5b1000a6a37f3e412151cc63c5d31165 Mon Sep 17 00:00:00 2001 From: gerg Date: Thu, 19 Sep 2024 16:56:17 -0400 Subject: [PATCH 25/54] fix name of return variable to reflect to docstrings and apparent intended value --- pkg/solidity-utils/contracts/math/WeightedMath.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/solidity-utils/contracts/math/WeightedMath.sol b/pkg/solidity-utils/contracts/math/WeightedMath.sol index ff686d190..3d68981d5 100644 --- a/pkg/solidity-utils/contracts/math/WeightedMath.sol +++ b/pkg/solidity-utils/contracts/math/WeightedMath.sol @@ -137,7 +137,7 @@ library WeightedMath { uint256 currentBalance, uint256 weight, uint256 invariantRatio - ) internal pure returns (uint256 invariant) { + ) internal pure returns (uint256 newBalance) { /****************************************************************************************** // calculateBalanceGivenInvariant // // o = balanceOut // From fca2e63614f1aef03ac60ab8ef71eab1ac32cc4e Mon Sep 17 00:00:00 2001 From: gerg Date: Fri, 20 Sep 2024 09:34:32 -0400 Subject: [PATCH 26/54] revert WeightedMath to what's in main branch --- .../contracts/math/WeightedMath.sol | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/pkg/solidity-utils/contracts/math/WeightedMath.sol b/pkg/solidity-utils/contracts/math/WeightedMath.sol index 3d68981d5..278cb9667 100644 --- a/pkg/solidity-utils/contracts/math/WeightedMath.sol +++ b/pkg/solidity-utils/contracts/math/WeightedMath.sol @@ -146,10 +146,21 @@ library WeightedMath { // i = invariantRatio // ******************************************************************************************/ - // Rounds result up overall. + // Rounds result up overall, rounding up the two individual steps: + // - balanceRatio = invariantRatio ^ (1 / weight) + // - newBalance = balance * balanceRatio + // + // Regarding `balanceRatio`, the exponent is always > FP(1), but the invariant ratio can be either greater or + // lower than FP(1) depending on whether this is solving an `add` or a `remove` operation. + // - For i > 1, we need to round the exponent up, as i^x is monotonically increasing for i > 1. + // - For i < 1, we need to round the exponent down, as as i^x is monotonically decreasing for i < 1. + + function(uint256, uint256) internal pure returns (uint256) divUpOrDown = invariantRatio > 1 + ? FixedPoint.divUp + : FixedPoint.divDown; // Calculate by how much the token balance has to increase to match the invariantRatio. - uint256 balanceRatio = invariantRatio.powUp(FixedPoint.ONE.divUp(weight)); + uint256 balanceRatio = invariantRatio.powUp(divUpOrDown(FixedPoint.ONE, weight)); return currentBalance.mulUp(balanceRatio); } From b65cf101690293207d8b578dfcb0b38cdbe05356 Mon Sep 17 00:00:00 2001 From: gerg Date: Fri, 20 Sep 2024 10:54:02 -0400 Subject: [PATCH 27/54] add LBPoolFactory.t.sol --- .../test/foundry/LBPoolFactory.t.sol | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol diff --git a/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol b/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol new file mode 100644 index 000000000..9447e45cd --- /dev/null +++ b/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol @@ -0,0 +1,77 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +pragma solidity ^0.8.24; + +import "forge-std/Test.sol"; + +import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; + +import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol"; +import { IVaultErrors } from "@balancer-labs/v3-interfaces/contracts/vault/IVaultErrors.sol"; +import { PoolRoleAccounts } from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; + +import { CastingHelpers } from "@balancer-labs/v3-solidity-utils/contracts/helpers/CastingHelpers.sol"; +import { ArrayHelpers } from "@balancer-labs/v3-solidity-utils/contracts/test/ArrayHelpers.sol"; +import { BaseVaultTest } from "@balancer-labs/v3-vault/test/foundry/utils/BaseVaultTest.sol"; + +import { LBPoolFactory } from "../../contracts/LBPoolFactory.sol"; + +contract LBPoolFactoryTest is BaseVaultTest { + using CastingHelpers for address[]; + using ArrayHelpers for *; + + uint64 public constant swapFee = 1e16; //1% + + LBPoolFactory internal lbPoolFactory; + + function setUp() public override { + super.setUp(); + + lbPoolFactory = new LBPoolFactory(IVault(address(vault)), 365 days, "Factory v1", "Pool v1", address(router)); + vm.label(address(lbPoolFactory), "LB pool factory"); + } + + function testFactoryPausedState() public view { + uint32 pauseWindowDuration = lbPoolFactory.getPauseWindowDuration(); + assertEq(pauseWindowDuration, 365 days); + } + + function testCreatePool() public { + address lbPool = _deployAndInitializeLBPool(); + + // Verify pool was created and initialized correctly + assertTrue(vault.isPoolRegistered(lbPool), "Pool not registered in the vault"); + } + + function testDonationNotAllowed() public { + address lbPool = _deployAndInitializeLBPool(); + + // Try to donate to the pool + vm.startPrank(bob); + vm.expectRevert(IVaultErrors.DoesNotSupportDonation.selector); + router.donate(lbPool, [poolInitAmount, poolInitAmount].toMemoryArray(), false, bytes("")); + vm.stopPrank(); + } + + function _deployAndInitializeLBPool() private returns (address) { + IERC20[] memory tokens = [address(dai), address(usdc)].toMemoryArray().asIERC20(); + uint256[] memory weights = [uint256(50e16), uint256(50e16)].toMemoryArray(); + + address lbPool = lbPoolFactory.create( + "LB Pool", + "LBP", + vault.buildTokenConfig(tokens), + weights, + swapFee, + bob, // owner + true, // swapEnabledOnStart + ZERO_BYTES32 + ); + + // Initialize pool. + vm.prank(bob); // Owner initializes the pool + router.initialize(lbPool, tokens, [poolInitAmount, poolInitAmount].toMemoryArray(), 0, false, bytes("")); + + return lbPool; + } +} \ No newline at end of file From f1b3b42baffe6c0cd504e75340f38bd82708145e Mon Sep 17 00:00:00 2001 From: gerg Date: Sun, 22 Sep 2024 15:56:23 -0400 Subject: [PATCH 28/54] move swap enabled check from the hook (onBeforeSwap) to the actual swap function (onSwap) to reduce multiple balance reads from storage --- pkg/pool-weighted/contracts/LBPool.sol | 21 ++++++++++---------- pkg/pool-weighted/contracts/WeightedPool.sol | 2 +- pkg/pool-weighted/test/foundry/LBPool.t.sol | 2 +- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/pkg/pool-weighted/contracts/LBPool.sol b/pkg/pool-weighted/contracts/LBPool.sol index ec3780767..be04de163 100644 --- a/pkg/pool-weighted/contracts/LBPool.sol +++ b/pkg/pool-weighted/contracts/LBPool.sol @@ -76,6 +76,9 @@ contract LBPool is WeightedPool, Ownable, BaseHooks { /// @dev Indicates that the router that called the Vault is not trusted, so any operations should revert. error RouterNotTrusted(); + /// @dev Indicates that the `owner` has disabled swaps. + error SwapsDisabled(); + constructor( NewPoolParams memory params, IVault vault, @@ -169,6 +172,13 @@ contract LBPool is WeightedPool, Ownable, BaseHooks { _startGradualWeightChange(startTime.toUint32(), endTime.toUint32(), _getNormalizedWeights(), endWeights); } + /// @inheritdoc WeightedPool + function onSwap(PoolSwapParams memory request) public view override onlyVault returns (uint256) { + if (!_getPoolSwapEnabled()) { + revert SwapsDisabled(); + } + return super.onSwap(request); + } /******************************************************************************* Hook Functions *******************************************************************************/ @@ -193,8 +203,6 @@ contract LBPool is WeightedPool, Ownable, BaseHooks { // Return HookFlags struct that indicates which hooks this contract supports function getHookFlags() public pure override returns (HookFlags memory hookFlags) { - // Check whether swaps are enabled in `onBeforeSwap`. - hookFlags.shouldCallBeforeSwap = true; // Ensure the caller is the owner, as only the owner can add liquidity. hookFlags.shouldCallBeforeAddLiquidity = true; } @@ -218,18 +226,9 @@ contract LBPool is WeightedPool, Ownable, BaseHooks { if (router == _TRUSTED_ROUTER) { return IRouterCommon(router).getSender() == owner(); } - revert RouterNotTrusted(); } - /** - * @notice Called before a swap to let the pool block swaps if not enabled. - * @return success True if the pool has swaps enabled. - */ - function onBeforeSwap(PoolSwapParams calldata, address) public view override onlyVault returns (bool) { - return _getPoolSwapEnabled(); - } - /******************************************************************************* Internal Functions *******************************************************************************/ diff --git a/pkg/pool-weighted/contracts/WeightedPool.sol b/pkg/pool-weighted/contracts/WeightedPool.sol index 45bf6e48c..0c3104741 100644 --- a/pkg/pool-weighted/contracts/WeightedPool.sol +++ b/pkg/pool-weighted/contracts/WeightedPool.sol @@ -134,7 +134,7 @@ contract WeightedPool is IWeightedPool, BalancerPoolToken, PoolInfo, Version { } /// @inheritdoc IBasePool - function onSwap(PoolSwapParams memory request) public view onlyVault returns (uint256) { + function onSwap(PoolSwapParams memory request) public view virtual onlyVault returns (uint256) { uint256 balanceTokenInScaled18 = request.balancesScaled18[request.indexIn]; uint256 balanceTokenOutScaled18 = request.balancesScaled18[request.indexOut]; diff --git a/pkg/pool-weighted/test/foundry/LBPool.t.sol b/pkg/pool-weighted/test/foundry/LBPool.t.sol index b51513b43..be4b065b9 100644 --- a/pkg/pool-weighted/test/foundry/LBPool.t.sol +++ b/pkg/pool-weighted/test/foundry/LBPool.t.sol @@ -325,7 +325,7 @@ contract LBPoolTest is BasePoolTest { // Test swap when disabled vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(IVaultErrors.BeforeSwapHookFailed.selector)); + vm.expectRevert(abi.encodeWithSelector(LBPool.SwapsDisabled.selector)); router.swapSingleTokenExactIn( address(pool), IERC20(dai), From aec77e9570b6dad5092cd79964d840dff948300f Mon Sep 17 00:00:00 2001 From: gerg Date: Sun, 22 Sep 2024 15:56:48 -0400 Subject: [PATCH 29/54] lint --- pkg/pool-weighted/contracts/LBPool.sol | 1 + pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/pool-weighted/contracts/LBPool.sol b/pkg/pool-weighted/contracts/LBPool.sol index be04de163..033e5906f 100644 --- a/pkg/pool-weighted/contracts/LBPool.sol +++ b/pkg/pool-weighted/contracts/LBPool.sol @@ -179,6 +179,7 @@ contract LBPool is WeightedPool, Ownable, BaseHooks { } return super.onSwap(request); } + /******************************************************************************* Hook Functions *******************************************************************************/ diff --git a/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol b/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol index 9447e45cd..6f3a61b0f 100644 --- a/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol +++ b/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol @@ -74,4 +74,4 @@ contract LBPoolFactoryTest is BaseVaultTest { return lbPool; } -} \ No newline at end of file +} From af0dcf2036431fc0df8fc17ec736ae2b913363e1 Mon Sep 17 00:00:00 2001 From: gerg Date: Sun, 22 Sep 2024 16:02:27 -0400 Subject: [PATCH 30/54] move LBP to pool-weighted/lbp directory --- pkg/pool-weighted/contracts/{ => lbp}/LBPool.sol | 4 ++-- pkg/pool-weighted/contracts/{ => lbp}/LBPoolFactory.sol | 2 +- pkg/pool-weighted/test/foundry/LBPool.t.sol | 4 ++-- pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) rename pkg/pool-weighted/contracts/{ => lbp}/LBPool.sol (99%) rename pkg/pool-weighted/contracts/{ => lbp}/LBPoolFactory.sol (98%) diff --git a/pkg/pool-weighted/contracts/LBPool.sol b/pkg/pool-weighted/contracts/lbp/LBPool.sol similarity index 99% rename from pkg/pool-weighted/contracts/LBPool.sol rename to pkg/pool-weighted/contracts/lbp/LBPool.sol index 033e5906f..3e30c219b 100644 --- a/pkg/pool-weighted/contracts/LBPool.sol +++ b/pkg/pool-weighted/contracts/lbp/LBPool.sol @@ -15,8 +15,8 @@ import { InputHelpers } from "@balancer-labs/v3-solidity-utils/contracts/helpers import { FixedPoint } from "@balancer-labs/v3-solidity-utils/contracts/math/FixedPoint.sol"; import { BaseHooks } from "@balancer-labs/v3-vault/contracts/BaseHooks.sol"; -import { GradualValueChange } from "./lib/GradualValueChange.sol"; -import { WeightedPool } from "./WeightedPool.sol"; +import { GradualValueChange } from "../lib/GradualValueChange.sol"; +import { WeightedPool } from "../WeightedPool.sol"; /** * @notice Weighted Pool with mutable weights, designed to support v3 Liquidity Bootstrapping. diff --git a/pkg/pool-weighted/contracts/LBPoolFactory.sol b/pkg/pool-weighted/contracts/lbp/LBPoolFactory.sol similarity index 98% rename from pkg/pool-weighted/contracts/LBPoolFactory.sol rename to pkg/pool-weighted/contracts/lbp/LBPoolFactory.sol index 4faab6476..099c3fa06 100644 --- a/pkg/pool-weighted/contracts/LBPoolFactory.sol +++ b/pkg/pool-weighted/contracts/lbp/LBPoolFactory.sol @@ -9,7 +9,7 @@ import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol" import { BasePoolFactory } from "@balancer-labs/v3-pool-utils/contracts/BasePoolFactory.sol"; import { Version } from "@balancer-labs/v3-solidity-utils/contracts/helpers/Version.sol"; -import { WeightedPool } from "./WeightedPool.sol"; +import { WeightedPool } from "../WeightedPool.sol"; import { LBPool } from "./LBPool.sol"; /** diff --git a/pkg/pool-weighted/test/foundry/LBPool.t.sol b/pkg/pool-weighted/test/foundry/LBPool.t.sol index be4b065b9..f52bdcd98 100644 --- a/pkg/pool-weighted/test/foundry/LBPool.t.sol +++ b/pkg/pool-weighted/test/foundry/LBPool.t.sol @@ -20,8 +20,8 @@ import { PoolHooksMock } from "@balancer-labs/v3-vault/contracts/test/PoolHooksM import { FixedPoint } from "@balancer-labs/v3-solidity-utils/contracts/math/FixedPoint.sol"; import { BasePoolTest } from "@balancer-labs/v3-vault/test/foundry/utils/BasePoolTest.sol"; -import { LBPoolFactory } from "../../contracts/LBPoolFactory.sol"; -import { LBPool } from "../../contracts/LBPool.sol"; +import { LBPoolFactory } from "../../contracts/lbp/LBPoolFactory.sol"; +import { LBPool } from "../../contracts/lbp/LBPool.sol"; contract LBPoolTest is BasePoolTest { using CastingHelpers for address[]; diff --git a/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol b/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol index 6f3a61b0f..1972bc43a 100644 --- a/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol +++ b/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol @@ -14,7 +14,7 @@ import { CastingHelpers } from "@balancer-labs/v3-solidity-utils/contracts/helpe import { ArrayHelpers } from "@balancer-labs/v3-solidity-utils/contracts/test/ArrayHelpers.sol"; import { BaseVaultTest } from "@balancer-labs/v3-vault/test/foundry/utils/BaseVaultTest.sol"; -import { LBPoolFactory } from "../../contracts/LBPoolFactory.sol"; +import { LBPoolFactory } from "../../contracts/lbp/LBPoolFactory.sol"; contract LBPoolFactoryTest is BaseVaultTest { using CastingHelpers for address[]; From 26fc3eda45226d30076bbcb66ebf001577104a71 Mon Sep 17 00:00:00 2001 From: gerg Date: Mon, 23 Sep 2024 11:35:14 -0400 Subject: [PATCH 31/54] increase test coverage --- pkg/pool-weighted/test/foundry/LBPool.t.sol | 58 +++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/pkg/pool-weighted/test/foundry/LBPool.t.sol b/pkg/pool-weighted/test/foundry/LBPool.t.sol index f52bdcd98..ab61d5b93 100644 --- a/pkg/pool-weighted/test/foundry/LBPool.t.sol +++ b/pkg/pool-weighted/test/foundry/LBPool.t.sol @@ -19,9 +19,11 @@ import { InputHelpers } from "@balancer-labs/v3-solidity-utils/contracts/helpers import { PoolHooksMock } from "@balancer-labs/v3-vault/contracts/test/PoolHooksMock.sol"; import { FixedPoint } from "@balancer-labs/v3-solidity-utils/contracts/math/FixedPoint.sol"; import { BasePoolTest } from "@balancer-labs/v3-vault/test/foundry/utils/BasePoolTest.sol"; +import { RouterMock } from "@balancer-labs/v3-vault/contracts/test/RouterMock.sol"; import { LBPoolFactory } from "../../contracts/lbp/LBPoolFactory.sol"; import { LBPool } from "../../contracts/lbp/LBPool.sol"; +import { WeightedPool } from "../../contracts/WeightedPool.sol"; contract LBPoolTest is BasePoolTest { using CastingHelpers for address[]; @@ -425,6 +427,62 @@ contract LBPoolTest is BasePoolTest { } } + function testGetGradualWeightUpdateParams() public { + uint256 startTime = block.timestamp + 1 days; + uint256 endTime = startTime + 7 days; + uint256[] memory endWeights = new uint256[](2); + endWeights[0] = 0.2e18; // 20% + endWeights[1] = 0.8e18; // 80% + + vm.prank(bob); + LBPool(address(pool)).updateWeightsGradually(startTime, endTime, endWeights); + + (uint256 returnedStartTime, uint256 returnedEndTime, uint256[] memory returnedEndWeights) = LBPool(address(pool)) + .getGradualWeightUpdateParams(); + + assertEq(returnedStartTime, startTime, "Start time should match"); + assertEq(returnedEndTime, endTime, "End time should match"); + assertEq(returnedEndWeights.length, endWeights.length, "End weights length should match"); + for (uint256 i = 0; i < endWeights.length; i++) { + assertEq(returnedEndWeights[i], endWeights[i], "End weight should match"); + } + } + + function testUpdateWeightsGraduallyMinWeightRevert() public { + uint256 startTime = block.timestamp + 1 days; + uint256 endTime = startTime + 7 days; + uint256[] memory endWeights = new uint256[](2); + endWeights[0] = 0.0001e18; // 0.01% + endWeights[1] = 0.9999e18; // 99.99% + + vm.prank(bob); + vm.expectRevert(WeightedPool.MinWeight.selector); + LBPool(address(pool)).updateWeightsGradually(startTime, endTime, endWeights); + } + + function testUpdateWeightsGraduallyNormalizedWeightInvariantRevert() public { + uint256 startTime = block.timestamp + 1 days; + uint256 endTime = startTime + 7 days; + uint256[] memory endWeights = new uint256[](2); + endWeights[0] = 0.6e18; // 60% + endWeights[1] = 0.5e18; // 50% + + vm.prank(bob); + vm.expectRevert(WeightedPool.NormalizedWeightInvariant.selector); + LBPool(address(pool)).updateWeightsGradually(startTime, endTime, endWeights); + } + + function testAddLiquidityRouterNotTrusted() public { + RouterMock mockRouter = new RouterMock(IVault(address(vault)), weth, permit2); + + uint256[] memory amounts = [TOKEN_AMOUNT, TOKEN_AMOUNT].toMemoryArray(); + + vm.startPrank(bob); + vm.expectRevert(abi.encodeWithSelector(LBPool.RouterNotTrusted.selector)); + mockRouter.addLiquidityUnbalanced(address(pool), amounts, 0, false, ""); + vm.stopPrank(); + } + function _executeAndUndoSwap(uint256 amountIn) internal returns (uint256) { // Create a storage checkpoint uint256 snapshot = vm.snapshot(); From b9e1d7ab6cb4a903de1c5bb3576caccf2e63cccc Mon Sep 17 00:00:00 2001 From: gerg Date: Mon, 23 Sep 2024 13:07:09 -0400 Subject: [PATCH 32/54] add input mismatch tests; add non-owner weight update fail test; add paused on start test --- pkg/pool-weighted/test/foundry/LBPool.t.sol | 135 ++++++++++++++++++++ 1 file changed, 135 insertions(+) diff --git a/pkg/pool-weighted/test/foundry/LBPool.t.sol b/pkg/pool-weighted/test/foundry/LBPool.t.sol index ab61d5b93..bf3dca903 100644 --- a/pkg/pool-weighted/test/foundry/LBPool.t.sol +++ b/pkg/pool-weighted/test/foundry/LBPool.t.sol @@ -21,6 +21,8 @@ import { FixedPoint } from "@balancer-labs/v3-solidity-utils/contracts/math/Fixe import { BasePoolTest } from "@balancer-labs/v3-vault/test/foundry/utils/BasePoolTest.sol"; import { RouterMock } from "@balancer-labs/v3-vault/contracts/test/RouterMock.sol"; +import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; + import { LBPoolFactory } from "../../contracts/lbp/LBPoolFactory.sol"; import { LBPool } from "../../contracts/lbp/LBPool.sol"; import { WeightedPool } from "../../contracts/WeightedPool.sol"; @@ -483,6 +485,139 @@ contract LBPoolTest is BasePoolTest { vm.stopPrank(); } + function testInvalidTokenCount() public { + IERC20[] memory sortedTokens1 = InputHelpers.sortTokens( + [address(dai)].toMemoryArray().asIERC20() + ); + IERC20[] memory sortedTokens3 = InputHelpers.sortTokens( + [address(dai), address(usdc), address(weth)].toMemoryArray().asIERC20() + ); + + TokenConfig[] memory tokenConfig1 = vault.buildTokenConfig(sortedTokens1); + TokenConfig[] memory tokenConfig3 = vault.buildTokenConfig(sortedTokens3); + + // Attempt to create a pool with 1 token + // Doesn't throw InputHelpers.InputLengthMismatch.selector b/c create3 intercepts error + vm.expectRevert("DEPLOYMENT_FAILED"); + LBPoolFactory(address(factory)).create( + "Invalid Pool 1", + "IP1", + tokenConfig1, + [uint256(1e18)].toMemoryArray(), + DEFAULT_SWAP_FEE, + bob, + true, + ZERO_BYTES32 + ); + + // Attempt to create a pool with 3 tokens + // Doesn't throw InputHelpers.InputLengthMismatch.selector b/c create3 intercepts error + vm.expectRevert("DEPLOYMENT_FAILED"); + LBPoolFactory(address(factory)).create( + "Invalid Pool 3", + "IP3", + tokenConfig3, + [uint256(0.3e18), uint256(0.3e18), uint256(0.4e18)].toMemoryArray(), + DEFAULT_SWAP_FEE, + bob, + true, + ZERO_BYTES32 + ); + } + + function testMismatchedWeightsAndTokens() public { + TokenConfig[] memory tokenConfig = vault.buildTokenConfig(poolTokens); + + vm.expectRevert("DEPLOYMENT_FAILED"); + LBPoolFactory(address(factory)).create( + "Mismatched Pool", + "MP", + tokenConfig, + [uint256(1e18)].toMemoryArray(), + DEFAULT_SWAP_FEE, + bob, + true, + ZERO_BYTES32 + ); + } + + function testInitializedWithSwapsDisabled() public { + LBPool swapsDisabledPool = LBPool( + LBPoolFactory(address(factory)).create( + "Swaps Disabled Pool", + "SDP", + vault.buildTokenConfig(poolTokens), + weights, + DEFAULT_SWAP_FEE, + bob, + false, // swapEnabledOnStart set to false + keccak256(abi.encodePacked(block.timestamp)) // generate pseudorandom salt to avoid collision + ) + ); + + assertFalse(swapsDisabledPool.getSwapEnabled(), "Swaps should be disabled on initialization"); + + // Initialize to make swapping (or at least trying) possible + vm.startPrank(bob); + bptAmountOut = _initPool( + address(swapsDisabledPool), + tokenAmounts, + // Account for the precision loss + expectedAddLiquidityBptAmountOut - DELTA + ); + vm.stopPrank(); + + vm.startPrank(alice); + vm.expectRevert(abi.encodeWithSelector(LBPool.SwapsDisabled.selector)); + router.swapSingleTokenExactIn( + address(swapsDisabledPool), + IERC20(dai), + IERC20(usdc), + TOKEN_AMOUNT / 10, + 0, + block.timestamp + 1 hours, + false, + "" + ); + vm.stopPrank(); + } + + function testUpdateWeightsGraduallyMismatchedEndWeightsTooFew() public { + uint256 startTime = block.timestamp + 1 days; + uint256 endTime = startTime + 7 days; + uint256[] memory endWeights = new uint256[](1); // Too few end weights + endWeights[0] = 1e18; + + vm.prank(bob); + vm.expectRevert(abi.encodeWithSelector(InputHelpers.InputLengthMismatch.selector)); + LBPool(address(pool)).updateWeightsGradually(startTime, endTime, endWeights); + } + + function testUpdateWeightsGraduallyMismatchedEndWeightsTooMany() public { + uint256 startTime = block.timestamp + 1 days; + uint256 endTime = startTime + 7 days; + uint256[] memory endWeights = new uint256[](3); // Too many end weights + endWeights[0] = 0.3e18; + endWeights[1] = 0.3e18; + endWeights[2] = 0.4e18; + + vm.prank(bob); + vm.expectRevert(abi.encodeWithSelector(InputHelpers.InputLengthMismatch.selector)); + LBPool(address(pool)).updateWeightsGradually(startTime, endTime, endWeights); + } + + function testNonOwnerCannotUpdateWeights() public { + uint256 startTime = block.timestamp + 1 days; + uint256 endTime = startTime + 7 days; + uint256[] memory endWeights = new uint256[](2); + endWeights[0] = 0.7e18; + endWeights[1] = 0.3e18; + + vm.prank(alice); // Non-owner + vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, address(alice))); + LBPool(address(pool)).updateWeightsGradually(startTime, endTime, endWeights); + } + function _executeAndUndoSwap(uint256 amountIn) internal returns (uint256) { // Create a storage checkpoint uint256 snapshot = vm.snapshot(); From d93f26535a1f0ba31d2003c5a623a917ae45e9bb Mon Sep 17 00:00:00 2001 From: gerg Date: Mon, 23 Sep 2024 13:36:11 -0400 Subject: [PATCH 33/54] lint --- pkg/pool-weighted/test/foundry/LBPool.t.sol | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pkg/pool-weighted/test/foundry/LBPool.t.sol b/pkg/pool-weighted/test/foundry/LBPool.t.sol index bf3dca903..e2490ecff 100644 --- a/pkg/pool-weighted/test/foundry/LBPool.t.sol +++ b/pkg/pool-weighted/test/foundry/LBPool.t.sol @@ -439,8 +439,9 @@ contract LBPoolTest is BasePoolTest { vm.prank(bob); LBPool(address(pool)).updateWeightsGradually(startTime, endTime, endWeights); - (uint256 returnedStartTime, uint256 returnedEndTime, uint256[] memory returnedEndWeights) = LBPool(address(pool)) - .getGradualWeightUpdateParams(); + (uint256 returnedStartTime, uint256 returnedEndTime, uint256[] memory returnedEndWeights) = LBPool( + address(pool) + ).getGradualWeightUpdateParams(); assertEq(returnedStartTime, startTime, "Start time should match"); assertEq(returnedEndTime, endTime, "End time should match"); @@ -486,9 +487,7 @@ contract LBPoolTest is BasePoolTest { } function testInvalidTokenCount() public { - IERC20[] memory sortedTokens1 = InputHelpers.sortTokens( - [address(dai)].toMemoryArray().asIERC20() - ); + IERC20[] memory sortedTokens1 = InputHelpers.sortTokens([address(dai)].toMemoryArray().asIERC20()); IERC20[] memory sortedTokens3 = InputHelpers.sortTokens( [address(dai), address(usdc), address(weth)].toMemoryArray().asIERC20() ); From a686bbab92f28c132f59fe2651dafb1a0a566027 Mon Sep 17 00:00:00 2001 From: gerg Date: Tue, 24 Sep 2024 11:05:34 -0400 Subject: [PATCH 34/54] add LBPool test for invalid token index (coverage for _getNormalizedWeight) and LBPoolFactory test for getPoolVersion --- pkg/pool-weighted/test/foundry/LBPool.t.sol | 24 ++++++++++++++++++- .../test/foundry/LBPoolFactory.t.sol | 8 ++++++- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/pkg/pool-weighted/test/foundry/LBPool.t.sol b/pkg/pool-weighted/test/foundry/LBPool.t.sol index e2490ecff..2365e694d 100644 --- a/pkg/pool-weighted/test/foundry/LBPool.t.sol +++ b/pkg/pool-weighted/test/foundry/LBPool.t.sol @@ -9,7 +9,12 @@ import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { IBasePool } from "@balancer-labs/v3-interfaces/contracts/vault/IBasePool.sol"; import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol"; -import { TokenConfig, PoolRoleAccounts } from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; +import { + TokenConfig, + PoolRoleAccounts, + PoolSwapParams, + SwapKind +} from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; import { IVaultErrors } from "@balancer-labs/v3-interfaces/contracts/vault/IVaultErrors.sol"; import { IBasePoolFactory } from "@balancer-labs/v3-interfaces/contracts/vault/IBasePoolFactory.sol"; @@ -617,6 +622,23 @@ contract LBPoolTest is BasePoolTest { LBPool(address(pool)).updateWeightsGradually(startTime, endTime, endWeights); } + function testOnSwapInvalidTokenIndex() public { + vm.prank(address(vault)); + + PoolSwapParams memory request = PoolSwapParams({ + kind: SwapKind.EXACT_IN, + amountGivenScaled18: 1e18, + balancesScaled18: new uint256[](3), // add an extra (non-existant) value to give the bad index a balance + indexIn: 2, // Invalid token index + indexOut: 0, + router: address(router), + userData: "" + }); + + vm.expectRevert(IVaultErrors.InvalidToken.selector); + LBPool(pool).onSwap(request); + } + function _executeAndUndoSwap(uint256 amountIn) internal returns (uint256) { // Create a storage checkpoint uint256 snapshot = vm.snapshot(); diff --git a/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol b/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol index 1972bc43a..b31c7c325 100644 --- a/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol +++ b/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol @@ -24,10 +24,12 @@ contract LBPoolFactoryTest is BaseVaultTest { LBPoolFactory internal lbPoolFactory; + string public constant poolVersion = "Pool v1"; + function setUp() public override { super.setUp(); - lbPoolFactory = new LBPoolFactory(IVault(address(vault)), 365 days, "Factory v1", "Pool v1", address(router)); + lbPoolFactory = new LBPoolFactory(IVault(address(vault)), 365 days, "Factory v1", poolVersion, address(router)); vm.label(address(lbPoolFactory), "LB pool factory"); } @@ -43,6 +45,10 @@ contract LBPoolFactoryTest is BaseVaultTest { assertTrue(vault.isPoolRegistered(lbPool), "Pool not registered in the vault"); } + function testGetPoolVersion() public view { + assert(keccak256(abi.encodePacked(lbPoolFactory.getPoolVersion())) == keccak256(abi.encodePacked(poolVersion))); + } + function testDonationNotAllowed() public { address lbPool = _deployAndInitializeLBPool(); From 3f82b06cbd98fb22b4b6ff4f3dbf1420b3bdd7f2 Mon Sep 17 00:00:00 2001 From: gerg Date: Wed, 25 Sep 2024 14:24:12 -0400 Subject: [PATCH 35/54] restore main's VaultFactory.{t.,}sol for this branch --- pkg/vault/contracts/VaultFactory.sol | 88 ++++++++++++++++------- pkg/vault/test/foundry/VaultFactory.t.sol | 78 ++++++++++++++++++-- 2 files changed, 136 insertions(+), 30 deletions(-) diff --git a/pkg/vault/contracts/VaultFactory.sol b/pkg/vault/contracts/VaultFactory.sol index 198e56373..5185a615b 100644 --- a/pkg/vault/contracts/VaultFactory.sol +++ b/pkg/vault/contracts/VaultFactory.sol @@ -5,9 +5,9 @@ pragma solidity ^0.8.24; import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol"; import { IAuthorizer } from "@balancer-labs/v3-interfaces/contracts/vault/IAuthorizer.sol"; import { Authentication } from "@balancer-labs/v3-solidity-utils/contracts/helpers/Authentication.sol"; +import { Create2 } from "@openzeppelin/contracts/utils/Create2.sol"; import { CREATE3 } from "@balancer-labs/v3-solidity-utils/contracts/solmate/CREATE3.sol"; -import { Vault } from "./Vault.sol"; import { VaultAdmin } from "./VaultAdmin.sol"; import { VaultExtension } from "./VaultExtension.sol"; import { ProtocolFeeController } from "./ProtocolFeeController.sol"; @@ -26,7 +26,12 @@ contract VaultFactory is Authentication { /// @notice The given salt does not match the generated address when attempting to create the Vault. error VaultAddressMismatch(); - bool public isDisabled; + /// @notice The bytecode for the given contract does not match the expected bytecode. + error InvalidBytecode(string contractName); + + bytes32 public immutable vaultCreationCodeHash; + bytes32 public immutable vaultAdminCreationCodeHash; + bytes32 public immutable vaultExtensionCreationCodeHash; IAuthorizer private immutable _authorizer; uint32 private immutable _pauseWindowDuration; @@ -35,8 +40,6 @@ contract VaultFactory is Authentication { uint256 private immutable _minWrapAmount; address private immutable _deployer; - bytes private _creationCode; - // solhint-disable not-rely-on-time constructor( @@ -44,10 +47,17 @@ contract VaultFactory is Authentication { uint32 pauseWindowDuration, uint32 bufferPeriodDuration, uint256 minTradeAmount, - uint256 minWrapAmount + uint256 minWrapAmount, + bytes32 vaultCreationCodeHash_, + bytes32 vaultAdminCreationCodeHash_, + bytes32 vaultExtensionCreationCodeHash_ ) Authentication(bytes32(uint256(uint160(address(this))))) { _deployer = msg.sender; - _creationCode = type(Vault).creationCode; + + vaultCreationCodeHash = vaultCreationCodeHash_; + vaultAdminCreationCodeHash = vaultAdminCreationCodeHash_; + vaultExtensionCreationCodeHash = vaultExtensionCreationCodeHash_; + _authorizer = authorizer; _pauseWindowDuration = pauseWindowDuration; _bufferPeriodDuration = bufferPeriodDuration; @@ -64,31 +74,65 @@ contract VaultFactory is Authentication { * @param targetAddress Expected Vault address. The function will revert if the given salt does not deploy the * Vault to the target address. */ - function create(bytes32 salt, address targetAddress) external authenticate { - if (isDisabled) { - revert VaultAlreadyCreated(); + function create( + bytes32 salt, + address targetAddress, + bytes calldata vaultCreationCode, + bytes calldata vaultAdminCreationCode, + bytes calldata vaultExtensionCreationCode + ) external authenticate { + if (vaultCreationCodeHash != keccak256(vaultCreationCode)) { + revert InvalidBytecode("Vault"); + } else if (vaultAdminCreationCodeHash != keccak256(vaultAdminCreationCode)) { + revert InvalidBytecode("VaultAdmin"); + } else if (vaultExtensionCreationCodeHash != keccak256(vaultExtensionCreationCode)) { + revert InvalidBytecode("VaultExtension"); } - isDisabled = true; address vaultAddress = getDeploymentAddress(salt); if (targetAddress != vaultAddress) { revert VaultAddressMismatch(); } - VaultAdmin vaultAdmin = new VaultAdmin( - IVault(vaultAddress), - _pauseWindowDuration, - _bufferPeriodDuration, - _minTradeAmount, - _minWrapAmount - ); - VaultExtension vaultExtension = new VaultExtension(IVault(vaultAddress), vaultAdmin); ProtocolFeeController feeController = new ProtocolFeeController(IVault(vaultAddress)); - address deployedAddress = _create(abi.encode(vaultExtension, _authorizer, feeController), salt); + VaultAdmin vaultAdmin = VaultAdmin( + payable( + Create2.deploy( + 0, + bytes32(0x00), + abi.encodePacked( + vaultAdminCreationCode, + abi.encode( + IVault(vaultAddress), + _pauseWindowDuration, + _bufferPeriodDuration, + _minTradeAmount, + _minWrapAmount + ) + ) + ) + ) + ); + + VaultExtension vaultExtension = VaultExtension( + payable( + Create2.deploy( + 0, + bytes32(uint256(0x01)), + abi.encodePacked(vaultExtensionCreationCode, abi.encode(vaultAddress, vaultAdmin)) + ) + ) + ); + + address deployedAddress = CREATE3.deploy( + salt, + abi.encodePacked(vaultCreationCode, abi.encode(vaultExtension, _authorizer, feeController)), + 0 + ); // This should always be the case, but we enforce the end state to match the expected outcome anyway. - if (deployedAddress != targetAddress) { + if (deployedAddress != vaultAddress) { revert VaultAddressMismatch(); } @@ -100,10 +144,6 @@ contract VaultFactory is Authentication { return CREATE3.getDeployed(salt); } - function _create(bytes memory constructorArgs, bytes32 finalSalt) internal returns (address) { - return CREATE3.deploy(finalSalt, abi.encodePacked(_creationCode, constructorArgs), 0); - } - function _canPerform(bytes32 actionId, address user) internal view virtual override returns (bool) { return _authorizer.canPerform(actionId, user, address(this)); } diff --git a/pkg/vault/test/foundry/VaultFactory.t.sol b/pkg/vault/test/foundry/VaultFactory.t.sol index 765efc986..3cf00f776 100644 --- a/pkg/vault/test/foundry/VaultFactory.t.sol +++ b/pkg/vault/test/foundry/VaultFactory.t.sol @@ -44,7 +44,13 @@ contract VaultFactoryTest is Test, VaultContractsDeployer { address vaultAddress = factory.getDeploymentAddress(salt); vm.prank(deployer); - factory.create(salt, vaultAddress); + factory.create( + salt, + vaultAddress, + type(Vault).creationCode, + type(VaultAdmin).creationCode, + type(VaultExtension).creationCode + ); // We cannot compare the deployed bytecode of the created vault against a second deployment of the Vault // because the actionIdDisambiguator of the authentication contract is stored in immutable storage. @@ -61,7 +67,13 @@ contract VaultFactoryTest is Test, VaultContractsDeployer { function testCreateNotAuthorized() public { vm.prank(deployer); vm.expectRevert(IAuthentication.SenderNotAllowed.selector); - factory.create(bytes32(0), address(0)); + factory.create( + bytes32(0), + address(0), + type(Vault).creationCode, + type(VaultAdmin).creationCode, + type(VaultExtension).creationCode + ); } function testCreateMismatch() public { @@ -71,7 +83,13 @@ contract VaultFactoryTest is Test, VaultContractsDeployer { address vaultAddress = factory.getDeploymentAddress(salt); vm.prank(deployer); vm.expectRevert(VaultFactory.VaultAddressMismatch.selector); - factory.create(bytes32(uint256(salt) + 1), vaultAddress); + factory.create( + bytes32(uint256(salt) + 1), + vaultAddress, + type(Vault).creationCode, + type(VaultAdmin).creationCode, + type(VaultExtension).creationCode + ); } function testCreateTwice() public { @@ -80,8 +98,56 @@ contract VaultFactoryTest is Test, VaultContractsDeployer { address vaultAddress = factory.getDeploymentAddress(salt); vm.startPrank(deployer); - factory.create(salt, vaultAddress); - vm.expectRevert(VaultFactory.VaultAlreadyCreated.selector); - factory.create(salt, vaultAddress); + factory.create( + salt, + vaultAddress, + type(Vault).creationCode, + type(VaultAdmin).creationCode, + type(VaultExtension).creationCode + ); + vm.expectRevert(); + factory.create( + salt, + vaultAddress, + type(Vault).creationCode, + type(VaultAdmin).creationCode, + type(VaultExtension).creationCode + ); + } + + function testInvalidVaultBytecode() public { + bytes32 salt = bytes32(uint256(123)); + authorizer.grantRole(factory.getActionId(VaultFactory.create.selector), deployer); + + address vaultAddress = factory.getDeploymentAddress(salt); + vm.prank(deployer); + vm.expectRevert(abi.encodeWithSelector(VaultFactory.InvalidBytecode.selector, "Vault")); + factory.create( + salt, + vaultAddress, + new bytes(0), + type(VaultAdmin).creationCode, + type(VaultExtension).creationCode + ); + } + + function testInvalidVaultAdminBytecode() public { + bytes32 salt = bytes32(uint256(123)); + authorizer.grantRole(factory.getActionId(VaultFactory.create.selector), deployer); + + address vaultAddress = factory.getDeploymentAddress(salt); + vm.prank(deployer); + vm.expectRevert(abi.encodeWithSelector(VaultFactory.InvalidBytecode.selector, "VaultAdmin")); + factory.create(salt, vaultAddress, type(Vault).creationCode, new bytes(0), type(VaultExtension).creationCode); + } + + function testInvalidVaultExtensionBytecode() public { + bytes32 salt = bytes32(uint256(123)); + authorizer.grantRole(factory.getActionId(VaultFactory.create.selector), deployer); + + address vaultAddress = factory.getDeploymentAddress(salt); + vm.prank(deployer); + vm.expectRevert(abi.encodeWithSelector(VaultFactory.InvalidBytecode.selector, "VaultExtension")); + factory.create(salt, vaultAddress, type(Vault).creationCode, type(VaultAdmin).creationCode, new bytes(0)); } } From 78780d5c24ca6b164b487b52b844a0fd7b37ea0c Mon Sep 17 00:00:00 2001 From: gerg Date: Wed, 25 Sep 2024 14:38:50 -0400 Subject: [PATCH 36/54] clean up BasePoolTest changes --- pkg/vault/test/foundry/utils/BasePoolTest.sol | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/pkg/vault/test/foundry/utils/BasePoolTest.sol b/pkg/vault/test/foundry/utils/BasePoolTest.sol index 7b2fa9622..d56a4255c 100644 --- a/pkg/vault/test/foundry/utils/BasePoolTest.sol +++ b/pkg/vault/test/foundry/utils/BasePoolTest.sol @@ -251,16 +251,14 @@ abstract contract BasePoolTest is BaseVaultTest { } function testSetSwapFeeTooLow() public { - address swapFeeManager = _getSwapFeeAdmin(); - vm.prank(swapFeeManager); + vm.prank(_getSwapFeeAdmin()); vm.expectRevert(IVaultErrors.SwapFeePercentageTooLow.selector); vault.setStaticSwapFeePercentage(pool, poolMinSwapFeePercentage - 1); } function testSetSwapFeeTooHigh() public { - address swapFeeManager = _getSwapFeeAdmin(); - vm.prank(swapFeeManager); + vm.prank(_getSwapFeeAdmin()); vm.expectRevert(abi.encodeWithSelector(IVaultErrors.SwapFeePercentageTooHigh.selector)); vault.setStaticSwapFeePercentage(pool, poolMaxSwapFeePercentage + 1); @@ -301,14 +299,13 @@ abstract contract BasePoolTest is BaseVaultTest { function _getSwapFeeAdmin() internal returns (address) { PoolRoleAccounts memory roleAccounts = vault.getPoolRoleAccounts(pool); - address swapFeeManager; + address swapFeeManager = roleAccounts.swapFeeManager; - if (roleAccounts.swapFeeManager != address(0)) { - return roleAccounts.swapFeeManager; - } else { + if (swapFeeManager == address(0)) { swapFeeManager = alice; authorizer.grantRole(vault.getActionId(IVaultAdmin.setStaticSwapFeePercentage.selector), swapFeeManager); - return swapFeeManager; } + return swapFeeManager; + } } From c5fc58e1d14b0305b02bdfea70ea57d67ee09fbe Mon Sep 17 00:00:00 2001 From: gerg Date: Wed, 25 Sep 2024 14:41:04 -0400 Subject: [PATCH 37/54] lint --- pkg/vault/test/foundry/utils/BasePoolTest.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/vault/test/foundry/utils/BasePoolTest.sol b/pkg/vault/test/foundry/utils/BasePoolTest.sol index d56a4255c..cdefc489b 100644 --- a/pkg/vault/test/foundry/utils/BasePoolTest.sol +++ b/pkg/vault/test/foundry/utils/BasePoolTest.sol @@ -306,6 +306,5 @@ abstract contract BasePoolTest is BaseVaultTest { authorizer.grantRole(vault.getActionId(IVaultAdmin.setStaticSwapFeePercentage.selector), swapFeeManager); } return swapFeeManager; - } } From eb7e4f46ceca8246572c5bbcadee16483bb7dc3d Mon Sep 17 00:00:00 2001 From: Jeff Bennett Date: Wed, 9 Oct 2024 12:45:38 -0700 Subject: [PATCH 38/54] chore: update gas --- ...atchRouter] swapExactIn - with buffer liquidity - warm slots | 2 +- ...BatchRouter] swapExactOut - no buffer liquidity - warm slots | 2 +- ...With rate] swap single token exact in with fees - cold slots | 2 +- ...With rate] swap single token exact in with fees - warm slots | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - ERC4626 - BatchRouter] swapExactIn - with buffer liquidity - warm slots b/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - ERC4626 - BatchRouter] swapExactIn - with buffer liquidity - warm slots index 6ba047320..e54e23c5a 100644 --- a/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - ERC4626 - BatchRouter] swapExactIn - with buffer liquidity - warm slots +++ b/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - ERC4626 - BatchRouter] swapExactIn - with buffer liquidity - warm slots @@ -1 +1 @@ -230.8k \ No newline at end of file +230.9k \ No newline at end of file diff --git a/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - ERC4626 - BatchRouter] swapExactOut - no buffer liquidity - warm slots b/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - ERC4626 - BatchRouter] swapExactOut - no buffer liquidity - warm slots index 96fecf1be..8a42d2a95 100644 --- a/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - ERC4626 - BatchRouter] swapExactOut - no buffer liquidity - warm slots +++ b/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - ERC4626 - BatchRouter] swapExactOut - no buffer liquidity - warm slots @@ -1 +1 @@ -317.7k \ No newline at end of file +317.8k \ No newline at end of file diff --git a/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - With rate] swap single token exact in with fees - cold slots b/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - With rate] swap single token exact in with fees - cold slots index cdee8effd..2047abbb9 100644 --- a/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - With rate] swap single token exact in with fees - cold slots +++ b/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - With rate] swap single token exact in with fees - cold slots @@ -1 +1 @@ -201.2k \ No newline at end of file +201.3k \ No newline at end of file diff --git a/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - With rate] swap single token exact in with fees - warm slots b/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - With rate] swap single token exact in with fees - warm slots index b6426f9ae..60bcf4864 100644 --- a/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - With rate] swap single token exact in with fees - warm slots +++ b/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - With rate] swap single token exact in with fees - warm slots @@ -1 +1 @@ -167.0k \ No newline at end of file +167.1k \ No newline at end of file From 4ddeaa062e193f09f2cefc893b1bc8de49654191 Mon Sep 17 00:00:00 2001 From: Jeff Bennett Date: Wed, 27 Nov 2024 10:57:13 -0500 Subject: [PATCH 39/54] fix: hardhat router deployment --- pkg/pool-weighted/test/LBPool.test.ts | 3 ++- pkg/pool-weighted/test/WeightedPool.test.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/pool-weighted/test/LBPool.test.ts b/pkg/pool-weighted/test/LBPool.test.ts index 239e66f79..5924f4c52 100644 --- a/pkg/pool-weighted/test/LBPool.test.ts +++ b/pkg/pool-weighted/test/LBPool.test.ts @@ -46,6 +46,7 @@ describe('LBPool', function () { const FACTORY_VERSION = 'LBPool Factory v1'; const POOL_VERSION = 'LBPool v1'; + const ROUTER_VERSION = 'Router v11'; const WEIGHTS = [fp(0.5), fp(0.5)]; const INITIAL_BALANCES = [TOKEN_AMOUNT, TOKEN_AMOUNT]; @@ -62,7 +63,7 @@ describe('LBPool', function () { const WETH = await deploy('v3-solidity-utils/WETHTestToken'); permit2 = await deployPermit2(); - router = await deploy('v3-vault/Router', { args: [vault, WETH, permit2] }); + router = await deploy('v3-vault/Router', { args: [vault, WETH, permit2, ROUTER_VERSION] }); tokenA = await deploy('v3-solidity-utils/ERC20TestToken', { args: ['Token A', 'TKNA', 18] }); tokenB = await deploy('v3-solidity-utils/ERC20TestToken', { args: ['Token B', 'TKNB', 6] }); diff --git a/pkg/pool-weighted/test/WeightedPool.test.ts b/pkg/pool-weighted/test/WeightedPool.test.ts index 6a724394b..3a044b196 100644 --- a/pkg/pool-weighted/test/WeightedPool.test.ts +++ b/pkg/pool-weighted/test/WeightedPool.test.ts @@ -30,7 +30,7 @@ import { TokenConfigStruct } from '../typechain-types/@balancer-labs/v3-interfac describe('WeightedPool', function () { const FACTORY_VERSION = 'Weighted Factory v1'; const POOL_VERSION = 'Weighted Pool v1'; - const ROUTER_VERSION = 'Router v9'; + const ROUTER_VERSION = 'Router v11'; const POOL_SWAP_FEE = fp(0.01); const TOKEN_AMOUNT = fp(100); From 9405b036eaf59282da40dcd0f4294d2b5c46ff1e Mon Sep 17 00:00:00 2001 From: Jeff Bennett Date: Wed, 27 Nov 2024 11:22:54 -0500 Subject: [PATCH 40/54] fix: test that triggered roundtrip fee --- pkg/pool-weighted/test/foundry/LBPool.t.sol | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/pool-weighted/test/foundry/LBPool.t.sol b/pkg/pool-weighted/test/foundry/LBPool.t.sol index 2365e694d..d13dba861 100644 --- a/pkg/pool-weighted/test/foundry/LBPool.t.sol +++ b/pkg/pool-weighted/test/foundry/LBPool.t.sol @@ -196,6 +196,9 @@ contract LBPoolTest is BasePoolTest { minAmountsOut[i] = less(tokenAmounts[i], 1e4); } + // Prevent roundtrip fee + vault.manualSetAddLiquidityCalledFlag(pool, false); + uint256[] memory amountsOut = router.removeLiquidityProportional( pool, bptAmountIn, From bf4a35b3680bc275cd3f7627c2b77160f9e5e733 Mon Sep 17 00:00:00 2001 From: Jeff Bennett Date: Mon, 9 Dec 2024 09:52:21 -0500 Subject: [PATCH 41/54] fix: tests --- pkg/pool-weighted/test/foundry/LBPool.t.sol | 65 +++++++++++++-------- 1 file changed, 41 insertions(+), 24 deletions(-) diff --git a/pkg/pool-weighted/test/foundry/LBPool.t.sol b/pkg/pool-weighted/test/foundry/LBPool.t.sol index d13dba861..f1d6d76f6 100644 --- a/pkg/pool-weighted/test/foundry/LBPool.t.sol +++ b/pkg/pool-weighted/test/foundry/LBPool.t.sol @@ -4,9 +4,12 @@ pragma solidity ^0.8.24; import "forge-std/Test.sol"; -import { Strings } from "@openzeppelin/contracts/utils/Strings.sol"; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import { Create2 } from "@openzeppelin/contracts/utils/Create2.sol"; +import { Strings } from "@openzeppelin/contracts/utils/Strings.sol"; +import { IBasePoolFactory } from "@balancer-labs/v3-interfaces/contracts/vault/IBasePoolFactory.sol"; +import { IVaultErrors } from "@balancer-labs/v3-interfaces/contracts/vault/IVaultErrors.sol"; import { IBasePool } from "@balancer-labs/v3-interfaces/contracts/vault/IBasePool.sol"; import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol"; import { @@ -15,22 +18,20 @@ import { PoolSwapParams, SwapKind } from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; -import { IVaultErrors } from "@balancer-labs/v3-interfaces/contracts/vault/IVaultErrors.sol"; -import { IBasePoolFactory } from "@balancer-labs/v3-interfaces/contracts/vault/IBasePoolFactory.sol"; import { CastingHelpers } from "@balancer-labs/v3-solidity-utils/contracts/helpers/CastingHelpers.sol"; -import { ArrayHelpers } from "@balancer-labs/v3-solidity-utils/contracts/test/ArrayHelpers.sol"; import { InputHelpers } from "@balancer-labs/v3-solidity-utils/contracts/helpers/InputHelpers.sol"; -import { PoolHooksMock } from "@balancer-labs/v3-vault/contracts/test/PoolHooksMock.sol"; +import { ArrayHelpers } from "@balancer-labs/v3-solidity-utils/contracts/test/ArrayHelpers.sol"; import { FixedPoint } from "@balancer-labs/v3-solidity-utils/contracts/math/FixedPoint.sol"; import { BasePoolTest } from "@balancer-labs/v3-vault/test/foundry/utils/BasePoolTest.sol"; +import { PoolHooksMock } from "@balancer-labs/v3-vault/contracts/test/PoolHooksMock.sol"; import { RouterMock } from "@balancer-labs/v3-vault/contracts/test/RouterMock.sol"; import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; import { LBPoolFactory } from "../../contracts/lbp/LBPoolFactory.sol"; -import { LBPool } from "../../contracts/lbp/LBPool.sol"; import { WeightedPool } from "../../contracts/WeightedPool.sol"; +import { LBPool } from "../../contracts/lbp/LBPool.sol"; contract LBPoolTest is BasePoolTest { using CastingHelpers for address[]; @@ -58,7 +59,7 @@ contract LBPoolTest is BasePoolTest { poolMaxSwapFeePercentage = 10e16; } - function createPool() internal override returns (address) { + function createPool() internal override returns (address newPool, bytes memory poolArgs) { IERC20[] memory sortedTokens = InputHelpers.sortTokens( [address(dai), address(usdc)].toMemoryArray().asIERC20() ); @@ -67,25 +68,41 @@ contract LBPoolTest is BasePoolTest { tokenAmounts.push(TOKEN_AMOUNT); } - factory = new LBPoolFactory(IVault(address(vault)), 365 days, "Factory v1", "Pool v1", address(router)); + string memory poolVersion = "Pool v1"; + + factory = new LBPoolFactory(IVault(address(vault)), 365 days, "Factory v1", poolVersion, address(router)); weights = [uint256(50e16), uint256(50e16)].toMemoryArray(); // Allow pools created by `factory` to use poolHooksMock hooks PoolHooksMock(poolHooksContract).allowFactory(address(factory)); - LBPool newPool = LBPool( - LBPoolFactory(address(factory)).create( - "LB Pool", - "LBPOOL", - vault.buildTokenConfig(sortedTokens), - weights, - DEFAULT_SWAP_FEE, - bob, - true, - ZERO_BYTES32 - ) + string memory name = "LB Pool"; + string memory symbol = "LB_POOL"; + + poolArgs = abi.encode( + WeightedPool.NewPoolParams({ + name: name, + symbol: symbol, + numTokens: sortedTokens.length, + normalizedWeights: weights, + version: poolVersion + }), + vault, + bob, + true, + address(router) + ); + + newPool = LBPoolFactory(address(factory)).create( + name, + symbol, + vault.buildTokenConfig(sortedTokens), + weights, + DEFAULT_SWAP_FEE, + bob, + true, + ZERO_BYTES32 ); - return address(newPool); } function testInitialize() public view override { @@ -505,7 +522,7 @@ contract LBPoolTest is BasePoolTest { // Attempt to create a pool with 1 token // Doesn't throw InputHelpers.InputLengthMismatch.selector b/c create3 intercepts error - vm.expectRevert("DEPLOYMENT_FAILED"); + vm.expectRevert(Create2.Create2FailedDeployment.selector); LBPoolFactory(address(factory)).create( "Invalid Pool 1", "IP1", @@ -519,7 +536,7 @@ contract LBPoolTest is BasePoolTest { // Attempt to create a pool with 3 tokens // Doesn't throw InputHelpers.InputLengthMismatch.selector b/c create3 intercepts error - vm.expectRevert("DEPLOYMENT_FAILED"); + vm.expectRevert(Create2.Create2FailedDeployment.selector); LBPoolFactory(address(factory)).create( "Invalid Pool 3", "IP3", @@ -535,7 +552,7 @@ contract LBPoolTest is BasePoolTest { function testMismatchedWeightsAndTokens() public { TokenConfig[] memory tokenConfig = vault.buildTokenConfig(poolTokens); - vm.expectRevert("DEPLOYMENT_FAILED"); + vm.expectRevert(Create2.Create2FailedDeployment.selector); LBPoolFactory(address(factory)).create( "Mismatched Pool", "MP", @@ -631,7 +648,7 @@ contract LBPoolTest is BasePoolTest { PoolSwapParams memory request = PoolSwapParams({ kind: SwapKind.EXACT_IN, amountGivenScaled18: 1e18, - balancesScaled18: new uint256[](3), // add an extra (non-existant) value to give the bad index a balance + balancesScaled18: new uint256[](3), // add an extra (non-existent) value to give the bad index a balance indexIn: 2, // Invalid token index indexOut: 0, router: address(router), From 93246a9570657d982684bd076d2036a6c9a39581 Mon Sep 17 00:00:00 2001 From: gerg Date: Wed, 18 Dec 2024 00:18:01 -0500 Subject: [PATCH 42/54] invert logic in onBeforeAddLiquidity to be more readable --- pkg/pool-weighted/contracts/lbp/LBPool.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/pool-weighted/contracts/lbp/LBPool.sol b/pkg/pool-weighted/contracts/lbp/LBPool.sol index 59a7e0f55..b8e65184a 100644 --- a/pkg/pool-weighted/contracts/lbp/LBPool.sol +++ b/pkg/pool-weighted/contracts/lbp/LBPool.sol @@ -224,10 +224,10 @@ contract LBPool is WeightedPool, Ownable, BaseHooks { uint256[] memory, bytes memory ) public view override onlyVault returns (bool) { - if (router == _TRUSTED_ROUTER) { - return IRouterCommon(router).getSender() == owner(); + if (router != _TRUSTED_ROUTER) { + revert RouterNotTrusted(); } - revert RouterNotTrusted(); + return IRouterCommon(router).getSender() == owner(); } /******************************************************************************* From 8a86a64fb311c89afc8a2f5c44b645e279e77e41 Mon Sep 17 00:00:00 2001 From: gerg Date: Wed, 18 Dec 2024 00:29:30 -0500 Subject: [PATCH 43/54] change Ownable to Ownable2Step --- pkg/pool-weighted/contracts/lbp/LBPool.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/pool-weighted/contracts/lbp/LBPool.sol b/pkg/pool-weighted/contracts/lbp/LBPool.sol index b8e65184a..a1d966c33 100644 --- a/pkg/pool-weighted/contracts/lbp/LBPool.sol +++ b/pkg/pool-weighted/contracts/lbp/LBPool.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.24; import { SafeCast } from "@openzeppelin/contracts/utils/math/SafeCast.sol"; import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; +import { Ownable2Step } from "@openzeppelin/contracts/access/Ownable2Step.sol"; import { IBasePoolFactory } from "@balancer-labs/v3-interfaces/contracts/vault/IBasePoolFactory.sol"; import { IRouterCommon } from "@balancer-labs/v3-interfaces/contracts/vault/IRouterCommon.sol"; @@ -24,7 +25,7 @@ import { WeightedPool } from "../WeightedPool.sol"; * which will not be used later), and it is tremendously helpful for pool validation and any potential future * base contract changes. */ -contract LBPool is WeightedPool, Ownable, BaseHooks { +contract LBPool is WeightedPool, Ownable2Step, BaseHooks { using SafeCast for *; // Since we have max 2 tokens and the weights must sum to 1, we only need to store one weight. From 2f0acafcdc84e0945ff2332b815a9378064f76a8 Mon Sep 17 00:00:00 2001 From: gerg Date: Wed, 18 Dec 2024 00:57:06 -0500 Subject: [PATCH 44/54] use MINUTE instead of 60 --- pkg/pool-weighted/test/LBPool.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/pool-weighted/test/LBPool.test.ts b/pkg/pool-weighted/test/LBPool.test.ts index 5924f4c52..8481d6f4b 100644 --- a/pkg/pool-weighted/test/LBPool.test.ts +++ b/pkg/pool-weighted/test/LBPool.test.ts @@ -247,7 +247,7 @@ describe('LBPool', function () { it('should always sum weights to 1', async () => { const currentTime = await time.latest(); - const startTime = currentTime + 60; // Set startTime 60 seconds in the future + const startTime = currentTime + MINUTE; // Set startTime 1 min in the future const endTime = startTime + MONTH; const startWeights = [fp(0.5), fp(0.5)]; const endWeights = [fp(0.7), fp(0.3)]; From c604ba210395b0610a2073125c964b7be41b7a40 Mon Sep 17 00:00:00 2001 From: Jeff Bennett Date: Tue, 31 Dec 2024 17:14:42 -0500 Subject: [PATCH 45/54] test: add missing import --- pkg/pool-weighted/test/LBPool.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/pool-weighted/test/LBPool.test.ts b/pkg/pool-weighted/test/LBPool.test.ts index 8481d6f4b..249813337 100644 --- a/pkg/pool-weighted/test/LBPool.test.ts +++ b/pkg/pool-weighted/test/LBPool.test.ts @@ -13,7 +13,7 @@ import TypesConverter from '@balancer-labs/v3-helpers/src/models/types/TypesConv import { buildTokenConfig } from '@balancer-labs/v3-helpers/src/models/tokens/tokenConfig'; import { LBPool, LBPoolFactory } from '../typechain-types'; import { actionId } from '@balancer-labs/v3-helpers/src/models/misc/actions'; -import { MONTH } from '@balancer-labs/v3-helpers/src/time'; +import { MONTH, MINUTE } from '@balancer-labs/v3-helpers/src/time'; import * as expectEvent from '@balancer-labs/v3-helpers/src/test/expectEvent'; import { sortAddresses } from '@balancer-labs/v3-helpers/src/models/tokens/sortingHelper'; import { deployPermit2 } from '@balancer-labs/v3-vault/test/Permit2Deployer'; From 9b64acbd04e5ba3a2fba36ad48d59c2757821e7b Mon Sep 17 00:00:00 2001 From: Jeff Bennett Date: Tue, 31 Dec 2024 17:14:56 -0500 Subject: [PATCH 46/54] test: adjust to new pool factory function --- pkg/pool-weighted/test/foundry/LBPool.t.sol | 39 ++++++++++++++------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/pkg/pool-weighted/test/foundry/LBPool.t.sol b/pkg/pool-weighted/test/foundry/LBPool.t.sol index f1d6d76f6..3c355bc33 100644 --- a/pkg/pool-weighted/test/foundry/LBPool.t.sol +++ b/pkg/pool-weighted/test/foundry/LBPool.t.sol @@ -41,6 +41,9 @@ contract LBPoolTest is BasePoolTest { uint256 constant DEFAULT_SWAP_FEE = 1e16; // 1% uint256 constant TOKEN_AMOUNT = 1e3 * 1e18; + string constant factoryVersion = "Factory v1"; + string constant poolVersion = "Pool v1"; + uint256[] internal weights; uint256 internal daiIdx; @@ -59,6 +62,19 @@ contract LBPoolTest is BasePoolTest { poolMaxSwapFeePercentage = 10e16; } + function createPoolFactory() internal override returns (address) { + LBPoolFactory factory = new LBPoolFactory( + IVault(address(vault)), + 365 days, + factoryVersion, + poolVersion, + address(router) + ); + vm.label(address(factory), "LBPoolFactory"); + + return address(factory); + } + function createPool() internal override returns (address newPool, bytes memory poolArgs) { IERC20[] memory sortedTokens = InputHelpers.sortTokens( [address(dai), address(usdc)].toMemoryArray().asIERC20() @@ -68,13 +84,10 @@ contract LBPoolTest is BasePoolTest { tokenAmounts.push(TOKEN_AMOUNT); } - string memory poolVersion = "Pool v1"; - - factory = new LBPoolFactory(IVault(address(vault)), 365 days, "Factory v1", poolVersion, address(router)); weights = [uint256(50e16), uint256(50e16)].toMemoryArray(); - // Allow pools created by `factory` to use poolHooksMock hooks - PoolHooksMock(poolHooksContract).allowFactory(address(factory)); + // Allow pools created by `poolFactory` to use poolHooksMock hooks + PoolHooksMock(poolHooksContract).allowFactory(poolFactory); string memory name = "LB Pool"; string memory symbol = "LB_POOL"; @@ -93,7 +106,7 @@ contract LBPoolTest is BasePoolTest { address(router) ); - newPool = LBPoolFactory(address(factory)).create( + newPool = LBPoolFactory(poolFactory).create( name, symbol, vault.buildTokenConfig(sortedTokens), @@ -111,7 +124,7 @@ contract LBPoolTest is BasePoolTest { for (uint256 i = 0; i < poolTokens.length; ++i) { // Tokens are transferred from bob (lp/owner) assertEq( - defaultBalance - poolTokens[i].balanceOf(bob), + defaultAccountBalance() - poolTokens[i].balanceOf(bob), tokenAmounts[i], string.concat("LP: Wrong balance for ", Strings.toString(i)) ); @@ -159,7 +172,7 @@ contract LBPoolTest is BasePoolTest { for (uint256 i = 0; i < poolTokens.length; ++i) { // Tokens are transferred from Bob assertEq( - defaultBalance - poolTokens[i].balanceOf(bob), + defaultAccountBalance() - poolTokens[i].balanceOf(bob), tokenAmounts[i] * 2, // x2 because bob (as owner) did init join and subsequent join string.concat("LP: Wrong token balance for ", Strings.toString(i)) ); @@ -232,7 +245,7 @@ contract LBPoolTest is BasePoolTest { // Tokens are transferred to Bob assertApproxEqAbs( poolTokens[i].balanceOf(bob) + TOKEN_AMOUNT, //add TOKEN_AMOUNT to account for init join - defaultBalance, + defaultAccountBalance(), DELTA, string.concat("LP: Wrong token balance for ", Strings.toString(i)) ); @@ -523,7 +536,7 @@ contract LBPoolTest is BasePoolTest { // Attempt to create a pool with 1 token // Doesn't throw InputHelpers.InputLengthMismatch.selector b/c create3 intercepts error vm.expectRevert(Create2.Create2FailedDeployment.selector); - LBPoolFactory(address(factory)).create( + LBPoolFactory(poolFactory).create( "Invalid Pool 1", "IP1", tokenConfig1, @@ -537,7 +550,7 @@ contract LBPoolTest is BasePoolTest { // Attempt to create a pool with 3 tokens // Doesn't throw InputHelpers.InputLengthMismatch.selector b/c create3 intercepts error vm.expectRevert(Create2.Create2FailedDeployment.selector); - LBPoolFactory(address(factory)).create( + LBPoolFactory(poolFactory).create( "Invalid Pool 3", "IP3", tokenConfig3, @@ -553,7 +566,7 @@ contract LBPoolTest is BasePoolTest { TokenConfig[] memory tokenConfig = vault.buildTokenConfig(poolTokens); vm.expectRevert(Create2.Create2FailedDeployment.selector); - LBPoolFactory(address(factory)).create( + LBPoolFactory(poolFactory).create( "Mismatched Pool", "MP", tokenConfig, @@ -567,7 +580,7 @@ contract LBPoolTest is BasePoolTest { function testInitializedWithSwapsDisabled() public { LBPool swapsDisabledPool = LBPool( - LBPoolFactory(address(factory)).create( + LBPoolFactory(poolFactory).create( "Swaps Disabled Pool", "SDP", vault.buildTokenConfig(poolTokens), From 5de89455478743f46d5c58d1fd11684717cbbb11 Mon Sep 17 00:00:00 2001 From: Jeff Bennett Date: Tue, 31 Dec 2024 17:30:08 -0500 Subject: [PATCH 47/54] chore: update gas --- ...tchRouter] swapExactOut - with buffer liquidity - warm slots | 2 +- .../[WeightedPool - Standard] initialize with ETH | 2 +- ...BatchRouter] remove liquidity using swapExactIn - warm slots | 2 +- ...ithRate] remove liquidity single token exact in - warm slots | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - ERC4626 - BatchRouter] swapExactOut - with buffer liquidity - warm slots b/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - ERC4626 - BatchRouter] swapExactOut - with buffer liquidity - warm slots index 5e97e74e1..74805369f 100644 --- a/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - ERC4626 - BatchRouter] swapExactOut - with buffer liquidity - warm slots +++ b/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - ERC4626 - BatchRouter] swapExactOut - with buffer liquidity - warm slots @@ -1 +1 @@ -244.7k \ No newline at end of file +244.8k \ No newline at end of file diff --git a/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - Standard] initialize with ETH b/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - Standard] initialize with ETH index 13e58083b..6afcf8ec2 100644 --- a/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - Standard] initialize with ETH +++ b/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - Standard] initialize with ETH @@ -1 +1 @@ -348.8k \ No newline at end of file +348.9k \ No newline at end of file diff --git a/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - WithRate - BatchRouter] remove liquidity using swapExactIn - warm slots b/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - WithRate - BatchRouter] remove liquidity using swapExactIn - warm slots index 04c51970a..a5dc1053f 100644 --- a/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - WithRate - BatchRouter] remove liquidity using swapExactIn - warm slots +++ b/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - WithRate - BatchRouter] remove liquidity using swapExactIn - warm slots @@ -1 +1 @@ -198.2k \ No newline at end of file +198.3k \ No newline at end of file diff --git a/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - WithRate] remove liquidity single token exact in - warm slots b/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - WithRate] remove liquidity single token exact in - warm slots index d3fa14d41..61e5d8518 100644 --- a/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - WithRate] remove liquidity single token exact in - warm slots +++ b/pkg/pool-weighted/test/gas/.hardhat-snapshots/[WeightedPool - WithRate] remove liquidity single token exact in - warm slots @@ -1 +1 @@ -174.0k \ No newline at end of file +174.1k \ No newline at end of file From 46719fcea161acc3fb80455e6f277c7b07f13da4 Mon Sep 17 00:00:00 2001 From: Juan Ignacio Ubeira Date: Thu, 2 Jan 2025 11:54:13 -0300 Subject: [PATCH 48/54] First approximation to create and init. --- pkg/pool-weighted/contracts/lbp/LBPool.sol | 12 +- .../contracts/lbp/LBPoolFactory.sol | 125 ++++++++++++++++-- pkg/pool-weighted/test/foundry/LBPool.t.sol | 3 +- .../test/foundry/LBPoolFactory.t.sol | 56 +++++++- 4 files changed, 178 insertions(+), 18 deletions(-) diff --git a/pkg/pool-weighted/contracts/lbp/LBPool.sol b/pkg/pool-weighted/contracts/lbp/LBPool.sol index a1d966c33..b35935ed9 100644 --- a/pkg/pool-weighted/contracts/lbp/LBPool.sol +++ b/pkg/pool-weighted/contracts/lbp/LBPool.sol @@ -100,6 +100,11 @@ contract LBPool is WeightedPool, Ownable2Step, BaseHooks { _setSwapEnabled(swapEnabledOnStart); } + /// @notice Returns trusted router, which is the gateway to add liquidity to the pool. + function getTrustedRouter() external view returns (address) { + return _TRUSTED_ROUTER; + } + /** * @notice Return start time, end time, and endWeights as an array. * @dev Current weights should be retrieved via `getNormalizedWeights()`. @@ -194,12 +199,11 @@ contract LBPool is WeightedPool, Ownable2Step, BaseHooks { function onRegister( address, address pool, - TokenConfig[] memory, + TokenConfig[] memory tokenConfig, LiquidityManagement calldata ) public view override onlyVault returns (bool) { - // Since in this case the pool is the hook, we don't need to check anything else. - // We *could* check that it's two tokens, but better to let that be caught later, as it will fail with a more - // descriptive error. + InputHelpers.ensureInputLengthMatch(_NUM_TOKENS, tokenConfig.length); + return pool == address(this); } diff --git a/pkg/pool-weighted/contracts/lbp/LBPoolFactory.sol b/pkg/pool-weighted/contracts/lbp/LBPoolFactory.sol index 797270aa0..fea038287 100644 --- a/pkg/pool-weighted/contracts/lbp/LBPoolFactory.sol +++ b/pkg/pool-weighted/contracts/lbp/LBPoolFactory.sol @@ -2,12 +2,22 @@ pragma solidity ^0.8.24; +import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; +import { SafeCast } from "@openzeppelin/contracts/utils/math/SafeCast.sol"; +import { IPermit2 } from "permit2/src/interfaces/IPermit2.sol"; + import { IPoolVersion } from "@balancer-labs/v3-interfaces/contracts/solidity-utils/helpers/IPoolVersion.sol"; +import { IRouter } from "@balancer-labs/v3-interfaces/contracts/vault/IRouter.sol"; import { TokenConfig, PoolRoleAccounts } from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol"; import { BasePoolFactory } from "@balancer-labs/v3-pool-utils/contracts/BasePoolFactory.sol"; +import { InputHelpers } from "@balancer-labs/v3-solidity-utils/contracts/helpers/InputHelpers.sol"; import { Version } from "@balancer-labs/v3-solidity-utils/contracts/helpers/Version.sol"; +import { + ReentrancyGuardTransient +} from "@balancer-labs/v3-solidity-utils/contracts/openzeppelin/ReentrancyGuardTransient.sol"; import { WeightedPool } from "../WeightedPool.sol"; import { LBPool } from "./LBPool.sol"; @@ -16,23 +26,31 @@ import { LBPool } from "./LBPool.sol"; * @notice LBPool Factory. * @dev This is a factory specific to LBPools, allowing only 2 tokens. */ -contract LBPoolFactory is IPoolVersion, BasePoolFactory, Version { +contract LBPoolFactory is IPoolVersion, ReentrancyGuardTransient, BasePoolFactory, Version { + using SafeERC20 for IERC20; + using SafeCast for uint256; + + // LBPs are constrained to two tokens. + uint256 private constant _NUM_TOKENS = 2; + string private _poolVersion; - // solhint-disable-next-line var-name-mixedcase - address internal immutable _TRUSTED_ROUTER; + address internal immutable _trustedRouter; + IPermit2 internal immutable _permit2; constructor( IVault vault, uint32 pauseWindowDuration, string memory factoryVersion, string memory poolVersion, - address trustedRouter + address trustedRouter, + IPermit2 permit2 ) BasePoolFactory(vault, pauseWindowDuration, type(LBPool).creationCode) Version(factoryVersion) { _poolVersion = poolVersion; // LBPools are deployed with a router known to reliably report the originating address on operations. - _TRUSTED_ROUTER = trustedRouter; + _trustedRouter = trustedRouter; + _permit2 = permit2; } /// @inheritdoc IPoolVersion @@ -40,12 +58,22 @@ contract LBPoolFactory is IPoolVersion, BasePoolFactory, Version { return _poolVersion; } + /// @notice Returns trusted router, which is the gateway to add liquidity to the pool. + function getTrustedRouter() external view returns (address) { + return _trustedRouter; + } + + /// @notice Returns permit2 address, used to initialize pools. + function getPermit2() external view returns (IPermit2) { + return _permit2; + } + /** * @notice Deploys a new `LBPool`. * @dev Tokens must be sorted for pool registration. * @param name The name of the pool * @param symbol The symbol of the pool - * @param tokens An array of descriptors for the tokens the pool will manage + * @param tokenConfig An array of descriptors for the tokens the pool will manage * @param normalizedWeights The pool weights (must add to FixedPoint.ONE) * @param swapFeePercentage Initial swap fee percentage * @param owner The owner address for pool; sole LP with swapEnable/swapFee change permissions @@ -54,13 +82,86 @@ contract LBPoolFactory is IPoolVersion, BasePoolFactory, Version { function create( string memory name, string memory symbol, - TokenConfig[] memory tokens, + TokenConfig[] memory tokenConfig, uint256[] memory normalizedWeights, uint256 swapFeePercentage, address owner, bool swapEnabledOnStart, bytes32 salt - ) external returns (address pool) { + ) external nonReentrant returns (address pool) { + return + _create(name, symbol, tokenConfig, normalizedWeights, swapFeePercentage, owner, swapEnabledOnStart, salt); + } + + /** + * @notice Deploys a new `LBPool` and seeds it with initial liquidity in the same tx. + * @dev Tokens must be sorted for pool registration. + * Use this method in case pool initialization frontrunning is an issue. + * If the owner is the only address with liquidity of one of the tokens, this should not be necessary. + * This method does not support native ETH management; WETH needs to be used instead. + * @param name The name of the pool + * @param symbol The symbol of the pool + * @param tokenConfig An array of descriptors for the tokenConfig the pool will manage + * @param normalizedWeights The pool weights (must add to FixedPoint.ONE) + * @param swapFeePercentage Initial swap fee percentage + * @param owner The owner address for pool; sole LP with swapEnable/swapFee change permissions + * @param salt The salt value that will be passed to create3 deployment + * @param exactAmountsIn Token amounts in, matching token order + */ + function createAndInitialize( + string memory name, + string memory symbol, + TokenConfig[] memory tokenConfig, + uint256[] memory normalizedWeights, + uint256 swapFeePercentage, + address owner, + bool swapEnabledOnStart, + bytes32 salt, + uint256[] memory exactAmountsIn + ) external nonReentrant returns (address pool) { + // `create` checks token config length already + pool = _create( + name, + symbol, + tokenConfig, + normalizedWeights, + swapFeePercentage, + owner, + swapEnabledOnStart, + salt + ); + + IERC20[] memory tokens = new IERC20[](_NUM_TOKENS); + for (uint256 i = 0; i < _NUM_TOKENS; ++i) { + tokens[i] = tokenConfig[i].token; + + // Pull necessary tokens and approve permit2 to use them via the router + tokens[i].safeTransferFrom(msg.sender, address(this), exactAmountsIn[i]); + tokens[i].forceApprove(address(_permit2), exactAmountsIn[i]); + _permit2.approve( + address(tokens[i]), + address(_trustedRouter), + exactAmountsIn[i].toUint160(), + type(uint48).max + ); + } + + IRouter(_trustedRouter).initialize(pool, tokens, exactAmountsIn, 0, false, ""); + } + + function _create( + string memory name, + string memory symbol, + TokenConfig[] memory tokenConfig, + uint256[] memory normalizedWeights, + uint256 swapFeePercentage, + address owner, + bool swapEnabledOnStart, + bytes32 salt + ) internal returns (address pool) { + InputHelpers.ensureInputLengthMatch(_NUM_TOKENS, tokenConfig.length); + InputHelpers.ensureInputLengthMatch(_NUM_TOKENS, normalizedWeights.length); + PoolRoleAccounts memory roleAccounts; // It's not necessary to set the pauseManager, as the owner can already effectively pause the pool by disabling // swaps. There is also no poolCreator, as the owner is already using this to earn revenue directly. @@ -71,23 +172,23 @@ contract LBPoolFactory is IPoolVersion, BasePoolFactory, Version { WeightedPool.NewPoolParams({ name: name, symbol: symbol, - numTokens: tokens.length, + numTokens: tokenConfig.length, normalizedWeights: normalizedWeights, version: _poolVersion }), getVault(), owner, swapEnabledOnStart, - _TRUSTED_ROUTER + _trustedRouter ), salt ); _registerPoolWithVault( pool, - tokens, + tokenConfig, swapFeePercentage, - true, // protocol fee exempt + false, // protocol fee exempt roleAccounts, pool, // register the pool itself as the hook contract getDefaultLiquidityManagement() diff --git a/pkg/pool-weighted/test/foundry/LBPool.t.sol b/pkg/pool-weighted/test/foundry/LBPool.t.sol index 3c355bc33..d5f41c695 100644 --- a/pkg/pool-weighted/test/foundry/LBPool.t.sol +++ b/pkg/pool-weighted/test/foundry/LBPool.t.sol @@ -68,7 +68,8 @@ contract LBPoolTest is BasePoolTest { 365 days, factoryVersion, poolVersion, - address(router) + address(router), + permit2 ); vm.label(address(factory), "LBPoolFactory"); diff --git a/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol b/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol index b31c7c325..411000c0f 100644 --- a/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol +++ b/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol @@ -6,6 +6,7 @@ import "forge-std/Test.sol"; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import { IRouter } from "@balancer-labs/v3-interfaces/contracts/vault/IRouter.sol"; import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol"; import { IVaultErrors } from "@balancer-labs/v3-interfaces/contracts/vault/IVaultErrors.sol"; import { PoolRoleAccounts } from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol"; @@ -29,7 +30,7 @@ contract LBPoolFactoryTest is BaseVaultTest { function setUp() public override { super.setUp(); - lbPoolFactory = new LBPoolFactory(IVault(address(vault)), 365 days, "Factory v1", poolVersion, address(router)); + lbPoolFactory = new LBPoolFactory(IVault(address(vault)), 365 days, "Factory v1", poolVersion, address(router), permit2); vm.label(address(lbPoolFactory), "LB pool factory"); } @@ -45,6 +46,59 @@ contract LBPoolFactoryTest is BaseVaultTest { assertTrue(vault.isPoolRegistered(lbPool), "Pool not registered in the vault"); } + function testCreateAndInitializePool() public { + vm.startPrank(bob); + + uint256 snapshotId = vm.snapshot(); + + IERC20[] memory tokens = [address(dai), address(usdc)].toMemoryArray().asIERC20(); + uint256[] memory weights = [uint256(50e16), uint256(50e16)].toMemoryArray(); + uint256[] memory exactAmountsIn = [poolInitAmount, poolInitAmount].toMemoryArray(); + + address expectedLbPoolAddress = lbPoolFactory.create( + "LB Pool", + "LBP", + vault.buildTokenConfig(tokens), + weights, + swapFee, + bob, // owner + true, // swapEnabledOnStart + ZERO_BYTES32 + ); + + vm.revertTo(snapshotId); + + dai.approve(address(lbPoolFactory), poolInitAmount); + usdc.approve(address(lbPoolFactory), poolInitAmount); + + vm.expectCall( + address(router), + abi.encodeCall(IRouter.initialize, (expectedLbPoolAddress, tokens, exactAmountsIn, 0, false, "")) + ); + address lbPool = lbPoolFactory.createAndInitialize( + "LB Pool", + "LBP", + vault.buildTokenConfig(tokens), + weights, + swapFee, + bob, // owner + true, // swapEnabledOnStart + ZERO_BYTES32, + exactAmountsIn + ); + vm.stopPrank(); + + // Verify pool was created and initialized correctly + assertTrue(vault.isPoolRegistered(lbPool), "Pool not registered in the vault"); + assertTrue(vault.isPoolInitialized(lbPool), "Pool not initialized in the vault"); + assertEq(expectedLbPoolAddress, lbPool, "Unexpected pool address"); + + (, , uint256[] memory balancesRaw, ) = vault.getPoolTokenInfo(lbPool); + assertEq(balancesRaw.length, 2, "Unexpected balances raw length"); + assertEq(balancesRaw[0], exactAmountsIn[0], "Unexpected balances raw [0]"); + assertEq(balancesRaw[1], exactAmountsIn[1], "Unexpected balances raw [1]"); + } + function testGetPoolVersion() public view { assert(keccak256(abi.encodePacked(lbPoolFactory.getPoolVersion())) == keccak256(abi.encodePacked(poolVersion))); } From be1c8e8438416539699c76115953d4ca10e713dd Mon Sep 17 00:00:00 2001 From: Juan Ignacio Ubeira Date: Thu, 2 Jan 2025 12:03:22 -0300 Subject: [PATCH 49/54] Improve errors, fix hardhat tests. --- pkg/pool-weighted/contracts/lib/GradualValueChange.sol | 4 ++-- pkg/pool-weighted/test/LBPool.test.ts | 2 +- pkg/pool-weighted/test/foundry/GradualValueChange.t.sol | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/pool-weighted/contracts/lib/GradualValueChange.sol b/pkg/pool-weighted/contracts/lib/GradualValueChange.sol index 04b5e9228..5e2dfa733 100644 --- a/pkg/pool-weighted/contracts/lib/GradualValueChange.sol +++ b/pkg/pool-weighted/contracts/lib/GradualValueChange.sol @@ -21,7 +21,7 @@ pragma solidity ^0.8.24; library GradualValueChange { /// @dev Indicates that the start time is after the end time - error GradualUpdateTimeTravel(); + error GradualUpdateTimeTravel(uint256 resolvedStartTime, uint256 endTime); using FixedPoint for uint256; @@ -43,7 +43,7 @@ library GradualValueChange { resolvedStartTime = Math.max(block.timestamp, startTime); if (resolvedStartTime > endTime) { - revert GradualUpdateTimeTravel(); + revert GradualUpdateTimeTravel(resolvedStartTime, endTime); } } diff --git a/pkg/pool-weighted/test/LBPool.test.ts b/pkg/pool-weighted/test/LBPool.test.ts index 249813337..53f1366a5 100644 --- a/pkg/pool-weighted/test/LBPool.test.ts +++ b/pkg/pool-weighted/test/LBPool.test.ts @@ -77,7 +77,7 @@ describe('LBPool', function () { sharedBeforeEach('create and initialize pool', async () => { factory = await deploy('LBPoolFactory', { - args: [await vault.getAddress(), MONTH * 12, FACTORY_VERSION, POOL_VERSION, router], + args: [await vault.getAddress(), MONTH * 12, FACTORY_VERSION, POOL_VERSION, router, permit2], }); poolTokens = sortAddresses([tokenAAddress, tokenBAddress]); diff --git a/pkg/pool-weighted/test/foundry/GradualValueChange.t.sol b/pkg/pool-weighted/test/foundry/GradualValueChange.t.sol index d3d7916d1..7bc85e67d 100644 --- a/pkg/pool-weighted/test/foundry/GradualValueChange.t.sol +++ b/pkg/pool-weighted/test/foundry/GradualValueChange.t.sol @@ -43,7 +43,7 @@ contract GradualValueChangeTest is Test { "Should return current time for past start time" ); - vm.expectRevert(GradualValueChange.GradualUpdateTimeTravel.selector); + vm.expectRevert(abi.encodeWithSelector(GradualValueChange.GradualUpdateTimeTravel.selector, futureTime + 200, futureTime + 100)); mock.resolveStartTime(futureTime + 200, futureTime + 100); } From dfd563c0ea1012b9297cf675b5efbddd6ff0da3c Mon Sep 17 00:00:00 2001 From: Juan Ignacio Ubeira Date: Thu, 2 Jan 2025 12:04:01 -0300 Subject: [PATCH 50/54] Lint. --- pkg/pool-weighted/test/foundry/GradualValueChange.t.sol | 8 +++++++- pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol | 9 ++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/pkg/pool-weighted/test/foundry/GradualValueChange.t.sol b/pkg/pool-weighted/test/foundry/GradualValueChange.t.sol index 7bc85e67d..7afcec209 100644 --- a/pkg/pool-weighted/test/foundry/GradualValueChange.t.sol +++ b/pkg/pool-weighted/test/foundry/GradualValueChange.t.sol @@ -43,7 +43,13 @@ contract GradualValueChangeTest is Test { "Should return current time for past start time" ); - vm.expectRevert(abi.encodeWithSelector(GradualValueChange.GradualUpdateTimeTravel.selector, futureTime + 200, futureTime + 100)); + vm.expectRevert( + abi.encodeWithSelector( + GradualValueChange.GradualUpdateTimeTravel.selector, + futureTime + 200, + futureTime + 100 + ) + ); mock.resolveStartTime(futureTime + 200, futureTime + 100); } diff --git a/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol b/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol index 411000c0f..c43718566 100644 --- a/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol +++ b/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol @@ -30,7 +30,14 @@ contract LBPoolFactoryTest is BaseVaultTest { function setUp() public override { super.setUp(); - lbPoolFactory = new LBPoolFactory(IVault(address(vault)), 365 days, "Factory v1", poolVersion, address(router), permit2); + lbPoolFactory = new LBPoolFactory( + IVault(address(vault)), + 365 days, + "Factory v1", + poolVersion, + address(router), + permit2 + ); vm.label(address(lbPoolFactory), "LB pool factory"); } From 4c29466b707f9782865e0d2ade2e44275f75c5b2 Mon Sep 17 00:00:00 2001 From: Juan Ignacio Ubeira Date: Thu, 2 Jan 2025 12:39:33 -0300 Subject: [PATCH 51/54] Fix tests. --- pkg/pool-weighted/test/foundry/LBPool.t.sol | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/pkg/pool-weighted/test/foundry/LBPool.t.sol b/pkg/pool-weighted/test/foundry/LBPool.t.sol index d5f41c695..009a0d462 100644 --- a/pkg/pool-weighted/test/foundry/LBPool.t.sol +++ b/pkg/pool-weighted/test/foundry/LBPool.t.sol @@ -227,9 +227,6 @@ contract LBPoolTest is BasePoolTest { minAmountsOut[i] = less(tokenAmounts[i], 1e4); } - // Prevent roundtrip fee - vault.manualSetAddLiquidityCalledFlag(pool, false); - uint256[] memory amountsOut = router.removeLiquidityProportional( pool, bptAmountIn, @@ -245,7 +242,7 @@ contract LBPoolTest is BasePoolTest { for (uint256 i = 0; i < poolTokens.length; ++i) { // Tokens are transferred to Bob assertApproxEqAbs( - poolTokens[i].balanceOf(bob) + TOKEN_AMOUNT, //add TOKEN_AMOUNT to account for init join + poolTokens[i].balanceOf(bob) + TOKEN_AMOUNT, // add TOKEN_AMOUNT to account for init join defaultAccountBalance(), DELTA, string.concat("LP: Wrong token balance for ", Strings.toString(i)) @@ -535,8 +532,7 @@ contract LBPoolTest is BasePoolTest { TokenConfig[] memory tokenConfig3 = vault.buildTokenConfig(sortedTokens3); // Attempt to create a pool with 1 token - // Doesn't throw InputHelpers.InputLengthMismatch.selector b/c create3 intercepts error - vm.expectRevert(Create2.Create2FailedDeployment.selector); + vm.expectRevert(InputHelpers.InputLengthMismatch.selector); LBPoolFactory(poolFactory).create( "Invalid Pool 1", "IP1", @@ -549,8 +545,7 @@ contract LBPoolTest is BasePoolTest { ); // Attempt to create a pool with 3 tokens - // Doesn't throw InputHelpers.InputLengthMismatch.selector b/c create3 intercepts error - vm.expectRevert(Create2.Create2FailedDeployment.selector); + vm.expectRevert(InputHelpers.InputLengthMismatch.selector); LBPoolFactory(poolFactory).create( "Invalid Pool 3", "IP3", @@ -566,7 +561,7 @@ contract LBPoolTest is BasePoolTest { function testMismatchedWeightsAndTokens() public { TokenConfig[] memory tokenConfig = vault.buildTokenConfig(poolTokens); - vm.expectRevert(Create2.Create2FailedDeployment.selector); + vm.expectRevert(InputHelpers.InputLengthMismatch.selector); LBPoolFactory(poolFactory).create( "Mismatched Pool", "MP", From b870b77eaeb64fec7a1b7cab3be1bda8391f9ea9 Mon Sep 17 00:00:00 2001 From: Juan Ignacio Ubeira Date: Thu, 2 Jan 2025 16:17:15 -0300 Subject: [PATCH 52/54] Address comments. --- pkg/pool-weighted/contracts/lbp/LBPool.sol | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/pool-weighted/contracts/lbp/LBPool.sol b/pkg/pool-weighted/contracts/lbp/LBPool.sol index b35935ed9..55ecf8d47 100644 --- a/pkg/pool-weighted/contracts/lbp/LBPool.sol +++ b/pkg/pool-weighted/contracts/lbp/LBPool.sol @@ -49,7 +49,7 @@ contract LBPool is WeightedPool, Ownable2Step, BaseHooks { // could spoof the address of the owner, allowing anyone to call permissioned functions. // solhint-disable-next-line var-name-mixedcase - address private immutable _TRUSTED_ROUTER; + address private immutable _trustedRouter; PoolState private _poolState; @@ -74,7 +74,7 @@ contract LBPool is WeightedPool, Ownable2Step, BaseHooks { uint256[] endWeights ); - /// @dev Indicates that the router that called the Vault is not trusted, so any operations should revert. + /// @dev Indicates that the router that called the Vault is not trusted, so liquidity operations should revert. error RouterNotTrusted(); /// @dev Indicates that the `owner` has disabled swaps. @@ -92,7 +92,7 @@ contract LBPool is WeightedPool, Ownable2Step, BaseHooks { InputHelpers.ensureInputLengthMatch(_NUM_TOKENS, params.numTokens); // Set the trusted router (passed down from the factory). - _TRUSTED_ROUTER = trustedRouter; + _trustedRouter = trustedRouter; // solhint-disable-next-line not-rely-on-time uint32 currentTime = block.timestamp.toUint32(); @@ -100,9 +100,9 @@ contract LBPool is WeightedPool, Ownable2Step, BaseHooks { _setSwapEnabled(swapEnabledOnStart); } - /// @notice Returns trusted router, which is the gateway to add liquidity to the pool. + /// @notice Returns the trusted router, which is the gateway to add liquidity to the pool. function getTrustedRouter() external view returns (address) { - return _TRUSTED_ROUTER; + return _trustedRouter; } /** @@ -229,7 +229,7 @@ contract LBPool is WeightedPool, Ownable2Step, BaseHooks { uint256[] memory, bytes memory ) public view override onlyVault returns (bool) { - if (router != _TRUSTED_ROUTER) { + if (router != _trustedRouter) { revert RouterNotTrusted(); } return IRouterCommon(router).getSender() == owner(); From 8ef057213b15d533faf7e7eb8416b5a1f1a225d7 Mon Sep 17 00:00:00 2001 From: Juan Ignacio Ubeira Date: Thu, 2 Jan 2025 16:27:42 -0300 Subject: [PATCH 53/54] Add test for getters. --- pkg/pool-weighted/test/foundry/LBPool.t.sol | 4 ++++ pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/pkg/pool-weighted/test/foundry/LBPool.t.sol b/pkg/pool-weighted/test/foundry/LBPool.t.sol index 009a0d462..2260a604c 100644 --- a/pkg/pool-weighted/test/foundry/LBPool.t.sol +++ b/pkg/pool-weighted/test/foundry/LBPool.t.sol @@ -119,6 +119,10 @@ contract LBPoolTest is BasePoolTest { ); } + function testGetTrustedRouter() public view { + assertEq(LBPool(pool).getTrustedRouter(), address(router), "Wrong trusted router"); + } + function testInitialize() public view override { (, , uint256[] memory balances, ) = vault.getPoolTokenInfo(address(pool)); diff --git a/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol b/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol index c43718566..16045a75f 100644 --- a/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol +++ b/pkg/pool-weighted/test/foundry/LBPoolFactory.t.sol @@ -41,6 +41,10 @@ contract LBPoolFactoryTest is BaseVaultTest { vm.label(address(lbPoolFactory), "LB pool factory"); } + function testGetTrustedRouter() public view { + assertEq(lbPoolFactory.getTrustedRouter(), address(router), "Wrong trusted router"); + } + function testFactoryPausedState() public view { uint32 pauseWindowDuration = lbPoolFactory.getPauseWindowDuration(); assertEq(pauseWindowDuration, 365 days); From 429c75a551365aecab0ff68bd1f48ef582e89aca Mon Sep 17 00:00:00 2001 From: Juan Ignacio Ubeira Date: Thu, 2 Jan 2025 19:42:06 -0300 Subject: [PATCH 54/54] Apply suggestions from code review Co-authored-by: EndymionJkb --- pkg/pool-weighted/contracts/lbp/LBPool.sol | 5 +++++ pkg/pool-weighted/contracts/lbp/LBPoolFactory.sol | 1 + 2 files changed, 6 insertions(+) diff --git a/pkg/pool-weighted/contracts/lbp/LBPool.sol b/pkg/pool-weighted/contracts/lbp/LBPool.sol index 55ecf8d47..26d5fd527 100644 --- a/pkg/pool-weighted/contracts/lbp/LBPool.sol +++ b/pkg/pool-weighted/contracts/lbp/LBPool.sol @@ -47,6 +47,11 @@ contract LBPool is WeightedPool, Ownable2Step, BaseHooks { // originating account on operations. This is important for liquidity operations, as these are permissioned // operations that can only be performed by the owner of the pool. Without this check, a malicious router // could spoof the address of the owner, allowing anyone to call permissioned functions. + // + // Since the initialization mechanism does not allow verification of the router, it is technically possible + // to front-run `initialize`. This should not be a concern in the typical LBP use case of a new token launch, + // where there is no existing liquidity. In the unlikely event it is a concern, `LBPoolFactory` provides the + // `createAndInitialize` function, which does both operations in a single step. // solhint-disable-next-line var-name-mixedcase address private immutable _trustedRouter; diff --git a/pkg/pool-weighted/contracts/lbp/LBPoolFactory.sol b/pkg/pool-weighted/contracts/lbp/LBPoolFactory.sol index fea038287..13e54dbcc 100644 --- a/pkg/pool-weighted/contracts/lbp/LBPoolFactory.sol +++ b/pkg/pool-weighted/contracts/lbp/LBPoolFactory.sol @@ -99,6 +99,7 @@ contract LBPoolFactory is IPoolVersion, ReentrancyGuardTransient, BasePoolFactor * Use this method in case pool initialization frontrunning is an issue. * If the owner is the only address with liquidity of one of the tokens, this should not be necessary. * This method does not support native ETH management; WETH needs to be used instead. + * @param name The name of the pool * @param symbol The symbol of the pool * @param tokenConfig An array of descriptors for the tokenConfig the pool will manage