Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider passing constructor parameters instead of calling initialize. #33

Closed
MicahZoltu opened this issue Dec 27, 2019 · 1 comment
Closed

Comments

@MicahZoltu
Copy link

https://github.com/Uniswap/uniswap-v2-core/blob/d2ddbd95e8500cfd3669ab81b64f48956521ea6b/contracts/UniswapV2.sol#L46-L50

I think I agree with your current solution, but I wanted to bring this up just to make sure the conversation was had about using a constructor for the exchange rather than a separate initialize function. You can append the abi.encode(token0, token1) to the exchange deployment bytes in order to call the constructor with parameters. This way you wouldn't need to separately call an initialize function after deployment with CREATE2. This would also make it so you wouldn't need a separate salt variable for CREATE2, since the bytecode for each exchange would be different.

The downside here is that concatenating bytes requires assembly. The assembly required for simple bytes concatenation is pretty simple (just put the bytes in memory, followed by the constructor arguments, then change the length to include both), but this is assembly which is generally much more complicated and harder to audit.

@NoahZinsmeister
Copy link
Contributor

unless @haydenadams feels strongly about this, my preference is to avoid the assembly required to concat bytestrings in solidity, and to instead continue to rely on initialize. also i think there is something nice about having the tokens be the CREATE2 salt, since it makes it very clear that the exchange bytecode is shared across all exchanges

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants