Solidity Payable Functions - Discussion

@bulletfingers

I dont think that is a problem to transfer first then subtract, however it is good practice to subtract first then send.

Happy learning :grinning:
Abel S

1 Like

Check this post out. It’s to prevent something called a re-entrancy attack which you will learn about in a later course.

1 Like

Thanks Jon, I had some suspicion about this, but couldn’t remember what the concept was called or if it applied in smart contract world.

In a centralized world, I can imagine that a server might shut off or fail at an exact second where you sent money but didn’t reduce the balance, then someone might be able to double spend. This is probably different than a re-entrancy attack, but an example I thought of that made me suspicious initially.

Excited to learn more about that in the future security course.

1 Like

Hey @bulletfingers,

You are certainly thinking along the right lines :ok_hand: In a re-entrancy attack, external code hijacks your contract during execution making it behave differently to how it should. A transfer of funds to an external contract could trigger something known as a fallback function in the external contract, which, if malicious, could call the withdraw function in our contract multiple times until the contract’s ether balance is reduced to zero. If our code reducing the caller’s balance for the amount transferred is placed after this “attack loop”, then the require statement placed at the beginning of the function body will never throw, because the caller’s recorded balance in the mapping isn’t reduced each time the function is re-entered. We can prevent this type of attack by adjusting the internal state of our contract for the effect of the transfer before the actual external transfer takes place: the require statement would then throw whenever the caller’s fund balance is insufficient to meet the amount requested during one the attack iterations.

Hi everyone,

So my function looks like:

function withdraw(uint amount) public {
        require(balance[msg.sender] >= amount, "Balance not sufficient!");
        msg.sender.transfer(amount); //build-in error handeling
        balance[msg.sender] -= amount;
        emit withdrawComplete(msg.sender, amount);
    }

Also @jon_m I have a question. How this function suppose to look like in compiler version 0.8.0? If I use the same function in that version the following error popup:

TypeError: “send” and “transfer” are only available for objects of type “address payable”, not “address”.

I tried to use “payable” in different places but I can’t figure that out.

Thanks.

1 Like

Hey @D0tmask,

You have added all of the additional lines of code needed to solve the problem with the withdraw function :ok_hand:

Now have a look at this post which explains an important security modification that should be made to the order of the statements within your withdraw function body.

The original assignment code includes returns(uint) in the withdraw function header. This is not mandatory to include, and the function can still operate effectively without returning a value. But if it was included, you may just want to think about the return statement you would also need to add to the function body.

That’s great that you are already experimenting with using the very latest version 0.8.0 :muscle: Here is a link to the relevant page in the Solidity v0.8.0 documentation, where you can find out about the main changes that have been introduced with this version, and how to code for them.
https://docs.soliditylang.org/en/v0.8.0/080-breaking-changes.html
The answer to your specific question can be found in the 9th bullet point in this section:
https://docs.soliditylang.org/en/v0.8.0/080-breaking-changes.html#new-restrictions
You will see that the issue is that msg.sender is no longer a payable address by default, and so when using it with transfer() or send() you need to convert it to a payable address, which can be done as follows:

payable(msg.sender).transfer(amountToTransfer);

Let us know if you have any further questions, but, hopefully, now that I’ve pointed you in the right direction in terms of the relevant documentation, you’ll be able to find the answers to these sorts of questions more easily yourself from now on :slight_smile:

3 Likes

Hi @jon_m,

The code is working fine now. Thanks for your help and references :slight_smile:

Cheers.

1 Like

Hi @filip,

Should the withdraw function also be payable? So the contract would send out ether as opposed to just adjusting the sender’s balance?

Thanks!

1 Like

Hi @Jose_Hernandez, hope you are great.

Payable functions provide a mechanism to collect / receive funds in ethers to your contract .
Meaning the keyword payable should only be used when a function should receive funds.

If you have any more questions, please let us know so we can help you! :slight_smile:

Carlos Z.

1 Like

Thank you very much!

1 Like

Am I correct in understanding that using modifiers is (instead of, for example, using a function that would accomplish the same output goal) a way to reduce overall gas fee costs per contract?

1 Like

Indeed it is, modifiers are merely a way to reuse conditions, so code redundancy is optimized, that way you are also reducing the overall gas fees of your functions that requires conditionals that can be just reused with a modifier.

Carlos Z

Does anyone have an example of an actual smart contract that I can look at?

With the Payable Function - “function deposit() public payable returns (uint)” -
does this really need to be written -" function deposit() external payable returns (uint)" - replacing public with external?

I am reading this needs to be the case from soliditylang.org which is quoting v0.6.2.

1 Like

Hi @Mrbryce , hope you are great.

Payable functions provide a mechanism to collect / receive funds in ethers to your contract .
Meaning the keyword payable should only be used when a function should receive funds.

External functions are part of the contract interface, which means they can be called from other contracts and via transactions. An external function f cannot be called internally (i.e. f() does not work, but this.f() works). External functions are sometimes more efficient when they receive large arrays of data, because the data is not copied from calldata to memory.

https://docs.soliditylang.org/en/v0.8.1/contracts.html#visibility-and-getters

If you have any more questions, please let us know so we can help you! :slight_smile:

Carlos Z.

Thanks for the Carlos. Do you have any examples of a smart contract as this would be helpful in trying to understand what it is I am trying to build? Is the Metamask wallet or Uniswap examples of smart contracts. Thanks

Something to note. I had to backtrack from solidity 0.8.1 -> 0.7.5 to follow this section. 0.8.1 was complaining that msg.sender.transfer() was invalid due to the type not being a payable address.

Had the same issue, thanks for posting this!

1 Like

Payable Assignment Solution

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);
    
    uint256 public contractBalance;

    address public owner;

    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);
      contractBalance += 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];
   }
}
}
1 Like

need some help here. I keep getting an error on line 26 saying ‘send and transfer are only available for address payable’ however i don’t recall how to define an address as payable. i assumed from the video that msg.sender should be automatically payable

pragma solidity 0.8.1;

contract goinBack{

mapping (address => uint) balance;

address owner;

event depositDone(uint amoubt, address indexed depositedTo);

modifier onlyOwner {
require (msg.sender == owner, “Only the owner can add and send money”);
_;
}

constructor(){
owner = msg.sender ;
}

function deposit() public payable returns(uint){
balance [msg.sender] += msg.value;
emit depositDone (msg.value, msg.sender);
return balance [msg.sender];
}
function withdraw(uint amount) public returns (uint){
msg.sender.transfer (amount);
}

function getBalance ( )public view returns(uint){
return balance[msg.sender];
}

function transfer (address recipient, uint amount) public{
require(balance [msg.sender]>=amount, “you broke bro”);
require(msg.sender!= recipient, “dont send money to yourself”);

  uint previousSenderBalance = balance[msg.sender]; // this variable was made just to assert that the balance is acurrate
  
  _transfer(msg.sender, recipient, amount);

  assert(balance[msg.sender]== previousSenderBalance - amount); // assert is used to check that the balance is what it should be after the ftransfer

}

function _transfer (address from, address to, uint amount) onlyOwner private {
balance [from]-= amount;
balance [to] += amount;

}

}