From f9c8a9b5cac602d93bcf7a1b0b5026dcda71dc01 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 31 May 2016 13:56:05 +0200 Subject: Removed some problems in documentation examples. --- docs/common-patterns.rst | 4 -- docs/introduction-to-smart-contracts.rst | 4 -- docs/solidity-by-example.rst | 78 ++++++++++++++++++++++++-------- 3 files changed, 58 insertions(+), 28 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..5c1810c1 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,15 @@ of votes. // Forward the delegation as long as // `to` also delegated. + // In general, such loops are very dangerous, + // because if they run too long, they might + // need more gas that 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; + voters[to].delegate != msg.sender) { + to = voters[to].delegate; } // We found a loop in the delegation, not allowed. @@ -199,8 +203,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 +217,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 +263,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 +295,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 +348,6 @@ Bidders can confuse competition by placing several high or low invalid bids. -.. {% include open_link gist="70528429c2cd867dd1d6" %} - :: contract BlindAuction { @@ -349,6 +367,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 +447,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 +462,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 +490,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 +506,6 @@ high or low invalid bids. Safe Remote Purchase ******************** -.. {% include open_link gist="b16e8e76a423b7671e99" %} - :: contract Purchase { @@ -521,8 +553,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 +578,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() { -- cgit v1.2.3