diff options
Diffstat (limited to 'docs/solidity-by-example.rst')
| -rw-r--r-- | docs/solidity-by-example.rst | 24 |
1 files changed, 21 insertions, 3 deletions
diff --git a/docs/solidity-by-example.rst b/docs/solidity-by-example.rst index 6fa70be4..7dd51f00 100644 --- a/docs/solidity-by-example.rst +++ b/docs/solidity-by-example.rst @@ -291,15 +291,32 @@ activate themselves. /// End the auction and send the highest bid /// to the beneficiary. function auctionEnd() { + // It is a good guideline to structure functions that interact + // with other contracts (i.e. they call functions or send Ether) + // into three phases: + // 1. checking conditions + // 2. performing actions (potentially changing conditions) + // 3. interacting with other contracts + // If these phases are mixed up, the other contract could call + // back into the current contract and modify the state or cause + // effects (ether payout) to be perfromed multiple times. + // If functions called internally include interaction with external + // contracts, they also have to be considered interaction with + // external contracts. + + // 1. Conditions if (now <= auctionStart + biddingTime) throw; // auction did not yet end if (ended) throw; // this function has already been called + + // 2. Effects + ended = true; AuctionEnded(highestBidder, highestBid); + // 3. Interaction if (!beneficiary.send(highestBid)) throw; - ended = true; } function () { @@ -476,7 +493,8 @@ high or low invalid bids. var amount = pendingReturns[msg.sender]; // It is important to set this to zero because the recipient // can call this function again as part of the receiving call - // before `send` returns. + // before `send` returns (see the remark above about + // conditions -> effects -> interaction). pendingReturns[msg.sender] = 0; if (!msg.sender.send(amount)) throw; // If anything fails, this will revert the changes above @@ -490,11 +508,11 @@ high or low invalid bids. if (ended) throw; AuctionEnded(highestBidder, highestBid); + ended = true; // We send all the money we have, because some // of the refunds might have failed. if (!beneficiary.send(this.balance)) throw; - ended = true; } function () { |