Nice solution @InterstellarX 
You have added both of the essential lines of code needed to solve the problem with the withdraw function, and your statements are also in the correct order within the function body to reduce security risk:
- Check inputs (require statements)
- Effects (update the contract state)
- External Interactions (e.g.
transfer method)
Just as you have done, it’s important to modify the contract state for the reduction in the individual user’s balance…
balance[msg.sender] -= amount;
before actually transferring the funds out of the contract to the external address…
msg.sender.transfer(amount);
This isn’t, as you say, just “to ensure that is OK and possible” …
Our require() statement has already checked whether the withdrawal amount can be covered by the user’s individual balance, and it would have thrown an error and triggered revert if it couldn’t.
Instead, this is to prevent what is known as a re-entrancy attack from occuring after the transfer, but before the user’s individual balance (effectively, the share of the total contract balance they are entitled to withdraw) is reduced to reflect this operation. You’ll learn more about this type of attack, and how the above order of operations helps to prevent it, in the courses which follow this one.
The transfer method doesn’t return a Boolean true or false. As you have said in one of your comments within your code…
The point here is that if the transfer method itself fails, it will throw an error and trigger revert in the same way that require() does if that fails. When revert is triggered, the function doesn’t finish executing, and any changes made to the contract state — by the code in the function body that has already been executed before revert was triggered — are reversed (i.e. reverted). This is what we mean by “built-in error handling” — the transfer method doesn’t need to return a Boolean, because we don’t have to handle its success or error with any additional code of our own.
Returning true doesn’t prevent your solution from working, because the return statement will only be reached if the withdrawal has been successful; but it’s unnecessary to include. If the transaction is successful, then the transfer method will also have executed successfully.
The send method, on the other hand, doesn’t revert if it fails, and it does return a Boolean true or false, which we have to handle with additional code e.g…
function withdraw(uint amount) public {
require(balance[msg.sender] >= amount, "Insufficient funds");
balance[msg.sender] -= amount;
bool success = msg.sender.send(amount);
require(success, "Withdrawal failed");
}
If you want to emit an event and log specific data associated with the withdrawal, then you need to do this with an event declaration and an emit statement e.g.
event Withdrawal(uint amount, address indexed withdrawnBy);
function withdraw(uint amount) public {
require(balance[msg.sender] >= amount, "Insufficient funds");
balance[msg.sender] -= amount;
msg.sender.transfer(amount);
emit Withdrawal(amount, msg.sender);
}
To clarify …
A function is marked payable when it needs to receive ether from an external address to add to the contract address balance. Including payable in a function header enables the ether value sent to the function to be added to the contract address balance, without us having to add any further code for this. In the deposit() function, this happens automatically because we have marked the function as payable. The line …
balance[msg.sender] += msg.value;
… then adds the same value to the individual user’s balance in the mapping, in order to keep track of their share of the total funds held in the contract.
The balance mapping keeps track of each individual user’s share of the total contract balance, effectively the amount of ether each is entitled to withdraw from the contract. Each user’s share (or entitlement) changes with each deposit or withdrawal they make, and also with each internal transfer they are involved in as either the sender or the recipient.
Just let me know if you have any questions 
