PatrickAlphaC / brownie_fund_me

26 stars 64 forks source link

Lesson 6: ValueError: execution reverted: VM Exception while processing transaction: revert #10

Closed etsea117 closed 2 years ago

etsea117 commented 2 years ago

I am getting this error when attempting to run brownie run scripts/fund_and_withdraw.py --network ganache-local

based on this response on ethereum.stackexchange it seems like the issue is with the price returned by the MockV3Aggregator making the getPrice() function return a value less than 1. StackExchange link

Is there a way to alter either the getPrice() function to include an "if < 0 return 1" or the MockV3Aggregator code?

PatrickAlphaC commented 2 years ago

I'm not sure I follow. Can you verify that the price is returning wrong? What happens when you hop into the brownie console?

singhKaramjeet commented 2 years ago

i am getting same problem I have tried everything but it get resolved @PatrickAlphaC

ArvidSU commented 2 years ago

I got this error when I had forgotten to remove the hardcoded price feeds in the getPrice() and getVersion() functions.

AggregatorV3Interface priceFeed = AggregatorV3Interface( 0x8A753747A1Fa494EC906cE90E9f37563A8AF630e );

michaelclubman515 commented 2 years ago

Hi,I met the same error,the MockV3Aggregator deployed successfully, but the getEntrancePrice is 0 Googled it find this answer,don't quite follow.(https://ethereum.stackexchange.com/questions/114889/deploying-ganache-local-w-brownie-vm-exception-while-processing-transaction-in)

getPrice() isn't returning a number you want from the mock, somewhere in the vicinity of 2B. Think this is a bug with the Ganache implementation- performing the calculation (minimumUSD * precision) / price in getEntranceFee() gives you a number less than 1- and, since Solidity can't handle floats, Solidity simply sees it as a 0, and the whole thing errors out.

deploy.py

from brownie import FundMe, network, config, MockV3Aggregator
from scripts.helpful_scripts import (
    get_account,
    deploy_mocks,
    LOCAL_BLOCKCHAIN_ENVIRONMENT,
)

def deploy_fund_me():
    account = get_account()
    # if we have a persistent network like rinkeby,use the associated address
    # otherwise ,deploy mocks
    if network.show_active() not in LOCAL_BLOCKCHAIN_ENVIRONMENT:
        price_feed_address = config["networks"][network.show_active()][
            "eth_usd_price_feed"
        ]
    else:
        deploy_mocks()
        # just use the latest mockV3Aggregator
        price_feed_address = MockV3Aggregator[-1].address
        print("***********************************************************")
        print(f"MockVeAggrator's address is {price_feed_address}")

    fund_me = FundMe.deploy(
        price_feed_address,
        {"from": account},
        publish_source=config["networks"][network.show_active()].get("verify"),
    )
    print("***********************************************************")
    print(f"The Ether price is :{fund_me.getPrice()}\n")
    print(f"Contract deployed to {fund_me.address}\n")
    entrance_fee = fund_me.getEntranceFee()
    print("***********************************************************")
    print(f"The entrance fee is : {entrance_fee} !\n")
    return fund_me

def main():
    deploy_fund_me()

FundMe.sol

// SPDX-License-Identifier: MIT

pragma solidity 0.8.0;

// we need tell brownie @chainlink means == what input in config,need to tell compiler

import "@chainlink/contracts/src/v0.6/interfaces/AggregatorV3Interface.sol";

import "@chainlink/contracts/src/v0.6/vendor/SafeMathChainlink.sol";

contract FundMe {
    //using SafeMathChainlink for uint256;

    mapping(address => uint256) public addressToAmountFunded;
    address[] public funders;
    address public owner;
    AggregatorV3Interface public priceFeed;

    // if you're following along with the freecodecamp video
    // Please see https://github.com/PatrickAlphaC/fund_me
    // to get the starting solidity contract code, it'll be slightly different than this!
    constructor(address _priceFeed) {
        // make price feed a parameter
        priceFeed = AggregatorV3Interface(_priceFeed);
        owner = msg.sender;
    }

    function fund() public payable {
        uint256 mimimumUSD = 50 * 10**18;
        require(
            getConversionRate(msg.value) >= mimimumUSD,
            "You need to spend more ETH!"
        );
        addressToAmountFunded[msg.sender] += msg.value;
        funders.push(msg.sender);
    }

    function getVersion() public view returns (uint256) {
        return priceFeed.version();
    }

    function getPrice() public view returns (uint256) {
        (, int256 answer, , , ) = priceFeed.latestRoundData();
        return uint256(answer * 10000000000);
    }

    // 1000000000
    function getConversionRate(uint256 ethAmount)
        public
        view
        returns (uint256)
    {
        uint256 ethPrice = getPrice();
        uint256 ethAmountInUsd = (ethPrice * ethAmount) / 1000000000000000000;
        return ethAmountInUsd;
    }

    function getEntranceFee() public view returns (uint256) {
        // mimimumUSD
        uint256 mimimumUSD = 50 * 10**18;
        uint256 price = getPrice();
        uint256 precision = 1 * 10**18;
        return (mimimumUSD * precision) / price;
    }

    modifier onlyOwner() {
        require(msg.sender == owner);
        _;
    }

    function withdraw() public payable onlyOwner {
        payable(msg.sender).transfer(address(this).balance);

        for (
            uint256 funderIndex = 0;
            funderIndex < funders.length;
            funderIndex++
        ) {
            address funder = funders[funderIndex];
            addressToAmountFunded[funder] = 0;
        }
        funders = new address[](0);
    }
}
michaelclubman515 commented 2 years ago

the error disappear magicly (may be come back later),right now the error info is :list out of index actually i faced the same error in another project According the answer and brownie docs, I need to add account when I connect to a remote network via a hosted node such as Infura,. what confused me is if I launched the ganache local blockchain already ,why I still need to add account ?

Or it's some thing else cause this error?

7xAquarius commented 2 years ago

Hey, I ran into the same problem last night. After some digging, I found out it was due to a rounding error. Let me explain.

Explanation

First, your transaction gets reverted. The error you get could lead you to think that this is a gas issue due to "Gas estimation failed" But in fact, what reverts your transaction might be the 'require' in fund(). Setting an appropriate message in the 'require' can save you a lot of time.

So, your transaction is reverted because 'getConversionRate(msg.value) >= minimumUSD' is evaluated to False \ \ Since you're using fund_and_withdraw.py, your fund() interaction is sent with msg.value = entrance_fee = fund_me.getEntranceFee()

Let's say we deployed a mock pricefeed which returns 91e18 (91 000 000 000 000 000 000) fund_me.getEntranceFee() should return: (minimumUSD precision) / price = (50e18 1e18) / 91e18

And (50e18 * 1e18) / 91e18 = 549 450 549 450 549 450,54945054945055

BUT we are working with integers, and integer division is rounded towards zero in Solidity. Which means that our fund_me.getEntranceFee() will only return 549 450 549 450 549 450, forgetting about the '0,54945054945055' behind

Back to our 'require': getConversionRate(msg.value) = getConversionRate(549450549450549450) = (91e18 * 549450549450549450) / 1e18 = 49 999 999 999 999 999 950

which is inferior to 50e18, hence why the 'require' reverts the transaction. \ \ Without rounding it would have been: (91e18 * 549450549450549450,54945054945055) / 1e18 = 50 000 000 000 000 000 000

Why it doesnt happen in the video

The mock pricefeed it set with a value of 2000e18 so: fund_me.getEntranceFee() = (50e18 * 1e18) / 2000e18 = 25 000 000 000 000 000

No rounding to be made, so convenient :)

How to fix

Honestly, I think we cannot do much about rounding.

So I thought about a quick fix:

Instead of getEntranceFee() returning the value rounded towards 0, we could make it return the value rounded up by simply adding +1 after the division :)

now we get fund_me.getEntranceFee() = ((minimumUSD precision) / price) + 1 = ((50e18 1e18) / 91e18) + 1 = 549 450 549 450 549 451

and getConversionRate(549450549450549451) = (91e18 * 549450549450549451) / 1e18 = 50 000 000 000 000 000 041

which is now SUPERIOR to 50e18, the 'require' is no longer reverting the transaction

TL;DR

PatrickAlphaC commented 2 years ago

Thank you so much for this!!! Loved your explainer too!!