Upgradeable NFT Smart Contract

You need to write some test cases before deploying contracts, I think no one can guarantee a contract is absolutely safe.

Well it’s simply to ensure that if we want to add new features, fix bugs or mitigate attacks, we can do so through the proxy pattern.

Sorry, where do I need to place that?

Yes of course that’s the plan

function initialize() initializer {}

Don’t you need to use onlyInitializing to mitigate security? Doesn’t that ensure that it can only be called once and also not from other outside contracts/malicious actors?

I am sorry for misleading you, for the final deployed contract, just use initializer

ok so like this..

function initializable() public initializer {

Correct?

And do i include also?


Constructor()
_disableinitializers
}

``

See https://wizard.openzeppelin.com/#erc721 and enable Upgradeability to see examples on how to set up the initializer and that constructor statement.

Ok cool thanks got it!

To provide more context:

Your initializer could be named literally as "initialize" so that it is called automatically when you use Upgrades Plugins e.g.

function initialize() initializer public {

(otherwise you'd need to specify the initializer function name when using the plugin).

The initializer should also call the _init() methods of its parent contracts, which seems to be missing from your example.

The constructor is needed due to this reason.

OK great thanks for your input.

I've provided a snippet of the update below.

function initialize() initializer public {
    __ERC721_init("MyToken", "MTK");
    _baseUri = "";
    _setupRole(DEFAULT_ADMIN_ROLE, msg.sender);
    _setupRole(PAUSER_ROLE, msg.sender);
    _setupRole(MINTER_ROLE, msg.sender);
    _setupRole(BURNER_ROLE, msg.sender);
    _setupRole(OPERATOR_ROLE, msg.sender);
  }

What else should i be defining with _init()?

You should also call _init() for all of the other parent contracts that you are using. You can also see this in Wizard if you select those options on the left side.

From your example, that may include at least:

        __ERC721URIStorage_init();
        __Pausable_init();
        __AccessControl_init();

That's great thank you.

Do you know of any material i can seek to help me write tests on this contract?

Perhaps there's an NFT Upgradeable Smart Contract tutorial somewhere you know of?

Do you have a development environment set up already? If not, on Wizard you can click Download -> Development Package (Hardhat). That gives you a sample project including basic tests. Then you can make changes to that project for your contract and tests.

I do, i am using VS with the truffle suite. Is there a development package for Truffle instead of Hardhat anywhere?

Not at the moment. For Truffle tests with upgrades, see https://docs.openzeppelin.com/upgrades-plugins/1.x/truffle-upgrades#test-usage for some examples.

OK thanks..

On another note, it worth mentioning that when i compile/migrate with Truffle the contract above, including the Initializer, it doesn't compile successfully.

Error: Contract `project:/contracts/ERC721.sol:ERC721` is not upgrade safe

../project:/contracts/ERC721.sol:30: Contract `` has a constructor
    Define an initializer instead

The constructor is simply

Constructor() {
_disableInitializers;
}

If this is part of the Openzeppelin wizard, why am i not able to compile and migrate?

Thanks

The constructor needs to include the /// @custom:oz-upgrades-unsafe-allow constructor annotation:

    /// @custom:oz-upgrades-unsafe-allow constructor
    constructor() {
        _disableInitializers();
    }

This is because constructors are not generally allowed during upgrades, but in this case we specifically want to use the constructor to disable initialization at the implementation address. So we need to tell the plugin to allow it using this annotation.

1 Like

Great. This works perfectly. Thanks @ericglau