Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"constants" expressions are expressions, not constants. #9232

Closed
drortirosh opened this issue Jun 18, 2020 · 2 comments
Closed

"constants" expressions are expressions, not constants. #9232

drortirosh opened this issue Jun 18, 2020 · 2 comments

Comments

@drortirosh
Copy link

Constant expressions are left as expressions, not constants.

Description

a constant declared as:

bytes32 public constant EIP712DOMAIN_TYPEHASH = keccak256(
    "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"
);

Is re-calculated each time it is in use.

expected

the value should be converted into a constant value at compile time.

###a ctual
The expression is re-calculated each time the constant is referenced.

consequences:

  • each usage of a "constant" costs ~100gas more on each access (it is still a little better than storing the result in storage, but not much..)
  • since these are not real constants, they can't be referenced from a real constant environment (e.g. from assembly, or from another library )

Environment

-- solc 0.6.10
-- checked in remix, with optimization enabled.

@drortirosh drortirosh changed the title "constants" expressions are not optimized "constants" expressions are expressions, not constants. Jun 18, 2020
@chriseth
Copy link
Contributor

Duplicate of #4024 / #3157

@chriseth
Copy link
Contributor

Ah and by the way: You could use immutables until the referenced issues are implemented, then you only pay the gas costs for the computation at deploy time.

TomAFrench added a commit to balancer/balancer-v2-monorepo that referenced this issue Oct 27, 2021
nventuro added a commit to balancer/balancer-v2-monorepo that referenced this issue Nov 3, 2021
* Add temp storage read/writes

* Add chained reference detection and key extraction

* Add chained reference functions

* Add chained references to simple swap

* Add Math.abs

* Add chained references to batch swap

* Add some docs

* Remove unnecessary distinction between references and keys

* Copy toChainedReference helper

* refactor: make _TEMP_STORAGE_PREFIX constant and use as a prefix

* fix: revert swapping ref and _TEMP_STORAGE_PREFIX and rename to _TEMP_STORAGE_SUFFIX

Swapping the order meant that we were no longer replicating the standard mapping mechanism

* revert: change _TEMP_STORAGE_SUFFIX to immutable

see: ethereum/solidity#9232

* Add joinPool output chained reference

* Add join support

* Add more join tests

* Add test helpers

* Fix linter

* Update pkg/standalone-utils/contracts/relayer/VaultActions.sol

Co-authored-by: Tom French <[email protected]>

* refactor: remove unused imports

Co-authored-by: Tom French <[email protected]>
Co-authored-by: Tom French <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants