PatrickAlphaC / hardhat-fund-me-fcc

82 stars 183 forks source link

Issues with testing and debugging Error: VM Exception while processing transaction: reverted with reason string 'You need to spend more ETH!' #72

Closed olekmorawski closed 1 year ago

olekmorawski commented 2 years ago

Can someone please help me? I tried debugging it on my own for like 3 hours now and im losing my mind over it now. I get an error while testing :

  1) FundMe
       fund
         Updates the amount funded data structure:
     Error: VM Exception while processing transaction: reverted with reason string 'You need to spend more ETH!' 

This is my FundMe.test.js:

const { assert, expect } = require("chai")
const { deployments, ethers, getNamedAccounts } = require("hardhat")

describe("FundMe", async function () {
    let fundMe
    let deployer
    let mockV3Aggregator
    const sendValue = ethers.utils.parseEther("1") // 1 eth
    beforeEach(async function () {
        //const accounts = await ethers.getSigners()
        //const accountZero = accounts[0]
        deployer = (await getNamedAccounts()).deployer
        await deployments.fixture(["all"])
        fundMe = await ethers.getContract("FundMe", deployer)
        mockV3Aggregator = await ethers.getContract(
            "MockV3Aggregator",
            deployer
        )
    })

    describe("constructor", async function () {
        it("sets the aggregator addresses correctly", async function () {
            const response = await fundMe.priceFeed()
            assert.equal(response, mockV3Aggregator.address)
        })
    })

    describe("fund", async function () {
        it("Fails if you dont send enough ethereum", async function () {
            await expect(fundMe.fund()).to.be.rejectedWith(
                "You need to spend more ETH!"
            )
        })
        it("Updates the amount funded data structure", async function () {
            await fundMe.fund({ value: sendValue })
            const response = await fundMe.getAddressToAmountFunded(deployer)
            assert.equal(response.toString(), sendValue.toString())

This is my FundMe.sol:

// SPDX-License-Identifier: MIT
//Pragma
pragma solidity ^0.8.7;
// Imports
import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";
import "./PriceConverter.sol";
// Error Codes
error FundMe__NotOwner();

// Interfaces, Libraries, Contracts

/** @title A contrac for crowd funding
 * @author Aleksander Morawski
 * @notice This contract is a demo of a simple funding contract
 * @dev This implements price feeds as our library
 */
contract FundMe {
    // Type delarations
    using PriceConverter for uint256;

    // State variables
    uint256 public constant MINIMUM_USD = 50 * 10**18;
    mapping(address => uint256) public addressToAmountFunded;
    address[] public funders;
    address public immutable owner;
    AggregatorV3Interface public priceFeed;

    modifier onlyOwner() {
        // require(msg.sender == i_owner, "Sender is not the owner");
        if (msg.sender != owner) revert FundMe__NotOwner();
        _;
    }

    constructor(address priceFeedAddress) {
        owner = msg.sender;
        priceFeed = AggregatorV3Interface(priceFeedAddress);
    }

    /**
     * @notice This funds this contract
     * @dev This implements price feeds as our library
     */
    function fund() public payable {
        require(msg.value.getConversionRate(priceFeed) >= MINIMUM_USD, "You need to spend more ETH!");
        // require(PriceConverter.getConversionRate(msg.value) >= MINIMUM_USD, "You need to spend more ETH!");
        addressToAmountFunded[msg.sender] += msg.value;
        funders.push(msg.sender);
    }

    function withdraw() public onlyOwner {
        // require(msg.sender == owner, "Sender is not an owner!");
        for (
            uint256 funderIndex = 0;
            funderIndex < funders.length;
            funderIndex++
        ) {
            address funder = funders[funderIndex];
            addressToAmountFunded[funder] = 0;
        }
        // reset the array
        funders = new address[](0);

        (bool callSuccess, ) = payable(msg.sender).call{
            value: address(this).balance
        }("");
        require(callSuccess, "Call failed");
    }
}
byte14 commented 2 years ago

Everything you have posted here seems fine. The error you are getting is likely because you have not converted the eth/usd price to 18 digits. In your PriceCoverter contract you need to multiply the eth/usd price by 10**10 since the eth/usd price you initially get have only 8 additional zeros. If your eth/usd price do not have 18 digits then you end up checking for way too much USD rather than 50.

olekmorawski commented 2 years ago

Everything you have posted here seems fine. The error you are getting is likely because you have not converted the eth/usd price to 18 digits. In your PriceCoverter contract you need to multiply the eth/usd price by 10**10 since the eth/usd price you initially get have only 8 additional zeros. If your eth/usd price do not have 18 digits then you end up checking for way too much USD rather than 50.

This is how My price converter looks like and i cant see anything wrong with it. I have to say, moments like this really discourage me from thinking that i could become a solidity dev. Just because of how helpless i feel - especially when i dont think that anything is wrong with my code. Thanks for help btw

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.8;

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

library PriceConverter {
    function getPrice(AggregatorV3Interface priceFeed)
        internal
        view
        returns (uint256)
    {
        (, int256 price, , , ) = priceFeed.latestRoundData();
        // ETH in USD
        return uint256(price * 1e10);
    }

    function getConversionRate(
        uint256 ethAmont,
        AggregatorV3Interface priceFeed
    ) internal view returns (uint256) {
        uint256 ethPrice = getPrice(priceFeed);
        uint256 ethAmontInUSD = (ethPrice * ethAmont) / 1e18;
        return ethAmontInUSD;
    }
}
byte14 commented 2 years ago

Everything seems fine to me as well, did you try sending more ether than 1 and see if its pass? And don't be discourage, even professional developer goes through problems for days, totally normal. You will eventually solve this problem and probably gonna realize how silly mistake it was.

olekmorawski commented 2 years ago

Everything seems fine to me as well, did you try sending more ether than 1 and see if its pass? And don't be discourage, even professional developer goes through problems for days, totally normal. You will eventually solve this problem and probably gonna realize how silly mistake it was.

I tried sending 10 and it gives me the same error. I'm totally confused. The only thing i can think of is to start over the whole project... :(

olekmorawski commented 2 years ago

I also tried to debug it with remix and remix prints me an error like this : creation of FundMe errored: Error encoding arguments: Error: invalid address (argument="address", value="", code=INVALID_ARGUMENT, version=address/5.5.0) (argument=null, value="", code=INVALID_ARGUMENT, version=abi/5.5.0)

olekmorawski commented 2 years ago

image

Maybe its that pricefeedaddress thats here empty in remix? and thats the reason why it does not work?

olekmorawski commented 2 years ago

Okay so im a noob and remix only deploys contracts and i have no way of testing them with js so the fact i got it runnin there does not matter because thats not the problem, the test seems to be off the...i dont know im confused

byte14 commented 2 years ago

image

Maybe its that pricefeedaddress thats here empty in remix? and thats the reason why it does not work?

Yes, you will need a pricefeed address from chainlink data feeds and pass it in constructor

olekmorawski commented 2 years ago

image Maybe its that pricefeedaddress thats here empty in remix? and thats the reason why it does not work?

Yes, you will need a pricefeed address from chainlink data feeds and pass it in constructor

Yeah i figured that out, does not help my test tho :(. I think im going to start all over

byte14 commented 2 years ago

image Maybe its that pricefeedaddress thats here empty in remix? and thats the reason why it does not work?

Yes, you will need a pricefeed address from chainlink data feeds and pass it in constructor

Yeah i figured that out, does not help my test tho :(. I think im going to start all over

Maybe delete artifacts and cache and then run again or Maybe clone the project and try to run test and see what 's different

olekmorawski commented 2 years ago

image Maybe its that pricefeedaddress thats here empty in remix? and thats the reason why it does not work?

Yes, you will need a pricefeed address from chainlink data feeds and pass it in constructor

Yeah i figured that out, does not help my test tho :(. I think im going to start all over

Maybe delete artifacts and cache and then run again or Maybe clone the project and try to run test and see what 's different

Hey if anyone here is curious to see my weird errors i have pushed my repo on git here : https://github.com/olekmorawski/hardhat-fund-me-fcc

Feel free to check this out and lmk if anyone managed to fix it or know what the problem might be. The problem still persists in the repo

byte14 commented 2 years ago

You are passing 1 eth = 2 usd in your MockV3Aggregator constructor when you are deploying mocks. so passing 1 eth will check 2 usd >= MINIMUM_USD and fails. In your helper-hardhat-config you can change the INITITAL_ANSWER from 200000000 to 20000000000. It's 8 decimals so you are setting ETH/USD to 2 , add 3 zeros so it will be 2000 USD, now 50/2000 = 0.025. Now your test will pass even if you send 0.025 ETH.

byte14 commented 2 years ago

And one more thing, since you have install hardhat-toolbox plugin and imported it in your hardhat.config.js, you can remove any of these imports, they all redundant now:

@nomiclabs/hardhat-ethers
@nomiclabs/hardhat-etherscan
hardhat-gas-reporter
solidity-coverage
@typechain/hardhat

You can read it on https://hardhat.org/hardhat-runner/plugins/nomicfoundation-hardhat-toolbox

olekmorawski commented 1 year ago

oh yeah! it fixed the problem! thanks man!