foundry-rs / foundry

Foundry is a blazing fast, portable and modular toolkit for Ethereum application development written in Rust.
https://getfoundry.sh
Apache License 2.0
7.89k stars 1.59k forks source link

Bytecode verification mismatches on the explorers when possible division by 0 #7211

Open shideneyu opened 4 months ago

shideneyu commented 4 months ago

Component

Forge

Have you ensured that all of these are up to date?

What version of Foundry are you on?

forge 0.2.0 (a436a0d 2024-02-20T00:16:37.237839814Z)

What command(s) is the bug in?

FOUNDRY_PROFILE=release forge script DeployBlastSepolia.s.sol:DeployBlast --broadcast --rpc-url https://compatible-blue-shadow.blast-sepolia.quiknode.pro/xxx " --private-key $DEPLOYER_PRIVATE_KEY

Operating System

Linux

Describe the bug

I can get my contracts to deploy properly , but I was not able to verify them at all (despite I've put viaIR in the json file)

My foundry.toml

[profile.release]
optimizer = true
runs = 100
viaIR = true
forge-std = "lib/forge-std/src"
remappings = [
    "openzeppelin-contracts=lib/openzeppelin-contracts",
    "chainlink=lib/chainlink",
    "pyth-sdk-solidity=lib/pyth-sdk-solidity"
]

After endless redeploying and reverifying the contracts, I managed to pinpoint which part of my code was the culprit.

It turned out that when doing divisions in solidity, Forge accepted potential division by 0. I could deploy and my contracts would work.

But the contract was absolutely not verifyable on etherscan.

Let's take this function:

    function _getUsdAmount(address asset, uint256 amount) internal view returns (uint256) {
        AssetStore.Asset memory assetInfo = assetStore.get(asset);
        uint256 chainlinkPrice = chainlink.getPrice(assetInfo.chainlinkFeed);
        uint256 decimals = 18;
        if (asset != address(0)) {
            decimals = IERC20Metadata(asset).decimals();
        }
        return (amount * chainlinkPrice) / 10 ** decimals;
    }

Putting this function in any solidity contract will make the contract unverifyable when compiling through forge. If I compile it with Remix IDE, it is verifyable.

To make it verifyable, I had to do this fix:

    function _getUsdAmount(address asset, uint256 amount) internal view returns (uint256) {
        AssetStore.Asset memory assetInfo = assetStore.get(asset);
        uint256 chainlinkPrice = chainlink.getPrice(assetInfo.chainlinkFeed);
        uint256 decimals = 18;
        if (asset != address(0)) {
            decimals = IERC20Metadata(asset).decimals();
        }
        // amount is in the asset's decimals, convert to 18. Price is 18 decimals
        if (chainlinkPrice > 0) {
            // amount is in the asset's decimals, convert to 18. Price is 18 decimals
            return (amount * chainlinkPrice) / 10 ** decimals;
        }
        else {
            return uint256(0);
        }
    }

I am pretty sure that it will help countless people not being able to verify their contract on the explorers. To make this issue appear on google, I'll write the code error explorers usually give:

Unable to verify. Compiled contract runtime bytecode does NOT match the on-chain runtime bytecode.

0xSmit commented 4 months ago

On which line is division by zero happening? because 10**0 equals to 1.

shideneyu commented 4 months ago

Indeed. The bug title and description were misleading sorry. The problem lays on this line:

            return (amount * chainlinkPrice) / 10 ** decimals;

the numerator can be 0 or undefined, leading to a verifciation bytecode mismatch. I think it can also happens if divising by 0 or undefined.

I am just trying to figure out what is wrong. I just know that I could't verify two contracts and each time I needed to add checks as to prevent this kind of weird situation where potential division by or from 0 / undefined during compilation work (the code can be deployed and used, the transactions work, no problem) but the verifications on the explorer do not.