smartcontractkit / full-blockchain-solidity-course-py

Ultimate Solidity, Blockchain, and Smart Contract - Beginner to Expert Full Course | Python Edition
MIT License
10.67k stars 2.89k forks source link

Lesson 7: abstract contract and override issue with VRFConsumerBase.sol #1792

Closed emilrueh closed 1 year ago

emilrueh commented 1 year ago

Around the 7h mark in the tutorial I have encountered a Contract "Lottery" should be marked as abstract. and a Function has override specified but does not override anything. issue.

After looking through multiple forums of people encountering similar issues no solution was helpful to my problem.

emilrueh commented 1 year ago

I realized it was an import issue or rather a version issue. As soon as I changed the VRFConsumerBase import from: import "@chainlink/contracts/src/v0.6/VRFConsumerBase.sol"; to: import "@chainlink/contracts/src/v0.7/VRFConsumerBase.sol"; both issues resolved.


EDIT:

New Issue!

When compiling I now get a CompilerError requesting 0.7.0:

Brownie v1.19.2 - Python development framework for Ethereum

Compiling contracts...
  Solc version: 0.6.12
  Optimizer: Enabled  Runs: 200
  EVM Version: Istanbul        
CompilerError: solc returned the following errors:

C:/Users/T440s/.brownie/packages/smartcontractkit/chainlink-brownie-contracts@0.4.2/contracts/src/v0.7/VRFConsumerBase.sol:2:1: ParserError: Source file requires different compiler version (current compiler is 0.6.12+commit.27d51765.Windows.msvc) - note that nightly builds are considered to be strictly less than the released version
pragma solidity ^0.7.0;
^---------------------^

I then changed the solidity compiler version to 0.7.0 and pragma solidity from: pragma solidity ^0.6.6; to: pragma solidity >=0.6.6; and after compiling again I get a huge mess of multiple different compiler version required for compiling all the different imports correctly:

Brownie v1.19.2 - Python development framework for Ethereum

Compiling contracts...
  Solc version: 0.8.17
  Optimizer: Enabled  Runs: 200
  EVM Version: Istanbul        
CompilerError: solc returned the following errors:

ParserError: Source file requires different compiler version (current compiler is 0.8.17+commit.8df45f5f.Windows.msvc) - note that nightly builds are considered to be strictly less than the released version
 --> C:/Users/T440s/.brownie/packages/OpenZeppelin/openzeppelin-contracts@3.4.0/contracts/access/Ownable.sol:3:1:
  |
3 | pragma solidity >=0.6.0 <0.8.0;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

ParserError: Source file requires different compiler version (current compiler is 0.8.17+commit.8df45f5f.Windows.msvc) - note that nightly builds are considered to be strictly less than the released version
 --> C:/Users/T440s/.brownie/packages/smartcontractkit/chainlink-brownie-contracts@0.4.2/contracts/src/v0.6/interfaces/AggregatorV3Interface.sol:2:1:       
  |
2 | pragma solidity ^0.6.0;
  | ^^^^^^^^^^^^^^^^^^^^^^^

ParserError: Source file requires different compiler version (current compiler is 0.8.17+commit.8df45f5f.Windows.msvc) - note that nightly builds are considered to be strictly less than the released version
 --> C:/Users/T440s/.brownie/packages/smartcontractkit/chainlink-brownie-contracts@0.4.2/contracts/src/v0.7/VRFConsumerBase.sol:2:1:
  |
2 | pragma solidity ^0.7.0;
  | ^^^^^^^^^^^^^^^^^^^^^^^

If @cromewar could have a look I'd highly appreciate the help :) (If tagging is inappropriate pls let me know!)


EDIT: I'm switching inbetween versions of imported contracts, repositories, and solc versions for an hour now but just can't seem to find the right combination. It's either ^0.7.0 or ^0.8.0 canceling each other out and if it works I get reverted to my initial abstract and override issues..

(I have posted the issue here on stackoverflow as well.)

cromewar commented 1 year ago

Hello @emilrueh Well this tends to happen when your contract depends on imports which as well depends on other contracts, as they might and most probably will depend on different solidity versions is common to encounter this problems.

So, the question here is do we really need to update the solidity version? Most of the cases the answer is no, newer solidity version have some changes yes, but not that significant and if the contracts you are going to import from rely on older versions is ok to use them.

Let's suppose you have some complex math on a contract with a version minor to 0.8, you might consider upgrading to 0.8 and above as since that version SafeMath is not needed anymore and you can manage your math equations better. But this is just a specific case, not something that you will actually have to dealt with.

Anyways as you are on lesson 7 @Neftyr just published an updated working version for VRF V2 you can check out: https://github.com/smartcontractkit/full-blockchain-solidity-course-py/discussions/1791

emilrueh commented 1 year ago

Ah, so you suggest switching to VRF V2 is a solution. Do you also think it solves the Contract "Lottery" should be marked as abstract. and the Function has override specified but does not override anything. issues?

Nlferu commented 1 year ago

Hi @emilrueh

You can do it both on VRF_V1 and VRF_V2, which is way more advanced. Issues you have mentioned can be solved on any version, but as @cromewar said I would suggest you to use version 0.8 for all contracts you are implementing.

I had same issues while coding Lottery on VRF_V2 and as I remember, abstract error is cased because:

// We have to override fulfillRandomWords() as it is "virtual" -> which means it expecting to be overwritten, otherwise we cant compile code.
     function fulfillRandomWords(uint256 requestId, uint256[] memory randomWords) internal override {

     }

If you make your Lottery is VRFConsumerBase you have to check VRFConsumerBase code and look for functions, which are virtual and looks like below: image

And second issue is related to what it says exactly, so you just have to override something in that function to get rid of this issue.

emilrueh commented 1 year ago

Does that mean I change the imported code's virtual functions and add override to fulfillRandomness() in the imported contracts as well as the overriding function's declaration has to looks exactly like the function to override?

Really appreciate the help btw.

Nlferu commented 1 year ago

Just check the difference on vrf_v2 example: Below you can see original code of VRFConsumerBaseV2: https://github.com/smartcontractkit/chainlink/blob/develop/contracts/src/v0.8/VRFConsumerBaseV2.sol

As you can see it has following line:

function fulfillRandomWords(uint256 requestId, uint256[] memory randomWords) internal virtual;

So if you want to make your contract use it you write smth like this:

contract MyContract is VRFConsumerBaseV2 {
}

And now if you want to compile it you need to add line in your contract, which overrides function which is internal virtual, so in this case fulfillRandomWords function.

Override example below: image

emilrueh commented 1 year ago

I believe to have had all the things you mention already properly coded in my lottery contract and do not see where to start to fix the two abstract and override issues.

EDIT: I found that I used bytes32 randomness and uint256 requestId even though the types should be to:

function fulfillRandomness(bytes32 _requestId, uint256 _randomness) internal override {}

Now I encounter the warning: Unused function parameter. Remove or comment out the variable name to silence this warning. but I believe this one can be ignored as the VRFCoordinator calls it from outside.

Thank you for clarifying that the issues can be solved in either version as I was certain it had to do with depreciated code...


// SPDX-License-Identifier: MIT

pragma solidity ^0.6.6;
import "@chainlink/contracts/src/v0.6/interfaces/AggregatorV3Interface.sol";
import "@openzeppelin/contracts/access/Ownable.sol";
import "@chainlink/contracts/src/v0.6/VRFConsumerBase.sol";

contract Lottery is VRFConsumerBase, Ownable {
    address payable[] public players;
    address public recentWinner;
    uint256 public usdEntryFee;
    AggregatorV3Interface internal ethUsdPriceFeed;
    uint256 public fee;
    bytes32 public keyHash;

    enum LOTTERY_STATE {
        OPEN,
        CLOSED,
        CALCULATING_WINNER
    }

    LOTTERY_STATE public lottery_state;

    constructor(
        address _priceFeedAddress,
        address _vrfCoordinator,
        address _link,
        uint256 _fee,
        bytes32 _keyHash
    ) public VRFConsumerBase(_vrfCoordinator, _link) {
        fee = _fee;
        keyHash = _keyHash;
        lottery_state = LOTTERY_STATE.CLOSED;
        usdEntryFee = 50 * (10**18);
        ethUsdPriceFeed = AggregatorV3Interface(_priceFeedAddress);
    }

    function endLottery() public onlyOwner {
        lottery_state = LOTTERY_STATE.CALCULATING_WINNER;
        requestRandomness(keyHash, fee);
    }

    function fulfillRandomness(uint256 _requestId, bytes32 _randomness)
        internal
        override
    {
        // WINNER SELECTION
        uint256 indexOfWinner = uint256(_randomness) % players.length;
        recentWinner = players[indexOfWinner];
        payable(recentWinner).transfer(address(this).balance);
    }
}
Nlferu commented 1 year ago

Hi @emilrueh

You can also use below:

function fulfillRandomness(bytes32 /*_requestId*/, uint256 /*_randomness*/) internal override {}

To avoid Unused function parameter. Remove or comment out the variable name to silence this warning. error. I'm glad I could help ;)

emilrueh commented 1 year ago

Interesting, I have not yet learned about this comment writing but will look into it. Thanks again! :)