allo-protocol / allo-v2

Core Allo V2 Contracts
GNU Affero General Public License v3.0
86 stars 64 forks source link

feat/core-base-strategy #617

Closed 0xOneTony closed 2 months ago

0xOneTony commented 2 months ago

Closes #591 , #590

Most Basic Strategy contract that should be used for creating extensions or more complex strategies.

⚠️ It's important to note that I didn't remove the current BaseStrategy yet, as it would break all the strategies

owocki commented 2 months ago

you love to see it :)

what are we thinking the taxonomy of strategies should be

is it

basestrategy
strategy strategy strategy strategy strategy strategy 

or will there be multiple layers like

basestrategy
quadratic_straegy                              retro_strategy                        xyz_strategy
vanillaQAstrategy MACIQFstrategy            vanilla_retro                          strawberry_xyz_strategy  
ilpepepig commented 2 months ago

Thinking again about this, I'm not sure about the use of modifiers. I think their current logic belong in the _beforeX() functions and we should give custom strategy devs more flexibility.

For example, why is it assumed that withdraw() should be called by the Pool Manager? _checkOnlyPoolManager could be called from _beforeWithdraw if needed, and maybe by default. Also, why is the onlyAllo modifier not used here?

In addition, if custom strategies are not whitelisted, enforcing onlyAllo is not really possible but should be convinient. Can we simply remove this requirement? What would be the consequences?

Regarding onlyInitialized, currently strategies are cloned by Allo and initialized in the same call, which means that all strategies are by default initialized. Can we remove this modifier then? If a custom strategy requires a different initialization logic, checking that it happened could be added to _beforeX().

0xOneTony commented 2 months ago

what are we thinking the taxonomy of strategies should be

@owocki So we should be providing 2 things to devs

  1. tools to build easier their custom strategy (making use of base strategy, libraries, extensions)
  2. use one of our example strategies (QVStrategy, QFStrategy, RetroPGFStrategy, RFPStrategy ....)

so the taxonomy should be

1. BaseStrategy    /libraries   /extensions
2. /QuadraticVotingStrategies         /RFPStrategies      /DonationVotingStrategies       /RetroStrategies         /LTIPStrategies   
   SimpleQuadraticVoting              SimpleRFP           SimpleDonationVoting            SimpleRetro             LTIPStrategy 
   QuadraticVotingWithReview          RFPCommittee        DonationVotingMerkleDistr