Description
ViewPureChecker infers a modifier's state mutability as a single ordered maximum over
StateMutability { Pure, View, NonPayable, Payable }. Payable and NonPayable are not ordered
requirements though — they are independent obligations ("may receive value" vs "writes state").
When a modifier does both, Payable is numerically greater and the state write is silently
discarded.
Payable is then deliberately permitted on non-public functions, so an internal or private
function declared view receives no diagnostic at all and is compiled with the modifier's
SSTORE inlined into it.
The practical effect is that adding an unrelated msg.value read to a modifier suppresses an
error that solc otherwise reports.
Environment
- Compiler version: 0.8.36 (source of
develop is identical at these lines)
- Both pipelines (legacy and
--via-ir)
Reproducer
// SPDX-License-Identifier: GPL-3.0
pragma solidity ^0.8.0;
contract C {
uint public x;
modifier m() { uint v = msg.value; v; x = 1; _; }
function f() internal view m returns (uint) { return x; }
function g() public view returns (uint) { return f(); }
}
Compiles with no error and no warning (solc 0.8.36, legacy and via-IR). solc --asm shows
sstore in the runtime code, and solc --abi reports "stateMutability": "view" for g.
Executed (py-evm, Shanghai):
x() -> 0
g() -> 1 // g is declared `public view`
x() -> 1 // ... and it wrote storage
Control
Delete uint v = msg.value; v; from the modifier and leave everything else identical:
Error 8961: Function cannot be declared as view because this expression (potentially) modifies the state.
--> control.sol:10:32:
|
10 | function f() internal view m returns (uint) { return x; }
| ^
Full matrix (solc 0.8.36), modifier = { uint v = msg.value; v; x = 1; }:
| function declaration |
result |
function f() internal view m |
accepted (bug) |
function f() private view m |
accepted (bug) |
function f() internal pure m |
rejected (2527) — caught only incidentally, by the Payable && Pure branch |
function f() public view m |
rejected (4006), but the message is about msg.value and never mentions the state write |
Cause
libsolidity/analysis/ViewPureChecker.cpp:239-240:
if (_mutability > m_bestMutabilityAndLocation.mutability)
m_bestMutabilityAndLocation = MutabilityAndLocation{_mutability, _location};
While inferring the modifier, x = 1 reports NonPayable and msg.value reports Payable;
Payable > NonPayable (libsolidity/ast/ASTEnums.h:37), so only Payable survives.
modifierMutability() stores that single value, and endVisit(ModifierInvocation)
(:462-468) passes it to reportMutability, whose Payable branch (:270-295) errors only when
m_currentFunction->isConstructor() || m_currentFunction->isPublic() — intentionally, so internal
functions may use msg.value. The discarded NonPayable requirement is never reported by anyone.
A fix needs the payable requirement tracked separately from the view/pure requirement rather than
folded into one maximum.
Relationship to #16931
#16931 reports the same symptom class — a
pure/view function silently gaining state access through a modifier — but by a different
mechanism: overriding a virtual modifier, where resolveVirtual() re-resolves past the modifier
ViewPureChecker validated against. This report needs no inheritance, no virtual and no
override; the statically bound modifier's own inferred mutability is already wrong. A fix in
OverrideChecker for #16931 would not address this.
Description
ViewPureCheckerinfers a modifier's state mutability as a single ordered maximum overStateMutability { Pure, View, NonPayable, Payable }.PayableandNonPayableare not orderedrequirements though — they are independent obligations ("may receive value" vs "writes state").
When a modifier does both,
Payableis numerically greater and the state write is silentlydiscarded.
Payableis then deliberately permitted on non-public functions, so aninternalorprivatefunction declared
viewreceives no diagnostic at all and is compiled with the modifier'sSSTOREinlined into it.The practical effect is that adding an unrelated
msg.valueread to a modifier suppresses anerror that solc otherwise reports.
Environment
developis identical at these lines)--via-ir)Reproducer
Compiles with no error and no warning (solc 0.8.36, legacy and via-IR).
solc --asmshowssstorein the runtime code, andsolc --abireports"stateMutability": "view"forg.Executed (py-evm, Shanghai):
Control
Delete
uint v = msg.value; v;from the modifier and leave everything else identical:Full matrix (solc 0.8.36), modifier =
{ uint v = msg.value; v; x = 1; }:function f() internal view mfunction f() private view mfunction f() internal pure mPayable && Purebranchfunction f() public view mmsg.valueand never mentions the state writeCause
libsolidity/analysis/ViewPureChecker.cpp:239-240:if (_mutability > m_bestMutabilityAndLocation.mutability) m_bestMutabilityAndLocation = MutabilityAndLocation{_mutability, _location};While inferring the modifier,
x = 1reportsNonPayableandmsg.valuereportsPayable;Payable > NonPayable(libsolidity/ast/ASTEnums.h:37), so onlyPayablesurvives.modifierMutability()stores that single value, andendVisit(ModifierInvocation)(
:462-468) passes it toreportMutability, whosePayablebranch (:270-295) errors only whenm_currentFunction->isConstructor() || m_currentFunction->isPublic()— intentionally, so internalfunctions may use
msg.value. The discardedNonPayablerequirement is never reported by anyone.A fix needs the payable requirement tracked separately from the view/pure requirement rather than
folded into one maximum.
Relationship to #16931
#16931 reports the same symptom class — a
pure/viewfunction silently gaining state access through a modifier — but by a differentmechanism: overriding a
virtualmodifier, whereresolveVirtual()re-resolves past the modifierViewPureCheckervalidated against. This report needs no inheritance, novirtualand nooverride; the statically bound modifier's own inferred mutability is already wrong. A fix inOverrideCheckerfor #16931 would not address this.