Merge pull request #616 from chriseth/docs

Removed some problems in documentation examples.

3 files changed, 61 insertions, 29 deletions

diff --git a/docs/common-patterns.rst b/docs/common-patterns.rst
index c8751fd1..9096571e 100644
--- a/docs/common-patterns.rst
+++ b/docs/common-patterns.rst
@@ -28,8 +28,6 @@ functions and this is what this page is about.
 The use of **function modifiers** makes these
 restrictions highly readable.
 
-.. {% include open_link gist="fe4ef267cbdeac151b98" %}
-
 ::
 
     contract AccessRestriction {
@@ -167,8 +165,6 @@ function finishes.
     return. If you want to do that, make sure
     to call nextStage manually from those functions.
 
-.. {% include open_link gist="0a221eaceb6d708bf271" %}
-
 ::
 
     contract StateMachine {
diff --git a/docs/introduction-to-smart-contracts.rst b/docs/introduction-to-smart-contracts.rst
index fe51abe0..0cb2b0d0 100644
--- a/docs/introduction-to-smart-contracts.rst
+++ b/docs/introduction-to-smart-contracts.rst
@@ -14,8 +14,6 @@ right now, we will go into more detail later.
 Storage
 =======
 
-.. Gist: a4532ce30246847b371b
-
 ::
 
     contract SimpleStorage {
@@ -63,8 +61,6 @@ Furthermore, anyone can send coins to each other without any need for
 registering with username and password - all you need is an Ethereum keypair.
 
 
-.. Gist: ad490694f3e5b3de47ab
-
 ::
 
     contract Coin {
diff --git a/docs/solidity-by-example.rst b/docs/solidity-by-example.rst
index 52ce9849..6fa70be4 100644
--- a/docs/solidity-by-example.rst
+++ b/docs/solidity-by-example.rst
@@ -34,8 +34,6 @@ At the end of the voting time, ``winningProposal()``
 will return the proposal with the largest number
 of votes.
 
-.. Gist: 618560d3f740204d46a5
-
 ::
 
     /// @title Voting with delegation.
@@ -108,9 +106,17 @@ of votes.
 
             // Forward the delegation as long as
             // `to` also delegated.
-            while (voters[to].delegate != address(0) &&
-                   voters[to].delegate != msg.sender) {
-                       to = voters[to].delegate;
+            // In general, such loops are very dangerous,
+            // because if they run too long, they might
+            // need more gas than is available in a block.
+            // In this case, the delegation will not be executed,
+            // but in other situations, such loops might
+            // cause a contract to get "stuck" completely.
+            while (
+                voters[to].delegate != address(0) &&
+                voters[to].delegate != msg.sender
+            ) {
+                to = voters[to].delegate;
             }
 
             // We found a loop in the delegation, not allowed.
@@ -199,8 +205,6 @@ contract has to be called manually for the
 beneficiary to receive his money - contracts cannot
 activate themselves.
 
-.. {% include open_link gist="48cd2b65ff83bd04f7af" %}
-
 ::
 
     contract SimpleAuction {
@@ -215,6 +219,9 @@ activate themselves.
         address public highestBidder;
         uint public highestBid;
 
+        // Allowed withdrawals of previous bids
+        mapping(address => uint) pendingReturns;
+
         // Set to true at the end, disallows any change
         bool ended;
 
@@ -258,13 +265,29 @@ activate themselves.
                 throw;
             }
             if (highestBidder != 0) {
-                highestBidder.send(highestBid);
+                // Sending back the money by simply using
+                // highestBidder.send(highestBid) is a security risk
+                // because it can be prevented by the caller by e.g.
+                // raising the call stack to 1023. It is always safer
+                // to let the recipient withdraw their money themselves. 
+                pendingReturns[highestBidder] += highestBid;
             }
             highestBidder = msg.sender;
             highestBid = msg.value;
             HighestBidIncreased(msg.sender, msg.value);
         }
 
+        /// Withdraw a bid that was overbid.
+        function withdraw() {
+            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.
+            pendingReturns[msg.sender] = 0;
+            if (!msg.sender.send(amount))
+                throw; // If anything fails, this will revert the changes above
+        }
+
         /// End the auction and send the highest bid
         /// to the beneficiary.
         function auctionEnd() {
@@ -274,9 +297,8 @@ activate themselves.
                 throw; // this function has already been called
             AuctionEnded(highestBidder, highestBid);
 
-            // We send all the money we have, because some
-            // of the refunds might have failed.
-            beneficiary.send(this.balance);
+            if (!beneficiary.send(highestBid))
+                throw;
             ended = true;
         }
 
@@ -328,8 +350,6 @@ Bidders can confuse competition by placing several
 high or low invalid bids.
 
 
-.. {% include open_link gist="70528429c2cd867dd1d6" %}
-
 ::
 
     contract BlindAuction {
@@ -349,6 +369,9 @@ high or low invalid bids.
         address public highestBidder;
         uint public highestBid;
 
+        // Allowed withdrawals of previous bids
+        mapping(address => uint) pendingReturns;
+
         event AuctionEnded(address winner, uint highestBid);
 
         /// Modifiers are a convenient way to validate inputs to
@@ -426,7 +449,8 @@ high or low invalid bids.
                 // the same deposit.
                 bid.blindedBid = 0;
             }
-            msg.sender.send(refund);
+            if (!msg.sender.send(refund))
+                throw;
         }
 
         // This is an "internal" function which means that it
@@ -440,13 +464,24 @@ high or low invalid bids.
             }
             if (highestBidder != 0) {
                 // Refund the previously highest bidder.
-                highestBidder.send(highestBid);
+                pendingReturns[highestBidder] += highestBid;
             }
             highestBid = value;
             highestBidder = bidder;
             return true;
         }
 
+        /// Withdraw a bid that was overbid.
+        function withdraw() {
+            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.
+            pendingReturns[msg.sender] = 0;
+            if (!msg.sender.send(amount))
+                throw; // If anything fails, this will revert the changes above
+        }
+
         /// End the auction and send the highest bid
         /// to the beneficiary.
         function auctionEnd()
@@ -457,7 +492,8 @@ high or low invalid bids.
             AuctionEnded(highestBidder, highestBid);
             // We send all the money we have, because some
             // of the refunds might have failed.
-            beneficiary.send(this.balance);
+            if (!beneficiary.send(this.balance))
+                throw;
             ended = true;
         }
 
@@ -472,8 +508,6 @@ high or low invalid bids.
 Safe Remote Purchase
 ********************
 
-.. {% include open_link gist="b16e8e76a423b7671e99" %}
-
 ::
 
     contract Purchase {
@@ -521,8 +555,9 @@ Safe Remote Purchase
             inState(State.Created)
         {
             aborted();
-            seller.send(this.balance);
             state = State.Inactive;
+            if (!seller.send(this.balance))
+                throw;
         }
 
         /// Confirm the purchase as buyer.
@@ -545,9 +580,14 @@ Safe Remote Purchase
             inState(State.Locked)
         {
             itemReceived();
-            buyer.send(value); // We ignore the return value on purpose
-            seller.send(this.balance);
+            // It is important to change the state first because
+            // otherwise, the contracts called using `send` below
+            // can call in again here.
             state = State.Inactive;
+            // This actually allows both the buyer and the seller to
+            // block the refund.
+            if (!buyer.send(value) || !seller.send(this.balance))
+                throw;
         }
 
         function() {