Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: Add StableSurge hook example with tests. #934

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
278 changes: 278 additions & 0 deletions pkg/pool-hooks/contracts/StableSurgeHookExample.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,278 @@
// SPDX-License-Identifier: GPL-3.0-or-later

pragma solidity ^0.8.24;

import { IBasePoolFactory } from "@balancer-labs/v3-interfaces/contracts/vault/IBasePoolFactory.sol";
import { IHooks } from "@balancer-labs/v3-interfaces/contracts/vault/IHooks.sol";
import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol";
import { IStablePool } from "@balancer-labs/v3-interfaces/contracts/pool-stable/IStablePool.sol";
import {
LiquidityManagement,
TokenConfig,
PoolSwapParams,
HookFlags,
SwapKind
} from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol";
import { VaultGuard } from "@balancer-labs/v3-vault/contracts/VaultGuard.sol";
import { BaseHooks } from "@balancer-labs/v3-vault/contracts/BaseHooks.sol";

import { StableMath } from "@balancer-labs/v3-solidity-utils/contracts/math/StableMath.sol";
import { FixedPoint } from "@balancer-labs/v3-solidity-utils/contracts/math/FixedPoint.sol";

/**
* @notice Hook that applies a fee for out of range or undesirable amounts of tokens in relation to a threshold.
* @dev Uses the dynamic fee mechanism to apply a directional fee.
*/
contract StableSurgeHookExample is BaseHooks, VaultGuard {
using FixedPoint for uint256;
// Only pools from a specific factory are able to register and use this hook.
address private immutable _allowedFactory;
// Defines the range in which surging will not occur
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Defines the range in which surging will not occur
// Defines the top of the "constant fee" range, below which surging will not occur.

mapping(address pool => uint256 threshold) public poolThresholdPercentage;
// An amplification coefficient to amplify the degree to which a fee increases after the threshold is met.
mapping(address pool => uint256 surgeCoefficient) public poolSurgeCoefficient;
uint256 public constant DEFAULT_SURGECOEFFICIENT = 50e18;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
uint256 public constant DEFAULT_SURGECOEFFICIENT = 50e18;
uint256 public constant DEFAULT_SURGE_COEFFICIENT = 50e18;

Would separate all the words. Also, what is the range here? If it's a percentage, should also be e16 (but maybe it isn't?) You're comparing it to 100e18 later, so for consistency I'd say this should just be 50e16 here, and compared to FixedPoint.ONE later.

// A threshold of 0.1 for a 2 token pool means surging occurs if any token reaches 60% of the total of balances.
uint256 public constant DEFAULT_THRESHOLD = 0.1e18;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
uint256 public constant DEFAULT_THRESHOLD = 0.1e18;
uint256 public constant DEFAULT_THRESHOLD = 10e16; // 10%

We express percentages as 1-99e16.


// Note on StableSurge calculations:
// Relevant Variables inherited from Stable Math:
// n: number of tokens or assets
// Bi: balance of token in after the swap
// Wa: Weight after swap is defined as: Bi / SumOfAllTokenBalancesAfterSwap
// Surging fee will be applied when:
// Wa > 1/n + _thresholdPercentage
// Surging fee is calculated as: staticSwapFee * surgeCoefficient * (Wa/(1/n + thresholdPercentage))

/// @notice The sender does not have permission to call a function.
error SenderNotAllowed();
/// @notice Thrown when attempting to set the threshold percentage to an invalid value.
error ThresholdPercentageNotAllowed();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider InvalidThresholdPercentage and InvalidSurgeCoefficient. Usually "NotAllowed" is related to permissions, not numerical values being out-of-range.

/// @notice Thrown when attempting to set the surge coefficient to an invalid value.
error SurgeCoefficientNotAllowed();

/**
* @notice A new `StableSurgeHookExample` contract has been registered successfully.
* @dev If the registration fails the call will revert, so there will be no event.
* @param hooksContract This contract
* @param factory The factory (must be the allowed factory, or the call will revert)
* @param pool The pool on which the hook was registered
*/
event StableSurgeHookExampleRegistered(
address indexed hooksContract,
address indexed factory,
address indexed pool
);

/**
* @notice The threshold percentage has been changed in a `StableSurgeHookExample` contract.
* @dev Note, the initial threshold percentage is set on deployment and an event is emitted.
* @param hooksContract This contract
* @param thresholdPercentage The new threshold percentage
*/
event ThresholdPercentageChanged(address indexed hooksContract, uint256 indexed thresholdPercentage);

/**
* @notice The surgeCoefficient has been changed in a `StableSurgeHookExample` contract.
* @dev Note, the initial surgeCoefficient is set on deployment and an event is emitted.
* @param hooksContract This contract
* @param surgeCoefficient The new surgeCoefficient
*/
event SurgeCoefficientChanged(address indexed hooksContract, uint256 indexed surgeCoefficient);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Per our style doc (https://www.notion.so/Code-Style-51bb09b3192f457aa61bd8dec77847d2), events should come before errors, etc. Should be:

uint256 public constant DEFAULT_SURGE_COEFFICIENT = 50e18;
// A threshold of 0.1 for a 2 token pool means surging occurs if any token reaches 60% of the total of balances.
uint256 public constant DEFAULT_THRESHOLD = 0.1e18;

// Defines the range in which surging will not occur
mapping(address pool => uint256 threshold) public poolThresholdPercentage;
// An amplification coefficient to amplify the degree to which a fee increases after the threshold is met.
mapping(address pool => uint256 surgeCoefficient) public poolSurgeCoefficient;

// Only pools from a specific factory are able to register and use this hook.
address private immutable _allowedFactory;

Then events, then errors.


constructor(IVault vault, address allowedFactory) VaultGuard(vault) {
_allowedFactory = allowedFactory;
}

/// @inheritdoc IHooks
function getHookFlags() public pure override returns (HookFlags memory hookFlags) {
hookFlags.shouldCallComputeDynamicSwapFee = true;
}

/// @inheritdoc IHooks
function onRegister(
address factory,
address pool,
TokenConfig[] memory,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't use rates anywhere when calculating values. Does this only work with STANDARD tokens then? If so, we should also check that here. Revert if there's a WITH_RATE token. (Or handle rates, if that's a valid case... but then you need to worry about rounding, etc.)

The factory (presumably our standard StableFactory) already restricts it to stable pools.

LiquidityManagement calldata
) public override onlyVault returns (bool) {
// This hook implements a restrictive approach, where we check if the factory is an allowed factory and if
// the pool was created by the allowed factory.
emit StableSurgeHookExampleRegistered(address(this), factory, pool);
EndymionJkb marked this conversation as resolved.
Show resolved Hide resolved

// Initially set the pool threshold and surge coefficient to
// defaults (can be set by pool swapFeeManager in future).
Comment on lines +103 to +104
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Initially set the pool threshold and surge coefficient to
// defaults (can be set by pool swapFeeManager in future).
// Initially set the pool threshold and surge coefficient to default values. These can be overridden by the
// pool's swapFeeManager in the future.

_setThresholdPercentage(pool, DEFAULT_THRESHOLD);
_setSurgeCoefficient(pool, DEFAULT_SURGECOEFFICIENT);

return factory == _allowedFactory && IBasePoolFactory(factory).isPoolFromFactory(pool);
}

/// @inheritdoc IHooks
function onComputeDynamicSwapFeePercentage(
PoolSwapParams calldata params,
address pool,
uint256 staticSwapFeePercentage
) public view override onlyVault returns (bool, uint256) {
uint256 amp;
(amp, , ) = IStablePool(pool).getAmplificationParameter();
EndymionJkb marked this conversation as resolved.
Show resolved Hide resolved

// In order to calculate `weightAfterSwap` we need balances after swap, so we must compute the swap amount.
uint256 invariant = StableMath.computeInvariant(amp, params.balancesScaled18);
uint256 weightAfterSwap;
if (params.kind == SwapKind.EXACT_IN) {
uint256 amountCalculatedScaled18 = StableMath.computeOutGivenExactIn(
amp,
params.balancesScaled18,
params.indexIn,
params.indexOut,
params.amountGivenScaled18,
invariant
);
// Swap fee is always a percentage of the amountCalculated. On ExactIn, subtract it from the calculated
// amountOut. Round up to avoid losses during precision loss.
uint256 swapFeeAmountScaled18 = amountCalculatedScaled18.mulUp(staticSwapFeePercentage);
amountCalculatedScaled18 -= swapFeeAmountScaled18;
weightAfterSwap = getWeightAfterSwap(
params.balancesScaled18,
params.indexIn,
params.amountGivenScaled18,
amountCalculatedScaled18
);
} else {
uint256 amountCalculatedScaled18 = StableMath.computeInGivenExactOut(
amp,
params.balancesScaled18,
params.indexIn,
params.indexOut,
params.amountGivenScaled18,
invariant
);
// To ensure symmetry with EXACT_IN, the swap fee used by ExactOut is
// `amountCalculated * fee% / (100% - fee%)`. Add it to the calculated amountIn. Round up to avoid losses
// during precision loss.
uint256 swapFeeAmountScaled18 = amountCalculatedScaled18.mulDivUp(
staticSwapFeePercentage,
staticSwapFeePercentage.complement()
);

amountCalculatedScaled18 += swapFeeAmountScaled18;
weightAfterSwap = getWeightAfterSwap(
params.balancesScaled18,
params.indexIn,
amountCalculatedScaled18,
params.amountGivenScaled18
);
}

uint256 thresholdBoundary = getThresholdBoundary(params.balancesScaled18.length, poolThresholdPercentage[pool]);
if (weightAfterSwap > thresholdBoundary) {
return (
true,
getSurgeFee(weightAfterSwap, thresholdBoundary, staticSwapFeePercentage, poolSurgeCoefficient[pool])
);
} else {
return (true, staticSwapFeePercentage);
}
}

/**
* Defines the range in which surging will not occur.
* @dev An expected value for threshold in a 2 token (n=2) would be 0.1.
* This would mean surging would occur if any token reaches 60% of the total of balances.
* @param numberOfAssets Number of assets in the pool.
* @param thresholdPercentage Thershold percentage value.
Comment on lines +180 to +184
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* Defines the range in which surging will not occur.
* @dev An expected value for threshold in a 2 token (n=2) would be 0.1.
* This would mean surging would occur if any token reaches 60% of the total of balances.
* @param numberOfAssets Number of assets in the pool.
* @param thresholdPercentage Thershold percentage value.
* @notice Defines the top of the "constant fee" range, below which surging will not occur.
* @dev An expected value for the threshold in a two-token pool (n=2) would be 0.1.
* This would mean surging would occur if any token balance reaches 60% of the total pool value.
* @param numberOfAssets Number of assets in the pool
* @param thresholdPercentage Threshold percentage value

*/
function getThresholdBoundary(uint256 numberOfAssets, uint256 thresholdPercentage) public pure returns (uint256) {
return FixedPoint.ONE / numberOfAssets + thresholdPercentage;
}

/**
* The weight after swap, used to determine if surge fee should be applied.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* The weight after swap, used to determine if surge fee should be applied.
* @notice The weight after swap, used to determine whether the surge fee should be applied.

* @param balancesScaled18 Balances of pool
* @param indexIn Index of token in
* @param amountInScaled18 Amount in of swap
* @param amountOutScaled18 Amount out of swap
*/
function getWeightAfterSwap(
uint256[] memory balancesScaled18,
uint256 indexIn,
uint256 amountInScaled18,
uint256 amountOutScaled18
) public pure returns (uint256) {
uint256 balancesTotal;
for (uint256 i = 0; i < balancesScaled18.length; ++i) {
balancesTotal += balancesScaled18[i];
}
uint256 balanceTokenInAfterSwap = balancesScaled18[indexIn] + amountInScaled18;
uint256 balancesTotalAfterSwap = balancesTotal + amountInScaled18 - amountOutScaled18;
return balanceTokenInAfterSwap.divDown(balancesTotalAfterSwap);
}

/**
* A fee based on the virtual weights of the tokens.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* A fee based on the virtual weights of the tokens.
* @notice A fee based on the virtual weights of the tokens.

* @param weightAfterSwap Weight after swap
* @param thresholdBoundary Threshold that surge fee will be applied
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* @param thresholdBoundary Threshold that surge fee will be applied
* @param thresholdBoundary Threshold at which the surge fee will be applied

* @param swapFeePercentage Pools static swap fee
* @param surgeCoefficient Amplification coefficient to amplify the degree a fee increases
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* @param surgeCoefficient Amplification coefficient to amplify the degree a fee increases
* @param surgeCoefficient Amplification coefficient to amplify the magnitude of the fee increase

*/
function getSurgeFee(
uint256 weightAfterSwap,
uint256 thresholdBoundary,
uint256 swapFeePercentage,
uint256 surgeCoefficient
) public pure returns (uint256) {
uint256 weightRatio = weightAfterSwap.divDown(thresholdBoundary);
return swapFeePercentage.mulDown(surgeCoefficient).mulDown(weightRatio);
}

// Permissioned functions

/**
* @notice Sets the hook threshold percentage.
* @dev This function must be permissioned.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The other examples derive from Ownable and use the onlyOwner modifier for permissioned functions. Here, we're using the swapManager for the pool, vs. the "owner" of the hook, so we could state that explicitly here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* @dev This function must be permissioned.
* @dev This function can only be called by the pool's `swapManager`.

*/
function setThresholdPercentage(address pool, uint256 newThresholdPercentage) external {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider a modifier for clarity. (Modifiers go after errors.)

modifier onlySwapManager(address pool) {
    _ensureSwapManager(pool);
    _;
}

function setThresholdPercentage(address pool, uint256 newThresholdPercentage)
    external onlySwapManager(pool) { ... }

function _ensureSwapManager(address pool) private view {
    if (_vault.getPoolRoleAccounts(pool).swapFeeManager != msg.sender) {
        revert SenderNotAllowed();
    }
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

If this check were in the critical path, I'd consider storing the addresses (as they're immutable in the Vault) - but as they're only used in permissioned functions, that would be vast overkill.

if (_vault.getPoolRoleAccounts(pool).swapFeeManager != msg.sender) {
revert SenderNotAllowed();
}
// New threshold should be < 1 - 1/number_of_assets
uint256 thresholdPercentageCheck = FixedPoint.ONE - FixedPoint.ONE / _vault.getPoolTokens(pool).length;

if (newThresholdPercentage > thresholdPercentageCheck) {
revert ThresholdPercentageNotAllowed();
}
_setThresholdPercentage(pool, newThresholdPercentage);
}

/**
* @notice Sets the hook surgeCoefficient.
* @dev This function must be permissioned.
*/
function setSurgeCoefficient(address pool, uint256 newSurgeCoefficient) external {
if (_vault.getPoolRoleAccounts(pool).swapFeeManager != msg.sender) {
revert SenderNotAllowed();
}

// Check that baseFee * newSurgeCoefficient / (1/number_of_assets) < 100
uint256 surgeCoefficientCheck = (_vault.getStaticSwapFeePercentage(pool) * newSurgeCoefficient) /
(FixedPoint.ONE / _vault.getPoolTokens(pool).length);
Comment on lines +258 to +259
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
uint256 surgeCoefficientCheck = (_vault.getStaticSwapFeePercentage(pool) * newSurgeCoefficient) /
(FixedPoint.ONE / _vault.getPoolTokens(pool).length);
uint256 surgeCoefficientCheck = (_vault.getStaticSwapFeePercentage(pool) * newSurgeCoefficient).mulDown(
_vault.getPoolTokens(pool).length
);

Have suggestions for refactoring this more, but the above can be simplified to this to start with.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Another problem here is you're reading the static fee from the Vault (here and in the dynamic fee hook), but it can be changed, invalidating the surge coefficient (e.g., could change to something that would fail).

So you'd have to fix the static fee percentage on registration, and store it. There could be a permissioned "update static fee percentage" that could update it from the Vault to allow it to recognize changes, but it would then have to check that the coefficient is correct (or reset it to 1 or something known valid).

Seems complex and error-prone.


if (surgeCoefficientCheck > 100e18) {
revert SurgeCoefficientNotAllowed();
}
_setSurgeCoefficient(pool, newSurgeCoefficient);
}

function _setThresholdPercentage(address pool, uint256 newThresholdPercentage) private {
poolThresholdPercentage[pool] = newThresholdPercentage;

emit ThresholdPercentageChanged(address(this), newThresholdPercentage);
}

function _setSurgeCoefficient(address pool, uint256 newSurgeCoefficient) private {
poolSurgeCoefficient[pool] = newSurgeCoefficient;

emit SurgeCoefficientChanged(address(this), newSurgeCoefficient);
}
}
Loading
Loading