Upgradeable Contract Failure on Mainnet, but success on Rinkeby

Support Upgrades
, ,
1

Hi everyone,

A few weeks ago we had a mishap where our contract worked on Rinkeby Test net but failed on Mainnet, costing us around $1000 USD. The "owner" function on Rinkeby correctly returned our minting address, however on Mainnet we got the 0x000... address for the owner, effectively locking us out. We made sure to switch out the Oracles as well.

Would be lovely if some experienced Eth Devs could look over the contract and see if they can spot the issue, or any other vulnerabilities.

Another user pointed out that the issue was likely that the proxy did not deploy, and therefore did not call the initialize function when deploying. So we essentially only deployed the implementation, somehow.

Here is the contract:

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "@openzeppelin/contracts-upgradeable/token/ERC721/ERC721Upgradeable.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721BurnableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721PausableUpgradeable.sol";



import "@openzeppelin/contracts-upgradeable/access/AccessControlEnumerableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";

import "@openzeppelin/contracts-upgradeable/utils/ContextUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/utils/CountersUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";

import "@openzeppelin/contracts-upgradeable/utils/cryptography/draft-EIP712Upgradeable.sol";
import "@openzeppelin/contracts-upgradeable/utils/cryptography/SignatureCheckerUpgradeable.sol";
import "@chainlink/contracts/src/v0.6/interfaces/AggregatorV3Interface.sol";


//Rarible
import "@rarible/royalties/contracts/impl/RoyaltiesV2Impl.sol";
import "@rarible/royalties/contracts/LibPart.sol";
import "@rarible/royalties/contracts/LibRoyaltiesV2.sol";


contract NPortContract is Initializable, ContextUpgradeable, AccessControlEnumerableUpgradeable,OwnableUpgradeable, ERC721EnumerableUpgradeable,
 ERC721BurnableUpgradeable, ERC721PausableUpgradeable, EIP712Upgradeable, RoyaltiesV2Impl {
    function initialize(string memory name, string memory symbol, string memory baseTokenURI) public virtual initializer {
        __NPortContract_init(name, symbol, baseTokenURI);
    }
    using CountersUpgradeable for CountersUpgradeable.Counter;

    bytes32 public constant MOD_ROLE = keccak256("MOD_ROLE");

    bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
    bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");

    mapping(uint256 => bytes) unlistedSignatures;

    CountersUpgradeable.Counter private _tokenIdTracker;

    string private _baseTokenURI;

    AggregatorV3Interface internal priceFeed;

         string[] public URIs;
         mapping(string => bool) usedURIs;

    /**
     * @dev Grants `DEFAULT_ADMIN_ROLE`, `MINTER_ROLE` and `PAUSER_ROLE` to the
     * account that deploys the contract.
     *
     * Token URIs will be autogenerated based on `baseURI` and their token IDs.
     * See {ERC721-tokenURI}.
     */
    function __NPortContract_init(string memory name, string memory symbol, string memory baseTokenURI) internal initializer {
        __Context_init_unchained();
        __ERC165_init_unchained();
        __AccessControl_init_unchained();
        __AccessControlEnumerable_init_unchained();
        __Ownable_init_unchained();
        __ERC721_init_unchained(name, symbol);
        __ERC721Enumerable_init_unchained();
        __ERC721Burnable_init_unchained();
        __Pausable_init_unchained();
        __ERC721Pausable_init_unchained();
        __EIP712_init_unchained(name,  "1.0.0");
        __NPortContract_init_unchained(name, symbol, baseTokenURI);


    }

    function __NPortContract_init_unchained(string memory name, string memory symbol, string memory baseTokenURI) internal initializer {
        _baseTokenURI = baseTokenURI;

        _setupRole(DEFAULT_ADMIN_ROLE, _msgSender());
        
        _setupRole(MOD_ROLE, _msgSender());

        _setupRole(MINTER_ROLE, _msgSender());
        _setRoleAdmin(MINTER_ROLE, MOD_ROLE);

        _setupRole(PAUSER_ROLE, _msgSender());
        _setRoleAdmin(PAUSER_ROLE, MOD_ROLE);

        
        //This needs to be changed to mainnet before final deployment, currently rinkeby
        //0x5f4eC3Df9cbd43714FE2740f5E3616155c5b8419
        //Rinkeby --> 0x8A753747A1Fa494EC906cE90E9f37563A8AF630e
        //Mainnet --> 0x5f4eC3Df9cbd43714FE2740f5E3616155c5b8419
        priceFeed = AggregatorV3Interface(0x8A753747A1Fa494EC906cE90E9f37563A8AF630e);
        //Allow contract to transfer any and all of original owners tokens from now on.
        setApprovalForAll(address(this), true);

        
    }


    function _baseURI() internal view virtual override returns (string memory) {
        return _baseTokenURI;
    }
  
    

function getPriceInUSD(uint256 val) public view returns (uint256) {
uint256 msgValueInUSD = ((val * (uint256)(getLatestPrice())/ (10 ** 18)));
return msgValueInUSD;
}

event RedeemAndMint(uint256 _tokenId, address _from, address _to, uint256 _soldPrice, uint256 _nportCut, string _tokenUri);

    function redeemAndMint(uint256 key,uint256 deadline, address signer,bytes calldata signature, string memory tokenuri)
    external payable
    {
        //Deny if art owner isn't registered with us
        require(hasRole(MINTER_ROLE, signer), "Address is not verified with NPort");
        require(usedURIs[tokenuri] == false, "Token has already been minted");
        uint256 price = getPriceInUSD(msg.value);



        //Also pass in date, if date deadline matches signed date, then we check against this date.
        require(_verify(signer, _hash(key,deadline, price, tokenuri), signature), "Invalid signature");

        require(block.timestamp < deadline,"Expired");

        //Make sure this signature wasn't revoked by lister
        require(keccak256(abi.encode(unlistedSignatures[key])) != keccak256(abi.encode(signature)), "Signature has been revoked by owner");
        //mint can only be called from within this contract
        mint(signer);

        URIs.push(tokenuri);
        //Add to used signatures so this signature can't be reused
        usedURIs[tokenuri] = true;
        IERC721Upgradeable tokenContract = IERC721Upgradeable(address(this));

        tokenContract.approve(msg.sender,_tokenIdTracker.current() - 1);
        //need to do current -1 becaues mint increments the counter
        tokenContract.safeTransferFrom(signer, msg.sender, _tokenIdTracker.current() - 1);
        
        //Set the signer as the receiver for all royalties for this token from now on.
        //10 means 105%
        uint256 cut = (msg.value * 10) / 100;
        uint256 transferAmt = msg.value - cut;

        //set royalties to original token minter to 10% (/100)
        _internalSetRoyalties(_tokenIdTracker.current() - 1, payable(signer), 1000);
        //give cut to NPort account
        payable(owner()).transfer(cut);

        //transfer the rest to the seller
        payable(signer).transfer(transferAmt);
        unlistedSignatures[_tokenIdTracker.current() - 1] = abi.encode(0);
        emit RedeemAndMint(_tokenIdTracker.current() - 1,signer,msg.sender, msg.value, cut, tokenuri);
       
    }

event Redeem(uint256 _tokenId, address _from, address _to, uint256 _soldPrice, uint256 _nportCut);


     //if token which is ALREADY MINTED will be set for sale again. In this case, key is tokenId
    function redeem(uint256 key,uint256 deadline,string memory tokenuri, address signer,bytes calldata signature)
    external payable
    {
        //require signer is owner of token id

        require(_exists(key), "Token does not exist");
       
        uint256 price = getPriceInUSD(msg.value);

        //Also pass in date, if date deadline matches signed date, then we check against this date.
        require(_verify(signer, _hash(key,deadline, price, tokenuri), signature), "Invalid signature");

        require(block.timestamp < deadline,"Expired");

        require(ownerOf(key)==signer, "Signer does not own token");

        //Make sure this signature wasn't revoked by lister

        require(keccak256(abi.encode(unlistedSignatures[key])) != keccak256(abi.encode(signature)), "Signature has been revoked by owner");


        IERC721Upgradeable tokenContract = IERC721Upgradeable(address(this));
        //after transfer, approval is automatically revoked, no need to implement
        tokenContract.approve(msg.sender,key);

        tokenContract.safeTransferFrom(signer, msg.sender, key);
        
        //calcualte amount original token minter should receive
        (address royaltyReceiver, uint256 royaltyAmt) = this.royaltyInfo(key, msg.value);

        //5 means 5%
        uint256 nportCut = (msg.value * 2) / 100;
        //give cut to NPort account
        payable(owner()).transfer(nportCut);
        payable(royaltyReceiver).transfer(royaltyAmt);
        uint256 transferAmt = msg.value - nportCut - royaltyAmt;

        //give cut to original minter account
        //transfer the rest to the seller
        payable(signer).transfer(transferAmt);
        unlistedSignatures[key] = abi.encode(0);

        emit Redeem(key,signer,msg.sender, msg.value, nportCut);
       
    }

event Unlist(uint256 _tokenId, address _from);

     //Invalidate authorized signature, requires fee
    function unlist(uint256 key,uint256 deadline,string memory tokenuri, address signer,bytes calldata signature, uint256 listingPrice)
    external
    {
        uint256 price = getPriceInUSD(listingPrice);
        require(_verify(signer, _hash(key,deadline, price,tokenuri), signature), "Invalid signature");

        require(signer == msg.sender, "Must own token to unlist it");
        //require signer is owner of token id
        unlistedSignatures[key] = signature;


        emit Unlist(key,msg.sender);
       
    }

    function _hash(uint256 key, uint256 deadline, uint256 price, string memory tokenuri)
    internal view returns (bytes32)
    {
        return _hashTypedDataV4(keccak256(abi.encode(
            keccak256("NFT(uint256 deadline,uint256 price,uint256 key,string tokenuri)"),
            deadline,
            price,
            key,
            keccak256(bytes(tokenuri))
        )));
    }

    function _verify(address signer, bytes32 digest, bytes memory signature)
    internal view returns (bool)
    {
        return SignatureCheckerUpgradeable.isValidSignatureNow(signer, digest, signature);
    }

    //Standard function required by minting platforms. Returns metadata uri if given token
    //is found.
    function tokenURI(uint256 tokenId) public view override returns (string memory) {
        require(_exists(tokenId), "ERC721Metadata: URI query for nonexistent token");
        return string(abi.encodePacked(_baseURI(), URIs[tokenId]));
    }

    

    function getLatestPrice() public view returns (int) {
    (
      uint80 roundID,
      int price,
      uint startedAt,
      uint timeStamp,
      uint80 answeredInRound
  ) = priceFeed.latestRoundData();
    return (price/ 10 ** 8); //price of 1 ether in USD
  }

    //From is spoofed address, to is msg sender
    event PowerMint(uint256 _tokenId,string tokenURI, address from, address to);

    //Signer should be the person we want to mock mint to
    function _powerMint(address signer,string memory tokenuri)public onlyOwner{
        require(usedURIs[tokenuri] == false, "Token has already been minted");

        mint(signer);
        URIs.push(tokenuri);
        usedURIs[tokenuri] = true;

        IERC721Upgradeable tokenContract = IERC721Upgradeable(address(this));

        tokenContract.approve(msg.sender,_tokenIdTracker.current() - 1);
        //need to do current -1 becaues mint increments the counter
        tokenContract.safeTransferFrom(signer, msg.sender, _tokenIdTracker.current() - 1);
          
        _internalSetRoyalties(_tokenIdTracker.current() - 1, payable(signer), 1000);

        emit PowerMint(_tokenIdTracker.current() - 1,tokenuri, signer, msg.sender);
    }

    function mint(address to) private {
        // We cannot just use balanceOf to create the new tokenId because tokens
        // can be burned (destroyed), so we need a separate counter.

        _mint(to, _tokenIdTracker.current());
        _tokenIdTracker.increment();
    }


    function pause() public virtual {
        require(hasRole(PAUSER_ROLE, _msgSender()), "Must have pauser role to pause");
        _pause();
    }

    function unpause() public virtual {
        require(hasRole(PAUSER_ROLE, _msgSender()), "Must have pauser role to unpause");
        _unpause();
    }

    function _beforeTokenTransfer(address from, address to, uint256 tokenId) internal virtual override(ERC721Upgradeable, ERC721EnumerableUpgradeable, ERC721PausableUpgradeable) {
        super._beforeTokenTransfer(from, to, tokenId);
    }

      //Rarible royalties

    function setRoyalties(uint _tokenId, address payable _royaltiesRecipientAddress, uint96 _percentageBasisPoints) public onlyOwner{
        _internalSetRoyalties(_tokenId, _royaltiesRecipientAddress, _percentageBasisPoints);
    }

    function _internalSetRoyalties(uint _tokenId, address payable _royaltiesRecipientAddress, uint96 _percentageBasisPoints) internal{
        LibPart.Part[] memory _royalties = new LibPart.Part[](1);
        _royalties[0].value = _percentageBasisPoints;
        _royalties[0].account = _royaltiesRecipientAddress;
        _saveRoyalties(_tokenId,_royalties);
    }

    //EIP 2981 royalties for Opensea and mintable
      function royaltyInfo(uint256 _tokenId,uint256 _salePrice) external view returns (address receiver,uint256 royaltyAmount){
          LibPart.Part[] memory _royalties = royalties[_tokenId];
          if(_royalties.length > 0){
              return (_royalties[0].account,(_salePrice * _royalties[0].value)/10000);
          }
          return (address(0), 0);
      }
      

    /**
     * @dev See {IERC165-supportsInterface}.
     */
     bytes4 private constant _INTERFACE_ID_ERC2981 = 0x2a55205a;
    function supportsInterface(bytes4 interfaceId) public view virtual override(AccessControlEnumerableUpgradeable, ERC721Upgradeable, ERC721EnumerableUpgradeable) returns (bool) {
        if(interfaceId == LibRoyaltiesV2._INTERFACE_ID_ROYALTIES){
            return true;
        }

        if(interfaceId == _INTERFACE_ID_ERC2981){
            return true;
        }

        return super.supportsInterface(interfaceId);
    }


    uint256[48] private __gap;
}

and here is the code we use to deploy:

  this.timeout(110000);
    this.accounts = await ethers.getSigners();
    ({ chainId: this.chainId } = await ethers.provider.getNetwork());
    NPort = await ethers.getContractFactory("NPortContract");
    console.log(this.accounts);
    console.log("about to deploy contract");

    nport = await upgrades.deployProxy(NPort, ["NPortContract", "NPORT", "https://ipfs.io/ipfs/"], {initializer: 'initialize'});



    await nport.deployed();
    console.log("contract deployed to " + nport.address);
    console.log("initialized contract");

And the transaction: On etherscan

1 Like
2

Hi, I think the OpenZeppelin Hardhat Upgrades plugin provides the deployProxy function to deploy upgradeable contract. This deploys implementation contract, ProxyAdmin contract and the proxy contract. But for the link you shared, I only find one contract.

And I think for the script, it should be:

this.timeout(110000);
    this.accounts = await ethers.getSigners();
    ({ chainId: this.chainId } = await ethers.provider.getNetwork());
    NPort = await ethers.getContractFactory("NPortContract");
    console.log(this.accounts);
    console.log("about to deploy contract");

    const nport = await upgrades.deployProxy(NPort, ["NPortContract", "NPORT", "https://ipfs.io/ipfs/"], {initializer: 'initialize'});



   // await nport.deployed(); // <<<<<------ I think you can delete this line.
    console.log("contract deployed to " + nport.address);
    console.log("initialized contract");
3

I see. So it looks like only the implementation contract was deployed, for some reason, is it possible to deploy the proxy and attach it to the implementation without re-deploying the implementation?

4

Emmmm, maybe you can do this manually. For the plugin, I am not sure.

5

Yes, you can deploy the proxy and attach it to the existing implementation.

You can hack around the plugin to get it to reuse it. My recommendation would be to run deployProxy against a local Hardhat Node, and then copy the resulting .openzeppelin/unknown-31337.json to .openzeppelin/mainnet.json. You will need to delete the "admin" and "proxies" keys, and there should be a single value under "impls", whose address value you should replace with the address you have on mainnet and want to reuse.

After you're done setting this up, you should test it on a local mainnet fork.

1 Like