Merge pull request #3646 from ethereum/blockhash-global

Move blockhash from block.blockhash to global level.

8 files changed, 105 insertions, 14 deletions

diff --git a/docs/miscellaneous.rst b/docs/miscellaneous.rst
index a7d5c445..01154854 100644
--- a/docs/miscellaneous.rst
+++ b/docs/miscellaneous.rst
@@ -314,7 +314,7 @@ The following is the order of precedence for operators, listed in order of evalu
 Global Variables
 ================
 
-- ``block.blockhash(uint blockNumber) returns (bytes32)``: hash of the given block - only works for 256 most recent blocks
+- ``block.blockhash(uint blockNumber) returns (bytes32)``: hash of the given block - only works for 256 most recent, excluding current, blocks - deprecated in version 0.4.22 and replaced by ``blockhash(uint blockNumber)``.
 - ``block.coinbase`` (``address``): current block miner's address
 - ``block.difficulty`` (``uint``): current block difficulty
 - ``block.gaslimit`` (``uint``): current block gaslimit
@@ -331,6 +331,7 @@ Global Variables
 - ``assert(bool condition)``: abort execution and revert state changes if condition is ``false`` (use for internal error)
 - ``require(bool condition)``: abort execution and revert state changes if condition is ``false`` (use for malformed input or error in external component)
 - ``revert()``: abort execution and revert state changes
+- ``blockhash(uint blockNumber) returns (bytes32)``: hash of the given block - only works for 256 most recent blocks
 - ``keccak256(...) returns (bytes32)``: compute the Ethereum-SHA-3 (Keccak-256) hash of the :ref:`(tightly packed) arguments <abi_packed_mode>`
 - ``sha3(...) returns (bytes32)``: an alias to ``keccak256``
 - ``sha256(...) returns (bytes32)``: compute the SHA-256 hash of the :ref:`(tightly packed) arguments <abi_packed_mode>`
@@ -346,6 +347,23 @@ Global Variables
 - ``<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
 
+.. note::
+    Do not rely on ``block.timestamp``, ``now`` and ``blockhash`` as a source of randomness,
+    unless you know what you are doing.
+
+    Both the timestamp and the block hash can be influenced by miners to some degree.
+    Bad actors in the mining community can for example run a casino payout function on a chosen hash
+    and just retry a different hash if they did not receive any money.
+
+    The current block timestamp must be strictly larger than the timestamp of the last block,
+    but the only guarantee is that it will be somewhere between the timestamps of two
+    consecutive blocks in the canonical chain.
+
+.. note::
+    The block hashes are not available for all blocks for scalability reasons.
+    You can only access the hashes of the most recent 256 blocks, all other
+    values will be zero.
+
 .. index:: visibility, public, private, external, internal
 
 Function Visibility Specifiers
diff --git a/docs/units-and-global-variables.rst b/docs/units-and-global-variables.rst
index d789e87f..2571f20a 100644
--- a/docs/units-and-global-variables.rst
+++ b/docs/units-and-global-variables.rst
@@ -52,7 +52,7 @@ namespace and are mainly used to provide information about the blockchain.
 Block and Transaction Properties
 --------------------------------
 
-- ``block.blockhash(uint blockNumber) returns (bytes32)``: hash of the given block - only works for 256 most recent blocks excluding current
+- ``block.blockhash(uint blockNumber) returns (bytes32)``: hash of the given block - only works for 256 most recent, excluding current, blocks - deprecated in version 0.4.22 and replaced by ``blockhash(uint blockNumber)``.
 - ``block.coinbase`` (``address``): current block miner's address
 - ``block.difficulty`` (``uint``): current block difficulty
 - ``block.gaslimit`` (``uint``): current block gaslimit
@@ -74,7 +74,7 @@ Block and Transaction Properties
     This includes calls to library functions.
 
 .. note::
-    Do not rely on ``block.timestamp``, ``now`` and ``block.blockhash`` as a source of randomness,
+    Do not rely on ``block.timestamp``, ``now`` and ``blockhash`` as a source of randomness,
     unless you know what you are doing.
 
     Both the timestamp and the block hash can be influenced by miners to some degree.
diff --git a/libsolidity/analysis/GlobalContext.cpp b/libsolidity/analysis/GlobalContext.cpp
index 34cb61d8..6a858d36 100644
--- a/libsolidity/analysis/GlobalContext.cpp
+++ b/libsolidity/analysis/GlobalContext.cpp
@@ -38,6 +38,7 @@ m_magicVariables(vector<shared_ptr<MagicVariableDeclaration const>>{
 	make_shared<MagicVariableDeclaration>("addmod", make_shared<FunctionType>(strings{"uint256", "uint256", "uint256"}, strings{"uint256"}, FunctionType::Kind::AddMod, false, StateMutability::Pure)),
 	make_shared<MagicVariableDeclaration>("assert", make_shared<FunctionType>(strings{"bool"}, strings{}, FunctionType::Kind::Assert, false, StateMutability::Pure)),
 	make_shared<MagicVariableDeclaration>("block", make_shared<MagicType>(MagicType::Kind::Block)),
+	make_shared<MagicVariableDeclaration>("blockhash", make_shared<FunctionType>(strings{"uint256"}, strings{"bytes32"}, FunctionType::Kind::BlockHash, false, StateMutability::View)),
 	make_shared<MagicVariableDeclaration>("ecrecover", make_shared<FunctionType>(strings{"bytes32", "uint8", "bytes32", "bytes32"}, strings{"address"}, FunctionType::Kind::ECRecover, false, StateMutability::Pure)),
 	make_shared<MagicVariableDeclaration>("gasleft", make_shared<FunctionType>(strings(), strings{"uint256"}, FunctionType::Kind::GasLeft, false, StateMutability::View)),
 	make_shared<MagicVariableDeclaration>("keccak256", make_shared<FunctionType>(strings(), strings{"bytes32"}, FunctionType::Kind::SHA3, true, StateMutability::Pure)),
diff --git a/libsolidity/analysis/StaticAnalyzer.cpp b/libsolidity/analysis/StaticAnalyzer.cpp
index d4de219a..6aee260e 100644
--- a/libsolidity/analysis/StaticAnalyzer.cpp
+++ b/libsolidity/analysis/StaticAnalyzer.cpp
@@ -142,6 +142,7 @@ bool StaticAnalyzer::visit(MemberAccess const& _memberAccess)
 	bool const v050 = m_currentContract->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050);
 
 	if (MagicType const* type = dynamic_cast<MagicType const*>(_memberAccess.expression().annotation().type.get()))
+	{
 		if (type->kind() == MagicType::Kind::Message && _memberAccess.memberName() == "gas")
 		{
 			if (v050)
@@ -155,6 +156,20 @@ bool StaticAnalyzer::visit(MemberAccess const& _memberAccess)
 					"\"msg.gas\" has been deprecated in favor of \"gasleft()\""
 				);
 		}
+		if (type->kind() == MagicType::Kind::Block && _memberAccess.memberName() == "blockhash")
+		{
+			if (v050)
+				m_errorReporter.typeError(
+					_memberAccess.location(),
+					"\"block.blockhash()\" has been deprecated in favor of \"blockhash()\""
+				);
+			else
+				m_errorReporter.warning(
+					_memberAccess.location(),
+					"\"block.blockhash()\" has been deprecated in favor of \"blockhash()\""
+				);
+		}
+	}
 
 	if (m_nonPayablePublic && !m_library)
 		if (MagicType const* type = dynamic_cast<MagicType const*>(_memberAccess.expression().annotation().type.get()))
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index 44dc40f7..a866e46c 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -1805,6 +1805,18 @@ BOOST_AUTO_TEST_CASE(uncalled_blockhash)
 	BOOST_CHECK(result[0] != 0 || result[1] != 0 || result[2] != 0);
 }
 
+BOOST_AUTO_TEST_CASE(blockhash_shadow_resolution)
+{
+	char const* code = R"(
+		contract C {
+			function blockhash(uint256 blockNumber) public returns(bytes32) { bytes32 x; return x; }
+			function f() public returns(bytes32) { return blockhash(3); }
+		}
+	)";
+	compileAndRun(code, 0, "C");
+	ABI_CHECK(callContractFunction("f()"), encodeArgs(0));
+}
+
 BOOST_AUTO_TEST_CASE(log0)
 {
 	char const* sourceCode = R"(
diff --git a/test/libsolidity/SolidityExpressionCompiler.cpp b/test/libsolidity/SolidityExpressionCompiler.cpp
index c8adfc6e..90d8265c 100644
--- a/test/libsolidity/SolidityExpressionCompiler.cpp
+++ b/test/libsolidity/SolidityExpressionCompiler.cpp
@@ -503,12 +503,15 @@ BOOST_AUTO_TEST_CASE(blockhash)
 	char const* sourceCode = R"(
 		contract test {
 			function f() {
-				block.blockhash(3);
+				blockhash(3);
 			}
 		}
 	)";
-	bytes code = compileFirstExpression(sourceCode, {}, {},
-										{make_shared<MagicVariableDeclaration>("block", make_shared<MagicType>(MagicType::Kind::Block))});
+
+	auto blockhashFun = make_shared<FunctionType>(strings{"uint256"}, strings{"bytes32"}, 
+		FunctionType::Kind::BlockHash, false, StateMutability::View);
+	
+	bytes code = compileFirstExpression(sourceCode, {}, {}, {make_shared<MagicVariableDeclaration>("blockhash", blockhashFun)});
 
 	bytes expectation({byte(Instruction::PUSH1), 0x03,
 					   byte(Instruction::BLOCKHASH)});
diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp
index 50ee2b2e..e4e7b8d8 100644
--- a/test/libsolidity/SolidityNameAndTypeResolution.cpp
+++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp
@@ -8555,6 +8555,33 @@ BOOST_AUTO_TEST_CASE(require_visibility_specifiers)
 	CHECK_ERROR(text, SyntaxError, "No visibility specified.");
 }
 
+BOOST_AUTO_TEST_CASE(blockhash)
+{
+	char const* code = R"(
+		contract C {
+			function f() public view returns (bytes32) {
+				return block.blockhash(3);
+			}
+		}
+	)";
+	CHECK_WARNING(code, "\"block.blockhash()\" has been deprecated in favor of \"blockhash()\"");
+
+	code = R"(
+		contract C {
+			function f() public view returns (bytes32) { return blockhash(3); }
+		}
+	)";
+	CHECK_SUCCESS_NO_WARNINGS(code);
+
+	code = R"(
+		pragma experimental "v0.5.0";
+		contract C {
+			function f() public returns (bytes32) { return block.blockhash(3); }
+		}
+	)";
+	CHECK_ERROR(code, TypeError, "\"block.blockhash()\" has been deprecated in favor of \"blockhash()\"");
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }
diff --git a/test/libsolidity/ViewPureChecker.cpp b/test/libsolidity/ViewPureChecker.cpp
index a6ce6d91..cd0a0b01 100644
--- a/test/libsolidity/ViewPureChecker.cpp
+++ b/test/libsolidity/ViewPureChecker.cpp
@@ -25,6 +25,7 @@
 #include <boost/test/unit_test.hpp>
 
 #include <string>
+#include <tuple>
 
 using namespace std;
 
@@ -111,6 +112,7 @@ BOOST_AUTO_TEST_CASE(environment_access)
 		"block.difficulty",
 		"block.number",
 		"block.gaslimit",
+		"blockhash(7)",
 		"gasleft()",
 		"msg.gas",
 		"msg.value",
@@ -120,11 +122,12 @@ BOOST_AUTO_TEST_CASE(environment_access)
 		"this",
 		"address(1).balance"
 	};
+	// ``block.blockhash`` and ``blockhash`` are tested seperately below because their usage will
+	// produce warnings that can't be handled in a generic way.
 	vector<string> pure{
 		"msg.data",
 		"msg.data[0]",
 		"msg.sig",
-		"block.blockhash", // Not evaluating the function
 		"msg",
 		"block",
 		"tx"
@@ -132,20 +135,32 @@ BOOST_AUTO_TEST_CASE(environment_access)
 	for (string const& x: view)
 	{
 		CHECK_ERROR(
-			"contract C { function f() pure public { var x = " + x + "; x; } }",
+			"contract C { function f() pure public { " + x + "; } }",
 			TypeError,
 			"Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires \"view\""
 		);
 	}
 	for (string const& x: pure)
 	{
-		CHECK_WARNING_ALLOW_MULTI(
-			"contract C { function f() view public { var x = " + x + "; x; } }",
-			(std::vector<std::string>{
-				"Function state mutability can be restricted to pure",
-				"Use of the \"var\" keyword is deprecated."
-		}));
+		CHECK_WARNING(
+			"contract C { function f() view public { " + x + "; } }",
+			"Function state mutability can be restricted to pure"
+		);
 	}
+
+	CHECK_WARNING_ALLOW_MULTI(
+		"contract C { function f() view public { blockhash; } }",
+		(std::vector<std::string>{
+			"Function state mutability can be restricted to pure",
+			"Statement has no effect."
+	}));
+
+	CHECK_WARNING_ALLOW_MULTI(
+		"contract C { function f() view public { block.blockhash; } }",
+		(std::vector<std::string>{
+			"Function state mutability can be restricted to pure",
+			"\"block.blockhash()\" has been deprecated in favor of \"blockhash()\""
+	}));
 }
 
 BOOST_AUTO_TEST_CASE(view_error_for_050)