search

OpenZeppelin / openzeppelin-contracts

OpenZeppelin Contracts is a library for secure smart contract development.
https://openzeppelin.com/contracts
MIT License
24.44k stars 11.68k forks source link

Upgradeability issue for VotesUpgradeable.sol #5092

Closed njelich closed 1 week ago

njelich commented 1 week ago

A new simple ERC20 upgradeable contract that uses Votes is created using the OpenZeppelin official wizard. Adding that code to a simple hardhat project and trying to deploy it using openzeppelin upgrades library yields the following error:

@openzeppelin/contracts-upgradeable/governance/utils/VotesUpgradeable.sol:261: Variable `op` is an internal function
    Use external functions or avoid functions in storage.
     If you must use internal functions, skip this check with the `unsafeAllow.internal-function-storage`
     flag and ensure you always reassign internal functions in storage during upgrades
    https://zpl.in/upgrades/error-009
      at assertUpgradeSafe (node_modules/@openzeppelin/upgrades-core/src/validate/query.ts:20:11)
      at validateImpl (node_modules/@openzeppelin/hardhat-upgrades/src/utils/validate-impl.ts:42:20)
      at validateProxyImpl (node_modules/@openzeppelin/hardhat-upgrades/src/utils/validate-impl.ts:63:10)
      at deployProxyImpl (node_modules/@openzeppelin/hardhat-upgrades/src/utils/deploy-impl.ts:72:3)
      at Proxy.deployProxy (node_modules/@openzeppelin/hardhat-upgrades/src/deploy-proxy.ts:48:28)
      at deployDemoFixture (test/Demo.js:7:23)
      at loadFixture (node_modules/@nomicfoundation/hardhat-network-helpers/src/loadFixture.ts:59:18)
      at Context.<anonymous> (test/Demo.js:16:37)

💻 Environment

The following are all of the dependencies used for the project:

    "@nomicfoundation/hardhat-ethers": "^3.0.4",
    "@nomicfoundation/hardhat-toolbox": "^5.0.0",
    "@openzeppelin/contracts-upgradeable": "^5.0.2",
    "hardhat": "^2.22.5",
    "@openzeppelin/hardhat-upgrades": "^3.1.1"

📝 Details

This issue seems to come from the VotesUpgradeable implementation. Looking at the recent git blame in the source repo for transpilation, I noticed this optimization added 2 months ago.

https://github.com/OpenZeppelin/openzeppelin-contracts/commit/427b8bb0280b4e0946a76d417073e9a0c5e0b9ec

This function calls _push and was changed recently from private to internal, which might be triggering this, but I do not have time to dig through it.

Pinging @ernestognw

🔢 Code to reproduce bug

A demo repo with a PoC can be found here: https://github.com/njelich/oz-internal-function-poc

Simply run npm i and then npm test to reproduce the issue.

ericglau commented 1 week ago

Thanks for reporting this. I commented in https://github.com/OpenZeppelin/openzeppelin-upgrades/issues/1037 that this looks like a false positive from the Upgrades plugins validations. Closing this and will continue tracking in the other issue.