Solidity was started in October 2014 when neither the Ethereum network nor the virtual machine had any real world proof, gas costs at that time were even drastically different than they are now. Also, some of the earliest design decisions were made from Serpent. During the last months, examples and patterns that were initially considered best practices were exposed to reality and some of them turned out to be anti-patterns. Because of that, we recently updated some of the solidity documentationbut since most people probably don’t follow the github commit flow on that repository, I’d like to highlight some of the findings here.
I won’t talk about the minor problems here, please read about them in the documentation.
ether shipping
Sending Ether is supposed to be one of the simplest things in Solidity, but it turns out there are some subtleties that most people don’t realize.
It is important that, at best, the recipient of the ether initiates the payment. The following is a BAD Example of an auction contract:
// THIS IS A NEGATIVE EXAMPLE! DO NOT USE! contract auction { address highestBidder; uint highestBid; function bid() { if (msg.value < highestBid) throw; if (highestBidder != 0) highestBidder.send(highestBid); // refund previous bidder highestBidder = msg.sender; highestBid = msg.value; } }
Due to the maximum stack depth of 1024, the new bidder can always increase the stack size to 1023 and then call bidding() which will cause the submit (highest bid) call to fail silently (i.e. the previous bidder won’t get the refund), but the new bidder will still be the highest bidder. One way to check if Send succeeded is to check its return value:
/// THIS IS STILL A NEGATIVE EXAMPLE! DO NOT USE! if (highestBidder != 0) if (!highestBidder.send(highestBid)) throw;He
throwcauses the current call to be rolled back. This is a bad idea, because the recipient, for example, by implementing the fallback function as
function() { throw; }You can always make the Ether transfer fail and this would have the effect that no one can outbid you.
The only way to avoid both situations is to convert the send pattern to a withdrawal pattern by giving the recipient control over the transfer:
/// THIS IS STILL A NEGATIVE EXAMPLE! DO NOT USE! contract auction { address highestBidder; uint highestBid; mapping(address => uint) refunds; function bid() { if (msg.value < highestBid) throw; if (highestBidder != 0) refunds(highestBidder) += highestBid; highestBidder = msg.sender; highestBid = msg.value; } function withdrawRefund() { if (msg.sender.send(refunds(msg.sender))) refunds(msg.sender) = 0; } }
Why does it still say “negative example” above the contract? Due to the gas mechanics, the contract is fine, but still not a good example. The reason is that it is impossible to prevent code execution in the recipient as part of a shipment. This means that while the send function is still in progress, the recipient can call back to pick up a Refund. At that point, the refund amount remains the same, and therefore they would get the amount again, and so on. In this particular example, it doesn’t work, because the recipient only receives the gas stipend (gas 2100) and it is impossible to make another shipment with this amount of gas. However, the following code is vulnerable to this attack: msg.sender.call.value(refunds(msg.sender))().
With all this considered, the following code should be fine (of course, it’s still not a complete example of an auction contract):
contract auction { address highestBidder; uint highestBid; mapping(address => uint) refunds; function bid() { if (msg.value < highestBid) throw; if (highestBidder != 0) refunds(highestBidder) += highestBid; highestBidder = msg.sender; highestBid = msg.value; } function withdrawRefund() { uint refund = refunds(msg.sender); refunds(msg.sender) = 0; if (!msg.sender.send(refund)) refunds(msg.sender) = refund; } }
Note that we don’t use throw on a failed send because we can revert all state changes manually and not using throw has far fewer side effects.
using cast
The throw statement is usually quite convenient for reversing any changes to state made as part of the call (or the entire transaction, depending on how the function is called). However, you should be aware that it also causes all gas to be spent and is therefore expensive and will potentially kill current function calls. Therefore, I would like to recommend its use. only in the following situations:
1. Revert the Ether transfer to the current function
If a function is not intended to receive Ether or is not in the current state or with the current arguments, you should use throw to reject the Ether. Using throw is the only way to reliably send Ether due to gas and stack depth issues: the recipient may have a bug in the fallback function that consumes too much gas and therefore cannot receive Ether or the function could have been called maliciously. context with too high a stack depth (perhaps even before the calling function).
Note that accidentally sending Ether to a contract isn’t always a UX flaw: you can never predict in what order or at what time transactions are added to a block. If the contract is written to accept only the first transaction, the Ether included in the other transactions must be rejected.
2. Reverse the effects of called functions
If you call functions in other contracts, you can never know how they are implemented. This means that the effects of these calls are also not known, and therefore the only way to reverse these effects is to use throw. Of course, you should always write your contract not to call these functions in the first place, if you know you’ll have to reverse the effects, but there are some use cases where you only know that after the fact.
Loops and the block gas limit
There is a limit to how much gas can be spent in a single block. This limit is flexible, but it is quite difficult to increase it. This means that each individual feature in your contract must stay below a certain amount of gas in all (reasonable) situations. The following is a BAD example of a voting contract:
/// THIS IS STILL A NEGATIVE EXAMPLE! DO NOT USE! contract Voting { mapping(address => uint) voteWeight; address() yesVotes; uint requiredWeight; address beneficiary; uint amount; function voteYes() { yesVotes.push(msg.sender); } function tallyVotes() { uint yesVotes; for (uint i = 0; i < yesVotes.length; ++i) yesVotes += voteWeight(yesVotes(i)); if (yesVotes > requiredWeight) beneficiary.send(amount); } }
The contract actually has several problems, but the one I’d like to highlight here is the loop problem: suppose the vote weights are transferable and divisible like tokens (think DAO tokens as an example). This means that you can create an arbitrary number of clones of yourself. Creating such clones will increase the length of the loop in the tallyVotes function until it consumes more gas than is available within a single block.
This applies to everything that uses loops, also when the loops are not explicitly visible in the contract, for example when you copy arrays or strings into storage. Again, it’s fine to have loops of arbitrary length if the length of the loop is controlled by the caller, for example if it iterates over an array that was passed as a function argument. But Never create a situation where the cycle length is controlled by a party that would not be the only one to suffer its failure.
As a side note, this was one of the reasons why we now have the concept of locked accounts within the DAO contract: the weight of the vote is counted at the point where the vote is cast, to avoid the fact that the loop gets stuck. stuck, and if the vote weight wouldn’t be fixed until the end of the voting period, you could cast a second vote simply by transferring your tokens and then voting again.
Ether receiving / backup function
If you want your contract to receive Ether via the regular send() call, you should make your fallback function cheap. You can only use 2300, gas which does not allow any storage writes or function calls to be sent over Ether. Basically, all you have to do inside the fallback function is log an event so that external processes can react to it. Of course, any feature of a contract can receive ether and is not tied to that gas restriction. Functions actually have to reject Ether sent to them if they don’t want to receive any, but we’re thinking of potentially reversing this behavior in a future release.
