Solidity Payable Functions - Discussion

Ethereum Smart Contract Programming 101
#209

Hi @Al86,

If you post screen shots please be kind to our eyesight, and make sure they are readable and not blurred :pray: Otherwise it requires a lot of tortuous squinting, forensic analysis and guess work to try to work out what’s going on :mag_right: :face_with_monocle: :wink:

Even better is to copy and paste your code, format it and then post it. This makes it more readable and also means we can copy your code from your post and then run it ourselves to debug it, if necessary. Follow the instructions in this FAQ: How to post code in the forum

That said… it looks like you have a very similar issue to the previous one you posted. If you look at the error message, it says:  "Expected '{' but got identifier"
This means that on the line flagging the error, the compiler is expecting a curly brace, but instead it finds the name of a function:  function _transfer(...)
It’s not very clear, but it looks like you have an extra opening curly brace at the end of line 45 which pairs up with what you probably think is the closing curly brace of the transfer() function on line 47, but isn’t because of the extra one on line 45, which needs to be removed. This means that the compiler is still expecting another closing curly brace (for the function body) by the time it reaches the start of the next function header on line 49.

I think that’s probably the issue, but I can’t be 100% sure because of the picture quality.

If that doesn’t solve it, then post your code, and we’ll have another look at it for you :slight_smile:

1 Like
#210

Apologies for the bad screenshot I was trying to display the error msg along with the code but please see below code I have tried adding another closing { and it still comes up errors

#211 
pragma solidity 0.7.5;

contract Bank {
   
    mapping(address => uint) balance;
    
    address owner;
   
    event depositDone(uint amount, address indexed depositedTo);
    

    modifier onlyOwner {
        require(msg.sender == owner);
        _;
    }
    
    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 getBalance() public view returns (uint){
        return balance[msg.sender];
    }
   
    function transfer(address recipient, uint amount) public {
        // check balance of msg.sender
        require(balance[msg.sender] >= amount, "Balance not sufficient");
        require(msg.sender != recipient, "Do not send to yourself");

        // Save the senders balance
        uint previousSenderBalance = balance[msg.sender];

        _transfer(msg.sender, recipient, amount);
        
        // event logs and further checks
        // Confirm senders balance reduced by amount sent.
        assert(balance[msg.sender] == previousSenderBalance - amount);

        emit deposit(amount, msg.sender, recipient); 
        }
    

        function _transferaddress (from, address to, uint amount) private {

}
#212

Hi @Al86,

Thanks for the nice clear code :smiley:

When compiling this code, the 1st error that is flagged is because you don’t have a closing curly brace for your whole contract. Your last function has a closing curly brace, but then you also need to add another one.

When you correct that, the compiler will then flag two more errors (one after the other). You should be able to work out what the second one is from the error message. The third one is because you are trying to emit an event without having defined it. As you have with your depositDone event, you need an event declaration and a corresponding emit statement. I also wouldn’t use the name deposit for the event for the transfer function. This could cause confusion.

Let me know if you have any further problems with this, or if you have any questions :slight_smile:

#213

Hi Jon,

Thanks for your reply
Is there a link to a code where I can see the event declaration and the corresponding emit statement or an example of the code.

1 Like
#214

Hi @Al86

It’s the Events Assignment. I’ve just had a quick look and it doesn’t look like you went on to completed it after @thecil corrected your code.

I don’t think there is a model solution for this assignment, but if you browse through the discussion topic (link above) you will find other students’ solutions and the comments and feedback they’ve received. You’ll find a lot of helpful information posted in these discussion topics if you get stuck. And always make sure you attempt each assignment and post your solution (or an unfinished attempt with questions) because then you will get some personalised feedback, with helpful suggestions and pointers, if needed :slight_smile:

2 Likes
#215

Hi Jon

I just had a look at some of the answers and realised what you mean by emit and defining it will keep practicing this.

I did create this event; event coinsSent(uint amount, address from, address to);

looking at other peoples answers all I had to do was place it under;

function transfer(address recipient, uint amount) public {
emit coinsSent(amount, msg.sender, recipient);

I will post my answers moving forward instead of getting ahead of myself I think that’s where I am going wrong,

Thanks for your help

1 Like
#216

Hi @Al86

If you post your full code (that you were debugging) here (including the transfer event and emit statements) I’ll give it a final check and review for you.

1 Like
#217 
pragma solidity 0.7.5;

contract BankOfAl {
    
    mapping(address => uint) balance;
    address owner;
    
    event coinsCreated(uint amount, address indexed depositedTo); // indexed kryword to force node to index address of event
    event coinsSent(uint amount, address from, address to);
    
    modifier onlyOwner {
      require(msg.sender == owner, "Error! Only the owner of the address can transfer");
      _;
    }
    
    constructor(){
        owner = msg.sender;
    }
    
    function addBalance(uint _toAdd) public onlyOwner returns(uint) {
        balance[msg.sender] += _toAdd;
        emit coinsCreated(_toAdd, msg.sender);
        return balance[msg.sender];
    }
    
    function getBalance() public view returns(uint) {
        return balance[msg.sender];
    }
    
    function transfer(address recipient, uint amount) public {
        
        require(balance[msg.sender] >= amount, "Not enough funds!");
        require(msg.sender != recipient, "Don't sends money to yourself!");
        
        uint prevBalance = balance[msg.sender];
        
        _transfer(msg.sender, recipient, amount);
        
        emit coinsSent(amount, msg.sender, recipient);
        
        assert(balance[msg.sender] == prevBalance - amount);
    }
    
    function _transfer(address from, address to, uint amount) private {
        balance[from] -= amount;
        balance[to] += amount;
    }
}
1 Like
#218

Hi @filip how can someone concatenate on solidity

1 Like
#219

In the example when using ‘payble’ in the ‘Payable Functions’ lesson, how do you keep track of the balance if you’re not using the balance mapping function? How do you access the balance amount?

1 Like
#220

Hi Al,

Sorry for the delay in reviewing this for you :sweat_smile:

Your contract code is now looking really good :muscle:

Your transfer event (CoinsSent) and corresponding emit statement are both well coded, and the emit statement will log appropriate data when the transfer() function is successfully executed. The emit statement would be better placed at the very end of the transfer function, after assert(). If assert() throws, then the preceding operations in the function body will revert, but it is still safer to only log the event once as much of the other code in the function body as possible has executed successfully.

Just one other comment: I would make your error message for the onlyOwner modifier more general, because it doesn’t just apply to transfers. In fact, at the moment, it isn’t applied to the transfer() function, but to addBalance() instead. A good example of a more general error message, which will still be suitable if the modifier is applied to more functions, is something like: Only the contract owner is authorised to perform this operation.
Notice as well, that the restriction relates to the contract owner, not the owner of the address (which doesn’t actually make any sense).

Let me know if you have any questions :slight_smile:

#221

Hi @chim4us,

Here is a basic example of how to concatenate 2 strings using Solidity:

pragma solidity 0.8.4;

contract Concat {
    
    string public newString;
    
    function concatStrings(string memory str1, string memory str2) external pure returns(string memory) {
        return string(abi.encodePacked(str1, str2));
    }
    
    function setConcatStrings(string memory str1, string memory str2) external {
        newString = string(abi.encodePacked(str1, str2));
    }
    
}

Bear in mind that in order to avoid excessive gas costs, as much data manipulation (including string concatenation) as possible should be performed in the frontend, using JavaScript, for example.

1 Like
#222

Hi @ZacSed,

I think what you may be asking is related to the difference between an individual user’s account balance (stored in the mapping) and the total ether balance of the smart contract itself.

When a function is payable it is able to receive ether. Our deposit() function enables ether to be transferred from the caller’s external wallet address to the smart contract address. You can access the total ether balance of the contract address with   address(this).balance  e.g.

function getContractBalance() public view returns(uint) {
    return address(this).balance;
}

The individual user balances stored in the mapping perform an internal accounting role, and record each user’s share of the contract’s total ether balance. When ether is transferred into the contract, the total contract balance is automatically increased by msg.value and so we don’t have to add any code for that. However, the user’s individual balance in the mapping is not automatically adjusted, and so we do need to add code to increase their balance when they deposit ether in the contract, and do the opposite whenever they make a withdrawal. If we don’t do that, then there is no way of knowing each individual’s share of the total amount of ether held in the contract.

When the transfer() function is called, you should notice that the individual user balances increase/decrease, accordingly, by the amount transferred. But as this is just an internal adjustment to the allocation of funds within the contract, there is no change to the total amount of ether stored in the contract, and so  address(this).balance   will not change.

I hope this answers you question, and gives you the information you need to complete the picture. Just let me know if anything is still unclear, or if you have any further questions :slight_smile:

1 Like
#223

Hello,

For the amount i only did get 1 ant not 1 quadrillion as you. Has this changed since the course was created?

{
“0”: “uint256: 1”
}
[
{
“from”: “0xd9145CCE52D386f254917e481eB44e9943F39138”,
“topic”: “0x55b8d2fc1aaf27628691a3336b517d7b40bee36ba595a24e893d1e872afb82d1”,
“event”: “depositDone”,
“args”: {
“0”: “1”,
“1”: “0x5B38Da6a701c568545dCfcB03FcB875f56beddC4”,
“amount”: “1”,
“depositedTo”: “0x5B38Da6a701c568545dCfcB03FcB875f56beddC4”
}
}
]

Regards
Jimmy

1 Like
#224

Hi @Jimmyjim

Any of these inputs will log the following deposit amount in wei:

"amount": "1000000000000000000"

When you deposit 1 ether, you will also see that the sender’s address account balance in the Account field reduces by 1 ether (and not just by 1 wei, which is probably what has been happening).

Let me know if anything is unclear, or if you have any further questions :slight_smile:

#225

I had the exact same issue and thought I would need to create a temp address as payable and assign it to msg.sender before using the transfer function on that variable. I wasn’t quite sure how to define an address variable as payable, but now that I see you can cast msg.sender as payable, this makes a ton of sense! Thank you!.

1 Like
#226

I created a withdraw feature by adding:

event withdrawalDone(uint amount, address indexed withdrawFrom);

function withdraw(uint amount) public returns (uint) {
require(balance[msg.sender] >= amount);
balance[msg.sender] -= amount;
payable (msg.sender).transfer(amount); // Had to cast address as payable!
emit withdrawalDone(amount, msg.sender);
return balance[msg.sender];
}

Thanks to someone for the casting solution to making the address payable!

I also found it was easier to change my value type to wei in the contract header so I didn’t have to put so many zeros in the number!

1 Like
#227

An excellent solution @Samm :muscle:

You have added all of the additional lines of code needed to solve the problem with the withdraw function, and they are in the correct order to reduce security risk:

  1. check inputs (require statements)
  2. effects (update the contract state)
  3. external interactions

Just as you have done, it’s important to modify the contract state for the reduction in the balance…

balance[msg.sender] -= amount;

before actually transferring the funds out of the contract to the external wallet address…

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

… just in case there is an attack after the transfer, but before the state is modified to reflect this operation. The type of attack this specific order of operations prevents is a re-entrancy attack, and this is covered in detail in the Smart Contract Security course. But it’s great you’re already getting into good habits in terms of best practice in this area :+1:

You need to bear in mind that the syntax taught in this course is for Solidity v0.7. That’s absolutely fine to use v0.8, but one of the changes in syntax from v0.8 is that msg.sender is no longer a payable address by default, and so when it needs to refer to a payable address, it needs to be explicitly converted. As you have discovered, one of the ways to do this is with  payable(<address>)
You can also do it the way you initially thought of, by assigning msg.sender to a local payable-address variable, as follows:

address payable receiver = payable(msg.sender);
receiver.transfer(amount);

But as you can see, we would still have to explicitly convert msg.sender in order to be able to assign it to a payable address variable, so in our particular case it ends up being much less concise.

If you are using Solidity v0.8 for your assignments, then include that at the top of your code, so we know what syntax we are reviewing and checking for.

The solidity compiler will always generate an error when your syntax needs to be changed for v0.8, and the error message will tell you why, and often what you should change it to. In fact, msg.sender changing from payable to non-payable by default could well be the only change in syntax from v0.7 to v.0.8 that you will face during this initial course. So it shouldn’t cause much of an issue for you.

In order to receive ether, an address needs to be payable , so this is why in Solidity v0.8 you have to convert msg.sender when using it as the address in…

<address payable>.transfer(uint256 amount)

The withdrawal event (with corresponding emit statement), which you’ve added, is also appropriate and well coded. The emit statement will log relevant information when a call to the withdraw function has executed successfully.

I’m not too sure what you mean by this to be honest. What exactly did you add to the contract header?

contract Bank {  // This is the contract header. Is this actually what you mean?

Just let me know if you have any questions :slight_smile:

#228

Hi

Yes, now I see I used wei instead of ether when I transferred, Forgot to change that! Thank you :slight_smile:

Regards
JImmy

1 Like