upgradeable.patch 14 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267268269270271272273274275276277278279280281282283284285286287288289290291292293294295296297298299300301302303304305306307308309310311312313314315316317318319320321322323324325326327328329330331332333334335336337338
  1. diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md
  2. deleted file mode 100644
  3. index 2797a088..00000000
  4. --- a/.github/ISSUE_TEMPLATE/bug_report.md
  5. +++ /dev/null
  6. @@ -1,21 +0,0 @@
  7. ----
  8. -name: Bug report
  9. -about: Report a bug in OpenZeppelin Contracts
  10. -
  11. ----
  12. -
  13. -<!-- Briefly describe the issue you're experiencing. Tell us what you were trying to do and what happened instead. -->
  14. -
  15. -<!-- Remember, this is not a place to ask for help debugging code. For that, we welcome you in the OpenZeppelin Community Forum: https://forum.openzeppelin.com/. -->
  16. -
  17. -**💻 Environment**
  18. -
  19. -<!-- Tell us what version of OpenZeppelin Contracts you're using, and how you're using it: Truffle, Remix, etc. -->
  20. -
  21. -**📝 Details**
  22. -
  23. -<!-- Describe the problem you have been experiencing in more detail. Include as much information as you think is relevant. Keep in mind that transactions can fail for many reasons; context is key here. -->
  24. -
  25. -**🔢 Code to reproduce bug**
  26. -
  27. -<!-- We will be able to better help if you provide a minimal example that triggers the bug. -->
  28. diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml
  29. index 4018cef2..d343a53d 100644
  30. --- a/.github/ISSUE_TEMPLATE/config.yml
  31. +++ b/.github/ISSUE_TEMPLATE/config.yml
  32. @@ -1,4 +1,8 @@
  33. +blank_issues_enabled: false
  34. contact_links:
  35. + - name: Bug Reports & Feature Requests
  36. + url: https://github.com/OpenZeppelin/openzeppelin-contracts/issues/new/choose
  37. + about: Visit the OpenZeppelin Contracts repository
  38. - name: Questions & Support Requests
  39. url: https://forum.openzeppelin.com/c/support/contracts/18
  40. about: Ask in the OpenZeppelin Forum
  41. diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md
  42. deleted file mode 100644
  43. index ff596b0c..00000000
  44. --- a/.github/ISSUE_TEMPLATE/feature_request.md
  45. +++ /dev/null
  46. @@ -1,14 +0,0 @@
  47. ----
  48. -name: Feature request
  49. -about: Suggest an idea for OpenZeppelin Contracts
  50. -
  51. ----
  52. -
  53. -**🧐 Motivation**
  54. -<!-- Is your feature request related to a specific problem? Is it just a crazy idea? Tell us about it! -->
  55. -
  56. -**📝 Details**
  57. -<!-- Please describe your feature request in detail. -->
  58. -
  59. -<!-- Make sure that you have reviewed the OpenZeppelin Contracts Contributor Guidelines. -->
  60. -<!-- https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/CONTRIBUTING.md -->
  61. diff --git a/README.md b/README.md
  62. index 27627f43..e42a66e8 100644
  63. --- a/README.md
  64. +++ b/README.md
  65. @@ -19,6 +19,9 @@
  66. :building_construction: **Want to scale your decentralized application?** Check out [OpenZeppelin Defender](https://openzeppelin.com/defender) — a secure platform for automating and monitoring your operations.
  67. +> **Note**
  68. +> You are looking at the upgradeable variant of OpenZeppelin Contracts. Be sure to review the documentation on [Using OpenZeppelin Contracts with Upgrades](https://docs.openzeppelin.com/contracts/4.x/upgradeable).
  69. +
  70. ## Overview
  71. ### Installation
  72. @@ -26,7 +29,7 @@
  73. #### Hardhat, Truffle (npm)
  74. ```
  75. -$ npm install @openzeppelin/contracts
  76. +$ npm install @openzeppelin/contracts-upgradeable
  77. ```
  78. OpenZeppelin Contracts features a [stable API](https://docs.openzeppelin.com/contracts/releases-stability#api-stability), which means that your contracts won't break unexpectedly when upgrading to a newer minor version.
  79. @@ -38,7 +41,7 @@ OpenZeppelin Contracts features a [stable API](https://docs.openzeppelin.com/con
  80. > **Warning** Foundry installs the latest version initially, but subsequent `forge update` commands will use the `master` branch.
  81. ```
  82. -$ forge install OpenZeppelin/openzeppelin-contracts
  83. +$ forge install OpenZeppelin/openzeppelin-contracts-upgradeable
  84. ```
  85. ### Usage
  86. @@ -48,10 +51,11 @@ Once installed, you can use the contracts in the library by importing them:
  87. ```solidity
  88. pragma solidity ^0.8.19;
  89. -import {ERC721} from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
  90. +import {ERC721Upgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC721/ERC721Upgradeable.sol";
  91. -contract MyCollectible is ERC721 {
  92. - constructor() ERC721("MyCollectible", "MCO") {
  93. +contract MyCollectible is ERC721Upgradeable {
  94. + function initialize() initializer public {
  95. + __ERC721_init("MyCollectible", "MCO");
  96. }
  97. }
  98. ```
  99. diff --git a/contracts/package.json b/contracts/package.json
  100. index df141192..1cf90ad1 100644
  101. --- a/contracts/package.json
  102. +++ b/contracts/package.json
  103. @@ -1,5 +1,5 @@
  104. {
  105. - "name": "@openzeppelin/contracts",
  106. + "name": "@openzeppelin/contracts-upgradeable",
  107. "description": "Secure Smart Contract library for Solidity",
  108. "version": "4.9.2",
  109. "files": [
  110. @@ -13,7 +13,7 @@
  111. },
  112. "repository": {
  113. "type": "git",
  114. - "url": "https://github.com/OpenZeppelin/openzeppelin-contracts.git"
  115. + "url": "https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable.git"
  116. },
  117. "keywords": [
  118. "solidity",
  119. diff --git a/contracts/utils/cryptography/EIP712.sol b/contracts/utils/cryptography/EIP712.sol
  120. index ff34e814..a9d08d5c 100644
  121. --- a/contracts/utils/cryptography/EIP712.sol
  122. +++ b/contracts/utils/cryptography/EIP712.sol
  123. @@ -4,7 +4,6 @@
  124. pragma solidity ^0.8.19;
  125. import {ECDSA} from "./ECDSA.sol";
  126. -import {ShortStrings, ShortString} from "../ShortStrings.sol";
  127. import {IERC5267} from "../../interfaces/IERC5267.sol";
  128. /**
  129. @@ -27,28 +26,18 @@ import {IERC5267} from "../../interfaces/IERC5267.sol";
  130. * NOTE: In the upgradeable version of this contract, the cached values will correspond to the address, and the domain
  131. * separator of the implementation contract. This will cause the `_domainSeparatorV4` function to always rebuild the
  132. * separator from the immutable values, which is cheaper than accessing a cached version in cold storage.
  133. - *
  134. - * @custom:oz-upgrades-unsafe-allow state-variable-immutable state-variable-assignment
  135. */
  136. abstract contract EIP712 is IERC5267 {
  137. - using ShortStrings for *;
  138. -
  139. bytes32 private constant _TYPE_HASH =
  140. keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)");
  141. - // Cache the domain separator as an immutable value, but also store the chain id that it corresponds to, in order to
  142. - // invalidate the cached domain separator if the chain id changes.
  143. - bytes32 private immutable _cachedDomainSeparator;
  144. - uint256 private immutable _cachedChainId;
  145. - address private immutable _cachedThis;
  146. -
  147. + /// @custom:oz-renamed-from _HASHED_NAME
  148. bytes32 private immutable _hashedName;
  149. + /// @custom:oz-renamed-from _HASHED_VERSION
  150. bytes32 private immutable _hashedVersion;
  151. - ShortString private immutable _name;
  152. - ShortString private immutable _version;
  153. - string private _nameFallback;
  154. - string private _versionFallback;
  155. + string private _name;
  156. + string private _version;
  157. /**
  158. * @dev Initializes the domain separator and parameter caches.
  159. @@ -63,29 +52,23 @@ abstract contract EIP712 is IERC5267 {
  160. * contract upgrade].
  161. */
  162. constructor(string memory name, string memory version) {
  163. - _name = name.toShortStringWithFallback(_nameFallback);
  164. - _version = version.toShortStringWithFallback(_versionFallback);
  165. - _hashedName = keccak256(bytes(name));
  166. - _hashedVersion = keccak256(bytes(version));
  167. -
  168. - _cachedChainId = block.chainid;
  169. - _cachedDomainSeparator = _buildDomainSeparator();
  170. - _cachedThis = address(this);
  171. + _name = name;
  172. + _version = version;
  173. +
  174. + // Reset prior values in storage if upgrading
  175. + _hashedName = 0;
  176. + _hashedVersion = 0;
  177. }
  178. /**
  179. * @dev Returns the domain separator for the current chain.
  180. */
  181. function _domainSeparatorV4() internal view returns (bytes32) {
  182. - if (address(this) == _cachedThis && block.chainid == _cachedChainId) {
  183. - return _cachedDomainSeparator;
  184. - } else {
  185. - return _buildDomainSeparator();
  186. - }
  187. + return _buildDomainSeparator();
  188. }
  189. function _buildDomainSeparator() private view returns (bytes32) {
  190. - return keccak256(abi.encode(_TYPE_HASH, _hashedName, _hashedVersion, block.chainid, address(this)));
  191. + return keccak256(abi.encode(_TYPE_HASH, _EIP712NameHash(), _EIP712VersionHash(), block.chainid, address(this)));
  192. }
  193. /**
  194. @@ -124,6 +107,10 @@ abstract contract EIP712 is IERC5267 {
  195. uint256[] memory extensions
  196. )
  197. {
  198. + // If the hashed name and version in storage are non-zero, the contract hasn't been properly initialized
  199. + // and the EIP712 domain is not reliable, as it will be missing name and version.
  200. + require(_hashedName == 0 && _hashedVersion == 0, "EIP712: Uninitialized");
  201. +
  202. return (
  203. hex"0f", // 01111
  204. _EIP712Name(),
  205. @@ -138,22 +125,62 @@ abstract contract EIP712 is IERC5267 {
  206. /**
  207. * @dev The name parameter for the EIP712 domain.
  208. *
  209. - * NOTE: By default this function reads _name which is an immutable value.
  210. - * It only reads from storage if necessary (in case the value is too large to fit in a ShortString).
  211. + * NOTE: This function reads from storage by default, but can be redefined to return a constant value if gas costs
  212. + * are a concern.
  213. */
  214. - // solhint-disable-next-line func-name-mixedcase
  215. - function _EIP712Name() internal view returns (string memory) {
  216. - return _name.toStringWithFallback(_nameFallback);
  217. + function _EIP712Name() internal view virtual returns (string memory) {
  218. + return _name;
  219. }
  220. /**
  221. * @dev The version parameter for the EIP712 domain.
  222. *
  223. - * NOTE: By default this function reads _version which is an immutable value.
  224. - * It only reads from storage if necessary (in case the value is too large to fit in a ShortString).
  225. + * NOTE: This function reads from storage by default, but can be redefined to return a constant value if gas costs
  226. + * are a concern.
  227. */
  228. - // solhint-disable-next-line func-name-mixedcase
  229. - function _EIP712Version() internal view returns (string memory) {
  230. - return _version.toStringWithFallback(_versionFallback);
  231. + function _EIP712Version() internal view virtual returns (string memory) {
  232. + return _version;
  233. + }
  234. +
  235. + /**
  236. + * @dev The hash of the name parameter for the EIP712 domain.
  237. + *
  238. + * NOTE: In previous versions this function was virtual. In this version you should override `_EIP712Name` instead.
  239. + */
  240. + function _EIP712NameHash() internal view returns (bytes32) {
  241. + string memory name = _EIP712Name();
  242. + if (bytes(name).length > 0) {
  243. + return keccak256(bytes(name));
  244. + } else {
  245. + // If the name is empty, the contract may have been upgraded without initializing the new storage.
  246. + // We return the name hash in storage if non-zero, otherwise we assume the name is empty by design.
  247. + bytes32 hashedName = _hashedName;
  248. + if (hashedName != 0) {
  249. + return hashedName;
  250. + } else {
  251. + return keccak256("");
  252. + }
  253. + }
  254. + }
  255. +
  256. + /**
  257. + * @dev The hash of the version parameter for the EIP712 domain.
  258. + *
  259. + * NOTE: In previous versions this function was virtual. In this version you should override `_EIP712Version` instead.
  260. + */
  261. + function _EIP712VersionHash() internal view returns (bytes32) {
  262. + string memory version = _EIP712Version();
  263. + if (bytes(version).length > 0) {
  264. + return keccak256(bytes(version));
  265. + } else {
  266. + // If the version is empty, the contract may have been upgraded without initializing the new storage.
  267. + // We return the version hash in storage if non-zero, otherwise we assume the version is empty by design.
  268. + bytes32 hashedVersion = _hashedVersion;
  269. + if (hashedVersion != 0) {
  270. + return hashedVersion;
  271. + } else {
  272. + return keccak256("");
  273. + }
  274. + }
  275. }
  276. }
  277. diff --git a/package.json b/package.json
  278. index 9eae6732..b3a56366 100644
  279. --- a/package.json
  280. +++ b/package.json
  281. @@ -33,7 +33,7 @@
  282. },
  283. "repository": {
  284. "type": "git",
  285. - "url": "https://github.com/OpenZeppelin/openzeppelin-contracts.git"
  286. + "url": "https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable.git"
  287. },
  288. "keywords": [
  289. "solidity",
  290. diff --git a/test/utils/cryptography/EIP712.test.js b/test/utils/cryptography/EIP712.test.js
  291. index 7ea535b7..32e3a370 100644
  292. --- a/test/utils/cryptography/EIP712.test.js
  293. +++ b/test/utils/cryptography/EIP712.test.js
  294. @@ -47,26 +47,6 @@ contract('EIP712', function (accounts) {
  295. const rebuildDomain = await getDomain(this.eip712);
  296. expect(mapValues(rebuildDomain, String)).to.be.deep.equal(mapValues(this.domain, String));
  297. });
  298. -
  299. - if (shortOrLong === 'short') {
  300. - // Long strings are in storage, and the proxy will not be properly initialized unless
  301. - // the upgradeable contract variant is used and the initializer is invoked.
  302. -
  303. - it('adjusts when behind proxy', async function () {
  304. - const factory = await Clones.new();
  305. - const cloneReceipt = await factory.$clone(this.eip712.address);
  306. - const cloneAddress = cloneReceipt.logs.find(({ event }) => event === 'return$clone').args.instance;
  307. - const clone = new EIP712Verifier(cloneAddress);
  308. -
  309. - const cloneDomain = { ...this.domain, verifyingContract: clone.address };
  310. -
  311. - const reportedDomain = await getDomain(clone);
  312. - expect(mapValues(reportedDomain, String)).to.be.deep.equal(mapValues(cloneDomain, String));
  313. -
  314. - const expectedSeparator = await domainSeparator(cloneDomain);
  315. - expect(await clone.$_domainSeparatorV4()).to.equal(expectedSeparator);
  316. - });
  317. - }
  318. });
  319. it('hash digest', async function () {