feat: Add StableSurge hook example with tests. #934

johngrantuk · 2024-08-29T12:07:43Z

Description

Add StableSurge hook example based of ZenDragons initial doc.

Type of change

Checklist:

The diff is legible and has no extraneous changes
Complex code has been commented, including external interfaces
Tests have 100% code coverage
The base branch is either main, or there's a description of how to merge

Issue Resolution

pkg/pool-hooks/contracts/StableSurgeHookExample.sol

EndymionJkb

See questions in the comments; maybe we can sync and talk about the intent and how this should work.

EndymionJkb · 2024-09-10T19:34:50Z

pkg/pool-hooks/contracts/StableSurgeHookExample.sol

+    // 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; meaning surging will occur when:
+    // B(i-after swap) / (sum(B(n-after swap))) > 1/n + gamma


I'm not sure what all the variables mean: i, n, B(), etc. Range of what? Maybe give some more details / a numerical example.

Tried to clarify this a bit more in a more detailed comment. Let me know if this works?
(Also see the linked Notion in the description, not sure how much of that is practical to include in the contract so would be great to get guidance there.)

pkg/pool-hooks/contracts/StableSurgeHookExample.sol

EndymionJkb · 2024-09-10T19:41:54Z

pkg/pool-hooks/contracts/StableSurgeHookExample.sol

+        emit ThresholdChanged(address(this), threshold);
+        emit SurgeCoefficientChanged(address(this), surgeCoefficient);


What we typically do here is encapsulate the event with the set function.

Suggested change

emit ThresholdChanged(address(this), threshold);

emit SurgeCoefficientChanged(address(this), surgeCoefficient);

_setThreshold(threshold);

_setSurgeCoefficient(surgeCoefficient);

Then, for both the threshold and the surgeCoefficient:

function setThreshold(uint64 newThreshold) external onlyOwner { _setThreshold(newThreshold); } function _setThreshold(uint64 newThreshold) private { // This should be validated; e.g. <= FixedPoint.ONE or some max value? _threshold = newThreshold; emit ThresholdChanged(address(this), newThreshold); }

How is surge coefficient validated? (i.e., what is the range)? You can't go over 100% on a dynamic fee, or the Vault will revert.

Will check with Zen on best way to validate.

From Zen:

I don’t think the threshold should matter so much as long as it’s less than 1 - 1/n

surge coefficient: baseFee * 𝜇 / (1/n) < 100 would be the test then

pkg/pool-hooks/contracts/StableSurgeHookExample.sol

EndymionJkb · 2024-09-10T19:59:24Z

pkg/pool-hooks/test/foundry/StableSurgeExample.t.sol

+    uint256 amountInBelowThreshold = 1e18;
+    uint256 amountInAboveThreshold = 30e18;
+    // Return from GivenIn swap, input=amountInAboveThreshold, minus fee
+    uint256 amountOutAboveThreshold = 29.850106042102014697e18;


These would be calculated in the code based on the constants above, but we can look at the tests once the main contract is done.

Co-authored-by: EndymionJkb <[email protected]>

… be set by pool swapFeeManager.

EndymionJkb

See this Notion doc for my thoughts here:
https://www.notion.so/Stable-Surge-algorithm-113e395e2281801ba9f1d452e3a9be29

EndymionJkb · 2024-10-02T15:34:07Z

pkg/pool-hooks/contracts/StableSurgeHookExample.sol

+    /// @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();


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

EndymionJkb · 2024-10-02T15:36:13Z

pkg/pool-hooks/contracts/StableSurgeHookExample.sol

+    mapping(address pool => uint256 surgeCoefficient) public poolSurgeCoefficient;
+    uint256 public constant DEFAULT_SURGECOEFFICIENT = 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;


Suggested change

uint256 public constant DEFAULT_THRESHOLD = 0.1e18;

uint256 public constant DEFAULT_THRESHOLD = 10e16; // 10%

We express percentages as 1-99e16.

EndymionJkb · 2024-10-02T15:40:58Z

pkg/pool-hooks/contracts/StableSurgeHookExample.sol

+    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;


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.

EndymionJkb · 2024-10-02T15:43:11Z

pkg/pool-hooks/contracts/StableSurgeHookExample.sol

+    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


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.

EndymionJkb · 2024-10-02T15:44:43Z

pkg/pool-hooks/contracts/StableSurgeHookExample.sol

+     * 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.


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

EndymionJkb · 2024-10-02T16:04:21Z

pkg/pool-hooks/contracts/StableSurgeHookExample.sol

+    function onRegister(
+        address factory,
+        address pool,
+        TokenConfig[] memory,


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.

EndymionJkb · 2024-10-02T16:04:59Z

pkg/pool-hooks/contracts/StableSurgeHookExample.sol

+        // Initially set the pool threshold and surge coefficient to
+        // defaults (can be set by pool swapFeeManager in future).


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.

pkg/pool-hooks/contracts/StableSurgeHookExample.sol

EndymionJkb · 2024-10-02T16:10:58Z

pkg/pool-hooks/test/foundry/StableSurgeHookExample.t.sol

+
+    StablePoolFactory internal stablePoolFactory;
+
+    uint64 SWAP_FEE_PERCENTAGE = 0.0004e18;


Suggested change

uint64 SWAP_FEE_PERCENTAGE = 0.0004e18;

uint64 SWAP_FEE_PERCENTAGE = 0.04e16;

Percentages (< 100) are always e16

EndymionJkb · 2024-10-02T17:50:59Z

pkg/pool-hooks/contracts/StableSurgeHookExample.sol

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


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.

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.

feat: Add StableSurge hook example with tests.

8afdb62

johngrantuk requested a review from danielmkm August 29, 2024 12:07

johngrantuk self-assigned this Aug 29, 2024

fix: Remove forge test import.

301f8b8

johngrantuk requested a review from EndymionJkb September 6, 2024 16:07

MattPereira reviewed Sep 10, 2024

View reviewed changes

pkg/pool-hooks/contracts/StableSurgeHookExample.sol Show resolved Hide resolved

EndymionJkb reviewed Sep 10, 2024

View reviewed changes

johngrantuk and others added 8 commits September 13, 2024 09:43

Improve comment.

c2f55e7

Co-authored-by: EndymionJkb <[email protected]>

Improve comment.

bbe4ffd

Co-authored-by: EndymionJkb <[email protected]>

Use defined constant.

d6cf007

Co-authored-by: EndymionJkb <[email protected]>

chore: General tidy and improvement of comments.

b29dbba

refactor: Setting threshold and surge.

3521e0b

refactor: Change to have threshold and surge vars per pool. These can…

911df4d

… be set by pool swapFeeManager.

Prettier

bc50f6d

feat: Add validation for setting surge and threshold.

bec194c

johngrantuk requested a review from EndymionJkb September 26, 2024 08:20

EndymionJkb reviewed Oct 2, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add StableSurge hook example with tests. #934

feat: Add StableSurge hook example with tests. #934

johngrantuk commented Aug 29, 2024

EndymionJkb left a comment

EndymionJkb Sep 10, 2024

johngrantuk Sep 13, 2024 •

edited

Loading

EndymionJkb Sep 10, 2024

EndymionJkb Sep 10, 2024

johngrantuk Sep 13, 2024

johngrantuk Sep 25, 2024

EndymionJkb Sep 10, 2024

EndymionJkb left a comment

EndymionJkb Oct 2, 2024

EndymionJkb Oct 2, 2024

EndymionJkb Oct 2, 2024

EndymionJkb Oct 2, 2024

EndymionJkb Oct 2, 2024

EndymionJkb Oct 2, 2024

EndymionJkb Oct 2, 2024

EndymionJkb Oct 2, 2024

EndymionJkb Oct 2, 2024

EndymionJkb Oct 2, 2024

		emit ThresholdChanged(address(this), threshold);
		emit SurgeCoefficientChanged(address(this), surgeCoefficient);

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

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

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

		// Initially set the pool threshold and surge coefficient to
		// defaults (can be set by pool swapFeeManager in future).


		StablePoolFactory internal stablePoolFactory;

		uint64 SWAP_FEE_PERCENTAGE = 0.0004e18;

	uint64 SWAP_FEE_PERCENTAGE = 0.0004e18;
	uint64 SWAP_FEE_PERCENTAGE = 0.04e16;

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

feat: Add StableSurge hook example with tests. #934

Are you sure you want to change the base?

feat: Add StableSurge hook example with tests. #934

Conversation

johngrantuk commented Aug 29, 2024

Description

Type of change

Checklist:

Issue Resolution

EndymionJkb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johngrantuk Sep 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EndymionJkb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johngrantuk Sep 13, 2024 •

edited

Loading