0

I have this code and it gives me infinite gas. please help me improve it.

pragma solidity ^0.4.23;
// v6
contract owned {

    constructor() public { owner = msg.sender; }
    address owner;

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

}

contract mortal is owned {
    function kill() public onlyOwner {
        selfdestruct(owner);
    }
}

contract StudentCertificate is owned, mortal {

    struct Certificate {
        bytes32 title;
        bytes32 courseName;
        bytes32 issuedBy;
        bool _isRekoved;
    }

    struct Student {
        uint16 id;
        bytes32 name;
        uint certificateCount;
        mapping(uint => Certificate) certificates;
    }

    mapping(address => Student) public students;

    function addStudent(bytes32 _name, address _studentAddress, uint16 _userUId) public onlyOwner {
        require(students[_studentAddress].id==0);
        students[_studentAddress] = Student({
            id: _userUId,
            name: _name,
            certificateCount: 0
        });        
    }

    function removeStudent(address _studentAddress) public onlyOwner {
        require(students[_studentAddress].id!=0); // save gas
        // first delete certs if any
        for(uint i = 0; i < students[_studentAddress].certificateCount; i++){
            delete(students[_studentAddress].certificates[i]);
        }
        delete(students[_studentAddress]);  // will save gas. gas refund 
    }


    function addStudentCertificate(address _studentAddress, bytes32 _title, bytes32 _courseName, bytes32 _issuedBy) public onlyOwner {
        require(students[_studentAddress].id!=0);
        students[_studentAddress].certificates[students[_studentAddress].certificateCount] = Certificate({
            title: _title,
            courseName: _courseName,
            issuedBy: _issuedBy,
            _isRekoved: false
            });

        students[_studentAddress].certificateCount++;
    }


    function getStudentCertificateCount(address _studentAddress) public view returns (uint256){
        return students[_studentAddress].certificateCount;
    }

    function getStudentCertificateData(address _studentAddress, uint256 _certArrayIndex) public view returns (bytes32, bytes32, bytes32, bool){
        return (
            students[_studentAddress].certificates[_certArrayIndex].title,
            students[_studentAddress].certificates[_certArrayIndex].courseName,
            students[_studentAddress].certificates[_certArrayIndex].issuedBy,
            students[_studentAddress].certificates[_certArrayIndex]._isRekoved
            );
    }



}

Error/Warning message by Remix:

Gas requirement of function StudentCertificate.addStudent(bytes32,address,uint16) high: infinite. If the gas requirement of a function is higher than the block gas limit, it cannot be executed. Please avoid loops in your functions or actions that modify large areas of storage (this includes clearing or copying arrays in storage)
  • Works for me (only edited your code by removing onlyOwner): imgur.com/a/fxfHr4B Seems like you will need to share more code, or steps to reproduce. – Shawn Tabrizi Oct 3 '18 at 12:04
  • Hi Shawn, I have added the complete code to the post. – Vikram Oct 3 '18 at 14:41
  • Also I tried the code the way you did but I still get the warning of high gas – Vikram Oct 3 '18 at 14:54
  • 1
    Warnings are not errors. Compilers are not that intelligent. You need to evaluate if the warning is a good warning, and implies that you could fix something, or if the warning is a red herring. – Shawn Tabrizi Oct 3 '18 at 14:56
  • I get about 10 warnings for the code and some how wanted to reduce few of them. Is there any way I could do that? I can not think of any more improvements. Would you have any suggestions. Thank you – Vikram Oct 3 '18 at 15:08
0

This error is usually a misleading red herring.

You're onlyOwner modifier (not shown) and your require(student[....id==0); are conditionals that potentially consume all the gas when they do their job which is to prevent the function from running in unacceptable circumstances.

You can disable to this particular type of warning on the static analysis tab.

Hope it helps.

  • Hi Rob, I tried with removing onlyOwner and the require(student[....id==0); statement ..But I still get the same warning. – Vikram Oct 3 '18 at 14:37
  • On closer look, i think it's the require ... !=0 in the add function. Same logic. – Rob Hitchens - B9lab Oct 3 '18 at 15:25
  • I do not know any possibility for require() to consume “infinite” or “all” gas: even when the argument is false, require() abort and refund all the remaining gas back to msg.sender. Could you please Rob explain better your point? – Rick Park Oct 4 '18 at 3:02
  • Good point. I loaded it in Remix and found it doesn't raise that exact warning with 0.4.23 or 0.4.25. It raises a similar concern with the removeStudent() function that contains an unbounded for loop here: for(uint i = 0; i < students[_studentAddress].certificateCount; i++){ - which could be high/nearly infinite depending on the value of certificateCount. I didn't see a warning with the add function. Upon closer inspection, I think it's a valid concern with the delete function. – Rob Hitchens - B9lab Oct 4 '18 at 3:25
0

As a matter of fact this warning (yes: “warning”) is largely untrustable in Remix.

The proper way to use it is to have a detailed check of the code for (possibly) infinite loops, possible infinite storage use (for instance bounds to some dynamic array or string that, in theory, can be pushed without limit for growing, and so on. Another possibility to check is the case where the function can call some external service not easily predicible in term of gas used.

If all is ok and nothing strange appears, simply switch it off as Rob suggested.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.