From b6111faac8e22958ac031df305c7a5923063f543 Mon Sep 17 00:00:00 2001 From: Francisco Date: Mon, 11 Sep 2023 16:32:10 -0300 Subject: [PATCH] Use namespaced storage for upgradeable contracts (#4534) --- .changeset/wet-bears-heal.md | 5 ++ .github/actions/setup/action.yml | 2 +- contracts/governance/Governor.sol | 6 +- .../governance/extensions/GovernorVotes.sol | 17 ++++-- .../GovernorVotesQuorumFraction.sol | 2 +- package-lock.json | 56 ++++++++++++++++--- package.json | 2 +- scripts/upgradeable/transpile.sh | 6 +- 8 files changed, 75 insertions(+), 21 deletions(-) create mode 100644 .changeset/wet-bears-heal.md diff --git a/.changeset/wet-bears-heal.md b/.changeset/wet-bears-heal.md new file mode 100644 index 000000000..2df32f39a --- /dev/null +++ b/.changeset/wet-bears-heal.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': major +--- + +Upgradeable contracts now use namespaced storage (EIP-7201). diff --git a/.github/actions/setup/action.yml b/.github/actions/setup/action.yml index 680fba0c6..84ebe823a 100644 --- a/.github/actions/setup/action.yml +++ b/.github/actions/setup/action.yml @@ -5,7 +5,7 @@ runs: steps: - uses: actions/setup-node@v3 with: - node-version: 14.x + node-version: 16.x - uses: actions/cache@v3 id: cache with: diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol index b4abaa695..f5d68f654 100644 --- a/contracts/governance/Governor.sol +++ b/contracts/governance/Governor.sol @@ -299,8 +299,8 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72 bytes[] memory calldatas, string memory description, address proposer - ) internal virtual returns (uint256) { - uint256 proposalId = hashProposal(targets, values, calldatas, keccak256(bytes(description))); + ) internal virtual returns (uint256 proposalId) { + proposalId = hashProposal(targets, values, calldatas, keccak256(bytes(description))); if (targets.length != values.length || targets.length != calldatas.length || targets.length == 0) { revert GovernorInvalidProposalLength(targets.length, calldatas.length, values.length); @@ -329,7 +329,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72 description ); - return proposalId; + // Using a named return variable to avoid stack too deep errors } /** diff --git a/contracts/governance/extensions/GovernorVotes.sol b/contracts/governance/extensions/GovernorVotes.sol index 45447da51..3ea1bd2bf 100644 --- a/contracts/governance/extensions/GovernorVotes.sol +++ b/contracts/governance/extensions/GovernorVotes.sol @@ -12,10 +12,17 @@ import {SafeCast} from "../../utils/math/SafeCast.sol"; * @dev Extension of {Governor} for voting weight extraction from an {ERC20Votes} token, or since v4.5 an {ERC721Votes} token. */ abstract contract GovernorVotes is Governor { - IERC5805 public immutable token; + IERC5805 private immutable _token; constructor(IVotes tokenAddress) { - token = IERC5805(address(tokenAddress)); + _token = IERC5805(address(tokenAddress)); + } + + /** + * @dev The token that voting power is sourced from. + */ + function token() public view virtual returns (IERC5805) { + return _token; } /** @@ -23,7 +30,7 @@ abstract contract GovernorVotes is Governor { * does not implement EIP-6372. */ function clock() public view virtual override returns (uint48) { - try token.clock() returns (uint48 timepoint) { + try token().clock() returns (uint48 timepoint) { return timepoint; } catch { return SafeCast.toUint48(block.number); @@ -35,7 +42,7 @@ abstract contract GovernorVotes is Governor { */ // solhint-disable-next-line func-name-mixedcase function CLOCK_MODE() public view virtual override returns (string memory) { - try token.CLOCK_MODE() returns (string memory clockmode) { + try token().CLOCK_MODE() returns (string memory clockmode) { return clockmode; } catch { return "mode=blocknumber&from=default"; @@ -50,6 +57,6 @@ abstract contract GovernorVotes is Governor { uint256 timepoint, bytes memory /*params*/ ) internal view virtual override returns (uint256) { - return token.getPastVotes(account, timepoint); + return token().getPastVotes(account, timepoint); } } diff --git a/contracts/governance/extensions/GovernorVotesQuorumFraction.sol b/contracts/governance/extensions/GovernorVotesQuorumFraction.sol index cc2e85b45..100758e3b 100644 --- a/contracts/governance/extensions/GovernorVotesQuorumFraction.sol +++ b/contracts/governance/extensions/GovernorVotesQuorumFraction.sol @@ -71,7 +71,7 @@ abstract contract GovernorVotesQuorumFraction is GovernorVotes { * @dev Returns the quorum for a timepoint, in terms of number of votes: `supply * numerator / denominator`. */ function quorum(uint256 timepoint) public view virtual override returns (uint256) { - return (token.getPastTotalSupply(timepoint) * quorumNumerator(timepoint)) / quorumDenominator(); + return (token().getPastTotalSupply(timepoint) * quorumNumerator(timepoint)) / quorumDenominator(); } /** diff --git a/package-lock.json b/package-lock.json index 22fe93cb7..fdcda1e7b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -44,7 +44,7 @@ "semver": "^7.3.5", "solhint": "^3.3.6", "solhint-plugin-openzeppelin": "file:scripts/solhint-custom", - "solidity-ast": "^0.4.25", + "solidity-ast": "^0.4.50", "solidity-coverage": "^0.8.0", "solidity-docgen": "^0.6.0-beta.29", "undici": "^5.22.1", @@ -3221,6 +3221,25 @@ "url": "https://github.com/sponsors/ljharb" } }, + "node_modules/array.prototype.findlast": { + "version": "1.2.2", + "resolved": "https://registry.npmjs.org/array.prototype.findlast/-/array.prototype.findlast-1.2.2.tgz", + "integrity": "sha512-p1YDNPNqA+P6cPX9ATsxg7DKir7gOmJ+jh5dEP3LlumMNYVC1F2Jgnyh6oI3n/qD9FeIkqR2jXfd73G68ImYUQ==", + "dev": true, + "dependencies": { + "call-bind": "^1.0.2", + "define-properties": "^1.1.4", + "es-abstract": "^1.20.4", + "es-shim-unscopables": "^1.0.0", + "get-intrinsic": "^1.1.3" + }, + "engines": { + "node": ">= 0.4" + }, + "funding": { + "url": "https://github.com/sponsors/ljharb" + } + }, "node_modules/array.prototype.flat": { "version": "1.3.1", "resolved": "https://registry.npmjs.org/array.prototype.flat/-/array.prototype.flat-1.3.1.tgz", @@ -12569,10 +12588,13 @@ } }, "node_modules/solidity-ast": { - "version": "0.4.49", - "resolved": "https://registry.npmjs.org/solidity-ast/-/solidity-ast-0.4.49.tgz", - "integrity": "sha512-Pr5sCAj1SFqzwFZw1HPKSq0PehlQNdM8GwKyAVYh2DOn7/cCK8LUKD1HeHnKtTgBW7hi9h4nnnan7hpAg5RhWQ==", - "dev": true + "version": "0.4.50", + "resolved": "https://registry.npmjs.org/solidity-ast/-/solidity-ast-0.4.50.tgz", + "integrity": "sha512-WpIhaUibbjcBY4bg8TO2UXFWl8PQPhtH1QtMYJUqFUGxx0rRiEFsVLV+ow8XiWEnSPeu4xPp1/K43P4esxuK1Q==", + "dev": true, + "dependencies": { + "array.prototype.findlast": "^1.2.2" + } }, "node_modules/solidity-comments": { "version": "0.0.2", @@ -17853,6 +17875,19 @@ "es-shim-unscopables": "^1.0.0" } }, + "array.prototype.findlast": { + "version": "1.2.2", + "resolved": "https://registry.npmjs.org/array.prototype.findlast/-/array.prototype.findlast-1.2.2.tgz", + "integrity": "sha512-p1YDNPNqA+P6cPX9ATsxg7DKir7gOmJ+jh5dEP3LlumMNYVC1F2Jgnyh6oI3n/qD9FeIkqR2jXfd73G68ImYUQ==", + "dev": true, + "requires": { + "call-bind": "^1.0.2", + "define-properties": "^1.1.4", + "es-abstract": "^1.20.4", + "es-shim-unscopables": "^1.0.0", + "get-intrinsic": "^1.1.3" + } + }, "array.prototype.flat": { "version": "1.3.1", "resolved": "https://registry.npmjs.org/array.prototype.flat/-/array.prototype.flat-1.3.1.tgz", @@ -25144,10 +25179,13 @@ "version": "file:scripts/solhint-custom" }, "solidity-ast": { - "version": "0.4.49", - "resolved": "https://registry.npmjs.org/solidity-ast/-/solidity-ast-0.4.49.tgz", - "integrity": "sha512-Pr5sCAj1SFqzwFZw1HPKSq0PehlQNdM8GwKyAVYh2DOn7/cCK8LUKD1HeHnKtTgBW7hi9h4nnnan7hpAg5RhWQ==", - "dev": true + "version": "0.4.50", + "resolved": "https://registry.npmjs.org/solidity-ast/-/solidity-ast-0.4.50.tgz", + "integrity": "sha512-WpIhaUibbjcBY4bg8TO2UXFWl8PQPhtH1QtMYJUqFUGxx0rRiEFsVLV+ow8XiWEnSPeu4xPp1/K43P4esxuK1Q==", + "dev": true, + "requires": { + "array.prototype.findlast": "^1.2.2" + } }, "solidity-comments": { "version": "0.0.2", diff --git a/package.json b/package.json index ffa868ac7..c71139f3e 100644 --- a/package.json +++ b/package.json @@ -85,7 +85,7 @@ "semver": "^7.3.5", "solhint": "^3.3.6", "solhint-plugin-openzeppelin": "file:scripts/solhint-custom", - "solidity-ast": "^0.4.25", + "solidity-ast": "^0.4.50", "solidity-coverage": "^0.8.0", "solidity-docgen": "^0.6.0-beta.29", "undici": "^5.22.1", diff --git a/scripts/upgradeable/transpile.sh b/scripts/upgradeable/transpile.sh index c0cb9ff5e..05de96d34 100644 --- a/scripts/upgradeable/transpile.sh +++ b/scripts/upgradeable/transpile.sh @@ -22,6 +22,8 @@ fi # -i: use included Initializable # -x: exclude proxy-related contracts with a few exceptions # -p: emit public initializer +# -n: use namespaces +# -N: exclude from namespaces transformation npx @openzeppelin/upgrade-safe-transpiler@latest -D \ -b "$build_info" \ -i contracts/proxy/utils/Initializable.sol \ @@ -32,7 +34,9 @@ npx @openzeppelin/upgrade-safe-transpiler@latest -D \ -x '!contracts/proxy/ERC1967/ERC1967Utils.sol' \ -x '!contracts/proxy/utils/UUPSUpgradeable.sol' \ -x '!contracts/proxy/beacon/IBeacon.sol' \ - -p 'contracts/**/presets/**/*' + -p 'contracts/**/presets/**/*' \ + -n \ + -N 'contracts/mocks/**/*' # delete compilation artifacts of vanilla code npm run clean