Solidity function implementing `public onlyOwner` cannot be called even by the owner
Asked Answered
B

3

7

I am following along the documentation here: https://docs.alchemyapi.io/alchemy/tutorials/how-to-create-an-nft/how-to-mint-a-nft. And have a smart contract of form:

pragma solidity ^0.8.0;
import "@openzeppelin/contracts/token/ERC721/ERC721.sol";
import "@openzeppelin/contracts/utils/Counters.sol";
import "@openzeppelin/contracts/access/Ownable.sol";

  contract NFTA is ERC721, Ownable {

     using Counters for Counters.Counter;
     Counters.Counter public _tokenIds;
     mapping (uint256 => string) public _tokenURIs;
     mapping(string => uint8) public hashes;

     constructor() public ERC721("NFTA", "NFT") {}

     function mintNFT(address recipient, string memory tokenURI)
          public onlyOwner
          returns (uint256)
      {
          _tokenIds.increment();

          uint256 newItemId = _tokenIds.current();
          _mint(recipient, newItemId);
          _setTokenURI(newItemId, tokenURI);

          return newItemId;
     }

     /**
      * @dev Sets `_tokenURI` as the tokenURI of `tokenId`.
      *
      * Requirements:
      *
      * - `tokenId` must exist.
      */
     function _setTokenURI(uint256 tokenId, string memory _tokenURI) internal virtual {
        require(_exists(tokenId), "ERC721URIStorage: URI set of nonexistent token");
        _tokenURIs[tokenId] = _tokenURI;
     }    

  }

When I attempt to estimate the gas cost of minting with this:

    const MY_PUBLIC_KEY  = '..'
    const MY_PRIVATE_KEY = '..'

    const ALCHEMY = {
        http: '',
        websocket:'',
    }

    const { createAlchemyWeb3 } = require("@alch/alchemy-web3");
    const web3 = createAlchemyWeb3(ALCHEMY.http);

    const NFTA = require("../artifacts/contracts/OpenSea.sol/NFTA.json");
    const address_a   = '0x...';
    const nft_A = new web3.eth.Contract(NFTA.abi, address_a);


    async function mint({ tokenURI, run }){

        const nonce = await web3.eth.getTransactionCount(MY_PUBLIC_KEY, 'latest'); 
        const fn  = nft_A.methods.mintNFT(MY_PUBLIC_KEY, '')

        console.log( 'fn: ', fn.estimateGas() )
    }

    mint({ tokenURI: '', run: true })

I receive error:

(node:29262) UnhandledPromiseRejectionWarning: Error: Returned error: execution reverted: Ownable: caller is not the owner

Presumably because mintNFT is public onlyOwner. However, when I check Etherscan, the From field is the same as MY_PUBLIC_KEY, and I'm not sure what else can be done to sign the transaction as from MY_PUBLIC_KEY. The easy way to solve this is to remove the onlyOwner from function mintNFT, and everything runs as expected. But suppose we want to keep onlyOwner, how would I sign the transaction beyond what is already written above.

Note I'm using hardHat to compile the contracts and deploying them. That is: npx hardhat compile npx hardhat run scripts/deploy.js

=============================================

addendum

The exact code given by alchemy to deploy the mint is:

async function mintNFT(tokenURI) {
  const nonce = await web3.eth.getTransactionCount(PUBLIC_KEY, 'latest'); //get latest nonce

  //the transaction
  const tx = {
    'from': PUBLIC_KEY,
    'to': contractAddress,
    'nonce': nonce,
    'gas': 500000,
    'data': nftContract.methods.mintNFT(PUBLIC_KEY, tokenURI).encodeABI()
  };

Note in the transaction the from field is PUBLIC_KEY, the same PUBLIC_KEY that deployed the contract, and in this case the nftContract has public onlyOwner specified. This is exactly what I have done. So conceptually who owns this NFT code? On etherscan is it the to address ( the contract address ), or the from address, which is my public key, the address that deployed the contract, and the one that is calling mint, which is now failing with caller is not the owner error. enter image description here

Search the internet, I see others have encountered this problem here: https://ethereum.stackexchange.com/questions/94114/erc721-testing-transferfrom, for Truffle you can specify the caller with extra field:

 await nft.transferFrom(accounts[0], accounts[1], 1, { from: accounts[1] })

Extra parameters is not an option here because I'm using hardhat.

Blackmon answered 16/4, 2021 at 2:34 Comment(2)
I guess your need to pass the test account sender address when you call methods.mintNFT.Homeomorphism
@MikkoOhtamaa how would i do that? And is the test account sender address the same as my metamask wallet public key?Blackmon
B
1

I finally figured it out, the contract does not initialize the way I deploy it. So you have to initialize it after deployment.

Blackmon answered 20/4, 2021 at 17:8 Comment(0)
P
6

OpenZeppelin's Ownable.sol defines the default owner value as the contract deployer. You can later change it by calling transferOwnership() or renounce the owner (i.e. set to 0x0) by calling renounceOwnership().

The onlyOwner modifier reverts the transaction if it's not sent by the current owner. (see the code)

So you need to call the mintNFT() function from the same address that deployed the contract, because that's the current owner. Or you can change the owner first by calling transferOwnership() (from the current owner address).

Removing the onlyOwner modifier from the mintNFT() function would allow anyone to call the function.

Pressroom answered 16/4, 2021 at 8:43 Comment(9)
this is the question, I only have one public key going around, shouldn't that be the address that deployed the contract?Blackmon
And when I call transferOwnership I'm getting the same error: ` Error: Returned error: execution reverted: Ownable: caller is not the owner, stating that I'm not calling the fn from the address that deployed it. But again I only have one wallet and one pk, so Im not sure what else i can do. Mikko above suggest I call mintNFT` and presumably transferOwner using test account sender address, what is the syntax here? And conceptually on Etherscan I see a to and from field for the contract, is the owner the to field or the from field?Blackmon
@Blackmon (1/2) From your screenshot, I can see that the contract 0xa26c... was deployed by address 0xd2590..., effectively making this address the owner. And that this address was able to execute the mintNFT() function.Pressroom
(2/2) It seems like the value of your MY_PUBLIC_KEY is other than 0xd2590... - which gets the tx reverted, only the owner can execute it. Also pay attention to the value of MY_PUBLIC_KEY which might be different from the PUBLIC_KEY in your second snippet. But I'm not able to verify it, because your question doesn't show the values.Pressroom
The reason the contract was able to mint was because I removed the onlyOwner bit in the one that i deployed, i was trying to verify that it runs w/o the ownership constraint. And yeah I only have one pk that's 0xd2590.... The second snippet is a direct past from the docs, in my code PUBLIC_KEY is MY_PUBLIC_KEY. This is why it's so infuriatingly fraustrating, I only have one pk private key pair, and that's what I use to deploy the contract and sign transactions. @PetrHejdaBlackmon
Just to confirm, I redployed with onlyOwner set here: ropsten.etherscan.io/tx/…, and mintNFT fails with: Error: Returned error: execution reverted: Ownable: caller is not the owner as expectedBlackmon
"I removed the onlyOwner", I didn't notice that - my bad... But right now, I'm out of ideas. Hopefully someone will be able to help you better.Pressroom
I'm still not clear on the concept of onlyOwner, if your public key is public, then can't anyone call the fn with your pk provided they have it? it doesn't exactly make onlyOwner secure.Blackmon
They would have to sign the transaction with the private key that pairs with the owner public key.Pressroom
C
3

Answering this for anyone else who stumbles across the issue while using the Alchemy tutorial:

In the tutorial, it says to init the contract in your mint method like this:

const contract = require("../artifacts/contracts/MyNFT.sol/MyNFT.json");
const contractAddress = "0x81c587EB0fE773404c42c1d2666b5f557C470eED";
const nftContract = new web3.eth.Contract(contract.abi, contractAddress);

However, if you attempt to call estimateGas() or encodeABI() it will fail with the onlyOwner error.

The solution is to change the third line to:

const nftContract = new web3.eth.Contract(contract.abi, contractAddress, {
    from: PUBLIC_KEY
});

This will set a default value of "From" so that when you call estimateGas() on a mint function marked onlyOwner, it will be able to use that from field to see that its the owner calling the estimateGas.

Took forever to figure this out.

Cohort answered 16/8, 2021 at 22:27 Comment(1)
Thank you so much! You can also add from: PUBLIC_KEY directly to the data passed into estimateGas(): const estimatedGas = await web3.eth.estimateGas({ from: PUBLIC_KEY, to: contractAddress, ...}).Thirtytwomo
B
1

I finally figured it out, the contract does not initialize the way I deploy it. So you have to initialize it after deployment.

Blackmon answered 20/4, 2021 at 17:8 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.