feat: StrategyFactory contract #522

ChaoticWalrus · 2024-04-18T16:51:11Z

Proposal Doc: https://docs.google.com/document/d/1C1nN6BkCMS0Z9EnizU0pNYNNOiRd_FH6E2Vd-ZPULro/edit?usp=sharing

High-Level Notes on Design

This avoid making any changes to existing "core" protocol contracts, and is instead purely additive.
In particular, this change is effectively turning the strategyWhitelister role in the StrategyManager into a smart contract -- the ability to whitelist particular strategies is maintained with a pass-through function, while the ability to whitelist factory-generated strategy contracts is made permissionless.

Summary

The StrategyFactory allows any caller to deploy a StrategyBase contract for any ERC20 token

Technically, a BeaconProxy contract is deployed, pointing to a reference implementation
Upgrades are controlled by a new BeaconProxy, with the same control of upgrades as our existing contracts
Pausing power is also given to the same PauserRegistry as we use now
The token cannot already have a Strategy deployed for it (only one deployment from the factory per token)
We store a mapping(IERC20 token => IStrategy) public tokenStrategies and check against previous writes
Owner can pass-through this contract to whitelist new strategy contracts
The StrategyFactory contract is itself upgradeable, to potentially allow other feature additions in the future

Still needs a few things, including documentation that provides warnings against using reentrant tokens (e.g. ERC777), or other non-standard tokens, and more extensive testing

Deployment

Deploy Upgradeable Beacon for Beacon Proxies, with ownership given to the executor multisig (so upgrade rights work like our existing contracts)
Deploy StrategyFactory implementation, pointing to Strategy Manager (immutable variable)
Deploy StrategyFactory (proxy), giving ownership to Ops Multisig (so it maintains similar manual-whitelisting powers), pausing powers to the existing protocol-wide PausingRegistry, and proxy upgrade powers to the existing protocol-wide ProxyAdmin (+ pointing to the Upgradeable Beacon & StrategyFactory implementation deployed above)
Transfer strategyWhitelister role in the StrategyManager to the StrategyFactory (proxy) -- this will require an executor multisig action and thus be subject to a 10-day timelock delay if accomplished via the ops multisig

src/contracts/strategies/StrategyFactory.sol

wadealexc

Overall looks fine, but IMO we want the BeaconProxy pattern!

ChaoticWalrus · 2024-07-01T18:33:05Z

Overall looks fine, but IMO we want the BeaconProxy pattern!

switch to beacon proxies implemented in 5d19597

src/contracts/interfaces/IStrategyFactory.sol

scotthconner

These comments here mostly reflect what I put in the technical proposal. Nothing super critical, but understanding the core contract interactions and where state is managed is one piece that sticks out to me.

src/contracts/interfaces/IStrategyFactory.sol

src/contracts/strategies/StrategyFactory.sol

wadealexc · 2024-07-12T15:36:44Z

src/contracts/strategies/StrategyFactoryStorage.sol

+    // @notice Upgradeable beacon which new Strategies deployed by this contract point to
+    IBeacon public strategyBeacon;


Should add immutable?

src/contracts/strategies/StrategyFactory.sol

wadealexc

Needs docs. I'll put something together tomorrow :)

ypatil12

One comment

ypatil12 · 2024-07-26T18:08:49Z

src/contracts/strategies/StrategyFactory.sol

+     * @notice Owner-only function to prevent strategies from being created for given tokens.
+     * @param tokens An array of token addresses to blacklist.
+     */
+    function blacklistTokens(IERC20[] calldata tokens) external onlyOwner {


Is the plan to blacklist already deployed tokens via the initializer? We don't want to get frontrun on a permissionless deployment

Resolved offline -> the deployment requires us to wait to transfer the strategyWhitelister role so we won't need to add this functionality in the initializer

wadealexc · 2024-07-26T19:32:52Z

src/contracts/strategies/StrategyFactory.sol

+        IStrategy strategy = IStrategy(
+            address(
+                new BeaconProxy(
+                    address(strategyBeacon),
+                    abi.encodeWithSelector(StrategyBase.initialize.selector, token, pauserRegistry)
+                )
+            )
+        );


Should we be using Create2 like we do in the EigenPodManager?

As an outsider, I definitely +1 this

mpjunior92 · 2024-08-02T03:18:41Z

src/contracts/strategies/StrategyFactory.sol

+     * $dev Immense caution is warranted for non-standard ERC20 tokens, particularly "reentrant" tokens
+     * like those that conform to ERC777.
+     */
+    function deployNewStrategy(IERC20 token)


I could not find an event emitted when a new strategy gets deployed. EigenPodManager, for example, emits a PodDeployed event whenever an EigenPod is created, which can be used in subgraph templates to index instantiated contracts. Is it possible to do the same here?

We can use the StrategyAddedToDepositWhitelist event in the StrategyManager instead? Or do you want to track the count of contracts deployed from this contract?

StrategyAddedToDepositWhitelist tracks only EigenLayer approved strategies that are shown on the UI, right?

When we deploy a strategy through this method, it is added to the deposit whitelist. So it would be both EigenLayer approved AND permissionless strategies

github-actions · 2024-08-07T03:00:45Z

Reading tracefile ./lcov.info.pruned
                                             |Lines      |Functions|Branches  
Filename                                       |Rate    Num|Rate  Num|Rate   Num
================================================================================
[src/contracts/]
core/AVSDirectory.sol                          |77.8%    27|77.8%   9|    -    0
core/DelegationManager.sol                     |96.5%   198|92.3%  39|    -    0
core/RewardsCoordinator.sol                    |90.8%   119|81.2%  32|    -    0
core/StrategyManager.sol                       |95.2%    83|95.8%  24|    -    0
libraries/BeaconChainProofs.sol                |94.7%    38|91.7%  12|    -    0
libraries/BytesLib.sol                         | 2.6%   156| 7.1%  14|    -    0
libraries/EIP1271SignatureUtils.sol            | 100%     3| 100%   1|    -    0
libraries/Endian.sol                           | 100%     2| 100%   1|    -    0
libraries/Merkle.sol                           | 100%    38| 100%   5|    -    0
libraries/StructuredLinkedList.sol             | 0.0%    45| 0.0%  19|    -    0
permissions/Pausable.sol                       |95.7%    23|90.9%  11|    -    0
permissions/PauserRegistry.sol                 | 100%    12| 100%   6|    -    0
pods/DelayedWithdrawalRouter.sol               |93.2%    59|85.7%  14|    -    0
pods/EigenPod.sol                              | 100%   145| 100%  33|    -    0
pods/EigenPodManager.sol                       |98.8%    84|89.5%  19|    -    0
strategies/EigenStrategy.sol                   | 0.0%    10| 0.0%   5|    -    0
strategies/StrategyBase.sol                    |90.9%    44|78.9%  19|    -    0
strategies/StrategyBaseTVLLimits.sol           | 100%    12|83.3%   6|    -    0
strategies/StrategyFactory.sol                 |94.3%    35|88.9%   9|    -    0
token/BackingEigen.sol                         |72.0%    25|50.0%  10|    -    0
token/Eigen.sol                                |38.5%    39|50.0%  12|    -    0
utils/UpgradeableSignatureCheckingUtils.sol    | 0.0%     6| 0.0%   4|    -    0
================================================================================
                                       Total:|76.2%  1203|75.0% 304|    -    0

old

* fix: make the StrategyFactory actually whitelist new strategies * feat: add several unit tests for the StrategyFactory * feat: switch to beacon proxies * feat: split storage, create interface, and rename - separate storage contract - create interface -- storage contract inherits from it - rename `tokenStrategies` => `tokenStrategy` for clarity * chore: move events to interface * feat: additional unit tests, interface cleanup * feat: reduce owner powers, make mapping behavior strict - remove `editTokenStrategiesMapping` and `setStrategyBeacon` functions - make the `whitelistStrategies` pass-through function no longer edit the `tokenStrategy` mapping - delete and modify related tests, as necessary * feat: add several unit tests for the StrategyFactory * feat: switch to beacon proxies * feat: additional unit tests, interface cleanup * feat: reduce owner powers, make mapping behavior strict - remove `editTokenStrategiesMapping` and `setStrategyBeacon` functions - make the `whitelistStrategies` pass-through function no longer edit the `tokenStrategy` mapping - delete and modify related tests, as necessary * chore: formatting feat: add `blacklistTokens` fn refactor: separate mapping Feat: add `sharesToUnderlying` event (#644) * feat: add `sharesToUnderlying` event * refactor: cleanup * refactor: separate logic to new fn * test: sanity feat: emit exchange rate on deposits/withdrawals (#647) * feat: exchange rate in deposits/withdrawals * chore: forge fmt --------- Co-authored-by: clandestine.eth <96172957+0xClandestine@users.noreply.github.com> fix: strategy unit tests feat: add pass thoughts for all `strategyWhitelister` gated functions (#648) docs: add documentation for strategy factory methods chore: make bindings feat: change name of tokenStrategy mapping to deployedStrategies (#657) feat: auto remove strategies from whitelist (#658) feat: limit total shares on `StrategyBase` (#659) * feat: limit totalShares * chore: update natspec * chore: bindings & formatting * chore: bindings * docs: update docs with max total shares change * chore: fix formatting --------- Co-authored-by: wadealexc <pragma-services@proton.me> fix: natspec strategy factory (#660) * fix: natspec * chore: format * chore: bindings * chore: bindings * chore: bindings feat: integration tests docs: wip readme update docs: finalize deployment info in readme

wadealexc · 2024-08-07T18:55:38Z

All the activity you're seeing here is:

squashing the PR into a single commit to make rebasing easier (git rebase -i HEAD~18, then squashing all the commits into 1)
rebasing on top of dev to make merging clean (git rebase dev, then fix the conflicts)
force push when done (git push --force)

github-actions · 2024-08-07T18:57:04Z

Reading tracefile ./lcov.info.pruned
                                             |Lines      |Functions|Branches  
Filename                                       |Rate    Num|Rate  Num|Rate   Num
================================================================================
[src/contracts/]
core/AVSDirectory.sol                          |77.8%    27|77.8%   9|    -    0
core/DelegationManager.sol                     |96.5%   198|92.3%  39|    -    0
core/RewardsCoordinator.sol                    |90.8%   119|81.2%  32|    -    0
core/StrategyManager.sol                       |95.2%    83|95.8%  24|    -    0
libraries/BeaconChainProofs.sol                |94.7%    38|91.7%  12|    -    0
libraries/BytesLib.sol                         | 2.6%   156| 7.1%  14|    -    0
libraries/EIP1271SignatureUtils.sol            | 100%     3| 100%   1|    -    0
libraries/Endian.sol                           | 100%     2| 100%   1|    -    0
libraries/Merkle.sol                           | 100%    38| 100%   5|    -    0
libraries/StructuredLinkedList.sol             | 0.0%    45| 0.0%  19|    -    0
permissions/Pausable.sol                       |95.7%    23|90.9%  11|    -    0
permissions/PauserRegistry.sol                 | 100%    12| 100%   6|    -    0
pods/DelayedWithdrawalRouter.sol               |93.2%    59|85.7%  14|    -    0
pods/EigenPod.sol                              | 100%   145| 100%  33|    -    0
pods/EigenPodManager.sol                       |98.8%    84|89.5%  19|    -    0
strategies/EigenStrategy.sol                   | 0.0%    10| 0.0%   5|    -    0
strategies/StrategyBase.sol                    |90.9%    44|78.9%  19|    -    0
strategies/StrategyBaseTVLLimits.sol           | 100%    12|83.3%   6|    -    0
strategies/StrategyFactory.sol                 |94.3%    35|88.9%   9|    -    0
token/BackingEigen.sol                         |72.0%    25|50.0%  10|    -    0
token/Eigen.sol                                |38.5%    39|50.0%  12|    -    0
utils/UpgradeableSignatureCheckingUtils.sol    | 0.0%     6| 0.0%   4|    -    0
================================================================================
                                       Total:|76.2%  1203|75.0% 304|    -    0

github-actions · 2024-08-07T18:59:53Z

Reading tracefile ./lcov.info.pruned
                                             |Lines      |Functions|Branches  
Filename                                       |Rate    Num|Rate  Num|Rate   Num
================================================================================
[src/contracts/]
core/AVSDirectory.sol                          |77.8%    27|77.8%   9|    -    0
core/DelegationManager.sol                     |96.5%   198|92.3%  39|    -    0
core/RewardsCoordinator.sol                    |90.8%   119|81.2%  32|    -    0
core/StrategyManager.sol                       |95.2%    83|95.8%  24|    -    0
libraries/BeaconChainProofs.sol                |94.7%    38|91.7%  12|    -    0
libraries/BytesLib.sol                         | 2.6%   156| 7.1%  14|    -    0
libraries/EIP1271SignatureUtils.sol            | 100%     3| 100%   1|    -    0
libraries/Endian.sol                           | 100%     2| 100%   1|    -    0
libraries/Merkle.sol                           | 100%    38| 100%   5|    -    0
libraries/StructuredLinkedList.sol             | 0.0%    45| 0.0%  19|    -    0
permissions/Pausable.sol                       |95.7%    23|90.9%  11|    -    0
permissions/PauserRegistry.sol                 | 100%    12| 100%   6|    -    0
pods/DelayedWithdrawalRouter.sol               |93.2%    59|85.7%  14|    -    0
pods/EigenPod.sol                              | 100%   145| 100%  33|    -    0
pods/EigenPodManager.sol                       |98.8%    84|89.5%  19|    -    0
strategies/EigenStrategy.sol                   | 0.0%    10| 0.0%   5|    -    0
strategies/StrategyBase.sol                    |90.9%    44|78.9%  19|    -    0
strategies/StrategyBaseTVLLimits.sol           | 100%    12|83.3%   6|    -    0
strategies/StrategyFactory.sol                 |94.3%    35|88.9%   9|    -    0
token/BackingEigen.sol                         |72.0%    25|50.0%  10|    -    0
token/Eigen.sol                                |38.5%    39|50.0%  12|    -    0
utils/UpgradeableSignatureCheckingUtils.sol    | 0.0%     6| 0.0%   4|    -    0
================================================================================
                                       Total:|76.2%  1203|75.0% 304|    -    0

github-actions · 2024-08-07T19:03:48Z

Reading tracefile ./lcov.info.pruned
                                             |Lines      |Functions|Branches  
Filename                                       |Rate    Num|Rate  Num|Rate   Num
================================================================================
[src/contracts/]
core/AVSDirectory.sol                          |77.8%    27|77.8%   9|    -    0
core/DelegationManager.sol                     |96.5%   198|92.3%  39|    -    0
core/RewardsCoordinator.sol                    |90.8%   119|81.2%  32|    -    0
core/StrategyManager.sol                       |95.2%    83|95.8%  24|    -    0
libraries/BeaconChainProofs.sol                |94.7%    38|91.7%  12|    -    0
libraries/BytesLib.sol                         | 2.6%   156| 7.1%  14|    -    0
libraries/EIP1271SignatureUtils.sol            | 100%     3| 100%   1|    -    0
libraries/Endian.sol                           | 100%     2| 100%   1|    -    0
libraries/Merkle.sol                           | 100%    38| 100%   5|    -    0
libraries/StructuredLinkedList.sol             | 0.0%    45| 0.0%  19|    -    0
permissions/Pausable.sol                       |95.7%    23|90.9%  11|    -    0
permissions/PauserRegistry.sol                 | 100%    12| 100%   6|    -    0
pods/DelayedWithdrawalRouter.sol               |93.2%    59|85.7%  14|    -    0
pods/EigenPod.sol                              | 100%   145| 100%  33|    -    0
pods/EigenPodManager.sol                       |98.8%    84|89.5%  19|    -    0
strategies/EigenStrategy.sol                   | 0.0%    10| 0.0%   5|    -    0
strategies/StrategyBase.sol                    |90.9%    44|78.9%  19|    -    0
strategies/StrategyBaseTVLLimits.sol           | 100%    12|83.3%   6|    -    0
strategies/StrategyFactory.sol                 |94.3%    35|88.9%   9|    -    0
token/BackingEigen.sol                         |72.0%    25|50.0%  10|    -    0
token/Eigen.sol                                |38.5%    39|50.0%  12|    -    0
utils/UpgradeableSignatureCheckingUtils.sol    | 0.0%     6| 0.0%   4|    -    0
================================================================================
                                       Total:|76.2%  1203|75.0% 304|    -    0

github-actions · 2024-08-07T19:09:48Z

Reading tracefile ./lcov.info.pruned
                                             |Lines      |Functions|Branches  
Filename                                       |Rate    Num|Rate  Num|Rate   Num
================================================================================
[src/contracts/]
core/AVSDirectory.sol                          |77.8%    27|77.8%   9|    -    0
core/DelegationManager.sol                     |96.5%   198|92.3%  39|    -    0
core/RewardsCoordinator.sol                    |90.8%   119|81.2%  32|    -    0
core/StrategyManager.sol                       |95.2%    83|95.8%  24|    -    0
libraries/BeaconChainProofs.sol                |94.7%    38|91.7%  12|    -    0
libraries/BytesLib.sol                         | 2.6%   156| 7.1%  14|    -    0
libraries/EIP1271SignatureUtils.sol            | 100%     3| 100%   1|    -    0
libraries/Endian.sol                           | 100%     2| 100%   1|    -    0
libraries/Merkle.sol                           | 100%    38| 100%   5|    -    0
libraries/StructuredLinkedList.sol             | 0.0%    45| 0.0%  19|    -    0
permissions/Pausable.sol                       |95.7%    23|90.9%  11|    -    0
permissions/PauserRegistry.sol                 | 100%    12| 100%   6|    -    0
pods/DelayedWithdrawalRouter.sol               |93.2%    59|85.7%  14|    -    0
pods/EigenPod.sol                              | 100%   145| 100%  33|    -    0
pods/EigenPodManager.sol                       |98.8%    84|89.5%  19|    -    0
strategies/EigenStrategy.sol                   | 0.0%    10| 0.0%   5|    -    0
strategies/StrategyBase.sol                    |90.9%    44|78.9%  19|    -    0
strategies/StrategyBaseTVLLimits.sol           | 100%    12|83.3%   6|    -    0
strategies/StrategyFactory.sol                 |94.3%    35|88.9%   9|    -    0
token/BackingEigen.sol                         |72.0%    25|50.0%  10|    -    0
token/Eigen.sol                                |38.5%    39|50.0%  12|    -    0
utils/UpgradeableSignatureCheckingUtils.sol    | 0.0%     6| 0.0%   4|    -    0
================================================================================
                                       Total:|76.2%  1203|75.0% 304|    -    0

gpsanant reviewed Apr 21, 2024

View reviewed changes

src/contracts/strategies/StrategyFactory.sol Outdated Show resolved Hide resolved

gpsanant reviewed Apr 21, 2024

View reviewed changes

src/contracts/strategies/StrategyFactory.sol Show resolved Hide resolved

wadealexc force-pushed the dev branch from b087c53 to 4d05ba5 Compare May 20, 2024 17:14

ChaoticWalrus commented Jun 26, 2024

View reviewed changes

src/contracts/strategies/StrategyFactory.sol Outdated Show resolved Hide resolved

ChaoticWalrus commented Jun 26, 2024

View reviewed changes

src/contracts/strategies/StrategyFactory.sol Outdated Show resolved Hide resolved

This comment was marked as duplicate.

Sign in to view

wadealexc force-pushed the feat-StrategyFactory branch from fd751e3 to 6da040d Compare July 1, 2024 18:14

wadealexc reviewed Jul 1, 2024

View reviewed changes

src/contracts/strategies/StrategyFactory.sol Outdated Show resolved Hide resolved

wadealexc suggested changes Jul 1, 2024

View reviewed changes

ChaoticWalrus commented Jul 1, 2024

View reviewed changes

src/contracts/interfaces/IStrategyFactory.sol Outdated Show resolved Hide resolved

scotthconner previously requested changes Jul 10, 2024

View reviewed changes

This comment was marked as duplicate.

Sign in to view

wadealexc force-pushed the feat-StrategyFactory branch from ce213b4 to bb5f30b Compare July 12, 2024 15:16

wadealexc reviewed Jul 12, 2024

View reviewed changes

ypatil12 requested changes Jul 19, 2024

View reviewed changes

src/contracts/strategies/StrategyFactory.sol Show resolved Hide resolved

wadealexc suggested changes Jul 22, 2024

View reviewed changes

ypatil12 previously requested changes Jul 26, 2024

View reviewed changes

wadealexc reviewed Jul 26, 2024

View reviewed changes

Layr-Labs deleted a comment from github-actions bot Jul 26, 2024

mpjunior92 reviewed Aug 2, 2024

View reviewed changes

ypatil12 force-pushed the feat-StrategyFactory branch from 14aa656 to aa05081 Compare August 5, 2024 17:50

Layr-Labs deleted a comment from github-actions bot Aug 7, 2024

wadealexc approved these changes Aug 7, 2024

View reviewed changes

wadealexc force-pushed the feat-StrategyFactory branch from bd1cc42 to 0960524 Compare August 7, 2024 18:51

wadealexc marked this pull request as ready for review August 7, 2024 18:52

wadealexc force-pushed the feat-StrategyFactory branch from 0960524 to b8694e9 Compare August 7, 2024 18:54

wadealexc merged commit d3730b5 into dev Aug 7, 2024

wadealexc deleted the feat-StrategyFactory branch August 7, 2024 19:01

		// @notice Upgradeable beacon which new Strategies deployed by this contract point to
		IBeacon public strategyBeacon;

feat: StrategyFactory contract #522

feat: StrategyFactory contract #522

Uh oh!

Conversation

ChaoticWalrus commented Apr 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

High-Level Notes on Design

Summary

Deployment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as duplicate.

Uh oh!

wadealexc left a comment

Choose a reason for hiding this comment

Uh oh!

ChaoticWalrus commented Jul 1, 2024

Uh oh!

Uh oh!

scotthconner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as duplicate.

This comment was marked as duplicate.

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wadealexc left a comment

Choose a reason for hiding this comment

Uh oh!

ypatil12 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Aug 7, 2024

Uh oh!

wadealexc commented Aug 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 7, 2024

Uh oh!

github-actions bot commented Aug 7, 2024

Uh oh!

github-actions bot commented Aug 7, 2024

Uh oh!

github-actions bot commented Aug 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

ChaoticWalrus commented Apr 18, 2024 •

edited

Loading

wadealexc commented Aug 7, 2024 •

edited

Loading