Merge pull request #706 from Denton-L/withdrawal-pattern

Add Withdrawal Pattern Example

6 files changed, 113 insertions, 5 deletions

diff --git a/docs/common-patterns.rst b/docs/common-patterns.rst
index 322be3ef..74f9c078 100644
--- a/docs/common-patterns.rst
+++ b/docs/common-patterns.rst
@@ -2,6 +2,110 @@
 Common Patterns
 ###############
 
+.. index:: withdrawal
+
+.. _withdrawal_pattern:
+
+*************************
+Withdrawal from Contracts
+*************************
+
+The recommended method of sending funds after an effect
+is using the withdrawal pattern. Although the most intuitive
+method of sending Ether, as a result of an effect, is a
+direct ``send`` call, this is not recommended as it
+introduces a potential security risk. You may read
+more about this on the :ref:`security_considerations` page.
+
+This is an example of the withdrawal pattern in practice in
+a contract where the goal is to send the most money to the
+contract in order to become the "richest", inspired by
+`King of the Ether <https://www.kingoftheether.com/>`_.
+
+In the following contract, if you are usurped as the richest,
+you will recieve the funds of the person who has gone on to
+become the new richest.
+
+::
+
+    contract WithdrawalContract {
+        address public richest;
+        uint public mostSent;
+
+        mapping (address => uint) pendingWithdrawals;
+
+        function WithdrawalContract() {
+            richest = msg.sender;
+            mostSent = msg.value;
+        }
+
+        function becomeRichest() returns (bool) {
+            if (msg.value > mostSent) {
+                pendingWithdrawals[richest] += msg.value;
+                richest = msg.sender;
+                mostSent = msg.value;
+                return true;
+            }
+            else {
+                return false;
+            }
+        }
+
+        function withdraw() returns (bool) {
+            uint amount = pendingWithdrawals[msg.sender];
+            // Remember to zero the pending refund before
+            // sending to prevent re-entrancy attacks
+            pendingWithdrawals[msg.sender] = 0;
+            if (msg.sender.send(amount)) {
+                return true;
+            }
+            else {
+                pendingWithdrawals[msg.sender] = amount;
+                return false;
+            }
+        }
+    }
+
+This is as opposed to the more intuitive sending pattern.
+
+::
+
+    contract SendContract {
+        address public richest;
+        uint public mostSent;
+
+        function SendContract() {
+            richest = msg.sender;
+            mostSent = msg.value;
+        }
+
+        function becomeRichest() returns (bool) {
+            if (msg.value > mostSent) {
+                // Check if call succeeds to prevent an attacker
+                // from trapping the previous person's funds in
+                // this contract through a callstack attack
+                if (!richest.send(msg.value)) {
+                    throw;
+                }
+                richest = msg.sender;
+                mostSent = msg.value;
+                return true;
+            }
+            else {
+                return false;
+            }
+        }
+    }
+
+Notice that, in this example, an attacker could trap the
+contract into an unusable state by causing ``richest`` to be
+the address of a  contract that has a fallback function
+which consumes more than the 2300 gas stipend.  That way,
+whenever ``send`` is called to deliver funds to the
+"poisoned" contract, it will cause execution to always fail
+because there will not be enough gas to finish the execution
+of the fallback function.
+
 .. index:: access;restricting
 
 ******************
diff --git a/docs/index.rst b/docs/index.rst
index 4b3ace89..a330172e 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -70,7 +70,7 @@ Solidity Tools
 
 * `Solidity REPL <https://github.com/raineorshine/solidity-repl>`_
     Try Solidity instantly with a command-line Solidity console.
-    
+
 * `solgraph <https://github.com/raineorshine/solgraph>`_
     Visualize Solidity control flow and highlight potential security vulnerabilities.
 
diff --git a/docs/installing-solidity.rst b/docs/installing-solidity.rst
index 47388c25..16a02310 100644
--- a/docs/installing-solidity.rst
+++ b/docs/installing-solidity.rst
@@ -101,7 +101,7 @@ installed either by adding the Ethereum PPA (Option 1) or by backporting
     sudo apt-get -y update
     sudo apt-get -y upgrade
     sudo apt-get -y install libcryptopp-dev
-    
+
     ## (Option 2) For those willing to backport libcrypto++:
     #sudo apt-get -y install ubuntu-dev-tools
     #sudo pbuilder create
diff --git a/docs/layout-of-source-files.rst b/docs/layout-of-source-files.rst
index ef6fd656..ae1e0d26 100644
--- a/docs/layout-of-source-files.rst
+++ b/docs/layout-of-source-files.rst
@@ -101,7 +101,7 @@ and then run the compiler as
 
 As a more complex example, suppose you rely on some module that uses a
 very old version of dapp-bin. That old version of dapp-bin is checked
-out at ``/usr/local/dapp-bin_old``, then you can use 
+out at ``/usr/local/dapp-bin_old``, then you can use
 
 .. code-block:: bash
 
diff --git a/docs/security-considerations.rst b/docs/security-considerations.rst
index bae6e20b..eff3c5e8 100644
--- a/docs/security-considerations.rst
+++ b/docs/security-considerations.rst
@@ -1,3 +1,5 @@
+.. _security_considerations:
+
 #######################
 Security Considerations
 #######################
@@ -124,7 +126,7 @@ Sending and Receiving Ether
      because the operation is just too expensive) - it "runs out of gas" (OOG).
      If the return value of ``send`` is checked, this might provide a
      means for the recipient to block progress in the sending contract. Again, the best practice here is to use
-     a "withdraw" pattern instead of a "send" pattern.
+     a :ref:`"withdraw" pattern instead of a "send" pattern <withdrawal_pattern>`.
 
 Callstack Depth
 ===============
diff --git a/docs/solidity-by-example.rst b/docs/solidity-by-example.rst
index 7dd51f00..86d6f72b 100644
--- a/docs/solidity-by-example.rst
+++ b/docs/solidity-by-example.rst
@@ -191,6 +191,8 @@ contract into a blind auction where it is not
 possible to see the actual bid until the bidding
 period ends.
 
+.. _simple_auction:
+
 Simple Open Auction
 ===================
 
@@ -269,7 +271,7 @@ activate themselves.
                 // 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. 
+                // to let the recipient withdraw their money themselves.
                 pendingReturns[highestBidder] += highestBid;
             }
             highestBidder = msg.sender;