Merge branch 'master' into next-v5.0

.codecov.yml

@@ -6,6 +6,7 @@ coverage:
     patch:
       default:
         target: 95%
+        only_pulls: true
     project:
       default:
         threshold: 1%

.github/workflows/changelog.yml

@@ -0,0 +1,28 @@
+name: changelog
+
+on:
+  pull_request:
+    types:
+      - opened
+      - synchronize
+      - labeled
+      - unlabeled
+
+concurrency:
+  group: changelog-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  check:
+    runs-on: ubuntu-latest
+    if: ${{ !contains(github.event.pull_request.labels.*.name, 'ignore-changelog') }}
+    steps:
+      - uses: actions/checkout@v3
+      - name: Check diff
+        run: |
+          git fetch origin ${{ github.base_ref }} --depth=1
+          if git diff --exit-code origin/${{ github.base_ref }} -- CHANGELOG.md ; then
+            echo 'Missing changelog entry'
+            exit 1
+          fi
+

.github/workflows/checks.yml

@@ -14,19 +14,6 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
-  changelog:
-    if: github.event_name == 'pull_request' && github.repository != 'OpenZeppelin/openzeppelin-contracts-upgradeable'
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v3
-      - name: Check diff
-        run: |
-          git fetch origin ${{ github.base_ref }} --depth=1
-          if git diff --exit-code origin/${{ github.base_ref }} -- CHANGELOG.md ; then
-            echo 'Missing changelog entry'
-            exit 1
-          fi
-
   lint:
     if: github.repository != 'OpenZeppelin/openzeppelin-contracts-upgradeable'
     runs-on: ubuntu-latest
@@ -57,6 +44,20 @@ jobs:
         with:
           token: ${{ github.token }}
 
+  foundry-tests:
+    if: github.repository != 'OpenZeppelin/openzeppelin-contracts-upgradeable'
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+        with:
+          submodules: recursive
+      - name: Install Foundry
+        uses: foundry-rs/foundry-toolchain@v1
+        with:
+          version: nightly
+      - name: Run tests
+        run: forge test -vv
+
   coverage:
     if: github.repository != 'OpenZeppelin/openzeppelin-contracts-upgradeable'
     runs-on: ubuntu-latest
@@ -76,4 +77,15 @@ jobs:
       - uses: actions/checkout@v3
       - name: Set up environment
         uses: ./.github/actions/setup
-      - uses: crytic/slither-action@v0.1.1
+      - uses: crytic/slither-action@v0.2.0
+
+  codespell:
+    if: github.repository != 'OpenZeppelin/openzeppelin-contracts-upgradeable'
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - name: Run CodeSpell
+        uses: codespell-project/actions-codespell@v1.0
+        with:
+          check_filenames: true
+          skip: package-lock.json

.gitmodules

@@ -0,0 +1,3 @@
+[submodule "lib/forge-std"]
+	path = lib/forge-std
+	url = https://github.com/foundry-rs/forge-std

certora/specs/GovernorBase.spec

@@ -98,26 +98,26 @@ function helperFunctionsWithRevert(uint256 proposalId, method f, env e) {
  // To use env with general preserved block disable type checking [--disableLocalTypeChecking]
 invariant startAndEndDatesNonZero(uint256 pId)
         proposalSnapshot(pId) != 0 <=> proposalDeadline(pId) != 0
-        { preserved with (env e){   
+        { preserved with (env e){
                 require e.block.number > 0;
         }}
-        
+
 
 /*
- * If a proposal is canceled it must have a start and an end date 
+ * If a proposal is canceled it must have a start and an end date
  */
  // To use env with general preserved block disable type checking [--disableLocalTypeChecking]
 invariant canceledImplyStartAndEndDateNonZero(uint pId)
         isCanceled(pId) => proposalSnapshot(pId) != 0
-        {preserved with (env e){               
+        {preserved with (env e){
                 require e.block.number > 0;
         }}
 
 
 /*
- * If a proposal is executed it must have a start and an end date 
+ * If a proposal is executed it must have a start and an end date
  */
- // To use env with general preserved block disable type checking [--disableLocalTypeChecking] 
+ // To use env with general preserved block disable type checking [--disableLocalTypeChecking]
 invariant executedImplyStartAndEndDateNonZero(uint pId)
         isExecuted(pId) => proposalSnapshot(pId) != 0
         { preserved with (env e){
@@ -143,7 +143,7 @@ invariant voteStartBeforeVoteEnd(uint256 pId)
 /*
  * A proposal cannot be both executed and canceled simultaneously.
  */
-invariant noBothExecutedAndCanceled(uint256 pId) 
+invariant noBothExecutedAndCanceled(uint256 pId)
         !isExecuted(pId) || !isCanceled(pId)
 
 
@@ -154,10 +154,10 @@ rule executionOnlyIfQuoromReachedAndVoteSucceeded(uint256 pId, env e, method f){
     bool isExecutedBefore = isExecuted(pId);
     bool quorumReachedBefore = _quorumReached(e, pId);
     bool voteSucceededBefore = _voteSucceeded(pId);
-    
+
     calldataarg args;
     f(e, args);
-    
+
     bool isExecutedAfter = isExecuted(pId);
     assert (!isExecutedBefore && isExecutedAfter) => (quorumReachedBefore && voteSucceededBefore), "quorum was changed";
 }
@@ -177,16 +177,16 @@ rule executionOnlyIfQuoromReachedAndVoteSucceeded(uint256 pId, env e, method f){
  // the fact that the 3 functions themselves makes no changes, but rather call an internal function to execute.
  // That means that we do not check those 3 functions directly, however for castVote & castVoteWithReason it is quite trivial
  // to understand why this is ok. For castVoteBySig we basically assume that the signature referendum is correct without checking it.
- // We could check each function separately and pass the rule, but that would have uglyfied the code with no concrete 
+ // We could check each function separately and pass the rule, but that would have uglyfied the code with no concrete
  // benefit, as it is evident that nothing is happening in the first 2 functions (calling a view function), and we do not desire to check the signature verification.
 rule doubleVoting(uint256 pId, uint8 sup, method f) {
     env e;
-    address user = e.msg.sender;        
+    address user = e.msg.sender;
     bool votedCheck = hasVoted(e, pId, user);
 
     castVote@withrevert(e, pId, sup);
 
-    assert votedCheck => lastReverted, "double voting accured";
+    assert votedCheck => lastReverted, "double voting occurred";
 }
 
 
@@ -207,7 +207,7 @@ rule immutableFieldsAfterProposalCreation(uint256 pId, method f) {
     uint256 _voteEnd = proposalDeadline(pId);
 
     require proposalCreated(pId); // startDate > 0
-    
+
     env e; calldataarg arg;
     f(e, arg);
 
@@ -226,7 +226,7 @@ rule noStartBeforeCreation(uint256 pId) {
     // This line makes sure that we see only cases where start date is changed from 0, i.e. creation of proposal
     // We proved in immutableFieldsAfterProposalCreation that once dates set for proposal, it cannot be changed
     require !proposalCreated(pId); // previousStart == 0;
-    
+
     env e; calldataarg args;
     propose(e, args);
 
@@ -273,7 +273,7 @@ rule noExecuteOrCancelBeforeDeadline(uint256 pId, method f){
  * All proposal specific (non-view) functions should revert if proposal is executed
  */
  // In this rule we show that if a function is executed, i.e. execute() was called on the proposal ID,
- // non of the proposal specific functions can make changes again. In executedOnlyAfterExecuteFunc 
+ // non of the proposal specific functions can make changes again. In executedOnlyAfterExecuteFunc
  // we connected the executed attribute to the execute() function, showing that only execute() can
  // change it, and that it will always change it.
 rule allFunctionsRevertIfExecuted(method f) filtered { f ->
@@ -331,4 +331,3 @@ rule executedOnlyAfterExecuteFunc(address[] targets, uint256[] values, bytes[] c
     bool executedAfter = isExecuted(pId);
     assert(executedAfter != executedBefore => f.selector == execute(address[], uint256[], bytes[], bytes32).selector, "isExecuted only changes in the execute method");
 }
-

contracts/access/Ownable2Step.sol

@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: MIT
+// OpenZeppelin Contracts (last updated v4.7.0) (access/Ownable.sol)
+
+pragma solidity ^0.8.0;
+
+import "./Ownable.sol";
+
+/**
+ * @dev Contract module which provides access control mechanism, where
+ * there is an account (an owner) that can be granted exclusive access to
+ * specific functions.
+ *
+ * By default, the owner account will be the one that deploys the contract. This
+ * can later be changed with {transferOwnership} and {acceptOwnership}.
+ *
+ * This module is used through inheritance. It will make available all functions
+ * from parent (Ownable).
+ */
+abstract contract Ownable2Step is Ownable {
+    address private _pendingOwner;
+
+    event OwnershipTransferStarted(address indexed previousOwner, address indexed newOwner);
+
+    /**
+     * @dev Returns the address of the pending owner.
+     */
+    function pendingOwner() public view virtual returns (address) {
+        return _pendingOwner;
+    }
+
+    /**
+     * @dev Starts the ownership transfer of the contract to a new account. Replaces the pending transfer if there is one.
+     * Can only be called by the current owner.
+     */
+    function transferOwnership(address newOwner) public virtual override onlyOwner {
+        _pendingOwner = newOwner;
+        emit OwnershipTransferStarted(owner(), newOwner);
+    }
+
+    /**
+     * @dev Transfers ownership of the contract to a new account (`newOwner`) and deletes any pending owner.
+     * Internal function without access restriction.
+     */
+    function _transferOwnership(address newOwner) internal virtual override {
+        delete _pendingOwner;
+        super._transferOwnership(newOwner);
+    }
+
+    /**
+     * @dev The new owner accepts the ownership transfer.
+     */
+    function acceptOwnership() external {
+        address sender = _msgSender();
+        require(pendingOwner() == sender, "Ownable2Step: caller is not the new owner");
+        _transferOwnership(sender);
+    }
+}

contracts/crosschain/arbitrum/LibArbitrumL1.sol

@@ -4,7 +4,6 @@
 pragma solidity ^0.8.4;
 
 import {IBridge as ArbitrumL1_Bridge} from "../../vendor/arbitrum/IBridge.sol";
-import {IInbox as ArbitrumL1_Inbox} from "../../vendor/arbitrum/IInbox.sol";
 import {IOutbox as ArbitrumL1_Outbox} from "../../vendor/arbitrum/IOutbox.sol";
 import "../errors.sol";
 

contracts/finance/VestingWallet.sol

@@ -33,7 +33,7 @@ contract VestingWallet is Context {
         address beneficiaryAddress,
         uint64 startTimestamp,
         uint64 durationSeconds
-    ) {
+    ) payable {
         require(beneficiaryAddress != address(0), "VestingWallet: beneficiary is zero address");
         _beneficiary = beneficiaryAddress;
         _start = startTimestamp;

contracts/governance/Governor.sol

@@ -22,7 +22,7 @@ import "./IGovernor.sol";
  *
  * - A counting module must implement {quorum}, {_quorumReached}, {_voteSucceeded} and {_countVote}
  * - A voting module must implement {_getVotes}
- * - Additionanly, the {votingPeriod} must also be implemented
+ * - Additionally, the {votingPeriod} must also be implemented
  *
  * _Available since v4.3._
  */
@@ -544,8 +544,9 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
         address target,
         uint256 value,
         bytes calldata data
-    ) external virtual onlyGovernance {
-        Address.functionCallWithValue(target, data, value);
+    ) external payable virtual onlyGovernance {
+        (bool success, bytes memory returndata) = target.call{value: value}(data);
+        Address.verifyCallResult(success, returndata, "Governor: relay reverted without message");
     }
 
     /**

contracts/governance/IGovernor.sol

@@ -184,7 +184,7 @@ abstract contract IGovernor is IERC165 {
 
     /**
      * @notice module:voting
-     * @dev Returns weither `account` has cast a vote on `proposalId`.
+     * @dev Returns whether `account` has cast a vote on `proposalId`.
      */
     function hasVoted(uint256 proposalId, address account) public view virtual returns (bool);
 

contracts/governance/README.adoc

@@ -118,7 +118,7 @@ Both operations contain:
 
 * *Target*, the address of the smart contract that the timelock should operate on.
 * *Value*, in wei, that should be sent with the transaction. Most of the time this will be 0. Ether can be deposited before-end or passed along when executing the transaction.
-* *Data*, containing the encoded function selector and parameters of the call. This can be produced using a number of tools. For example, a maintenance operation granting role `ROLE` to `ACCOUNT` can be encode using web3js as follows:
+* *Data*, containing the encoded function selector and parameters of the call. This can be produced using a number of tools. For example, a maintenance operation granting role `ROLE` to `ACCOUNT` can be encoded using web3js as follows:
 
 ```javascript
 const data = timelock.contract.methods.grantRole(ROLE, ACCOUNT).encodeABI()
@@ -153,7 +153,7 @@ Operations status can be queried using the functions:
 [[timelock-admin]]
 ===== Admin
 
-The admins are in charge of managing proposers and executors. For the timelock to be self-governed, this role should only be given to the timelock itself. Upon deployment, both the timelock and the deployer have this role. After further configuration and testing, the deployer can renounce this role such that all further maintenance operations have to go through the timelock process.
+The admins are in charge of managing proposers and executors. For the timelock to be self-governed, this role should only be given to the timelock itself. Upon deployment, the admin role can be granted to any address (in addition to the timelock itself). After further configuration and testing, this optional admin should renounce its role such that all further maintenance operations have to go through the timelock process.
 
 This role is identified by the *TIMELOCK_ADMIN_ROLE* value: `0x5f58e3a2316349923ce3780f8d587db2d72378aed66a8261c916544fa6846ca5`
 

contracts/governance/TimelockController.sol

@@ -62,31 +62,37 @@ contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver
     event MinDelayChange(uint256 oldDuration, uint256 newDuration);
 
     /**
-     * @dev Initializes the contract with a given `minDelay`, and a list of
-     * initial proposers and executors. The proposers receive both the
-     * proposer and the canceller role (for backward compatibility). The
-     * executors receive the executor role.
+     * @dev Initializes the contract with the following parameters:
      *
-     * NOTE: At construction, both the deployer and the timelock itself are
-     * administrators. This helps further configuration of the timelock by the
-     * deployer. After configuration is done, it is recommended that the
-     * deployer renounces its admin position and relies on timelocked
-     * operations to perform future maintenance.
+     * - `minDelay`: initial minimum delay for operations
+     * - `proposers`: accounts to be granted proposer and canceller roles
+     * - `executors`: accounts to be granted executor role
+     * - `admin`: optional account to be granted admin role; disable with zero address
+     *
+     * IMPORTANT: The optional admin can aid with initial configuration of roles after deployment
+     * without being subject to delay, but this role should be subsequently renounced in favor of
+     * administration through timelocked proposals. Previous versions of this contract would assign
+     * this admin to the deployer automatically and should be renounced as well.
      */
     constructor(
         uint256 minDelay,
         address[] memory proposers,
-        address[] memory executors
+        address[] memory executors,
+        address admin
     ) {
         _setRoleAdmin(TIMELOCK_ADMIN_ROLE, TIMELOCK_ADMIN_ROLE);
         _setRoleAdmin(PROPOSER_ROLE, TIMELOCK_ADMIN_ROLE);
         _setRoleAdmin(EXECUTOR_ROLE, TIMELOCK_ADMIN_ROLE);
         _setRoleAdmin(CANCELLER_ROLE, TIMELOCK_ADMIN_ROLE);
 
-        // deployer + self administration
-        _setupRole(TIMELOCK_ADMIN_ROLE, _msgSender());
+        // self administration
         _setupRole(TIMELOCK_ADMIN_ROLE, address(this));
 
+        // optional admin
+        if (admin != address(0)) {
+            _setupRole(TIMELOCK_ADMIN_ROLE, admin);
+        }
+
         // register proposers and cancellers
         for (uint256 i = 0; i < proposers.length; ++i) {
             _setupRole(PROPOSER_ROLE, proposers[i]);
@@ -158,7 +164,7 @@ contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver
     }
 
     /**
-     * @dev Returns the timestamp at with an operation becomes ready (0 for
+     * @dev Returns the timestamp at which an operation becomes ready (0 for
      * unset operations, 1 for done operations).
      */
     function getTimestamp(bytes32 id) public view virtual returns (uint256 timestamp) {
@@ -252,7 +258,7 @@ contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver
     }
 
     /**
-     * @dev Schedule an operation that is to becomes valid after a given delay.
+     * @dev Schedule an operation that is to become valid after a given delay.
      */
     function _schedule(bytes32 id, uint256 delay) private {
         require(!isOperation(id), "TimelockController: operation already scheduled");