wighawag / clones-with-immutable-args

BSD 3-Clause "New" or "Revised" License
229 stars 44 forks source link

supporting clones that can receive ETH #6

Closed sbauch closed 2 years ago

sbauch commented 2 years ago

Thanks for this and all of your OSS work! I've been wanting to learn more about factory-like things so started playing around here.

Inspired by the vested-erc20 project, I wanted to see if we could create a clone factory that would create clones that could receive ETH. Everything is working as intended, but the cloned contracts cannot receive ETH.

I certainly don't understand the EVM as deeply as others here, my naive understanding is that by implementing a receive function, the contract would now be able to receive ether via send() or transfer() calls, but these calls are made to the contract with empty calldata. And so this is where my understanding breaks down, but it seems like the clone always appends the immutable args as calldata when delegating to the implementation contract, so we aren't ever able to actually call receive() with empty calldata?

I doubt I can help much, but will certainly try to learn! Here's our goerli contracts if helpful:

Factory: https://goerli.etherscan.io/address/0x9862d0fdb8b72df9ef22f3e80cf5d6b74511c8f5#code Implementation: https://goerli.etherscan.io/address/0xc0d8d49af874c2dfcd43b3b525801e621d70599e#code

wighawag commented 2 years ago

Actually adding a fallback in the implementation (instead of receive) should make it work

Else we would need to make a special case for empty call data, but this would mean that the receive callback would have no access to the immutable), which is fine in some case, but maybe not in others

I guess the best solution would be to provide 2 kind of clones, one that accept empty fallback (aka receive) and the other, like the current one which require the use of fallback

z0r0z commented 2 years ago

I believe this was resolved via https://github.com/wighawag/clones-with-immutable-args/pull/7 @sbauch

sbauch commented 2 years ago

incredible that my issue was about 100x longer than the solution, gotta love it. bless you both 🙏