Merge pull request #1651 from ethereum/transfer-method

Add address.transfer(value)

11 files changed, 85 insertions, 21 deletions

diff --git a/Changelog.md b/Changelog.md
index 1d61f09d..5e21819c 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -2,6 +2,7 @@
 
 Features:
  * Add ``assert(condition)``, which throws if condition is false.
+ * Introduce ``.transfer(value)`` for sending Ether.
  * Code generator: Support ``revert()`` to abort with rolling back, but not consuming all gas.
  * Inline assembly: Support ``revert`` (EIP140) as an opcode.
  * Type system: Support explicit conversion of external function to address.
diff --git a/docs/control-structures.rst b/docs/control-structures.rst
index df8ac729..ebc45965 100644
--- a/docs/control-structures.rst
+++ b/docs/control-structures.rst
@@ -395,6 +395,7 @@ Currently, Solidity automatically generates a runtime exception in the following
 #. If your contract receives Ether via a public function without ``payable`` modifier (including the constructor and the fallback function).
 #. If your contract receives Ether via a public getter function.
 #. If you call a zero-initialized variable of internal function type.
+#. If a ``.transfer()`` fails.
 
 While a user-provided exception is generated in the following situations:
 #. Calling ``throw``.
diff --git a/docs/frequently-asked-questions.rst b/docs/frequently-asked-questions.rst
index d2fc5b1c..8a68ae5b 100644
--- a/docs/frequently-asked-questions.rst
+++ b/docs/frequently-asked-questions.rst
@@ -660,16 +660,6 @@ https://github.com/ethereum/wiki/wiki/Subtleties
 After a successful CREATE operation's sub-execution, if the operation returns x, 5 * len(x) gas is subtracted from the remaining gas before the contract is created. If the remaining gas is less than 5 * len(x), then no gas is subtracted, the code of the created contract becomes the empty string, but this is not treated as an exceptional condition - no reverts happen.
 
 
-How do I use ``.send()``?
-=========================
-
-If you want to send 20 Ether from a contract to the address ``x``, you use ``x.send(20 ether);``.
-Here, ``x`` can be a plain address or a contract. If the contract already explicitly defines
-a function ``send`` (and thus overwrites the special function), you can use ``address(x).send(20 ether);``.
-
-Note that the call to ``send`` may fail in certain conditions, such as if you have insufficient funds, so you should always check the return value.
-``send`` returns ``true`` if the send was successful and ``false`` otherwise.
-
 What does the following strange check do in the Custom Token contract?
 ======================================================================
 
diff --git a/docs/miscellaneous.rst b/docs/miscellaneous.rst
index 3c57507e..7b0305d5 100644
--- a/docs/miscellaneous.rst
+++ b/docs/miscellaneous.rst
@@ -465,8 +465,9 @@ Global Variables
 - ``this`` (current contract's type): the current contract, explicitly convertible to ``address``
 - ``super``: the contract one level higher in the inheritance hierarchy
 - ``selfdestruct(address recipient)``: destroy the current contract, sending its funds to the given address
-- ``<address>.balance`` (``uint256``): balance of the address in Wei
-- ``<address>.send(uint256 amount) returns (bool)``: send given amount of Wei to address, returns ``false`` on failure
+- ``<address>.balance`` (``uint256``): balance of the :ref:`address` in Wei
+- ``<address>.send(uint256 amount) returns (bool)``: send given amount of Wei to :ref:`address`, returns ``false`` on failure
+- ``<address>.transfer(uint256 amount)``: send given amount of Wei to :ref:`address`, throws on failure
 
 .. index:: visibility, public, private, external, internal
 
diff --git a/docs/types.rst b/docs/types.rst
index 69c23e6e..f7d1d54f 100644
--- a/docs/types.rst
+++ b/docs/types.rst
@@ -64,7 +64,7 @@ expression ``x << y`` is equivalent to ``x * 2**y`` and ``x >> y`` is
 equivalent to ``x / 2**y``. This means that shifting negative numbers
 sign extends. Shifting by a negative amount throws a runtime exception.
 
-.. index:: address, balance, send, call, callcode, delegatecall
+.. index:: address, balance, send, call, callcode, delegatecall, transfer
 
 .. _address:
 
@@ -80,27 +80,31 @@ Operators:
 Members of Addresses
 ^^^^^^^^^^^^^^^^^^^^
 
-* ``balance`` and ``send``
+* ``balance`` and ``transfer``
 
 For a quick reference, see :ref:`address_related`.
 
 It is possible to query the balance of an address using the property ``balance``
-and to send Ether (in units of wei) to an address using the ``send`` function:
+and to send Ether (in units of wei) to an address using the ``transfer`` function:
 
 ::
 
     address x = 0x123;
     address myAddress = this;
-    if (x.balance < 10 && myAddress.balance >= 10) x.send(10);
+    if (x.balance < 10 && myAddress.balance >= 10) x.transfer(10);
 
 .. note::
-    If ``x`` is a contract address, its code (more specifically: its fallback function, if present) will be executed together with the ``send`` call (this is a limitation of the EVM and cannot be prevented). If that execution runs out of gas or fails in any way, the Ether transfer will be reverted. In this case, ``send`` returns ``false``.
+    If ``x`` is a contract address, its code (more specifically: its fallback function, if present) will be executed together with the ``transfer`` call (this is a limitation of the EVM and cannot be prevented). If that execution runs out of gas or fails in any way, the Ether transfer will be reverted and the current contract will stop with an exception.
+
+* ``send``
+
+Send is the low-level counterpart of ``transfer``. If the execution fails, the current contract will not stop with an exception, but ``send`` will return ``false``.
 
 .. warning::
     There are some dangers in using ``send``: The transfer fails if the call stack depth is at 1024
     (this can always be forced by the caller) and it also fails if the recipient runs out of gas. So in order
-    to make safe Ether transfers, always check the return value of ``send`` or even better:
-    Use a pattern where the recipient withdraws the money.
+    to make safe Ether transfers, always check the return value of ``send``, use ``transfer`` or even better:
+    use a pattern where the recipient withdraws the money.
 
 * ``call``, ``callcode`` and ``delegatecall``
 
diff --git a/docs/units-and-global-variables.rst b/docs/units-and-global-variables.rst
index 72741b67..49fe5d84 100644
--- a/docs/units-and-global-variables.rst
+++ b/docs/units-and-global-variables.rst
@@ -130,6 +130,8 @@ Address Related
     balance of the :ref:`address` in Wei
 ``<address>.send(uint256 amount) returns (bool)``:
     send given amount of Wei to :ref:`address`, returns ``false`` on failure
+``<address>.transfer(uint256 amount)``:
+    send given amount of Wei to :ref:`address`, throws on failure
 
 For more information, see the section on :ref:`address`.
 
diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp
index 96b3ed17..7fccccbc 100644
--- a/libsolidity/ast/Types.cpp
+++ b/libsolidity/ast/Types.cpp
@@ -464,7 +464,8 @@ MemberList::MemberMap IntegerType::nativeMembers(ContractDefinition const*) cons
 			{"call", make_shared<FunctionType>(strings(), strings{"bool"}, FunctionType::Location::Bare, true, false, true)},
 			{"callcode", make_shared<FunctionType>(strings(), strings{"bool"}, FunctionType::Location::BareCallCode, true, false, true)},
 			{"delegatecall", make_shared<FunctionType>(strings(), strings{"bool"}, FunctionType::Location::BareDelegateCall, true)},
-			{"send", make_shared<FunctionType>(strings{"uint"}, strings{"bool"}, FunctionType::Location::Send)}
+			{"send", make_shared<FunctionType>(strings{"uint"}, strings{"bool"}, FunctionType::Location::Send)},
+			{"transfer", make_shared<FunctionType>(strings{"uint"}, strings(), FunctionType::Location::Transfer)}
 		};
 	else
 		return MemberList::MemberMap();
@@ -2097,6 +2098,7 @@ string FunctionType::identifier() const
 	case Location::BareDelegateCall: id += "baredelegatecall"; break;
 	case Location::Creation: id += "creation"; break;
 	case Location::Send: id += "send"; break;
+	case Location::Transfer: id += "transfer"; break;
 	case Location::SHA3: id += "sha3"; break;
 	case Location::Selfdestruct: id += "selfdestruct"; break;
 	case Location::Revert: id += "revert"; break;
diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h
index 3546e522..022b67c4 100644
--- a/libsolidity/ast/Types.h
+++ b/libsolidity/ast/Types.h
@@ -826,6 +826,7 @@ public:
 		BareDelegateCall, ///< DELEGATECALL without function hash
 		Creation, ///< external call using CREATE
 		Send, ///< CALL, but without data and gas
+		Transfer, ///< CALL, but without data and throws on error
 		SHA3, ///< SHA3
 		Selfdestruct, ///< SELFDESTRUCT
 		Revert, ///< REVERT
diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp
index 2ed19a83..fd4d87a5 100644
--- a/libsolidity/codegen/ExpressionCompiler.cpp
+++ b/libsolidity/codegen/ExpressionCompiler.cpp
@@ -616,6 +616,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 			arguments.front()->accept(*this);
 			break;
 		case Location::Send:
+		case Location::Transfer:
 			_functionCall.expression().accept(*this);
 			// Provide the gas stipend manually at first because we may send zero ether.
 			// Will be zeroed if we send more than zero ether.
@@ -644,6 +645,12 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 				),
 				{}
 			);
+			if (function.location() == Location::Transfer)
+			{
+				// Check if zero (out of stack or not enough balance).
+				m_context << Instruction::ISZERO;
+				m_context.appendConditionalInvalid();
+			}
 			break;
 		case Location::Selfdestruct:
 			arguments.front()->accept(*this);
@@ -960,6 +967,7 @@ bool ExpressionCompiler::visit(MemberAccess const& _memberAccess)
 				case FunctionType::Location::Bare:
 				case FunctionType::Location::BareCallCode:
 				case FunctionType::Location::BareDelegateCall:
+				case FunctionType::Location::Transfer:
 					_memberAccess.expression().accept(*this);
 					m_context << funType->externalIdentifier();
 					break;
@@ -1041,7 +1049,7 @@ bool ExpressionCompiler::visit(MemberAccess const& _memberAccess)
 			);
 			m_context << Instruction::BALANCE;
 		}
-		else if ((set<string>{"send", "call", "callcode", "delegatecall"}).count(member))
+		else if ((set<string>{"send", "transfer", "call", "callcode", "delegatecall"}).count(member))
 			utils().convertType(
 				*_memberAccess.expression().annotation().type,
 				IntegerType(0, IntegerType::Modifier::Address),
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index 68f8fbef..cb0cc168 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -1681,6 +1681,42 @@ BOOST_AUTO_TEST_CASE(send_ether)
 	BOOST_CHECK_EQUAL(balanceAt(address), amount);
 }
 
+BOOST_AUTO_TEST_CASE(transfer_ether)
+{
+	char const* sourceCode = R"(
+		contract A {
+			function A() payable {}
+			function a(address addr, uint amount) returns (uint) {
+				addr.transfer(amount);
+				return this.balance;
+			}
+			function b(address addr, uint amount) {
+				addr.transfer(amount);
+			}
+		}
+
+		contract B {
+		}
+
+		contract C {
+			function () payable {
+				throw;
+			}
+		}
+	)";
+	compileAndRun(sourceCode, 0, "B");
+	u160 const nonPayableRecipient = m_contractAddress;
+	compileAndRun(sourceCode, 0, "C");
+	u160 const oogRecipient = m_contractAddress;
+	compileAndRun(sourceCode, 20, "A");
+	u160 payableRecipient(23);
+	BOOST_CHECK(callContractFunction("a(address,uint256)", payableRecipient, 10) == encodeArgs(10));
+	BOOST_CHECK_EQUAL(balanceAt(payableRecipient), 10);
+	BOOST_CHECK_EQUAL(balanceAt(m_contractAddress), 10);
+	BOOST_CHECK(callContractFunction("b(address,uint256)", nonPayableRecipient, 10) == encodeArgs());
+	BOOST_CHECK(callContractFunction("b(address,uint256)", oogRecipient, 10) == encodeArgs());
+}
+
 BOOST_AUTO_TEST_CASE(log0)
 {
 	char const* sourceCode = R"(
diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp
index a1ebc300..bb274614 100644
--- a/test/libsolidity/SolidityNameAndTypeResolution.cpp
+++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp
@@ -5108,6 +5108,24 @@ BOOST_AUTO_TEST_CASE(early_exit_on_fatal_errors)
 	CHECK_ERROR(text, DeclarationError, "Identifier not found or not unique");
 }
 
+BOOST_AUTO_TEST_CASE(address_methods)
+{
+	char const* text = R"(
+		contract C {
+			function f() {
+				address addr;
+				uint balance = addr.balance;
+				bool callRet = addr.call();
+				bool callcodeRet = addr.callcode();
+				bool delegatecallRet = addr.delegatecall();
+				bool sendRet = addr.send(1);
+				addr.transfer(1);
+			}
+		}
+	)";
+	CHECK_SUCCESS(text);
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }