From 21e01e796672fd289778ab869479b9231f6fda19 Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Thu, 3 Aug 2023 22:40:22 -0600
Subject: [PATCH 1/9] Use Ownable2Step in VestingWallet with restricted
 `release()`

---
 .changeset/healthy-gorillas-applaud.md |  5 +++
 contracts/finance/VestingWallet.sol    | 37 ++++++++--------
 test/finance/VestingWallet.behavior.js | 60 +++++++++++++++++---------
 test/finance/VestingWallet.test.js     | 13 ++++--
 4 files changed, 72 insertions(+), 43 deletions(-)
 create mode 100644 .changeset/healthy-gorillas-applaud.md

diff --git a/.changeset/healthy-gorillas-applaud.md b/.changeset/healthy-gorillas-applaud.md
new file mode 100644
index 00000000000..a7619714caa
--- /dev/null
+++ b/.changeset/healthy-gorillas-applaud.md
@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': major
+---
+
+`VestingWallet`: Use Ownable2Step instead of an immutable `beneficiary`. The initial owner is set to the benefactor (`msg.sender`) but transferred to the beneficiary address so that unclaimed tokens can be recovered by the benefactor.
diff --git a/contracts/finance/VestingWallet.sol b/contracts/finance/VestingWallet.sol
index 840837d27e0..081aebd8fd6 100644
--- a/contracts/finance/VestingWallet.sol
+++ b/contracts/finance/VestingWallet.sol
@@ -6,12 +6,17 @@ import {IERC20} from "../token/ERC20/IERC20.sol";
 import {SafeERC20} from "../token/ERC20/utils/SafeERC20.sol";
 import {Address} from "../utils/Address.sol";
 import {Context} from "../utils/Context.sol";
+import {Ownable2Step, Ownable} from "../access/Ownable2Step.sol";
 
 /**
  * @title VestingWallet
- * @dev This contract handles the vesting of Eth and ERC20 tokens for a given beneficiary. Custody of multiple tokens
- * can be given to this contract, which will release the token to the beneficiary following a given vesting schedule.
- * The vesting schedule is customizable through the {vestedAmount} function.
+ * @dev Handles the vesting of native currency and ERC20 tokens for a given beneficiary who gets the ownership of the
+ * contract by calling {Ownable2Step-acceptOwnership} to accept a 2-step ownership transfer setup at deployment with
+ * {Ownable2Step-transferOwnership}. The initial owner of this contract is the benefactor (`msg.sender`), enabling them
+ * to recover any unclaimed tokens.
+ *
+ * Custody of multiple tokens can be given to this contract, which will release the tokens to the beneficiary following
+ * a given vesting schedule that is customizable through the {vestedAmount} function.
  *
  * Any token transferred to this contract will follow the vesting schedule as if they were locked from the beginning.
  * Consequently, if the vesting has already started, any amount of tokens sent to this contract will (at least partly)
@@ -20,7 +25,7 @@ import {Context} from "../utils/Context.sol";
  * By setting the duration to 0, one can configure this contract to behave like an asset timelock that hold tokens for
  * a beneficiary until a specified time.
  */
-contract VestingWallet is Context {
+contract VestingWallet is Context, Ownable2Step {
     event EtherReleased(uint256 amount);
     event ERC20Released(address indexed token, uint256 amount);
 
@@ -31,18 +36,19 @@ contract VestingWallet is Context {
 
     uint256 private _released;
     mapping(address => uint256) private _erc20Released;
-    address private immutable _beneficiary;
     uint64 private immutable _start;
     uint64 private immutable _duration;
 
     /**
-     * @dev Set the beneficiary, start timestamp and vesting duration of the vesting wallet.
+     * @dev Sets the sender as the initial owner, the beneficiary as the pending owner, the start timestamp and the
+     * vesting duration of the vesting wallet.
      */
-    constructor(address beneficiaryAddress, uint64 startTimestamp, uint64 durationSeconds) payable {
+    constructor(address beneficiaryAddress, uint64 startTimestamp, uint64 durationSeconds) payable Ownable(msg.sender) {
         if (beneficiaryAddress == address(0)) {
             revert VestingWalletInvalidBeneficiary(address(0));
         }
-        _beneficiary = beneficiaryAddress;
+        transferOwnership(beneficiaryAddress);
+
         _start = startTimestamp;
         _duration = durationSeconds;
     }
@@ -52,13 +58,6 @@ contract VestingWallet is Context {
      */
     receive() external payable virtual {}
 
-    /**
-     * @dev Getter for the beneficiary address.
-     */
-    function beneficiary() public view virtual returns (address) {
-        return _beneficiary;
-    }
-
     /**
      * @dev Getter for the start timestamp.
      */
@@ -114,11 +113,11 @@ contract VestingWallet is Context {
      *
      * Emits a {EtherReleased} event.
      */
-    function release() public virtual {
+    function release() public virtual onlyOwner {
         uint256 amount = releasable();
         _released += amount;
         emit EtherReleased(amount);
-        Address.sendValue(payable(beneficiary()), amount);
+        Address.sendValue(payable(owner()), amount);
     }
 
     /**
@@ -126,11 +125,11 @@ contract VestingWallet is Context {
      *
      * Emits a {ERC20Released} event.
      */
-    function release(address token) public virtual {
+    function release(address token) public virtual onlyOwner {
         uint256 amount = releasable(token);
         _erc20Released[token] += amount;
         emit ERC20Released(token, amount);
-        SafeERC20.safeTransfer(IERC20(token), beneficiary(), amount);
+        SafeERC20.safeTransfer(IERC20(token), owner(), amount);
     }
 
     /**
diff --git a/test/finance/VestingWallet.behavior.js b/test/finance/VestingWallet.behavior.js
index afd4c0495e5..987c276a07b 100644
--- a/test/finance/VestingWallet.behavior.js
+++ b/test/finance/VestingWallet.behavior.js
@@ -1,12 +1,14 @@
-const { time } = require('@nomicfoundation/hardhat-network-helpers');
+const { time, setNextBlockBaseFeePerGas } = require('@nomicfoundation/hardhat-network-helpers');
 const { expectEvent } = require('@openzeppelin/test-helpers');
 const { expect } = require('chai');
+const { web3 } = require('hardhat');
+const { expectRevertCustomError } = require('../helpers/customError');
 
 function releasedEvent(token, amount) {
   return token ? ['ERC20Released', { token: token.address, amount }] : ['EtherReleased', { amount }];
 }
 
-function shouldBehaveLikeVesting(beneficiary) {
+function shouldBehaveLikeVesting(beneficiary, accounts) {
   it('check vesting schedule', async function () {
     const [vestedAmount, releasable, ...args] = this.token
       ? ['vestedAmount(address,uint64)', 'releasable(address)', this.token.address]
@@ -22,35 +24,51 @@ function shouldBehaveLikeVesting(beneficiary) {
     }
   });
 
-  it('execute vesting schedule', async function () {
-    const [release, ...args] = this.token ? ['release(address)', this.token.address] : ['release()'];
+  describe('execute vesting schedule', async function () {
+    beforeEach(async function () {
+      [this.release, ...this.args] = this.token ? ['release(address)', this.token.address] : ['release()'];
+    });
 
-    let released = web3.utils.toBN(0);
-    const before = await this.getBalance(beneficiary);
+    it('releases a linearly vested schedule', async function () {
+      let released = web3.utils.toBN(0);
+      const before = await this.getBalance(beneficiary);
+      const releaser = await this.mock.owner();
 
-    {
-      const receipt = await this.mock.methods[release](...args);
+      {
+        // Allows gas price to be 0 so no ETH is spent in the transaction.
+        await setNextBlockBaseFeePerGas(0);
 
-      await expectEvent.inTransaction(receipt.tx, this.mock, ...releasedEvent(this.token, '0'));
+        const receipt = await this.mock.methods[this.release](...this.args, { from: releaser, gasPrice: 0 });
+        await expectEvent.inTransaction(receipt.tx, this.mock, ...releasedEvent(this.token, '0'));
+        await this.checkRelease(receipt, beneficiary, '0');
 
-      await this.checkRelease(receipt, beneficiary, '0');
+        expect(await this.getBalance(beneficiary)).to.be.bignumber.equal(before);
+      }
 
-      expect(await this.getBalance(beneficiary)).to.be.bignumber.equal(before);
-    }
+      for (const timestamp of this.schedule) {
+        await time.setNextBlockTimestamp(timestamp);
+        const vested = this.vestingFn(timestamp);
 
-    for (const timestamp of this.schedule) {
-      await time.setNextBlockTimestamp(timestamp);
-      const vested = this.vestingFn(timestamp);
+        // Allows gas price to be 0 so no ETH is spent in the transaction.
+        await setNextBlockBaseFeePerGas(0);
 
-      const receipt = await this.mock.methods[release](...args);
-      await expectEvent.inTransaction(receipt.tx, this.mock, ...releasedEvent(this.token, vested.sub(released)));
+        const receipt = await this.mock.methods[this.release](...this.args, { from: releaser, gasPrice: 0 });
+        await expectEvent.inTransaction(receipt.tx, this.mock, ...releasedEvent(this.token, vested.sub(released)));
+        await this.checkRelease(receipt, beneficiary, vested.sub(released));
 
-      await this.checkRelease(receipt, beneficiary, vested.sub(released));
+        expect(await this.getBalance(beneficiary)).to.be.bignumber.equal(before.add(vested));
 
-      expect(await this.getBalance(beneficiary)).to.be.bignumber.equal(before.add(vested));
+        released = vested;
+      }
+    });
 
-      released = vested;
-    }
+    it('cannot be released by a non releaser', async function () {
+      await expectRevertCustomError(
+        this.mock.methods[this.release](...this.args, { from: accounts[0] }),
+        'OwnableUnauthorizedAccount',
+        [accounts[0]],
+      );
+    });
   });
 }
 
diff --git a/test/finance/VestingWallet.test.js b/test/finance/VestingWallet.test.js
index 91ca04da06b..77aaccf83be 100644
--- a/test/finance/VestingWallet.test.js
+++ b/test/finance/VestingWallet.test.js
@@ -18,6 +18,7 @@ contract('VestingWallet', function (accounts) {
   beforeEach(async function () {
     this.start = (await time.latest()).addn(3600); // in 1 hour
     this.mock = await VestingWallet.new(beneficiary, this.start, duration);
+    await this.mock.acceptOwnership({ from: beneficiary });
   });
 
   it('rejects zero address for beneficiary', async function () {
@@ -28,8 +29,14 @@ contract('VestingWallet', function (accounts) {
     );
   });
 
+  it('sets the initial owner as the sender (benefactor)', async function () {
+    this.mock = await VestingWallet.new(beneficiary, this.start, duration, { from: sender });
+    expect(await this.mock.owner()).to.be.equal(sender);
+    expect(await this.mock.pendingOwner()).to.be.equal(beneficiary);
+  });
+
   it('check vesting contract', async function () {
-    expect(await this.mock.beneficiary()).to.be.equal(beneficiary);
+    expect(await this.mock.owner()).to.be.equal(beneficiary);
     expect(await this.mock.start()).to.be.bignumber.equal(this.start);
     expect(await this.mock.duration()).to.be.bignumber.equal(duration);
     expect(await this.mock.end()).to.be.bignumber.equal(this.start.add(duration));
@@ -50,7 +57,7 @@ contract('VestingWallet', function (accounts) {
         this.checkRelease = () => {};
       });
 
-      shouldBehaveLikeVesting(beneficiary);
+      shouldBehaveLikeVesting(beneficiary, accounts);
     });
 
     describe('ERC20 vesting', function () {
@@ -63,7 +70,7 @@ contract('VestingWallet', function (accounts) {
         await this.token.$_mint(this.mock.address, amount);
       });
 
-      shouldBehaveLikeVesting(beneficiary);
+      shouldBehaveLikeVesting(beneficiary, accounts);
     });
   });
 });

From 9059ff137afce131f6c0c980d4e38edfaa126b45 Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Thu, 3 Aug 2023 23:13:44 -0600
Subject: [PATCH 2/9] Fix Create2 tests

---
 test/utils/Create2.test.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/utils/Create2.test.js b/test/utils/Create2.test.js
index afbcc3db958..941ea4a2777 100644
--- a/test/utils/Create2.test.js
+++ b/test/utils/Create2.test.js
@@ -59,7 +59,7 @@ contract('Create2', function (accounts) {
         addr: offChainComputed,
       });
 
-      expect(await VestingWallet.at(offChainComputed).then(instance => instance.beneficiary())).to.be.equal(other);
+      expect(await VestingWallet.at(offChainComputed).then(instance => instance.owner())).to.be.equal(other);
     });
 
     it('deploys a contract with funds deposited in the factory', async function () {

From f73699b213ad4611042404e35365910786cd4184 Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Fri, 4 Aug 2023 02:05:35 -0600
Subject: [PATCH 3/9] Actually fix tests

---
 test/utils/Create2.test.js | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/test/utils/Create2.test.js b/test/utils/Create2.test.js
index 941ea4a2777..44aac343b1f 100644
--- a/test/utils/Create2.test.js
+++ b/test/utils/Create2.test.js
@@ -59,7 +59,12 @@ contract('Create2', function (accounts) {
         addr: offChainComputed,
       });
 
-      expect(await VestingWallet.at(offChainComputed).then(instance => instance.owner())).to.be.equal(other);
+      const instance = await VestingWallet.at(offChainComputed);
+
+      // Needs to be accepted
+      await instance.acceptOwnership({ from: other });
+
+      expect(await instance.owner()).to.be.equal(other);
     });
 
     it('deploys a contract with funds deposited in the factory', async function () {

From 42a454edd9eb6c0506d74bbe64ab3567993b3df2 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Fri, 4 Aug 2023 14:26:12 -0300
Subject: [PATCH 4/9] use Ownable instead of Ownable2Step and remove onlyOwner

---
 contracts/finance/VestingWallet.sol | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/contracts/finance/VestingWallet.sol b/contracts/finance/VestingWallet.sol
index 081aebd8fd6..51f4ad45c1a 100644
--- a/contracts/finance/VestingWallet.sol
+++ b/contracts/finance/VestingWallet.sol
@@ -6,26 +6,20 @@ import {IERC20} from "../token/ERC20/IERC20.sol";
 import {SafeERC20} from "../token/ERC20/utils/SafeERC20.sol";
 import {Address} from "../utils/Address.sol";
 import {Context} from "../utils/Context.sol";
-import {Ownable2Step, Ownable} from "../access/Ownable2Step.sol";
+import {Ownable} from "../access/Ownable.sol";
 
 /**
- * @title VestingWallet
- * @dev Handles the vesting of native currency and ERC20 tokens for a given beneficiary who gets the ownership of the
- * contract by calling {Ownable2Step-acceptOwnership} to accept a 2-step ownership transfer setup at deployment with
- * {Ownable2Step-transferOwnership}. The initial owner of this contract is the benefactor (`msg.sender`), enabling them
- * to recover any unclaimed tokens.
+ * @dev A vesting wallet is an ownable contract that can receive native currency and ERC20 tokens, and release these
+ * assets to the wallet owner, also referred to as "beneficiary", according to a vesting schedule.
  *
- * Custody of multiple tokens can be given to this contract, which will release the tokens to the beneficiary following
- * a given vesting schedule that is customizable through the {vestedAmount} function.
- *
- * Any token transferred to this contract will follow the vesting schedule as if they were locked from the beginning.
+ * Any assets transferred to this contract will follow the vesting schedule as if they were locked from the beginning.
  * Consequently, if the vesting has already started, any amount of tokens sent to this contract will (at least partly)
  * be immediately releasable.
  *
  * By setting the duration to 0, one can configure this contract to behave like an asset timelock that hold tokens for
  * a beneficiary until a specified time.
  */
-contract VestingWallet is Context, Ownable2Step {
+contract VestingWallet is Context, Ownable {
     event EtherReleased(uint256 amount);
     event ERC20Released(address indexed token, uint256 amount);
 
@@ -43,11 +37,10 @@ contract VestingWallet is Context, Ownable2Step {
      * @dev Sets the sender as the initial owner, the beneficiary as the pending owner, the start timestamp and the
      * vesting duration of the vesting wallet.
      */
-    constructor(address beneficiaryAddress, uint64 startTimestamp, uint64 durationSeconds) payable Ownable(msg.sender) {
-        if (beneficiaryAddress == address(0)) {
+    constructor(address beneficiary, uint64 startTimestamp, uint64 durationSeconds) payable Ownable(beneficiary) {
+        if (beneficiary == address(0)) {
             revert VestingWalletInvalidBeneficiary(address(0));
         }
-        transferOwnership(beneficiaryAddress);
 
         _start = startTimestamp;
         _duration = durationSeconds;
@@ -113,7 +106,7 @@ contract VestingWallet is Context, Ownable2Step {
      *
      * Emits a {EtherReleased} event.
      */
-    function release() public virtual onlyOwner {
+    function release() public virtual {
         uint256 amount = releasable();
         _released += amount;
         emit EtherReleased(amount);
@@ -125,7 +118,7 @@ contract VestingWallet is Context, Ownable2Step {
      *
      * Emits a {ERC20Released} event.
      */
-    function release(address token) public virtual onlyOwner {
+    function release(address token) public virtual {
         uint256 amount = releasable(token);
         _erc20Released[token] += amount;
         emit ERC20Released(token, amount);

From 4f56d4d68c6c52a14da765ba9526f4583ca537e3 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Fri, 4 Aug 2023 15:14:10 -0300
Subject: [PATCH 5/9] add docs about transferability

---
 contracts/finance/VestingWallet.sol | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/contracts/finance/VestingWallet.sol b/contracts/finance/VestingWallet.sol
index 51f4ad45c1a..a7e311181e4 100644
--- a/contracts/finance/VestingWallet.sol
+++ b/contracts/finance/VestingWallet.sol
@@ -18,6 +18,11 @@ import {Ownable} from "../access/Ownable.sol";
  *
  * By setting the duration to 0, one can configure this contract to behave like an asset timelock that hold tokens for
  * a beneficiary until a specified time.
+ *
+ * NOTE: Since the wallet is ownable, and ownership can be transferred, it is possible to sell unvested tokens.
+ * Preventing this in a smart contract is difficult, considering that: 1) a beneficiary address could be a
+ * counterfactually deployed contract, 2) there is likely to be a migration path for EOAs to become contracts in the
+ * near future.
  */
 contract VestingWallet is Context, Ownable {
     event EtherReleased(uint256 amount);

From 085aaf6646c8a2fa906de2a41d202732eadf20f2 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Fri, 4 Aug 2023 15:17:16 -0300
Subject: [PATCH 6/9] revert test changes

---
 test/finance/VestingWallet.behavior.js | 60 +++++++++-----------------
 test/finance/VestingWallet.test.js     | 11 +----
 2 files changed, 23 insertions(+), 48 deletions(-)

diff --git a/test/finance/VestingWallet.behavior.js b/test/finance/VestingWallet.behavior.js
index 987c276a07b..afd4c0495e5 100644
--- a/test/finance/VestingWallet.behavior.js
+++ b/test/finance/VestingWallet.behavior.js
@@ -1,14 +1,12 @@
-const { time, setNextBlockBaseFeePerGas } = require('@nomicfoundation/hardhat-network-helpers');
+const { time } = require('@nomicfoundation/hardhat-network-helpers');
 const { expectEvent } = require('@openzeppelin/test-helpers');
 const { expect } = require('chai');
-const { web3 } = require('hardhat');
-const { expectRevertCustomError } = require('../helpers/customError');
 
 function releasedEvent(token, amount) {
   return token ? ['ERC20Released', { token: token.address, amount }] : ['EtherReleased', { amount }];
 }
 
-function shouldBehaveLikeVesting(beneficiary, accounts) {
+function shouldBehaveLikeVesting(beneficiary) {
   it('check vesting schedule', async function () {
     const [vestedAmount, releasable, ...args] = this.token
       ? ['vestedAmount(address,uint64)', 'releasable(address)', this.token.address]
@@ -24,51 +22,35 @@ function shouldBehaveLikeVesting(beneficiary, accounts) {
     }
   });
 
-  describe('execute vesting schedule', async function () {
-    beforeEach(async function () {
-      [this.release, ...this.args] = this.token ? ['release(address)', this.token.address] : ['release()'];
-    });
+  it('execute vesting schedule', async function () {
+    const [release, ...args] = this.token ? ['release(address)', this.token.address] : ['release()'];
 
-    it('releases a linearly vested schedule', async function () {
-      let released = web3.utils.toBN(0);
-      const before = await this.getBalance(beneficiary);
-      const releaser = await this.mock.owner();
+    let released = web3.utils.toBN(0);
+    const before = await this.getBalance(beneficiary);
 
-      {
-        // Allows gas price to be 0 so no ETH is spent in the transaction.
-        await setNextBlockBaseFeePerGas(0);
+    {
+      const receipt = await this.mock.methods[release](...args);
 
-        const receipt = await this.mock.methods[this.release](...this.args, { from: releaser, gasPrice: 0 });
-        await expectEvent.inTransaction(receipt.tx, this.mock, ...releasedEvent(this.token, '0'));
-        await this.checkRelease(receipt, beneficiary, '0');
+      await expectEvent.inTransaction(receipt.tx, this.mock, ...releasedEvent(this.token, '0'));
 
-        expect(await this.getBalance(beneficiary)).to.be.bignumber.equal(before);
-      }
+      await this.checkRelease(receipt, beneficiary, '0');
 
-      for (const timestamp of this.schedule) {
-        await time.setNextBlockTimestamp(timestamp);
-        const vested = this.vestingFn(timestamp);
+      expect(await this.getBalance(beneficiary)).to.be.bignumber.equal(before);
+    }
 
-        // Allows gas price to be 0 so no ETH is spent in the transaction.
-        await setNextBlockBaseFeePerGas(0);
+    for (const timestamp of this.schedule) {
+      await time.setNextBlockTimestamp(timestamp);
+      const vested = this.vestingFn(timestamp);
 
-        const receipt = await this.mock.methods[this.release](...this.args, { from: releaser, gasPrice: 0 });
-        await expectEvent.inTransaction(receipt.tx, this.mock, ...releasedEvent(this.token, vested.sub(released)));
-        await this.checkRelease(receipt, beneficiary, vested.sub(released));
+      const receipt = await this.mock.methods[release](...args);
+      await expectEvent.inTransaction(receipt.tx, this.mock, ...releasedEvent(this.token, vested.sub(released)));
 
-        expect(await this.getBalance(beneficiary)).to.be.bignumber.equal(before.add(vested));
+      await this.checkRelease(receipt, beneficiary, vested.sub(released));
 
-        released = vested;
-      }
-    });
+      expect(await this.getBalance(beneficiary)).to.be.bignumber.equal(before.add(vested));
 
-    it('cannot be released by a non releaser', async function () {
-      await expectRevertCustomError(
-        this.mock.methods[this.release](...this.args, { from: accounts[0] }),
-        'OwnableUnauthorizedAccount',
-        [accounts[0]],
-      );
-    });
+      released = vested;
+    }
   });
 }
 
diff --git a/test/finance/VestingWallet.test.js b/test/finance/VestingWallet.test.js
index 77aaccf83be..d79aea1955a 100644
--- a/test/finance/VestingWallet.test.js
+++ b/test/finance/VestingWallet.test.js
@@ -18,7 +18,6 @@ contract('VestingWallet', function (accounts) {
   beforeEach(async function () {
     this.start = (await time.latest()).addn(3600); // in 1 hour
     this.mock = await VestingWallet.new(beneficiary, this.start, duration);
-    await this.mock.acceptOwnership({ from: beneficiary });
   });
 
   it('rejects zero address for beneficiary', async function () {
@@ -29,12 +28,6 @@ contract('VestingWallet', function (accounts) {
     );
   });
 
-  it('sets the initial owner as the sender (benefactor)', async function () {
-    this.mock = await VestingWallet.new(beneficiary, this.start, duration, { from: sender });
-    expect(await this.mock.owner()).to.be.equal(sender);
-    expect(await this.mock.pendingOwner()).to.be.equal(beneficiary);
-  });
-
   it('check vesting contract', async function () {
     expect(await this.mock.owner()).to.be.equal(beneficiary);
     expect(await this.mock.start()).to.be.bignumber.equal(this.start);
@@ -57,7 +50,7 @@ contract('VestingWallet', function (accounts) {
         this.checkRelease = () => {};
       });
 
-      shouldBehaveLikeVesting(beneficiary, accounts);
+      shouldBehaveLikeVesting(beneficiary);
     });
 
     describe('ERC20 vesting', function () {
@@ -70,7 +63,7 @@ contract('VestingWallet', function (accounts) {
         await this.token.$_mint(this.mock.address, amount);
       });
 
-      shouldBehaveLikeVesting(beneficiary, accounts);
+      shouldBehaveLikeVesting(beneficiary);
     });
   });
 });

From 21bc8b4ff4ccceca2aa105fa94f644da0ccd4e17 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Fri, 4 Aug 2023 17:43:26 -0300
Subject: [PATCH 7/9] revert Create2 test

---
 test/utils/Create2.test.js | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/test/utils/Create2.test.js b/test/utils/Create2.test.js
index 44aac343b1f..2f66c155b5f 100644
--- a/test/utils/Create2.test.js
+++ b/test/utils/Create2.test.js
@@ -61,9 +61,6 @@ contract('Create2', function (accounts) {
 
       const instance = await VestingWallet.at(offChainComputed);
 
-      // Needs to be accepted
-      await instance.acceptOwnership({ from: other });
-
       expect(await instance.owner()).to.be.equal(other);
     });
 

From ecd30f6972e66cca429e5d038f5a4a63c2f20b0b Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Fri, 4 Aug 2023 17:43:30 -0300
Subject: [PATCH 8/9] adjust changeset

---
 .changeset/healthy-gorillas-applaud.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.changeset/healthy-gorillas-applaud.md b/.changeset/healthy-gorillas-applaud.md
index a7619714caa..1d4156ebfae 100644
--- a/.changeset/healthy-gorillas-applaud.md
+++ b/.changeset/healthy-gorillas-applaud.md
@@ -2,4 +2,4 @@
 'openzeppelin-solidity': major
 ---
 
-`VestingWallet`: Use Ownable2Step instead of an immutable `beneficiary`. The initial owner is set to the benefactor (`msg.sender`) but transferred to the beneficiary address so that unclaimed tokens can be recovered by the benefactor.
+`VestingWallet`: Use `Ownable` instead of an immutable `beneficiary`.

From 4ce81640c4935c26e44e883d35d48b7921542d28 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= <ernestognw@gmail.com>
Date: Fri, 4 Aug 2023 15:06:15 -0600
Subject: [PATCH 9/9] Update contracts/finance/VestingWallet.sol

---
 contracts/finance/VestingWallet.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contracts/finance/VestingWallet.sol b/contracts/finance/VestingWallet.sol
index abe7472714d..1edb0113ef0 100644
--- a/contracts/finance/VestingWallet.sol
+++ b/contracts/finance/VestingWallet.sol
@@ -19,7 +19,7 @@ import {Ownable} from "../access/Ownable.sol";
  * By setting the duration to 0, one can configure this contract to behave like an asset timelock that hold tokens for
  * a beneficiary until a specified time.
  *
- * NOTE: Since the wallet is ownable, and ownership can be transferred, it is possible to sell unvested tokens.
+ * NOTE: Since the wallet is {Ownable}, and ownership can be transferred, it is possible to sell unvested tokens.
  * Preventing this in a smart contract is difficult, considering that: 1) a beneficiary address could be a
  * counterfactually deployed contract, 2) there is likely to be a migration path for EOAs to become contracts in the
  * near future.