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

Syntax for creating copies of contracts #2296

Closed
chriseth opened this issue May 24, 2017 · 15 comments
Closed

Syntax for creating copies of contracts #2296

chriseth opened this issue May 24, 2017 · 15 comments
Labels
closed due inactivity The issue/PR was automatically closed due to inactivity. language design :rage4: Any changes to the language, e.g. new features stale The issue/PR was marked as stale because it has been open for too long.

Comments

@chriseth
Copy link
Contributor

chriseth commented May 24, 2017

The factory pattern that creates multiple instances of the same contract is quite common. Solidity only provides a way to implement it which requires full copies of the contracts and thus is rather costly. After metropolis, delegatecall-proxies are possible and they should also be supported for contracts created from contracts.

--

Alternative 1:

If we have an existing contract c of type C, then copyof c(arg1, ..., argn) creates a delegatecall proxy with constructor taken from C and supplied arguments arg1, ..., argn.

Drawbacks:
The programmer has to explicitly create a "master copy" of the contract and has to ensure that it will not be selfdestructed as long as there are other copies.

--

Alternative 2:

Another suggestion might be that the master copy M of all contracts created by a contract C is created as part of the constructor of C. This master copy ensures that it cannot be selfdestructed by storing a flag in its storage at a certain location. Every time the master copy runs, it reads that location. If the flag is set, it throws. The idea is that actual copies will have that flag set to false, while the master copy has it set to true. This way, the master copy can only be delegatecalled and will throw if it is called regularly.

Another way to ensure this would be to not use storage but instead store the address of the master copy inside its code at the time where the constructor of the master copy runs. At runtime, the current address is compared with this address and results in a reversion if they are equal.

The syntax here would rather be copyof C(arg1, ..., argn) where C is the type of the contract and the address is either stored at a location in storage or the master copy is created using CREATE2 and thus its address is known.

We could also change the way new creates contract to this method.

In all cases, the fallback function of a copy should probably stay as cheap as possible and thus it should not delegatecall into the copy, i.e. all functions it calls internally have to be copied.

@chriseth
Copy link
Contributor Author

Alternative 2 also has the benefit that the code of the contract to be created is only part of the creation transaction code and not part of the deployed code (as it is currently).

@veox
Copy link
Contributor

veox commented May 31, 2017

The programmer has to explicitly create a "master copy" of the contract and has to ensure that it will not be selfdestructed as long as there are other copies.

Seems to me that this would require storing a list of "child" contracts, and it is possible to block the destruction of "master" by creating a copy and throwing away the key.


EDIT: Won't work, see next comment.

Every time the master copy runs, it reads that location. If the flag is set, it throws.

Instead of a separate flag, can this be achieved by having the first five bytes of a master contract as

0x 60 0060ff 57(PUSH10x00PUSH10xff JUMPI)

instead of (current)

0x 60 606040 52(PUSH10x60PUSH10x40 MSTORE)

or is this too hackish?..

The master throws due to the infinite loop consuming all gas. Or Does doing DELEGATECALL execute that piece, too?


We could also change the way new creates contract to this method.

Bytecode will differ then if compiled with a previous version and the new one (SFTP). IMO it's best to avoid semantic changes in keywords if possible. This makes some forms of analysis more difficult. copyof looks fine to me.

@chriseth
Copy link
Contributor Author

chriseth commented May 31, 2017

@veox thanks for your comment! Unfortunately, just changing the code is not an option, also the regular call would run into an infinite loop. But it seems that there is a cheaper way: At deploy time, the address of the master is stored in its own code and that is checked for each call. (updated the description)

@chriseth
Copy link
Contributor Author

Suggestion for the syntax for alternative 2: create C(x1, ..., xn)

@chriseth chriseth added the Focus label Jun 22, 2017
@chriseth
Copy link
Contributor Author

One thing to keep in mind here is that the fallback function should probably stay as cheap as possible, i.e. it should not delegatecall into the copy contract. If we do this, it has to "pull in" all functions it calls to internally (the same holds true for the constructor).

@axic
Copy link
Member

axic commented Jul 19, 2017

@chriseth do you remember what was the design we agreed the last time (~5 meetings ago)?

@chriseth
Copy link
Contributor Author

Hm, I don't remember that we decided anything about this.

@axic axic added the feature label Sep 14, 2017
@axic
Copy link
Member

axic commented Sep 15, 2017

New proposal is to use new shared C(arg1, .. argN) from the BLWS'17.

@axic
Copy link
Member

axic commented Nov 11, 2017

We could consider introducing some kind of flag "in-master-copy", which would be accessible in the code and the user could code different behaviour using that (this also could work well with modifier areas).

contract Wallet {
  address owner;

  modifier ownerOnly { require(msg.sender == owner); _; }
  modifier onlyInInstance { require(isInMasterCopy == false); _; }

  onlyInInstance {
    function transfer(address to) ownerOnly {}
    function otherriskyfunction() {}
  }

  function Wallet(address _owner) { owner = _owner; }
}

contract walletFactory {
  function deployInstance() returns (address) {
    return new shared Wallet(msg.sender);
  }
}

@axic
Copy link
Member

axic commented Nov 11, 2017

Thinking more about this, it perhaps makes sense to have a special marker for such contracts/libraries: shared contract Wallet or shared library Wallet. The isInMasterCopy would only be available in such contracts and the new shared expression would only work with them.

@chriseth
Copy link
Contributor Author

As a first stage of this feature, I would propose the following:

It is possible to create contracts using new shared ContractName. The constructor of a contract that contains a new shared ContractName directive deploys master copies of such contracts. The master copy ignores the constructor code but retains the full deployed code. The location of the master copy is stored in the code. The deployed code will be call-protected. Whenever a new shared ContractName is executed, a special contract is created that:

  • runs the full constructor of ContractName
  • deploys a delegatecall-proxy to the stored master copy address (it delegates everything that has nonempty calldata)
  • still contains the fallback function and all functions called from there

@rayeaster
Copy link

any illustrative code for this proxy feature?

@axic axic added the language design :rage4: Any changes to the language, e.g. new features label Jul 28, 2018
@axic axic removed the Focus label Jul 30, 2018
@christianparpart
Copy link
Member

While I was coming from the CREATE2 ticket linked above, I wanted to give an example on how other languages (here: F#) provide a somewhat equivalent feature:

{ masterCopy with X = 1, Y = 2 }

whereas X and Y here could be the named parameters of the constructor of the type of masterCopy.

Linking this with the CREATE2 ticket, we could extend this to:

{ masterCopy with X = 1, Y = 2, salt = ... }

Alternatively

{ masterCopy <salt=...> with X = 1, Y = 2 }

@github-actions
Copy link

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Feb 12, 2023
@github-actions
Copy link

Hi everyone! This issue has been automatically 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 20, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 20, 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. language design :rage4: Any changes to the language, e.g. new features 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

6 participants