Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unreachable code warning for internal function called by an abstract contract #15426

Open
peterhorvat opened this issue Sep 11, 2024 · 1 comment
Labels

Comments

@peterhorvat
Copy link

Description

During compilation of a UUPSUpgradeable contract we receive the following 'Unreachable code' warning:

  --> @openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol:94:9:
   |
94 |         _upgradeToAndCallUUPS(newImplementation, data);
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The unreachable function is a private function that is called within an abstract contract so it doesn't apply to the bug where a pure internal function is called by an abstract contract.

Environment

  • Compiler version: 0.8.20
  • Target EVM version (as per compiler settings): paris
  • Framework/IDE (e.g. Truffle or Remix):
  • Operating system: MacOS

Steps to Reproduce

Create a simple upgradable contract that is UUPSUpgradeable and try to compile it.
Use the following version of Openzeppelin packages

        "@openzeppelin/contracts": "^5.0.0",
        "@openzeppelin/contracts-upgradeable": "^5.0.0",
// SPDX-License-Identifier: BUSL-1.1
pragma solidity ^0.8.20;

import { UUPSUpgradeable } from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
import { Ownable2StepUpgradeable } from "@openzeppelin/contracts-upgradeable/access/Ownable2StepUpgradeable.sol";

error DisabledFunction();

contract test is UUPSUpgradeable, Ownable2StepUpgradeable {
    //// @notice Upgrades the contract to a new implementation.
    function _authorizeUpgrade(address) internal view override onlyOwner {
        revert DisabledFunction();
    }
}

The following warning should be produced

Screenshot 2024-09-11 at 09 33 08
@Wayno1989
Copy link

The error you're seeing is due to the combination of overriding the _authorizeUpgrade function and the UUPSUpgradeable contract setup.

UUPSUpgradeable: The _authorizeUpgrade function is required in the UUPS proxy pattern, and it controls which addresses are authorized to upgrade the contract.
Your current implementation: You have overridden _authorizeUpgrade and set it to revert DisabledFunction();, which essentially disables the upgrade functionality.
Cause of the issue:
In your code, you're using UUPSUpgradeable, which expects the _authorizeUpgrade function to determine whether an upgrade is allowed. By reverting every time it is called, you prevent any upgrades from happening, causing the error.

How to fix it:
If you intend to disable upgrades permanently (for security reasons or to lock the contract), the way you are handling it is correct. However, if your intention is to allow upgrades under specific conditions or in the future, you'll need to modify the _authorizeUpgrade function to allow upgrades when appropriate.

Here are a few options to resolve the issue:

Option 1: Allow upgrades only for the owner
If you want only the contract owner to be able to upgrade, you can modify the _authorizeUpgrade function like this:

function _authorizeUpgrade(address newImplementation) internal override onlyOwner {
// Allow upgrade logic here, such as security checks.
}
Option 2: Permanently disable upgrades (which is what you are doing now)
If you truly want to disable upgrades, your code is fine, and the revert DisabledFunction(); will block any future upgrades. However, you will get an error if you're trying to upgrade this contract, because you've explicitly disallowed it.

If you're sure you want to keep the upgrade path blocked, then there's nothing wrong with your code as-is. If this was unintended and you want to perform upgrades, use option 1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants