Solidity Payable Functions - Discussion

Hi @Christopher_Duff,

Are you sure the error starts…

… and not “transact to HelloWorld.createPerson errored:…” ?

This error normally occurs when calling a function marked payable once the contract has already been deployed. What you’ve written suggests that you are getting this error on creation of your contract, which doesn’t make sense to me.

When this error occurs when calling a function marked payable (and starts with the different text I’ve mentioned above), it is normally because the function is called without sending a payment, or specifying a payment value which is lower than the minimum required. This needs to be entered in the Value field below the Gas Limit. Make sure you have set the value to ether if the minimum required is a value in ether, and not wei. Wei is often the default, and so this can send an insufficient payment if not changed to ether.

If this doesn’t help resolve the problem, and the beginning of the error message is as you’ve stated (and not the alternative I’ve suggested), then post a copy of your complete contract code, and explain which specific transaction is throwing the error (i.e. on contract deployment, or on calling a specific function). I’ll then take a detailed look and find out what’s going on.

1 Like

Here’s my code and image of my transaction. The transaction gets stopped even though I put more than 1 ether in the value setting. I notice the account balance is still less than 100 so the gas transaction still goes through.

pragma solidity 0.5.12;

contract HelloWorld{
string public message = “Hello World”;
uint[] public numbers = [1,20,45];

// storage: eveything that's saved permanetly like the owner/mapping - lifetime of contract
//memory: only saved during function execution. such as-- string memory name. will be deleted after function done
//stack  store local memory of value types <256 bits - int256, bool . deleted w/ function is executed

struct Person {
    uint id;
    string name;
    uint age;
    uint height;
    bool senior;
    
} // definition of person --> object

address public owner; //avail for lifetime of contract
uint public balance;

event personCreated(string name, bool senior); //data any interested party may need -> can send event info
event personDeleteded(string name, bool senior, address deletedBy);

modifier onlyOwner(){
require(msg.sender == owner,“only the owner can do this!”);
_;
} // continue execution. have a piece of code that can be used by lots of functions

modifier costs(uint cost){
require(msg.value > cost);
_;

}

constructor() public{
owner = msg.sender; //person that initiated contract
}

mapping(address => Person) private people; //mapping better than array - dictionary/key pair
address[] private creators; //only want owner to access address

//Person[] public people;//array of people have some place to save new people public makes getter function to query array

function createPerson(string memory name, uint age, uint height) public payable costs(1 ether){
  // address creator = msg.sender; // create separte function. don't want the user to add address in function - mistake instead use address of person creating contract
   
   require(age <= 150, "Age need to below 150");
 
   balance += msg.value;
   
    // creates a Person
    Person memory newPerson;
    //newPerson.id = people.length; can't get length in mapping
    newPerson.name = name;
    newPerson.age = age;
    newPerson.height = height;
    
    if(age >= 65){
        newPerson.senior = true;
    }
    else{
        newPerson.senior = false;
    }    
   // people.push(newPerson);
   // people.push(Person(people.length,name,age,height)); another way to create people
   
   insertPerson(newPerson);
   creators.push(msg.sender);
   
   // people[msg.sender] == newPerson, assert should always be true and never fire
   assert(
       keccak256
        (abi.encodePacked(
            people[msg.sender].name, 
            people[msg.sender].age, 
            people[msg.sender].height,
            people[msg.sender].senior 
         )
      ) 
       == keccak256
            (abi.encodePacked(
                newPerson.name,
                newPerson.age,
                newPerson.height,
                newPerson.senior
                 )
            )
        ); // cant just compare people have to hash then compare. same input of hash ->same o/p
    emit personCreated(newPerson.name,newPerson.senior); //    send data to event
}
   function insertPerson(Person memory newPerson) private {
       address creator = msg.sender;
       people[creator]=newPerson;
   }
   function getPerson() public view returns (string memory name, uint age, uint height, bool senior){
        address creator = msg.sender;
       return (people[creator].name,people[creator].age, people[creator].height,people[creator].senior);
   }
   
   function deletePerson(address creator) public onlyOwner{
       string memory name = people[creator].name;// got to store in memory before delete then call event handler
       bool senior = people[creator].senior;
       delete people[creator]; //delete value of key ,person in any mapping- limit by the admin =>address that deployed contract
       assert(people[creator].age == 0);
       emit personDeleteded(name,senior,msg.sender);
   }
   
   function getCreator(uint index) public view onlyOwner returns (address){
       return creators[index];
   }

}

1 Like

Hi @Christopher_Duff,

What do you exactly mean by “the transaction gets stopped” ? What error message are you seeing — the original one you quoted, or the other version I suggested it could be instead? Even so, it’s strange because I’ve just copied and pasted your exact code into Remix (same compiler version 0.5.12) and when I recreate the msg.value of 1.4 ether, and call the createPerson function (with name, age and height inputs) the transaction is successful. The transaction confirmation gives the value in wei, and the account balance for the address that called the function has reduced by 1.4 ether (and a bit more due to the gas cost). Then when I call the getter for the contract balance it displays 1.4 ether in wei (1,400,000,000,000,000,000). At first I thought the issue might be inputting a number with a decimal (1.4), as it’s not an integer, but as the value that’s transferred, processed and stored in the contract is in wei, this doesn’t seem to be a problem.

As expected, when I called the same function with 1 ether, the transaction reverts, because you have used a condition of msg.value > 1 ether instead of msg.value >= 1 ether — is this intentional?

If you still can’t get it to work with a value of 1.4 ether, try inputting this same value in wei, and see if you still get the same error.

1 Like

Hi filip @filip
I was doing the Ethereum programming 101, send & transfer lesson, and I have a doubt. Wouldn’t this code (below) didnt transfer the balance to the sender?? I mean the msg.sender.send() function is only executed as the condition of an if statement/loop, but my understanding is that it wont really send the funds to the address specified… maybe Iḿ wrong, but could you please answer to me? or anyone?

function withdrawAll() public onlyOwner returns (uint){
  uint toTransfer = balance;
  balance = 0;
  if(msg.sender.send(toTransfer)==true){
    return toTransfer;
  }else {
    balance = toTransfer;
    return 0;
  }
  return toTransfer;
}

Hey @javier_ortiz_mir

That function works and sends back funds to the user.
If your code is not working please paste it here fully so that I can have a look.

Also notice that return toTransfer; can never be reached and can be removed.
Because an if statement can be true or false, one for the two returns above will be triggered.

2 Likes

@filip You mentioned at the end of the video that with SEND that it is possible for the transaction to be a ‘valid’ transaction but that no money is sent. I’m not clear on what that means. If we try to send 10 eth with send function when 10 eth are not in the contract, how can that be a valid transaction?

Hey @CryptoEatsTheWorld

If you use transfer to send ether, it will revert if the transaction fails.
If you use send, the transaction won’t revert, it would just return false.

Use transfer is much more secure and it is surely the best practice.

Happy learning,
Dani

1 Like

Am I missing anything here?

pragma solidity 0.5.12;

contract HelloWorld{

    struct Person {
      uint id;
      string name;
      uint age;
      uint height;
      bool senior;
      uint balance
    }

    address public owner;

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

    mapping (address => Person) private people;
    address[] private creators;

    function createPerson(string memory name, uint age, uint height) public {
      require(age < 150, "Age needs to be below 150");
      require(msg.value = 1 ether);
      
        //This creates a person
        Person memory newPerson;
        newPerson.name = name;
        newPerson.age = age;
        newPerson.height = height;
        newPerson.balance = msg.value

        if(age >= 65){
           newPerson.senior = true;
       }
       else{
           newPerson.senior = false;
       }

        insertPerson(newPerson);
        creators.push(msg.sender);

        assert(
            keccak256(
                abi.encodePacked(
                    people[msg.sender].name,
                    people[msg.sender].age,
                    people[msg.sender].height,
                    people[msg.sender].senior
                )
            )
            ==
            keccak256(
                abi.encodePacked(
                    newPerson.name,
                    newPerson.age,
                    newPerson.height,
                    newPerson.senior
                )
            )
        );
    }
    function insertPerson(Person memory newPerson) private {
        address creator = msg.sender;
        people[creator] = newPerson;
    }
    function getPerson() public view returns(string memory name, uint age, uint height, bool senior){
        address creator = msg.sender;
        return (people[creator].name, people[creator].age, people[creator].height, people[creator].senior);
        people[creator].balance;
    }
    function deletePerson(address creator) public {
       require(msg.sender == owner, "Caller needs to be owner");
       delete people[creator];
       assert(people[creator].age == 0);
   }
   function getCreator(uint index) public view returns(address){
       require(msg.sender == owner, "Caller needs to be owner");
       return creators[index];
   }

}

Hey @keithra1948

I have edited your post to make your code readable.
When posting code, please remember to use:

There are some syntax errors in your code, I copied and pasted it into remix for a quick look and I notice that:

  • A semicolon is missing after uint balance declaration in struct Person (line 11).

  • require(msg.value = 1 ether); In order to compare two values you need to use ==, otherwise what the compiler understands is that you are trying to assign 1 ether to msg.value which is an invalid operation (line 25).

  • The function createPerson() should be payable if you want to send ether to it.

  • A semicolon is missing after newPerson.balance = msg.value (line 32).

Have you tested this contract with Remix or with another text editor / compiler?

Happy learning,
Dani

2 Likes
pragma solidity 0.5.12;

contract HelloWorld{

    struct Person {
      uint id;
      string name;
      uint age;
      uint height;
      bool senior;
    }

    event personCreated(string name, bool senior);
    event personDeleted(string name, bool senior, address deletedBy);

    address public owner;
    uint256 public donationTotal = 0;
    
    modifier onlyOwner(){
        require(msg.sender == owner);
        _; //Continue execution
    }

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

    mapping (address => Person) private people;
    address[] private creators;

    function createPerson(string memory name, uint age, uint height) public payable {
      require(age < 150, "Age needs to be below 150");
      require(msg.value >= 1 ether);
      
        //Store total donations to the contract
        donationTotal += msg.value;

        //This creates a person
        Person memory newPerson;
        newPerson.name = name;
        newPerson.age = age;
        newPerson.height = height;

        if(age >= 65){
           newPerson.senior = true;
       }
       else{
           newPerson.senior = false;
       }

        insertPerson(newPerson);
        creators.push(msg.sender);

        assert(
            keccak256(
                abi.encodePacked(
                    people[msg.sender].name,
                    people[msg.sender].age,
                    people[msg.sender].height,
                    people[msg.sender].senior
                )
            )
            ==
            keccak256(
                abi.encodePacked(
                    newPerson.name,
                    newPerson.age,
                    newPerson.height,
                    newPerson.senior
                )
            )
        );
        emit personCreated(newPerson.name, newPerson.senior);
    }
    function insertPerson(Person memory newPerson) private {
        address creator = msg.sender;
        people[creator] = newPerson;
    }
    function getPerson() public view returns(string memory name, uint age, uint height, bool senior){
        address creator = msg.sender;
        return (people[creator].name, people[creator].age, people[creator].height, people[creator].senior);
    }
    function deletePerson(address creator) public onlyOwner {
      string memory name = people[creator].name;
      bool senior = people[creator].senior;

       delete people[creator];
       assert(people[creator].age == 0);
       emit personDeleted(name, senior, owner);
   }
   function getCreator(uint index) public view onlyOwner returns(address){
       return creators[index];
   }
   
   //Allow owner to check total donations
   function getTotalDonation() public view onlyOwner returns(uint256){
       return donationTotal;
   }

}

1 Like

Hi @keithra1948,

In addition to the great feedback from @dan-i about some of the detail in your contract, I also just want to let you know that the overall idea here is to store and retrieve the contract balance, rather than individual user balances (which is what your contract aims to do). This can simply be done by creating a state variable…

uint public balance;
/* as this state variable has public visibility,
   a getter will be created automatically */

… and then assigning to this balance variable, the transaction costs paid to the smart contract address (in our contract, 1 ETH for creating each new person):

// createPerson() function
balance += msg.value;

Once you’ve made the corrections that Dani has told you about, maybe you can then develop 2 versions of your contract (one to handle individual user balances, and another for the overall contract balance) :muscle:

3 Likes

I’ve also just spotted what looks like an error in your getPerson function. I assume your intention is to also return the person’s balance (as well as their name, age, height and senior status). If that is the case, then you need to:
(i) Include people[creator].balancewithin the return statement (not on a separate line); and
(ii) Include  uint balance  at the end of  returns()  in the function header.

Hi @Marlic,

This is a good attempt :ok_hand:

If you want the donationTotal to only be viewed by the owner, then you need to mark the donationTotal state variable as private. Because it is currently marked with public visibility, a getter will automatically be created allowing all users to view this value (including the owner).

Conversely, if you don’t actually want to restrict access to the donationTotal value, then the owner could just view it in the same way as all the other users — by calling the automatically created public getter. And so this would mean that you don’t need the separate getTotalDonation function, and you can remove it.

1 Like
function getBalance() public view returns(uint){
return address(this).balance;
}type or paste code here
1 Like

Q). What is the base contract?
A). The base (or parent) contracts are the ones from which other contracts inherit. Contracts that inherit data are derived (or children).
Q). Which functions are available for derived contracts?
A). the child inherits all function and Variables from the parent Both public and internal functions and variables (the clue is in the name INHERITANCE) :grinning:
Q). What is hierarchical inheritance?
A). See above and below answer.

Please Note: Solidity follows the path of Python and uses C3 Linearization, also known as Method Resolution Order (MRO), to force a specific order in graphs of base contracts. The contracts should follow a specific order while inheriting, starting from the base contract through to the most derived contract. ( For Clarity, Easy to follow and understand). :wink:

First of all I made the address a payable address. After doing some research I found that you even don’t need a new state variable for getting the balance of the contract owner. return address(this).balance will do the task. To withdraw all of the balance you can first call the getContractBalancef unction, which is getting the current balance for you. The assert statement at the end of the withdrawAll function just checks if everything had been executed correctly.

    address payable public owner;
    
    modifier onlyOwner(){
        require(msg.sender == owner);
        _;  //Continue execution
    }
    
    constructor () public {
        owner = msg.sender;
    }
    
    function getContractBalance() public view onlyOwner returns(uint256){
        return address(this).balance;
    }
    
    function withdrawAll () public onlyOwner {
        msg.sender.transfer(getContractBalance());
        assert(getContractBalance()==0);
    }
1 Like

//created private variable to store contract balance
uint private contractBalance = 0;

//increment the contract balance after assertion
emit personCreated(newPerson.name, newPerson.senior);
contractBalance += msg.value;

//created a function to get contract balance to be called only by Contract Owner
function getContractBalance() public view onlyOwner returns (uint){
return contractBalance;
}

1 Like

Thanks for all the help Jon!

1 Like

Hi @NLUGNER,

Apologies for the long delay in providing you with some feedback on this assignment:

Q1 :ok_hand:

No, not all of them… but the next part of your answer is correct :ok_hand:

No, hierarchical inheritance is where more than one child contract is derived from the same parent contract.

1 Like

@jon_m No worries dude. Better Late than never :stuck_out_tongue_winking_eye:

1 Like