Problem as sometimes initialize() can't be called

[lil]

Hi,

I use OpenZeppelin SDK & and all the good things in @openzeppelin/contracts-ethereum-package/contracts/...

I have a simple function initialize() public initializer { ... } at the very beginning of my contract to setup everything.
But I inherit others in initializers from the OpenZeppelin contracts like the ERC721.

I noticed that sometime initialize() (meaning without args) can’t be call anymore what bring the hell everywhere (including in truffle tests where I call let init = await instance.initialize({from: accounts[0]}); or in the truffle console.

I noticed it happens randomly and when for eg when I call oz create initialize() is not first anymore in the list :

All contracts have been deployed
? Do you want to call a function on the instance after creating
 it? Yes
? Select which function (Use arrow keys)
❯   * initialize(sender: address) 
  * initialize(name: string, symbol: string)
* initialize() 

For now the solution I found is to create an initialize2() to be sure it is the only function with that name but it feels hacky, break the convention & might bite me sometime soon.

Do you experience the same behavior? if yes what is the best solution?

Many thanks

Environment
I am using the truffle, sdk version advised in the starter kit

npm install -g truffle@5.0.41 ganache-cli@6.7.0 @openzeppelin/cli@2.5.3

Node is v10.16.3
Solidity compiler is 0.5.11

Code to reproduce

Hard to tell, when you change some solidity code, sometimes after compiling initialize() won’t be first anymore.

[nventuro]

Hello @lil, thanks for posting!

I’m not sure I fully understand your issue: it sounds like you are trying to just call your own initialize function, despite there being other initializers present (like (name: string, symbol: string), which looks like ERC721Metadata's).

Like with constructors, you should make sure to call all base initializers from your own initialize function. If you’re inheriting from ERC721Metadata, you should make sure it is properly initialized:

contract MyContract is ERC721Metadata {
    function initialize(string memory name, string memory symbol) public initializer {
        ERC721Metadata(name, symbol);
    }
}

[lil]

Hello @nventuro,

Yes sorry…

I will describe the scenario:

State 1
Let’s say everything is going well, I run oz create
I get something like

All contracts have been deployed
? Do you want to call a function on the instance after creating
 it? Yes
? Select which function (Use arrow keys)
❯   * initialize()
  * initialize(name: string, symbol: string)
* initialize(sender: address) 

(note the order, initialize() is first)

I run my truffle tests & even beyond that everything works fine.

State 2
I have made some minor changes like adding a function at the end of the contract.
If I don’t oz upgradeI usually clean up everything (artifacts & co) rm build/contracts/* .openzeppelin/dev-*
I run oz create, everything goes well, but I notice that the functions order have changed… no problem it deploy and initialize anyway because I select my own initializer (initialize())

All contracts have been deployed
? Do you want to call a function on the instance after creating
 it? Yes
? Select which function (Use arrow keys)
❯   * initialize(sender: address) 
  * initialize(name: string, symbol: string)
* initialize() 

Then this time everything go wrong with the truffle tests because when it run let init = await instance.initialize({from: accounts[0]});

Error: invalid address (arg="sender", coderType="address", value={"from":"0x90F8bf6A479f320ead074411a4B0e7944Ea8c9C1"})
     at PromiEvent (/home/me/.npm-global/lib/node_modules/truffle/build/webpack:/packages/contract/lib/promievent.js:6:1)
      at TruffleContract.initialize (/home/me/.npm-global/lib/node_modules/truffle/build/webpack:/packages/contract/lib/execute.js:157:1)
      at Context.it (test/thing.truffle.test1.js:15:31)
      at process._tickCallback (internal/process/next_tick.js:68:7)

when I do

$ oz send-tx
? Pick a network development
? Pick an instance Thing at 0xA57B8a5584442B467b4689F1144D269d096A3daF
? Select which function pause()
✖ Calling: 'pause' with no arguments
Error while trying to send transaction to 0xA57B8a5584442B467b4689F1144D269d096A3daF. Error: Returned error: VM Exception while processing transaction: revert PauserRole: caller does not have the Pauser role

Here is my 'initialize()` (the only one in my contract)

    function initialize() public initializer {
        ERC721.initialize();
        ERC721Enumerable.initialize();
        ERC721Metadata.initialize("XXX", "XXX");
        ERC721MetadataMintable.initialize(msg.sender);
        Pausable.initialize(msg.sender);
        Ownable.initialize(msg.sender); 
        ReentrancyGuard.initialize();
    }

It took me a lot of time to notice that every time things go wrong I noticed the order changed in oz interactive command…

What it could be is that sometimes for eg ERC721.initialize() will be called instead of MyContract.initialize() ??? (I only see 1 initialize() in the oz interactive command).

Anyway my simple solution is to initialize with a initialize2() I hope it is ok.

Apart from that I am a very happy user of OpenZeppelin SDK

[nventuro]

Ah, I see. It seems like the issue is Truffle not handling function overloads by default. Truffle 5 added a way to do this, using the .methods proprty.

In your case, I believe the code should be something like:

await myContract.methods['initialize()']({ from: accounts[0] });

Alternatively, renaming your initializer to something else will also work. We’re working on a major improvement to the SDK that will remove the need for multiple initializers, and will make situations such as this one much easier to handle

[lil]

Thanks @nventuro its clear
(In fact the problem is not directly related to OpenZeppelin SDK…)

[abcoathup]

Hi @lil,

The CLI now supports deploying regular (non-upgradeable) contracts in the release candidate of OpenZeppelin CLI 2.8

Would appreciate if you could give the release candidate a try and let us know what you think!