Below is a simplified version of the transferPunk() function and its dependencies. Note, it's not the complete code and actually it's unsafe and buggy, but we'll build it back up and it will help us understand important security considerations.
contract CryptoPunksMarket { // Our storage that keeps track of punk ownership. mapping (uint => address) public punkIndexToAddress; // Transfer ownership of a punk to another user without requiring payment function transferPunk(address to, uint punkIndex) { // Assign the new owner punkIndexToAddress[punkIndex] = to; // Let outside applications know of the transfer by emitting an Event PunkTransfer(msg.sender, to, punkIndex); }}
Now we can call this transferPunk function, give it an address (the new owner) and a uint (the punk id), and the contract will store this address as the owner of the given punk within the punkIndexToAddress mapping. Lastly, we emit an Event so offchain applications are notified about the state change.
But hold on... There is a huge issue with the above code. Can you figure it out?
Yes! ANYONE can call this function and update the ownership of any punk!!!
Let's fix that immediately...
We need to make sure only the current owner of a given punk can transfer it. We can do this with an if-statement, just like we learned in the "Control Flow" section.
if (punkIndexToAddress[punkIndex] != msg.sender) throw;
Note: The msg.sender is a special constant that always refers to the account that's calling the current contract.
The entire function code now looks like this:
contract CryptoPunksMarket { // Our storage that keeps track of punk ownership. mapping (uint => address) public punkIndexToAddress; // Transfer ownership of a punk to another user without requiring payment function transferPunk(address to, uint punkIndex) { // Throw an error if called by anyone but the punk owner if (punkIndexToAddress[punkIndex] != msg.sender) throw; // Assign the new owner punkIndexToAddress[punkIndex] = to; // Let outside applications know of the transfer by emitting an Event PunkTransfer(msg.sender, to, punkIndex); }}
So far so good!
There are a few more issues with this function though. I'll give you a hint: Our contract should only allow trading the 10,000 punks (#0 – #9,999). If someone were to try to transfer a punk id greater than 9,999 our code should also throw an error.
That's an easy fix:
All we have to do is add another if-statement to throw an error if the given punk id is greater than (or equal to) 10,000.
if (punkIndex >= 10000) throw;
The entire function code now looks like this:
contract CryptoPunksMarket { // Our storage that keeps track of punk ownership. mapping (uint => address) public punkIndexToAddress; // Transfer ownership of a punk to another user without requiring payment function transferPunk(address to, uint punkIndex) { // Throw an error if called by anyone but the punk owner if (punkIndexToAddress[punkIndex] != msg.sender) throw; // Our contract only allows managing punks #0 – #9,999 if (punkIndex >= 10000) throw; // Assign the new owner punkIndexToAddress[punkIndex] = to; // Let outside applications know of the transfer by emitting an Event PunkTransfer(msg.sender, to, punkIndex); }}
Technically one could argue this check isn't requred if no punks beyond #9,999 are ever owned, the msg.sender could never be the current owner of the punk, so the first check would already throw an error. But LarvaLabs added this statement for good measure.
If transferring was the only thing an owner could do, this function would work as is. But if you compare the above code to the actual code in the contract, we're still missing a few things.