From efb48659dd61595e0841419543d919ca21d7854a Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Fri, 8 Jul 2016 12:21:57 -0400 Subject: [PATCH 01/12] Add section about withdrawal pattern --- docs/common-patterns.rst | 74 ++++++++++++++++++++++++++++++++ docs/security-considerations.rst | 2 + docs/solidity-by-example.rst | 2 + 3 files changed, 78 insertions(+) diff --git a/docs/common-patterns.rst b/docs/common-patterns.rst index 422e27584..1a9083d9a 100644 --- a/docs/common-patterns.rst +++ b/docs/common-patterns.rst @@ -2,6 +2,80 @@ Common Patterns ############### +.. index:: withdrawal + +************************* +Withdrawal from Contracts +************************* + +The recommended method of sending funds after an effect +is with the withdrawal pattern. Although the most intuitive +aethod 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 +an Ether storage contract. + +:: + + contract WithdrawalPattern { + + mapping (address => uint) etherStore; + mapping (address => uint) pendingReturns; + + function sendEther(uint amount) { + if (amount < etherStore[msg.sender]) { + throw; + } + etherStore[msg.sender] -= amount; + pendingReturns[msg.sender] += amount; + } + + function withdraw() { + uint amount = pendingReturns[msg.sender]; + // It is important to zero the mapping entry + // before sending otherwise this could open + // the contract to a re-entrancy attack + pendingReturns[msg.sender] = 0; + if (!msg.sender.send(amount)) { + throw; + } + } + + function () { + etherStore[msg.sender] += msg.value; + } + } + +This is as opposed to the more intuitive sending pattern. + +:: + + contract SendPattern { + + mapping (address => uint) etherStore; + + function sendEther(uint amount) { + if (amount < etherStore[msg.sender]) { + throw; + } + etherStore[msg.sender] -= amount; + if (!msg.sender.send(amount)) { + throw; + } + } + + function () { + etherStore[msg.sender] += msg.value; + } + } + +An example of this pattern in a less contrived +application can be found on the :ref:`simple_auction` +example. + .. index:: access;restricting ****************** diff --git a/docs/security-considerations.rst b/docs/security-considerations.rst index bae6e20b7..c5d206492 100644 --- a/docs/security-considerations.rst +++ b/docs/security-considerations.rst @@ -1,3 +1,5 @@ +.. _security_considerations: + ####################### Security Considerations ####################### diff --git a/docs/solidity-by-example.rst b/docs/solidity-by-example.rst index 7dd51f006..e68ce4487 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 =================== From 82365f21c0421bd20d60a1e544dd798ce4315b2f Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Sat, 9 Jul 2016 19:01:27 -0400 Subject: [PATCH 02/12] Link to withdraw pattern --- docs/common-patterns.rst | 2 ++ docs/security-considerations.rst | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/common-patterns.rst b/docs/common-patterns.rst index 1a9083d9a..0de692fc6 100644 --- a/docs/common-patterns.rst +++ b/docs/common-patterns.rst @@ -4,6 +4,8 @@ Common Patterns .. index:: withdrawal +.. _withdrawal_pattern: + ************************* Withdrawal from Contracts ************************* diff --git a/docs/security-considerations.rst b/docs/security-considerations.rst index c5d206492..eff3c5e8e 100644 --- a/docs/security-considerations.rst +++ b/docs/security-considerations.rst @@ -126,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 `. Callstack Depth =============== From 617daa1f004dba9960c0de57109d5b1554a9f599 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 11 Jul 2016 10:28:20 -0400 Subject: [PATCH 03/12] Fix withdrawal pattern documentation --- docs/common-patterns.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/common-patterns.rst b/docs/common-patterns.rst index 0de692fc6..e725e7862 100644 --- a/docs/common-patterns.rst +++ b/docs/common-patterns.rst @@ -11,8 +11,8 @@ Withdrawal from Contracts ************************* The recommended method of sending funds after an effect -is with the withdrawal pattern. Although the most intuitive -aethod of sending Ether as a result of an effect is a +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. @@ -28,7 +28,7 @@ an Ether storage contract. mapping (address => uint) pendingReturns; function sendEther(uint amount) { - if (amount < etherStore[msg.sender]) { + if (amount > etherStore[msg.sender]) { throw; } etherStore[msg.sender] -= amount; @@ -60,7 +60,7 @@ This is as opposed to the more intuitive sending pattern. mapping (address => uint) etherStore; function sendEther(uint amount) { - if (amount < etherStore[msg.sender]) { + if (amount > etherStore[msg.sender]) { throw; } etherStore[msg.sender] -= amount; From a6c9d85399d39548abcb7b0739849efab7647c52 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 11 Jul 2016 11:23:17 -0400 Subject: [PATCH 04/12] Remove trailing whitespace --- docs/index.rst | 2 +- docs/installing-solidity.rst | 2 +- docs/layout-of-source-files.rst | 2 +- docs/solidity-by-example.rst | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/index.rst b/docs/index.rst index 5ca5c4a96..555ffaa7e 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -67,7 +67,7 @@ Solidity Tools * `Solidity REPL `_ Try Solidity instantly with a command-line Solidity console. - + * `solgraph `_ Visualize Solidity control flow and highlight potential security vulnerabilities. diff --git a/docs/installing-solidity.rst b/docs/installing-solidity.rst index 33bba29ba..a98a1adeb 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 ef6fd656a..ae1e0d265 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/solidity-by-example.rst b/docs/solidity-by-example.rst index e68ce4487..86d6f72be 100644 --- a/docs/solidity-by-example.rst +++ b/docs/solidity-by-example.rst @@ -271,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; From b688d33055665f0eb8472f609532069a8cfb2fbc Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 11 Jul 2016 15:34:46 -0400 Subject: [PATCH 05/12] Change example to auction --- docs/common-patterns.rst | 130 +++++++++++++++++++++++++++++++-------- 1 file changed, 103 insertions(+), 27 deletions(-) diff --git a/docs/common-patterns.rst b/docs/common-patterns.rst index e725e7862..9f5e12c46 100644 --- a/docs/common-patterns.rst +++ b/docs/common-patterns.rst @@ -22,32 +22,70 @@ an Ether storage contract. :: - contract WithdrawalPattern { + contract WithdrawalPatternAuction { + address public beneficiary; + uint public auctionStart; + uint public biddingTime; - mapping (address => uint) etherStore; - mapping (address => uint) pendingReturns; + address public highestBidder; + uint public highestBid; - function sendEther(uint amount) { - if (amount > etherStore[msg.sender]) { - throw; - } - etherStore[msg.sender] -= amount; - pendingReturns[msg.sender] += amount; + mapping(address => uint) pendingReturns; + + bool ended; + + function WithdrawalPatternAuction( + uint _biddingTime, + address _beneficiary + ) { + beneficiary = _beneficiary; + auctionStart = now; + biddingTime = _biddingTime; } - function withdraw() { - uint amount = pendingReturns[msg.sender]; - // It is important to zero the mapping entry - // before sending otherwise this could open - // the contract to a re-entrancy attack - pendingReturns[msg.sender] = 0; - if (!msg.sender.send(amount)) { + function bid() { + if (now > auctionStart + biddingTime) { throw; } + if (msg.value <= highestBid) { + throw; + } + + // Note that funds for unsucessful + // bids are returned using the + // pendingReturns mapping + if (highestBidder != 0) { + pendingReturns[highestBidder] += highestBid; + } + highestBidder = msg.sender; + highestBid = 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 + } + + function auctionEnd() { + if (now <= auctionStart + biddingTime) + throw; + if (ended) + throw; + + ended = true; + + if (!beneficiary.send(this.balance)) + throw; } function () { - etherStore[msg.sender] += msg.value; + throw; } } @@ -55,28 +93,66 @@ This is as opposed to the more intuitive sending pattern. :: - contract SendPattern { + contract SendPatternAuction { + address public beneficiary; + uint public auctionStart; + uint public biddingTime; - mapping (address => uint) etherStore; + address public highestBidder; + uint public highestBid; - function sendEther(uint amount) { - if (amount > etherStore[msg.sender]) { + bool ended; + + function WithdrawalPatternAuction( + uint _biddingTime, + address _beneficiary + ) { + beneficiary = _beneficiary; + auctionStart = now; + biddingTime = _biddingTime; + } + + function bid() { + if (now > auctionStart + biddingTime) { throw; } - etherStore[msg.sender] -= amount; - if (!msg.sender.send(amount)) { + if (msg.value <= highestBid) { throw; } + + // Note that funds are + // immedietally sent back to + // unsucessful bidders + if (highestBidder != 0) { + msg.sender.send(amount);// DANGER - send is unchecked! + } + highestBidder = msg.sender; + highestBid = msg.value; + } + + function auctionEnd() { + if (now <= auctionStart + biddingTime) + throw; + if (ended) + throw; + + ended = true; + + if (!beneficiary.send(this.balance)) + throw; } function () { - etherStore[msg.sender] += msg.value; + throw; } } -An example of this pattern in a less contrived -application can be found on the :ref:`simple_auction` -example. +Notice that, in this example, an attacker could trap +the previous highest bidder's funds in the contract +by causing the execution of `send` to fail. + +The full auction example can be found at +:ref:`simple_auction`. .. index:: access;restricting From 666c46ac296531ba55fbd02243b4cecd3645186c Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Wed, 10 Aug 2016 11:11:13 -0400 Subject: [PATCH 06/12] Use store example --- docs/common-patterns.rst | 180 +++++++++++++++++---------------------- 1 file changed, 76 insertions(+), 104 deletions(-) diff --git a/docs/common-patterns.rst b/docs/common-patterns.rst index 9f5e12c46..870be0494 100644 --- a/docs/common-patterns.rst +++ b/docs/common-patterns.rst @@ -18,74 +18,57 @@ 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 -an Ether storage contract. +an Ether "store" contract. :: - contract WithdrawalPatternAuction { - address public beneficiary; - uint public auctionStart; - uint public biddingTime; + contract WithdrawalStore { - address public highestBidder; - uint public highestBid; - - mapping(address => uint) pendingReturns; - - bool ended; - - function WithdrawalPatternAuction( - uint _biddingTime, - address _beneficiary - ) { - beneficiary = _beneficiary; - auctionStart = now; - biddingTime = _biddingTime; + struct Item { + uint price; + uint quantity; } - function bid() { - if (now > auctionStart + biddingTime) { + modifier onlyOwner { + if (msg.sender == owner) { + _ + } + else { throw; } - if (msg.value <= highestBid) { - throw; + } + + address owner; + mapping (string => Item) inventory; + + function WithdrawalStore() { + owner = msg.sender; + } + + function updateInventory( + string _item, + uint _price, + uint _quantity + ) onlyOwner { + inventory[_item] = Item(_price, _quantity); + } + + // Notice that the owner withdraws their own Ether + function withdraw() onlyOwner { + owner.send(this.balance); + } + + function buy(string _item) returns (bool) { + if ( + inventory[_item].quantity > 0 && + inventory[_item].price <= msg.value + ) { + inventory[_item].quantity--; + return true; } - - // Note that funds for unsucessful - // bids are returned using the - // pendingReturns mapping - if (highestBidder != 0) { - pendingReturns[highestBidder] += highestBid; + else { + return false; } - highestBidder = msg.sender; - highestBid = 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 - } - - function auctionEnd() { - if (now <= auctionStart + biddingTime) - throw; - if (ended) - throw; - - ended = true; - - if (!beneficiary.send(this.balance)) - throw; - } - - function () { - throw; } } @@ -93,66 +76,55 @@ This is as opposed to the more intuitive sending pattern. :: - contract SendPatternAuction { - address public beneficiary; - uint public auctionStart; - uint public biddingTime; + contract SendStore { - address public highestBidder; - uint public highestBid; - - bool ended; - - function WithdrawalPatternAuction( - uint _biddingTime, - address _beneficiary - ) { - beneficiary = _beneficiary; - auctionStart = now; - biddingTime = _biddingTime; + struct Item { + uint price; + uint quantity; } - function bid() { - if (now > auctionStart + biddingTime) { + modifier onlyOwner { + if (msg.sender == owner) { + _ + } + else { throw; } - if (msg.value <= highestBid) { - throw; - } - - // Note that funds are - // immedietally sent back to - // unsucessful bidders - if (highestBidder != 0) { - msg.sender.send(amount);// DANGER - send is unchecked! - } - highestBidder = msg.sender; - highestBid = msg.value; } - function auctionEnd() { - if (now <= auctionStart + biddingTime) - throw; - if (ended) - throw; + address owner; + mapping (string => Item) inventory; - ended = true; - - if (!beneficiary.send(this.balance)) - throw; + function SendStore() { + owner = msg.sender; } - function () { - throw; + function updateInventory( + string _item, + uint _price, + uint _quantity + ) onlyOwner { + inventory[_item] = Item(_price, _quantity); + } + + function buy(string _item) returns (bool) { + if ( + inventory[_item].quantity > 0 && + inventory[_item].price <= msg.value + ) { + inventory[_item].quantity--; + owner.send(msg.value);// WARNING - this send is unchecked!! + return true; + } + else { + return false; + } } } Notice that, in this example, an attacker could trap -the previous highest bidder's funds in the contract -by causing the execution of `send` to fail. - -The full auction example can be found at -:ref:`simple_auction`. +the owner's funds in the contract by causing the +execution of `send` to fail through a callstack attack. .. index:: access;restricting From 4737100d005e99be5b45691d304e5efe1457d3df Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Thu, 11 Aug 2016 10:28:53 -0400 Subject: [PATCH 07/12] Change withdrawal example The example is now a "King of the Ether"-esque contract. This is actually relevant as they suffered an attack because of an almost identical issue. See the post-mortem here: https://www.kingoftheether.com/postmortem.html --- docs/common-patterns.rst | 123 ++++++++++++++------------------------- 1 file changed, 44 insertions(+), 79 deletions(-) diff --git a/docs/common-patterns.rst b/docs/common-patterns.rst index 870be0494..8bf9e3c0d 100644 --- a/docs/common-patterns.rst +++ b/docs/common-patterns.rst @@ -18,102 +18,67 @@ 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 -an Ether "store" contract. +a contract where the goal is to send the most money to the +contract in order to become the "richest". + +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 richest. :: - contract WithdrawalStore { + contract WithdrawalContract { + address public richest; + uint public mostSent; - struct Item { - uint price; - uint quantity; + mapping (address => uint) pending; + + function WithdrawalContract() { + richest = msg.sender; + mostSent = msg.value; } - modifier onlyOwner { - if (msg.sender == owner) { - _ - } - else { - throw; - } - } - - address owner; - mapping (string => Item) inventory; - - function WithdrawalStore() { - owner = msg.sender; - } - - function updateInventory( - string _item, - uint _price, - uint _quantity - ) onlyOwner { - inventory[_item] = Item(_price, _quantity); - } - - // Notice that the owner withdraws their own Ether - function withdraw() onlyOwner { - owner.send(this.balance); - } - - function buy(string _item) returns (bool) { - if ( - inventory[_item].quantity > 0 && - inventory[_item].price <= msg.value - ) { - inventory[_item].quantity--; + function becomeRichest() returns (bool) { + if (msg.value > mostSent) { + richest = msg.sender; + mostSent = msg.value; + pending[richest] = msg.value; return true; } else { return false; } } + + function withdraw() { + uint amount = pending[msg.sender]; + // Remember to zero the pending refund before sending + // to prevent re-entrancy attacks + pending[msg.sender] = 0; + if (!msg.sender.send(amount)) { + throw; + } + } } This is as opposed to the more intuitive sending pattern. :: - contract SendStore { - - struct Item { - uint price; - uint quantity; + contract SendContract { + address public richest; + uint public mostSent; + + function SendContract() { + richest = msg.sender; + mostSent = msg.value; } - modifier onlyOwner { - if (msg.sender == owner) { - _ - } - else { - throw; - } - } - - address owner; - mapping (string => Item) inventory; - - function SendStore() { - owner = msg.sender; - } - - function updateInventory( - string _item, - uint _price, - uint _quantity - ) onlyOwner { - inventory[_item] = Item(_price, _quantity); - } - - function buy(string _item) returns (bool) { - if ( - inventory[_item].quantity > 0 && - inventory[_item].price <= msg.value - ) { - inventory[_item].quantity--; - owner.send(msg.value);// WARNING - this send is unchecked!! + function becomeRichest() returns (bool) { + if (msg.value > mostSent) { + richest = msg.sender; + mostSent = msg.value; + richest.send(msg.value); return true; } else { @@ -122,9 +87,9 @@ This is as opposed to the more intuitive sending pattern. } } -Notice that, in this example, an attacker could trap -the owner's funds in the contract by causing the -execution of `send` to fail through a callstack attack. +Notice that, in this example, an attacker could trap the +previous richest person's funds in the contract by causing +the execution of `send` to fail through a callstack attack. .. index:: access;restricting From 058e5f0159dcad3e7349b2ab9873396fcc5894e5 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Thu, 11 Aug 2016 10:45:47 -0400 Subject: [PATCH 08/12] Update contracts and descriptions --- docs/common-patterns.rst | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/docs/common-patterns.rst b/docs/common-patterns.rst index 8bf9e3c0d..eb4e14f03 100644 --- a/docs/common-patterns.rst +++ b/docs/common-patterns.rst @@ -40,9 +40,9 @@ become the richest. function becomeRichest() returns (bool) { if (msg.value > mostSent) { + pending[richest] = msg.value; richest = msg.sender; mostSent = msg.value; - pending[richest] = msg.value; return true; } else { @@ -76,9 +76,14 @@ This is as opposed to the more intuitive sending pattern. 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; - richest.send(msg.value); return true; } else { @@ -88,8 +93,12 @@ This is as opposed to the more intuitive sending pattern. } Notice that, in this example, an attacker could trap the -previous richest person's funds in the contract by causing -the execution of `send` to fail through a callstack attack. +contract into an unusable state by causing the ``richest`` +to be 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 is not +enough gas to finish the execution of the fallback function. .. index:: access;restricting From 0f1fb33d58e55444474b75ed10e88bcd27567e6b Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Thu, 11 Aug 2016 14:34:36 -0400 Subject: [PATCH 09/12] Add minor corrections --- docs/common-patterns.rst | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/docs/common-patterns.rst b/docs/common-patterns.rst index eb4e14f03..51fb4e6ef 100644 --- a/docs/common-patterns.rst +++ b/docs/common-patterns.rst @@ -23,7 +23,7 @@ contract in order to become the "richest". 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 richest. +become the new richest. :: @@ -32,7 +32,7 @@ become the richest. uint public mostSent; mapping (address => uint) pending; - + function WithdrawalContract() { richest = msg.sender; mostSent = msg.value; @@ -68,7 +68,7 @@ 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; @@ -93,12 +93,13 @@ This is as opposed to the more intuitive sending pattern. } Notice that, in this example, an attacker could trap the -contract into an unusable state by causing the ``richest`` -to be 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 is not -enough gas to finish the execution of the fallback function. +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 From 269a6d1379166bd1e32129db467212bd7663d4af Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Fri, 12 Aug 2016 11:03:58 -0400 Subject: [PATCH 10/12] Reference inspiration --- docs/common-patterns.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/common-patterns.rst b/docs/common-patterns.rst index 51fb4e6ef..8d0b28af6 100644 --- a/docs/common-patterns.rst +++ b/docs/common-patterns.rst @@ -19,7 +19,8 @@ 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". +contract in order to become the "richest", inspired by +`King of the Ether `_. In the following contract, if you are usurped as the richest, you will recieve the funds of the person who has gone on to From 0b3ad9262cd99ae4788e14f399905952c16d5d27 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Fri, 12 Aug 2016 11:07:02 -0400 Subject: [PATCH 11/12] Fix code --- docs/common-patterns.rst | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/common-patterns.rst b/docs/common-patterns.rst index 8d0b28af6..6302af72e 100644 --- a/docs/common-patterns.rst +++ b/docs/common-patterns.rst @@ -32,7 +32,7 @@ become the new richest. address public richest; uint public mostSent; - mapping (address => uint) pending; + mapping (address => uint) pendingWithdrawals; function WithdrawalContract() { richest = msg.sender; @@ -41,7 +41,7 @@ become the new richest. function becomeRichest() returns (bool) { if (msg.value > mostSent) { - pending[richest] = msg.value; + pendingWithdrawals[richest] += msg.value; richest = msg.sender; mostSent = msg.value; return true; @@ -52,10 +52,10 @@ become the new richest. } function withdraw() { - uint amount = pending[msg.sender]; - // Remember to zero the pending refund before sending - // to prevent re-entrancy attacks - pending[msg.sender] = 0; + 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)) { throw; } From e27493aa8306da65d4cfc464b7867f1faaf72a9f Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Thu, 18 Aug 2016 12:56:39 -0400 Subject: [PATCH 12/12] Remove throw from withdrawal pattern --- docs/common-patterns.rst | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/docs/common-patterns.rst b/docs/common-patterns.rst index 6302af72e..12c6f20cc 100644 --- a/docs/common-patterns.rst +++ b/docs/common-patterns.rst @@ -51,13 +51,17 @@ become the new richest. } } - function withdraw() { + 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)) { - throw; + if (msg.sender.send(amount)) { + return true; + } + else { + pendingWithdrawals[msg.sender] = amount; + return false; } } }