AstrolabDAO / strats

Home of Astrolab strategies
Other
15 stars 11 forks source link

[A62-11M] Incorrect Implementation of EIP-7540 #10

Open pde-rent opened 7 months ago

pde-rent commented 7 months ago

Recommendation:

We advise either the code to be substantially updated to comply with the EIP-7540 standard, or to remove support of EIP-7540 and instead implement a custom EIP-7540 adaptation removing unnecessary traits such as the IERC7540RedeemReceiver callback.

We consider either of the two approaches as valid alleviations to this exhibit given that the EIP-7540 is not yet mature.

The standard callbacks have little use cases imo but we want to comply, we'll indeed change msg.sender for _owner here. Can you list out the other non-compliant functions please? Thanks

omniscia-core commented 7 months ago

Based on your use-case, we believe it may be impossible to strictly comply with the standard.

Specifically, EIP-7540 denotes that either asynchronous or synchronous flows can exist and they are mutually exclusive. This means that, for the asynchronous redemption flow the following functions must be overridden:

We can see that the overrides described in EIP-7540 do not align with the intents of the Astrolab DAO implementation. We understand that a hybrid model is desired, so we propose that both synchronous and asynchronous models are supported.

pde-rent commented 6 months ago

Thank you for raising this. After analysing both specs in-depth, I can confirm that being hybrid ultimately breaks both compliances in multiple ways. This is a compromise we'll happy to make if it allows for maximum redemption/depositing efficiency through netting out, and async compatibility is crucial to farming strategies with short to medium term lock-ins.

Despite not strictly sticking to the standard, we're maxing out ERC-7540 compatibility in cc9f72e29ac07f84a5433bb06f82c4c363b8f8dc