An ownable contract is not a cloneable contract

General
1

Hi there, I did a little experiment to testify my hypothesis that an ownable contract is not a cloneable contract. Here is what I found and what I did.

// SPDX-License-Identifier: MIT

pragma solidity 0.8.4;

import "@openzeppelin/contracts/proxy/Clones.sol";
import "openzeppelin-solidity/contracts/access/Ownable.sol";

contract Template is Ownable {

}

contract CheckOwnerableAndClone {

    address private templateAddr;

    function setTemplate(address addr) public {
        templateAddr = addr;
    }
    
    function getTemplate() public view returns(address) {
        return templateAddr;
    }

    function makeClone() public returns(address) {
        address newAddr = Clones.clone(templateAddr);
        return newAddr;
    }

}

interface IOwnable {
    function owner() external returns(address);
    function transferOwnership(address newOwner) external;
}

contract callContract {
    
    function checkOwner(address addr) public returns(address) {
        return IOwnable(addr).owner();
    }
    
    function transferOwner(address addr, address newOwner) public {
        IOwnable(addr).transferOwnership(newOwner);
    }
}

Basically the result is, the newly created contract by cloning cannot have the owner changed. The reason is once a contract is cloned and created, the new owner is 0x0000000000000000000000000000000000000000 and I believe there is no way to change it, since the function transferOwnership is onlyOwner modified.

Not the greatest discovery in the world, but this does have my concern verified. When cloning a contract, the template contract should not be a child of Ownable.

2 Likes
2

It goes further than this. Clones have to follow the same rules than upgradeable contracts. See Writing Upgradeable Contracts.

Notice the part about constructors vs initializers. Ownable is a contract that has a constructor. owner is a storage variable, and the constructor will initialize the storage of the "template", and not of the clone.

If you will Clone, you can't use any contract that relies on constructors. You shouldn't be using the @openzeppelin/contracts package for clonable contracts. You should be using our alternative package @openzeppelin/contracts-upgradeable, which has initializer functions instead of constructors. Read about it here.

4 Likes
3

The motivation behind this work is 1 verifying my concern that an ownable contract should not be cloned directly and 2 getting a practical example on how clone actually works.

If possible, I would suggest adding this example as an illustration to the quoted paragraph below. The plain text is understandable and having an accompanying example would work better.

In Solidity, code that is inside a constructor or part of a global variable declaration is not part of a deployed contract’s runtime bytecode. This code is executed only once, when the contract instance is deployed. As a consequence of this, the code within a logic contract’s constructor will never be executed in the context of the proxy’s state. To rephrase, proxies are completely oblivious to the existence of constructors. It’s simply as if they weren’t there for the proxy.

1 Like
4

Good question.

I think there is some basic knowledge before using the Clone. Actually, there are two kinds of code in the contract: Creation Code and Runtime Code. As for Runtime Code, they are the real logic to use in contract, and for Creation Code, it contains Runtime Code and Initialization

For Clone contract. It actually clones a contract’s runtime code. So just like frangio mentioned above, if you want to clone a contract has constructor, you should rewrite the contracts with the initializer functions or maybe you can also change all the variables that need to be assigned to constant.

As for example, I am not sure, I only know Uniswap uses the factory pattern to generate pair contract for each pair, each pair contract has the same code, but it seems like not the clones, maybe some multisig wallets use it, I am not sure.

1 Like
5

@Skyge @frangio These explanations are great. For a non-CS guy like myself, I definitely would appreciate more if the why part can be explained in a more intuitive way. Even the example by Uniswap give below is hard to digest. They actually use create2 to create a new contract with a deterministic address. (A side question, is it possible to achieve the same with Clones?)

Toy examples with reproducible results are much more friendly, can illustrate the why component more vividly and can capture your audience quickly if thinking from a publisher’s perspective.

By the way, is there a good source of reference for more information about Runtime Code and Creation Code?

    function createPair(address tokenA, address tokenB) external returns (address pair) {
        require(tokenA != tokenB, 'Pancake: IDENTICAL_ADDRESSES');
        (address token0, address token1) = tokenA < tokenB ? (tokenA, tokenB) : (tokenB, tokenA);
        require(token0 != address(0), 'Pancake: ZERO_ADDRESS');
        require(getPair[token0][token1] == address(0), 'Pancake: PAIR_EXISTS'); // single check is sufficient
        bytes memory bytecode = type(PancakePair).creationCode;
        bytes32 salt = keccak256(abi.encodePacked(token0, token1));
        assembly {
            pair := create2(0, add(bytecode, 32), mload(bytecode), salt)
        }
        IPancakePair(pair).initialize(token0, token1);
        getPair[token0][token1] = pair;
        getPair[token1][token0] = pair; // populate mapping in the reverse direction
        allPairs.push(pair);
        emit PairCreated(token0, token1, pair, allPairs.length);
    }
6

Not sure, but you can have a look at this article:

7

Yes! Clones has a function called cloneDeterministic that uses create2 under the hood.

8

Guys,

What about if the cloneable contract had a parameter "owner" at initialize function and call _transferOwnership. Wouln't it work?

contract CheckOwnerableAndClone is   Initializable, Ownable {

 constructor() Ownable() {}

 function initialize(address payable _owner) public initializer {
    _transferOwnership(_owner);
 }
}
9

See this issue for prior discussion: