Merge pull request #4590 from ethereum/msgValueModifier

Warn if modifier uses msg.value in non-payable function

7 files changed, 39 insertions, 1 deletions

diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index 63067b3c..85582ece 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -10537,11 +10537,18 @@ BOOST_AUTO_TEST_CASE(non_payable_throw)
 		contract C {
 			uint public a;
 			function f() public returns (uint) {
+				return msgvalue();
+			}
+			function msgvalue() internal returns (uint) {
 				return msg.value;
 			}
 			function() external {
+				update();
+			}
+			function update() internal {
 				a = msg.value + 1;
 			}
+
 		}
 	)";
 	compileAndRun(sourceCode, 0, "C");
@@ -10564,6 +10571,9 @@ BOOST_AUTO_TEST_CASE(no_nonpayable_circumvention_by_modifier)
 				if (false) _; // avoid the function, we should still not accept ether
 			}
 			function f() tryCircumvent public returns (uint) {
+				return msgvalue();
+			}
+			function msgvalue() internal returns (uint) {
 				return msg.value;
 			}
 		}
diff --git a/test/libsolidity/syntaxTests/memberLookup/msg_value_modifier_payable.sol b/test/libsolidity/syntaxTests/memberLookup/msg_value_modifier_payable.sol
new file mode 100644
index 00000000..6e93626f
--- /dev/null
+++ b/test/libsolidity/syntaxTests/memberLookup/msg_value_modifier_payable.sol
@@ -0,0 +1,4 @@
+contract C {
+    modifier costs(uint _amount) { require(msg.value >= _amount); _; }
+    function f() costs(1 ether) public payable {}
+}
diff --git a/test/libsolidity/syntaxTests/memberLookup/msg_value_modifier_pure.sol b/test/libsolidity/syntaxTests/memberLookup/msg_value_modifier_pure.sol
new file mode 100644
index 00000000..398c127d
--- /dev/null
+++ b/test/libsolidity/syntaxTests/memberLookup/msg_value_modifier_pure.sol
@@ -0,0 +1,6 @@
+contract C {
+    modifier costs(uint _amount) { require(msg.value >= _amount); _; }
+    function f() costs(1 ether) public pure {}
+}
+// ----
+// TypeError: (101-115): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view".
diff --git a/test/libsolidity/syntaxTests/memberLookup/msg_value_modifier_view.sol b/test/libsolidity/syntaxTests/memberLookup/msg_value_modifier_view.sol
new file mode 100644
index 00000000..8430c5c3
--- /dev/null
+++ b/test/libsolidity/syntaxTests/memberLookup/msg_value_modifier_view.sol
@@ -0,0 +1,6 @@
+contract C {
+    modifier costs(uint _amount) { require(msg.value >= _amount); _; }
+    function f() costs(1 ether) public view {}
+}
+// ----
+// TypeError: (101-115): This modifier uses "msg.value" and thus the function has to be payable or internal.
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/397_warns_msg_value_in_non_payable_public_function.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/397_warns_msg_value_in_non_payable_public_function.sol
index 4e1f62e1..c56ad25f 100644
--- a/test/libsolidity/syntaxTests/nameAndTypeResolution/397_warns_msg_value_in_non_payable_public_function.sol
+++ b/test/libsolidity/syntaxTests/nameAndTypeResolution/397_warns_msg_value_in_non_payable_public_function.sol
@@ -4,4 +4,4 @@ contract C {
     }
 }
 // ----
-// Warning: (52-61): "msg.value" used in non-payable function. Do you want to add the "payable" modifier to this function?
+// TypeError: (52-61): "msg.value" can only be used in payable public functions. Make the function "payable" or use an internal function to avoid this error.
diff --git a/test/libsolidity/syntaxTests/viewPureChecker/msg_value_modifier.sol b/test/libsolidity/syntaxTests/viewPureChecker/msg_value_modifier.sol
new file mode 100644
index 00000000..160b20a7
--- /dev/null
+++ b/test/libsolidity/syntaxTests/viewPureChecker/msg_value_modifier.sol
@@ -0,0 +1,6 @@
+contract C {
+    modifier m(uint _amount, uint _avail) { require(_avail >= _amount); _; }
+    function f() m(1 ether, msg.value) public pure {}
+}
+// ----
+// TypeError: (118-127): Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view".
diff --git a/test/libsolidity/syntaxTests/viewPureChecker/msg_value_modifier_view.sol b/test/libsolidity/syntaxTests/viewPureChecker/msg_value_modifier_view.sol
new file mode 100644
index 00000000..613b0198
--- /dev/null
+++ b/test/libsolidity/syntaxTests/viewPureChecker/msg_value_modifier_view.sol
@@ -0,0 +1,6 @@
+contract C {
+    modifier m(uint _amount, uint _avail) { require(_avail >= _amount); _; }
+    function f() m(1 ether, msg.value) public view {}
+}
+// ----
+// TypeError: (118-127): "msg.value" can only be used in payable public functions. Make the function "payable" or use an internal function to avoid this error.