Skip to content

Commit 0d665b1

Browse files
Transpile dd04dfe75
1 parent 076d900 commit 0d665b1

29 files changed

+507
-110
lines changed

.changeset/brave-islands-sparkle.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'openzeppelin-solidity': minor
3+
---
4+
5+
`GovernorSequentialProposalId`: Adds a `Governor` extension that sequentially numbers proposal ids instead of using the hash.

.changeset/cyan-taxis-travel.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'openzeppelin-solidity': minor
3+
---
4+
5+
`Address`: bubble up revert data on `sendValue` failed call

.changeset/long-walls-draw.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'openzeppelin-solidity': minor
3+
---
4+
5+
`IERC6909`: Add the interface for ERC-6909.

.changeset/ten-fishes-fold.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'openzeppelin-solidity': minor
3+
---
4+
5+
`IGovernor`: Add the `getProposalId` function to the governor interface.

.changeset/thin-eels-cross.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'openzeppelin-solidity': patch
3+
---
4+
5+
`ERC4626`: Use the `asset` getter in `totalAssets`, `_deposit` and `_withdraw`.

.github/workflows/checks.yml

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ jobs:
9797
uses: ./.github/actions/setup
9898
- name: Run coverage
9999
run: npm run coverage
100-
- uses: codecov/codecov-action@v4
100+
- uses: codecov/codecov-action@v5
101101
env:
102102
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
103103

@@ -118,11 +118,7 @@ jobs:
118118
- uses: actions/checkout@v4
119119
- name: Set up environment
120120
uses: ./.github/actions/setup
121-
- run: rm foundry.toml
122121
- uses: crytic/[email protected]
123-
with:
124-
node-version: 18.15
125-
slither-version: 0.10.1
126122

127123
codespell:
128124
runs-on: ubuntu-latest

.github/workflows/formal-verification.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ jobs:
5252
- name: Install python packages
5353
run: pip install -r fv-requirements.txt
5454
- name: Install java
55-
uses: actions/setup-java@v3
55+
uses: actions/setup-java@v4
5656
with:
5757
distribution: temurin
5858
java-version: ${{ env.JAVA_VERSION }}

contracts/governance/GovernorUpgradeable.sol

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ abstract contract GovernorUpgradeable is Initializable, ContextUpgradeable, ERC1
113113
function supportsInterface(bytes4 interfaceId) public view virtual override(IERC165, ERC165Upgradeable) returns (bool) {
114114
return
115115
interfaceId == type(IGovernor).interfaceId ||
116+
interfaceId == type(IGovernor).interfaceId ^ IGovernor.getProposalId.selector ||
116117
interfaceId == type(IERC1155Receiver).interfaceId ||
117118
super.supportsInterface(interfaceId);
118119
}
@@ -154,6 +155,18 @@ abstract contract GovernorUpgradeable is Initializable, ContextUpgradeable, ERC1
154155
return uint256(keccak256(abi.encode(targets, values, calldatas, descriptionHash)));
155156
}
156157

158+
/**
159+
* @dev See {IGovernor-getProposalId}.
160+
*/
161+
function getProposalId(
162+
address[] memory targets,
163+
uint256[] memory values,
164+
bytes[] memory calldatas,
165+
bytes32 descriptionHash
166+
) public view virtual returns (uint256) {
167+
return hashProposal(targets, values, calldatas, descriptionHash);
168+
}
169+
157170
/**
158171
* @dev See {IGovernor-state}.
159172
*/
@@ -346,7 +359,7 @@ abstract contract GovernorUpgradeable is Initializable, ContextUpgradeable, ERC1
346359
address proposer
347360
) internal virtual returns (uint256 proposalId) {
348361
GovernorStorage storage $ = _getGovernorStorage();
349-
proposalId = hashProposal(targets, values, calldatas, keccak256(bytes(description)));
362+
proposalId = getProposalId(targets, values, calldatas, keccak256(bytes(description)));
350363

351364
if (targets.length != values.length || targets.length != calldatas.length || targets.length == 0) {
352365
revert GovernorInvalidProposalLength(targets.length, calldatas.length, values.length);
@@ -388,7 +401,7 @@ abstract contract GovernorUpgradeable is Initializable, ContextUpgradeable, ERC1
388401
bytes32 descriptionHash
389402
) public virtual returns (uint256) {
390403
GovernorStorage storage $ = _getGovernorStorage();
391-
uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
404+
uint256 proposalId = getProposalId(targets, values, calldatas, descriptionHash);
392405

393406
_validateStateBitmap(proposalId, _encodeStateBitmap(ProposalState.Succeeded));
394407

@@ -437,7 +450,7 @@ abstract contract GovernorUpgradeable is Initializable, ContextUpgradeable, ERC1
437450
bytes32 descriptionHash
438451
) public payable virtual returns (uint256) {
439452
GovernorStorage storage $ = _getGovernorStorage();
440-
uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
453+
uint256 proposalId = getProposalId(targets, values, calldatas, descriptionHash);
441454

442455
_validateStateBitmap(
443456
proposalId,
@@ -499,8 +512,8 @@ abstract contract GovernorUpgradeable is Initializable, ContextUpgradeable, ERC1
499512
) public virtual returns (uint256) {
500513
// The proposalId will be recomputed in the `_cancel` call further down. However we need the value before we
501514
// do the internal call, because we need to check the proposal state BEFORE the internal `_cancel` call
502-
// changes it. The `hashProposal` duplication has a cost that is limited, and that we accept.
503-
uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
515+
// changes it. The `getProposalId` duplication has a cost that is limited, and that we accept.
516+
uint256 proposalId = getProposalId(targets, values, calldatas, descriptionHash);
504517

505518
// public cancel restrictions (on top of existing _cancel restrictions).
506519
_validateStateBitmap(proposalId, _encodeStateBitmap(ProposalState.Pending));
@@ -524,7 +537,7 @@ abstract contract GovernorUpgradeable is Initializable, ContextUpgradeable, ERC1
524537
bytes32 descriptionHash
525538
) internal virtual returns (uint256) {
526539
GovernorStorage storage $ = _getGovernorStorage();
527-
uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
540+
uint256 proposalId = getProposalId(targets, values, calldatas, descriptionHash);
528541

529542
_validateStateBitmap(
530543
proposalId,
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
// SPDX-License-Identifier: MIT
2+
3+
pragma solidity ^0.8.20;
4+
5+
import {GovernorUpgradeable} from "../GovernorUpgradeable.sol";
6+
import {Initializable} from "../../proxy/utils/Initializable.sol";
7+
8+
/**
9+
* @dev Extension of {Governor} that changes the numbering of proposal ids from the default hash-based approach to
10+
* sequential ids.
11+
*/
12+
abstract contract GovernorSequentialProposalIdUpgradeable is Initializable, GovernorUpgradeable {
13+
/// @custom:storage-location erc7201:openzeppelin.storage.GovernorSequentialProposalId
14+
struct GovernorSequentialProposalIdStorage {
15+
uint256 _latestProposalId;
16+
mapping(uint256 proposalHash => uint256 proposalId) _proposalIds;
17+
}
18+
19+
// keccak256(abi.encode(uint256(keccak256("openzeppelin.storage.GovernorSequentialProposalId")) - 1)) & ~bytes32(uint256(0xff))
20+
bytes32 private constant GovernorSequentialProposalIdStorageLocation = 0x4b8c47b641115bbb755a0530712d89d8042b41728d36570a6119c90ae1b76800;
21+
22+
function _getGovernorSequentialProposalIdStorage() private pure returns (GovernorSequentialProposalIdStorage storage $) {
23+
assembly {
24+
$.slot := GovernorSequentialProposalIdStorageLocation
25+
}
26+
}
27+
28+
/**
29+
* @dev The {latestProposalId} may only be initialized if it hasn't been set yet
30+
* (through initialization or the creation of a proposal).
31+
*/
32+
error GovernorAlreadyInitializedLatestProposalId();
33+
34+
function __GovernorSequentialProposalId_init() internal onlyInitializing {
35+
}
36+
37+
function __GovernorSequentialProposalId_init_unchained() internal onlyInitializing {
38+
}
39+
/**
40+
* @dev See {IGovernor-getProposalId}.
41+
*/
42+
function getProposalId(
43+
address[] memory targets,
44+
uint256[] memory values,
45+
bytes[] memory calldatas,
46+
bytes32 descriptionHash
47+
) public view virtual override returns (uint256) {
48+
GovernorSequentialProposalIdStorage storage $ = _getGovernorSequentialProposalIdStorage();
49+
uint256 proposalHash = hashProposal(targets, values, calldatas, descriptionHash);
50+
uint256 storedProposalId = $._proposalIds[proposalHash];
51+
if (storedProposalId == 0) {
52+
revert GovernorNonexistentProposal(0);
53+
}
54+
return storedProposalId;
55+
}
56+
57+
/**
58+
* @dev Returns the latest proposal id. A return value of 0 means no proposals have been created yet.
59+
*/
60+
function latestProposalId() public view virtual returns (uint256) {
61+
GovernorSequentialProposalIdStorage storage $ = _getGovernorSequentialProposalIdStorage();
62+
return $._latestProposalId;
63+
}
64+
65+
/**
66+
* @dev See {IGovernor-_propose}.
67+
* Hook into the proposing mechanism to increment proposal count.
68+
*/
69+
function _propose(
70+
address[] memory targets,
71+
uint256[] memory values,
72+
bytes[] memory calldatas,
73+
string memory description,
74+
address proposer
75+
) internal virtual override returns (uint256) {
76+
GovernorSequentialProposalIdStorage storage $ = _getGovernorSequentialProposalIdStorage();
77+
uint256 proposalHash = hashProposal(targets, values, calldatas, keccak256(bytes(description)));
78+
uint256 storedProposalId = $._proposalIds[proposalHash];
79+
if (storedProposalId == 0) {
80+
$._proposalIds[proposalHash] = ++$._latestProposalId;
81+
}
82+
return super._propose(targets, values, calldatas, description, proposer);
83+
}
84+
85+
/**
86+
* @dev Internal function to set the {latestProposalId}. This function is helpful when transitioning
87+
* from another governance system. The next proposal id will be `newLatestProposalId` + 1.
88+
*
89+
* May only call this function if the current value of {latestProposalId} is 0.
90+
*/
91+
function _initializeLatestProposalId(uint256 newLatestProposalId) internal virtual {
92+
GovernorSequentialProposalIdStorage storage $ = _getGovernorSequentialProposalIdStorage();
93+
if ($._latestProposalId != 0) {
94+
revert GovernorAlreadyInitializedLatestProposalId();
95+
}
96+
$._latestProposalId = newLatestProposalId;
97+
}
98+
}

contracts/mocks/WithInit.sol

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,13 @@ contract GovernorPreventLateQuorumMockUpgradeableWithInit is GovernorPreventLate
490490
__GovernorPreventLateQuorumMock_init(quorum_);
491491
}
492492
}
493+
import "./governance/GovernorSequentialProposalIdMockUpgradeable.sol";
494+
495+
contract GovernorSequentialProposalIdMockUpgradeableWithInit is GovernorSequentialProposalIdMockUpgradeable {
496+
constructor() payable initializer {
497+
__GovernorSequentialProposalIdMock_init();
498+
}
499+
}
493500
import "./governance/GovernorStorageMockUpgradeable.sol";
494501

495502
contract GovernorStorageMockUpgradeableWithInit is GovernorStorageMockUpgradeable {
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// SPDX-License-Identifier: MIT
2+
3+
pragma solidity ^0.8.20;
4+
5+
import {GovernorUpgradeable} from "../../governance/GovernorUpgradeable.sol";
6+
import {GovernorSettingsUpgradeable} from "../../governance/extensions/GovernorSettingsUpgradeable.sol";
7+
import {GovernorCountingSimpleUpgradeable} from "../../governance/extensions/GovernorCountingSimpleUpgradeable.sol";
8+
import {GovernorVotesQuorumFractionUpgradeable} from "../../governance/extensions/GovernorVotesQuorumFractionUpgradeable.sol";
9+
import {GovernorSequentialProposalIdUpgradeable} from "../../governance/extensions/GovernorSequentialProposalIdUpgradeable.sol";
10+
import {Initializable} from "../../proxy/utils/Initializable.sol";
11+
12+
abstract contract GovernorSequentialProposalIdMockUpgradeable is
13+
Initializable, GovernorSettingsUpgradeable,
14+
GovernorVotesQuorumFractionUpgradeable,
15+
GovernorCountingSimpleUpgradeable,
16+
GovernorSequentialProposalIdUpgradeable
17+
{
18+
function __GovernorSequentialProposalIdMock_init() internal onlyInitializing {
19+
}
20+
21+
function __GovernorSequentialProposalIdMock_init_unchained() internal onlyInitializing {
22+
}
23+
function proposalThreshold() public view override(GovernorUpgradeable, GovernorSettingsUpgradeable) returns (uint256) {
24+
return super.proposalThreshold();
25+
}
26+
27+
function getProposalId(
28+
address[] memory targets,
29+
uint256[] memory values,
30+
bytes[] memory calldatas,
31+
bytes32 descriptionHash
32+
) public view virtual override(GovernorUpgradeable, GovernorSequentialProposalIdUpgradeable) returns (uint256) {
33+
return super.getProposalId(targets, values, calldatas, descriptionHash);
34+
}
35+
36+
function _propose(
37+
address[] memory targets,
38+
uint256[] memory values,
39+
bytes[] memory calldatas,
40+
string memory description,
41+
address proposer
42+
) internal virtual override(GovernorUpgradeable, GovernorSequentialProposalIdUpgradeable) returns (uint256 proposalId) {
43+
return super._propose(targets, values, calldatas, description, proposer);
44+
}
45+
}

contracts/token/ERC20/extensions/ERC4626Upgradeable.sol

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,7 @@ abstract contract ERC4626Upgradeable is Initializable, ERC20Upgradeable, IERC462
136136

137137
/** @dev See {IERC4626-totalAssets}. */
138138
function totalAssets() public view virtual returns (uint256) {
139-
ERC4626Storage storage $ = _getERC4626Storage();
140-
return $._asset.balanceOf(address(this));
139+
return IERC20(asset()).balanceOf(address(this));
141140
}
142141

143142
/** @dev See {IERC4626-convertToShares}. */
@@ -260,15 +259,14 @@ abstract contract ERC4626Upgradeable is Initializable, ERC20Upgradeable, IERC462
260259
* @dev Deposit/mint common workflow.
261260
*/
262261
function _deposit(address caller, address receiver, uint256 assets, uint256 shares) internal virtual {
263-
ERC4626Storage storage $ = _getERC4626Storage();
264-
// If _asset is ERC-777, `transferFrom` can trigger a reentrancy BEFORE the transfer happens through the
262+
// If asset() is ERC-777, `transferFrom` can trigger a reentrancy BEFORE the transfer happens through the
265263
// `tokensToSend` hook. On the other hand, the `tokenReceived` hook, that is triggered after the transfer,
266264
// calls the vault, which is assumed not malicious.
267265
//
268266
// Conclusion: we need to do the transfer before we mint so that any reentrancy would happen before the
269267
// assets are transferred and before the shares are minted, which is a valid state.
270268
// slither-disable-next-line reentrancy-no-eth
271-
SafeERC20.safeTransferFrom($._asset, caller, address(this), assets);
269+
SafeERC20.safeTransferFrom(IERC20(asset()), caller, address(this), assets);
272270
_mint(receiver, shares);
273271

274272
emit Deposit(caller, receiver, assets, shares);
@@ -284,19 +282,18 @@ abstract contract ERC4626Upgradeable is Initializable, ERC20Upgradeable, IERC462
284282
uint256 assets,
285283
uint256 shares
286284
) internal virtual {
287-
ERC4626Storage storage $ = _getERC4626Storage();
288285
if (caller != owner) {
289286
_spendAllowance(owner, caller, shares);
290287
}
291288

292-
// If _asset is ERC-777, `transfer` can trigger a reentrancy AFTER the transfer happens through the
289+
// If asset() is ERC-777, `transfer` can trigger a reentrancy AFTER the transfer happens through the
293290
// `tokensReceived` hook. On the other hand, the `tokensToSend` hook, that is triggered before the transfer,
294291
// calls the vault, which is assumed not malicious.
295292
//
296293
// Conclusion: we need to do the transfer after the burn so that any reentrancy would happen after the
297294
// shares are burned and after the assets are transferred, which is a valid state.
298295
_burn(owner, shares);
299-
SafeERC20.safeTransfer($._asset, receiver, assets);
296+
SafeERC20.safeTransfer(IERC20(asset()), receiver, assets);
300297

301298
emit Withdraw(caller, receiver, owner, assets, shares);
302299
}

docs/modules/ROOT/pages/access-control.adoc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ WARNING: Removing the owner altogether will mean that administrative tasks that
2424

2525
Ownable is a simple and effective way to implement access control, but you should be mindful of the dangers associated with transferring the ownership to an incorrect account that can't interact with this contract anymore. An alternative to this problem is using xref:api:access.adoc#Ownable2Step[`Ownable2Step`]; a variant of Ownable that requires the new owner to explicitly accept the ownership transfer by calling xref:api:access.adoc#Ownable2Step-acceptOwnership--[`acceptOwnership`].
2626

27-
Note that *a contract can also be the owner of another one*! This opens the door to using, for example, a https://gnosis-safe.io[Gnosis Safe], an https://aragon.org[Aragon DAO], or a totally custom contract that _you_ create.
27+
Note that *a contract can also be the owner of another one*! This opens the door to using, for example, a https://safe.global[Gnosis Safe], an https://aragon.org[Aragon DAO], or a totally custom contract that _you_ create.
2828

2929
In this way, you can use _composability_ to add additional layers of access control complexity to your contracts. Instead of having a single regular Ethereum account (Externally Owned Account, or EOA) as the owner, you could use a 2-of-3 multisig run by your project leads, for example. Prominent projects in the space, such as https://makerdao.com[MakerDAO], use systems similar to this one.
3030

docs/modules/ROOT/pages/index.adoc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ The guides in the sidebar will teach about different concepts, and how to use th
6363

6464
The xref:api:token/ERC20.adoc[full API] is also thoroughly documented, and serves as a great reference when developing your smart contract application. You can also ask for help or follow Contracts' development in the https://forum.openzeppelin.com[community forum].
6565

66-
Finally, you may want to take a look at the https://blog.openzeppelin.com/guides/[guides on our blog], which cover several common use cases and good practices. The following articles provide great background reading, though please note, some of the referenced tools have changed as the tooling in the ecosystem continues to rapidly evolve.
66+
The following articles provide great background reading, though please note, some of the referenced tools have changed as the tooling in the ecosystem continues to rapidly evolve.
6767

6868
* https://blog.openzeppelin.com/the-hitchhikers-guide-to-smart-contracts-in-ethereum-848f08001f05[The Hitchhiker’s Guide to Smart Contracts in Ethereum] will help you get an overview of the various tools available for smart contract development, and help you set up your environment.
6969
* https://blog.openzeppelin.com/a-gentle-introduction-to-ethereum-programming-part-1-783cc7796094[A Gentle Introduction to Ethereum Programming, Part 1] provides very useful information on an introductory level, including many basic concepts from the Ethereum platform.

fv-requirements.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
certora-cli==4.13.1
22
# File uses a custom name (fv-requirements.txt) so that it isn't picked by Netlify's build
33
# whose latest Python version is 0.3.8, incompatible with most recent versions of Halmos
4-
halmos==0.2.0
4+
halmos==0.2.3

hardhat/common-contracts.js

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
const { task } = require('hardhat/config');
2+
const { TASK_TEST_SETUP_TEST_ENVIRONMENT } = require('hardhat/builtin-tasks/task-names');
3+
const { setCode } = require('@nomicfoundation/hardhat-network-helpers');
4+
5+
const fs = require('fs');
6+
const path = require('path');
7+
8+
const INSTANCES = {
9+
entrypoint: {
10+
address: '0x0000000071727De22E5E9d8BAf0edAc6f37da032',
11+
abi: JSON.parse(fs.readFileSync(path.resolve(__dirname, '../test/bin/EntryPoint070.abi'), 'utf-8')),
12+
bytecode: fs.readFileSync(path.resolve(__dirname, '../test/bin/EntryPoint070.bytecode'), 'hex'),
13+
},
14+
senderCreator: {
15+
address: '0xEFC2c1444eBCC4Db75e7613d20C6a62fF67A167C',
16+
abi: JSON.parse(fs.readFileSync(path.resolve(__dirname, '../test/bin/SenderCreator070.abi'), 'utf-8')),
17+
bytecode: fs.readFileSync(path.resolve(__dirname, '../test/bin/SenderCreator070.bytecode'), 'hex'),
18+
},
19+
};
20+
21+
task(TASK_TEST_SETUP_TEST_ENVIRONMENT).setAction((_, env, runSuper) =>
22+
runSuper().then(() =>
23+
Promise.all(
24+
Object.entries(INSTANCES).map(([name, { address, abi, bytecode }]) =>
25+
setCode(address, '0x' + bytecode.replace(/0x/, '')).then(() =>
26+
env.ethers.getContractAt(abi, address).then(instance => (env[name] = instance)),
27+
),
28+
),
29+
),
30+
),
31+
);

0 commit comments

Comments
 (0)