Importing non-upgradable contracts into upgradable contract

I have two contracts: HelloLogicV1, which is upgradeable, and HelloStore, which is not. HelloLogicV1's proxy owns HelloStore and references functions in HelloStore, so I’m importing HelloStore into HelloLogicV1.

    // HelloLogicV1.sol
    pragma solidity ^0.6.12;

    import "@openzeppelin/contracts-ethereum-package/contracts/access/Ownable.sol";
    import "./HelloStore.sol";

    contract HelloLogicV1 is OwnableUpgradeSafe {

        HelloStore public store;

        function initialize(HelloStore _store) public initializer {
            OwnableUpgradeSafe.__Ownable_init();
            store = _store;
        }

        function increment() external {
            store.increment();
        }
    }
    // HelloStore.sol
    pragma solidity ^0.6.12;

    import "@openzeppelin/contracts/access/Ownable.sol";

    contract HelloStore is Ownable {
        constructor() public {};

        function increment() external onlyOwner {
            // do stuff
        }
    }

It doesn’t seem appropriate to import an upgradeable and non-upgradeable Ownable.sol into HelloLogicV1. Using this pattern with other libraries throws DeclarationError: Identifier already declared. as well. Am I better off importing an interface HelloStore {} into HelloLogicV1, so I can just reference HelloStore's functions without importing all of its imports? There are many functions in HelloStore, so an interface may be redundant.

Which approach would you recommend?

EDIT: If I use an interface and import an upgradable contract in HelloLogicV1 and non-upgradable contract in HelloStore (eg SafeMath), oz check produces the warning: There is more than one contract named SafeMath. The compiled artifact for only one of them will be generated.

1 Like

Hi @dtp5,

I would recommend only making contracts upgradeable when they need to be. (Just like any other trusted functionality).

You can either import the regular (non-upgradeable) contract into the upgradeable contract or create an interface with only the functions that you need to call.

My preference would be to import an interface, though if there is type coupling between the contracts then this might not make sense.

As for the errors and warnings. For regular contracts you can import OpenZeppelin Contracts, and for upgradeable contracts you can import OpenZeppelin Contracts Ethereum Package with the UpgradeSafe suffix. The exception is libraries and interfaces where you can use the version from OpenZeppelin Contracts.

IHelloStore.sol

// IHelloStore.sol
pragma solidity ^0.6.12;

interface IHelloStore {

    function increment() external;
}

HelloStore.sol

// HelloStore.sol
pragma solidity ^0.6.12;

import "@openzeppelin/contracts/access/Ownable.sol";
import "@openzeppelin/contracts/math/SafeMath.sol";

import "./IHelloStore.sol";

contract HelloStore is IHelloStore, Ownable {
    constructor() public {}

    using SafeMath for uint256;

    function increment() external override onlyOwner {
        // do stuff
    }
}

HelloLogicV1.sol

// HelloLogicV1.sol
pragma solidity ^0.6.12;

import "@openzeppelin/contracts-ethereum-package/contracts/access/Ownable.sol";
import "@openzeppelin/contracts/math/SafeMath.sol";

import "./IHelloStore.sol";

contract HelloLogicV1 is OwnableUpgradeSafe {
    IHelloStore public store;

    using SafeMath for uint256;

    function initialize(IHelloStore _store) public initializer {
        OwnableUpgradeSafe.__Ownable_init();
        store = _store;
    }

    function increment() external {
        store.increment();
    }
}

As an aside, you may want to consider using AccessControl instead of Ownable for more finely grained control. https://docs.openzeppelin.com/contracts/3.x/access-control#role-based-access-control

Thank you @abcoathup. This was the exact interface solution that I implemented, until ultimately deciding to make HelloStore.sol upgradable for a separate reason. Thanks very much for your reply.

1 Like