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

docs: misleading outdated comment at EIP712Upgradeable #5607

Open
DefiCake opened this issue Mar 26, 2025 · 1 comment
Open

docs: misleading outdated comment at EIP712Upgradeable #5607

DefiCake opened this issue Mar 26, 2025 · 1 comment
Labels
documentation Inline comments, guides, and examples.

Comments

@DefiCake
Copy link

The EIP712Upgradeable contract seems to have misleading / outdated documentation in the comments.

💻 Environment

Latest (5.2.0)

📝 Details

At https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/master/contracts/utils/cryptography/EIP712Upgradeable.sol

/**
 ...
 * NOTE: In the upgradeable version of this contract, the cached values will correspond to the address, and the domain
 * separator of the implementation contract. This will cause the {_domainSeparatorV4} function to always rebuild the
 * separator from the immutable values, which is cheaper than accessing a cached version in cold storage.
 */

There are no use of immutables or constants in the contract. This note should go away.

🔢 Code to reproduce bug

Not relevant

@Amxx
Copy link
Collaborator

Amxx commented Mar 27, 2025

Hello @DefiCake

https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/master/contracts/utils/cryptography/EIP712Upgradeable.sol is the upgradeable version. Its true that in the transpilation process, the cache is removed completelly. Would it make more sens to you if we replaced it with

* NOTE: The upgradeable version of this contract does not use an immutable cache and recomputes the domain separator
* each time {_domainSeparatorV4} is called. That is cheaper than accessing a cached version in cold storage.

@Amxx Amxx added the documentation Inline comments, guides, and examples. label Mar 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Inline comments, guides, and examples.
Projects
None yet
Development

No branches or pull requests

2 participants