coinbase / smart-wallet

MIT License
258 stars 47 forks source link

Feature request: `delegateExecute` #75

Open clemlak opened 2 weeks ago

clemlak commented 2 weeks ago

The current implementation only allows "direct calls" to other contracts via execute and executeBatch. This is pretty limiting and introducing a way to delegatecall to external smart-contracts would make the smart-wallets smarter and allow them to perform specific actions that are not currently possible.

Here's an example: let's say that Alice deposited X and Y tokens into a liquidity pool, later on she wants to withdraw the liquidity and swap all her X tokens for Y tokens. Currently the only way to perform this is to prepare the calldata offchain and to call the executeBatch function to perform the 2 calls. However, these calls would be "static", since each call has been prepared offchain, they cannot be changed at execution time and the worst part is that returned values cannot be passed from one function call to another. Removing LP tokens from a pool could result in slightly different amounts than expected, same thing when it comes to swapping tokens, thus resulting in dust of tokens and unoptimized operations. On the other hand, a smart-contract could have been written with a function combining the liquidity withdrawal and the swap. This function would be more optimized since it has access to returned values, and the smart-wallet would simply need to call this function via delegatecall to perform this action.

However, introducing a delegateExecute function is not that simple and can introduce security risks. As you can see in the Solady implementation here, the biggest threat is malicious contracts trying to manipulate the storage slots. Anyway, I believe that performing safety checks and comparing the value of the storage slots before and after the call would be a way to prevent such attacks.

wilsoncusack commented 1 week ago

Thanks for bringing up! We intentionally chose to exclude this because of the security risks--it gets hard to reason about your storage layout, and any contract can change your owners! The safety checks that Solady has are less feasible in multi owner context.