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

Compiler should export constructor code #8803

Closed
rmeissner opened this issue Apr 29, 2020 · 10 comments
Closed

Compiler should export constructor code #8803

rmeissner opened this issue Apr 29, 2020 · 10 comments
Labels
closed due inactivity The issue/PR was automatically closed due to inactivity. stale The issue/PR was marked as stale because it has been open for too long.

Comments

@rmeissner
Copy link

Result from the open discussion about upgradable contracts at the Solidity Summit

See https://hackmd.io/zjYDbVL5TEqgpOgMXXNT1g?both

As proxies cannot make use of the constructor code if the "master copy" it is very common to use some initializer code. This can cause issues and often requires overhead (e.g. adding a init function to the master copy)

Currently it's not possible to grab the constructor bytecode from the deployBytecode since the constructor argument positions are hardcoded, so a different-length runtime code would yield differ.

Therefore it would be very valuable to expose the constructor in a way that it can be reused.

@chriseth
Copy link
Contributor

The problem with the constructor argument offset is that we do not know how many bytes it would need. Can you please elaborate on what you would do with the constructor code? Would it work to have a constructor routine that abi-decodes the parameters from calldata?

@frangio
Copy link
Contributor

frangio commented Apr 29, 2020

Different people may have different uses for it. One option is to deploy a contract whose runtime code is the constructor code. A proxy contract could then be created like this:

contract Proxy {
  constructor(address ctor, address runtime, bytes args) {
    ctor.delegatecall(args); // require success
    _setImplementation(runtime); // sets the delegatecall target in the fallback
  }
  ...
}

Keep in mind that there is an expectation that code on chain be verified by Etherscan or similar services, and the way they currently work they are not able to verify runtime code in isolation, or constructor code in isolation, so there would need to be a standard way to deploy these things that source code verification services can correctly detect. It's not clear whether defining this should be in the scope of the compiler, but I think it should be a consideration.

@chriseth
Copy link
Contributor

Disregarding the problem with the argument offset, something like type(contract).initCode should include

  • the callvalue check?
  • everything up to but excluding the actual deploy code?
  • what about immutables? I guess immutables are incompatible with upgradable contracts

@frangio
Copy link
Contributor

frangio commented Apr 30, 2020

Yes immutables seem incompatible. Maybe it would be an error to access or output the initCode of a contract with immutable state variables.

The other two things you mention should be included yes. This includes for example setting the initial values of state variables.

@chriseth
Copy link
Contributor

We just discussed that in the meeting and found that this is a rather complicated feature that is easy to get wrong both on the user and on the compiler side. If someone can come up with a cleaner specification, we are happy to implement it, but at the current time this does not seem like a good idea.

@frangio
Copy link
Contributor

frangio commented May 20, 2020

@chriseth Thanks for the update. Can you share some detail on the different ways you see this could go wrong? I'd like to understand what kind of problems we're talking about.

@axic
Copy link
Member

axic commented May 21, 2020

We also discussed that #2296 seems to be a safer way, in our opinion.

@chriseth
Copy link
Contributor

@frangio the problem is that the constructor code is deeply coupled with the code deposit routine. Maybe you could try if you can easily extract the required code from --ir?

@rmeissner
Copy link
Author

While #2296 is nice it requires that the "Proxy" code is agreed on, right? (e.g. https://eips.ethereum.org/EIPS/eip-1167)

Just as a reference how it looks like currently if you want to do this in solidity at runtime: https://gist.github.com/cag/41c68a6402fc7c80b03f72cbf42f27f6#file-erc1155toerc20adapter-sol-L113

@NunoFilipeSantos NunoFilipeSantos added the stale The issue/PR was marked as stale because it has been open for too long. label Nov 25, 2022
@github-actions
Copy link

github-actions bot commented Feb 3, 2023

Hi everyone! This issue has been closed due to inactivity.
If you think this issue is still relevant in the latest Solidity version and you have something to contribute, feel free to reopen.
However, unless the issue is a concrete proposal that can be implemented, we recommend starting a language discussion on the forum instead.

@github-actions github-actions bot added the closed due inactivity The issue/PR was automatically closed due to inactivity. label Feb 3, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed due inactivity The issue/PR was automatically closed due to inactivity. stale The issue/PR was marked as stale because it has been open for too long.
Projects
None yet
Development

No branches or pull requests

5 participants