zeppelinos/zos-lib

Upgrade example throws revert on initialize() call

Closed this issue · 4 comments

I'm trying to reproduce the example from this tutorial https://docs.zeppelinos.org/docs/low_level_contract.html.

Expected behaviour

I would have an initialized proxy to MyContract_v0 instance

Current behaviour

It throws revert when I invoke myContract.initialize() method:

const myContract_v0 = await MyContract_v0.new();
const proxy = await AdminUpgradeabilityProxy.new(myContract_v0.address);
const myContract = await MyContract_v0.at(proxy.address);
const value = 42;
await myContract.initialize(value);

I've tested on both embedded ganache-core and parity aura private net.

Detailed example

I've prepared this repo to help you reproduce my problem https://github.com/chebykin/zos-upgrade-example

I think that because you call initialize by admin (default account in truffle), which will revert in function _willFallback in AdminUpgradeabilityProxy.sol

/**
 * @dev Only fall back when the sender is not the admin.
 */
function _willFallback() internal {
  require(msg.sender != _admin(), "Cannot call fallback function from the proxy admin");
  super._willFallback();
}

Simply, you can call initialize by other address, it should work.

await myContract.initialize(value, { from: `OTHER ADDRESS` });

Or you can use the UpgradeabilityProxyFactory to create proxy, which is nice because it supports createProxyAndCall.

@han0110 Thank you!

Calling initialize() using a different than admin address works perfectly!

I think this should be noticed somehow in the examples/simple/index.js and the doc.

@chebykin You're welcome.

Agree, it should be noticed if AdminUpgradeabilityProxy is implemented this way.

By the way, I am wondering why the function _willFallback requires msg.sender not to be admin be default. In my use case, admin also needs to call delegate contract's function as well. Is there any security vulnerability if I disable the function _willFallback? Hope to get some advice.

@chebykin thanks for asking and sorry for the late response, we've been working on a new JS interface to work with upgradeable smart contracts which will abstract you from its complexity and improve your experience a lot: see OpenZeppelin/openzeppelin-sdk#70

Closing the issue