Use STATICCALL for pure function calls. by chriseth · Pull Request #2966 · argotorg/solidity

chriseth · 2017-09-26T13:27:36Z

No description provided.

chriseth · 2017-09-26T13:27:51Z

This still needs tests and requires an updated version of cpp-ethereum.

axic · 2017-09-26T13:59:42Z

Changelog.md


 Features:
 * Parser: Better error message for unexpected trailing comma in parameter lists.
+ * Code Generator: Use STATICCALL opcode for calling pure functions.


Why pure only?

Oh right. I'm not yet fully awake...

axic · 2017-09-26T13:59:55Z

libsolidity/codegen/ExpressionCompiler.cpp

 	if (isDelegateCall)
 		solAssert(!_functionType.valueSet(), "Value set for delegatecall");
+	else if (isStaticCall)
+		solAssert(!_functionType.valueSet(), "Value set for delegatecall");


Text is wrong.

axic · 2017-09-28T12:20:15Z

libsolidity/codegen/ExpressionCompiler.cpp

 	bool returnSuccessCondition = funKind == FunctionType::Kind::BareCall || funKind == FunctionType::Kind::BareCallCode || funKind == FunctionType::Kind::BareDelegateCall;
 	bool isCallCode = funKind == FunctionType::Kind::BareCallCode || funKind == FunctionType::Kind::CallCode;
 	bool isDelegateCall = funKind == FunctionType::Kind::BareDelegateCall || funKind == FunctionType::Kind::DelegateCall;
+	bool isStaticCall = (_functionType.stateMutability() <= StateMutability::View);


I'd prefer to have an experimental pragma for this.

axic · 2017-10-16T14:51:39Z

libsolidity/codegen/ExpressionCompiler.cpp

 	bool isDelegateCall = funKind == FunctionType::Kind::BareDelegateCall || funKind == FunctionType::Kind::DelegateCall;
+	bool useStaticCall =
+		_functionType.stateMutability() <= StateMutability::View &&
+		m_context.experimentalFeatureActive(ExperimentalFeature::STATICCALL;


axic · 2017-10-16T14:51:56Z

Changelog.md

   (previously, some amount was retained in order to work in pre-tangerine whistle
   EVM versions)
+ * Code Generator: Use STATICCALL opcode for calling view and pure functions. Enable
+   via `pragma experimental STATICCALL;`.


Why don't we go for pragma experimental Byzantium;?

This is still a question.

axic · 2017-10-16T14:53:03Z

libsolidity/codegen/ExpressionCompiler.cpp

 	bool returnSuccessCondition = funKind == FunctionType::Kind::BareCall || funKind == FunctionType::Kind::BareCallCode || funKind == FunctionType::Kind::BareDelegateCall;
 	bool isCallCode = funKind == FunctionType::Kind::BareCallCode || funKind == FunctionType::Kind::CallCode;
 	bool isDelegateCall = funKind == FunctionType::Kind::BareDelegateCall || funKind == FunctionType::Kind::DelegateCall;
+	bool useStaticCall =


I think use suits this better, but the other two are named is so perhaps just go with isStaticCall or alternatively rename the other two to use?

It's a different thing. I would call it isStaticCall out of the function type, but useStaticCall tells whether the compiler is also allowed to use the opcode due to pragmas.

axic

Needs tests, changelog update and a rebase.

axic · 2017-12-12T02:49:11Z

Still needs tests.

chriseth · 2018-01-16T09:50:26Z

Added tests and documentation. Should be ready to merge.

axic · 2018-01-16T16:13:06Z

Perhaps we should have a target pragma specifying the chain (related #1117). The question there becomes wether we enforce the latest chain and allow to go back to the previous one or we stay on longer on the "old chain".

In example:

stay on Homestead and enable Byzantium via pragma target byztantium or pragma byzantium
stay on Byzantium and enable Homestead via pragma homestead

chriseth · 2018-01-16T16:33:37Z

I would prefer a pragma homestead directive, or perhaps pragma target homestead

chriseth · 2018-01-25T15:50:45Z

Decision from discussion:

add a pragma target <x> pragma. The default is always "latest version of the EVM". Currently supported values are byzantium and homestead. Everything else is rejected.

homestead could also be used to force gas calculation for calls (which is currently still the default).

axic · 2018-02-14T13:20:25Z

Should use the new evmTarget from #1117.

chriseth · 2018-02-19T13:38:11Z

@axic this only requires the evm target check, or is something else missing?

chriseth · 2018-03-02T16:03:20Z

Rebased. Now depends on #3569 and #3600 .

axic · 2018-03-02T16:15:37Z

Changelog.md

 Features:
 * C99/C++-style scoping rules (instead of JavaScript function scoping) take effect as experimental v0.5.0 feature.
 * Code Generator: Assert that ``k != 0`` for ``molmod(a, b, k)`` and ``addmod(a, b, k)`` as experimental 0.5.0 feature.
+ * Code Generator: Use STATICCALL opcode for calling view and pure functions as experimenal 0.5.0 feature.


``STATICCALL`` :)

Also could fix the molmod above here.

axic · 2018-03-02T16:18:30Z

docs/contracts.rst

+  You can switch the compiler to use STATICCALL when calling such functions and thus
+  prevent modifications to the state on the level of the EVM by adding
+  ``pragma experimental "v0.5.0";``
+  It is not possible to prevent functions from reading the state at the level


Should this be a separate warning block?

axic · 2018-03-02T16:18:50Z

The static call changes look good.

axic

Apart from the changelog comment the staticcall changes looks fine. (Should of course merge the dependant PRs first.)

axic · 2018-03-06T13:29:12Z

libsolidity/codegen/ExpressionCompiler.cpp

 	bool isDelegateCall = funKind == FunctionType::Kind::BareDelegateCall || funKind == FunctionType::Kind::DelegateCall;
+	bool useStaticCall =
+		_functionType.stateMutability() <= StateMutability::View &&
+		m_context.experimentalFeatureActive(ExperimentalFeature::V050) &&


Actually this contradicts #3600.

Do we really need this line? We should just enable it based on the EMV version (which we also have as a condition below).

We perform changes in codegen from time to time without requiring a breaking release. The condition for that is that the actual semantics visible from outside do not change. This is only the case if people do not perform invalid type conversions and if they use the most recent EVM version. I think it is fine to assume the latter, so we can just remove it. We should highlight that in the release notes.

Ok but actually arguing the other way around: If we remove this condition, then we actually assume that it is perfectly fine to use. This means it is not an experimental feature. In turn, we don't have to warn about using experimental 0.5.0.

Copying this from the channel for the record:

The argument is that we specifically state in the changelog that pragma experimental v0.5.0 is analysis only and won’t change the codegen phase.

The second argument is whether staticcall is experiemental or not and I’m leaning towards to say it isn’t experimental. That code should work, if it doesn’t that is a broken VM or a broken implementation in the codegen.

There is one case where this could cause an issue if the user disregards the warnings from the ViewPureChecker and does expected state modification in a view function and calls it externally in another contract.

Luckily, the ViewPureChecker reports a warning for these cases (https://github.com/ethereum/solidity/blob/develop/libsolidity/analysis/ViewPureChecker.cpp#L254) so it should only fail as a matter of programmer error.

Though it is kind of a breaking change because if they disregard the warning the output will not work.

… 0.5.0 is activated.

axic · 2018-03-06T14:34:21Z

Rebased it.

axic reviewed Sep 26, 2017

View reviewed changes

chriseth force-pushed the useStaticCall branch from 7b13960 to e0f6c38 Compare September 26, 2017 14:36

axic reviewed Sep 28, 2017

View reviewed changes

chriseth force-pushed the useStaticCall branch from e0f6c38 to c10c6fa Compare October 16, 2017 13:17

axic reviewed Oct 16, 2017

View reviewed changes

axic suggested changes Nov 17, 2017

View reviewed changes

axic force-pushed the useStaticCall branch 2 times, most recently from e952b6f to 1e04cc3 Compare December 12, 2017 02:48

maurelian mentioned this pull request Jan 9, 2018

Native solidity staticcall #3388

Closed

HarryR mentioned this pull request Jan 10, 2018

Change constant and view to pure when compiler supports it. clearmatics/mobius#48

Open

chriseth force-pushed the useStaticCall branch from 1e04cc3 to 29a7cb6 Compare January 16, 2018 09:31

axic added the nextrelease label Feb 14, 2018

chriseth force-pushed the useStaticCall branch from 29a7cb6 to 583a931 Compare February 19, 2018 13:53

chriseth force-pushed the useStaticCall branch 2 times, most recently from e98d4e6 to 9955015 Compare March 2, 2018 16:02

axic reviewed Mar 2, 2018

View reviewed changes

axic approved these changes Mar 2, 2018

View reviewed changes

chriseth force-pushed the useStaticCall branch 2 times, most recently from 01390b2 to bec98c6 Compare March 5, 2018 18:31

axic reviewed Mar 6, 2018

View reviewed changes

chriseth added 3 commits March 6, 2018 15:30

Use STATICCALL for pure function calls if EVM version supports it and…

b467116

… 0.5.0 is activated.

Test for using staticcall for view and pure functions.

a3593df

Document STATICCALL usage in experimental 0.5.0.

3057aee

axic force-pushed the useStaticCall branch from bec98c6 to 3057aee Compare March 6, 2018 14:34

axic merged commit 14b12ae into develop Mar 6, 2018

axic deleted the useStaticCall branch March 6, 2018 16:07

Conversation

chriseth commented Sep 26, 2017

Uh oh!

chriseth commented Sep 26, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

axic Oct 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

axic left a comment

Choose a reason for hiding this comment

Uh oh!

axic commented Dec 12, 2017

Uh oh!

chriseth commented Jan 16, 2018

Uh oh!

axic commented Jan 16, 2018

Uh oh!

chriseth commented Jan 16, 2018

Uh oh!

chriseth commented Jan 25, 2018

Uh oh!

axic commented Feb 14, 2018

Uh oh!

chriseth commented Feb 19, 2018

Uh oh!

chriseth commented Mar 2, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

axic commented Mar 2, 2018

Uh oh!

axic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

axic commented Mar 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

axic Oct 16, 2017 •

edited

Loading

axic commented Mar 6, 2018 •

edited

Loading