Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Ownable2Step extension with 2-step transfer #3620

Merged
merged 38 commits into from Sep 1, 2022
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
fccb61f
Change _owner to internal
heldersepu Aug 16, 2022
918ab11
Revert change to _owner variable
heldersepu Aug 16, 2022
00aae11
New contract SafeOwnable
heldersepu Aug 16, 2022
16f6014
SafeOwnableMock
heldersepu Aug 16, 2022
fbc33a8
testing the new SafeOwnable contract
heldersepu Aug 16, 2022
229b1d0
format code
heldersepu Aug 16, 2022
4b3369a
Add entry to changelog
heldersepu Aug 16, 2022
6c7325d
Fix error 'safe_ownable' is not in camel case
heldersepu Aug 16, 2022
80b5260
Fix mix of tabs and spaces
heldersepu Aug 16, 2022
db4af81
Allow address(0) as new owner
heldersepu Aug 16, 2022
c21d53d
Merge branch 'master' into master
heldersepu Aug 17, 2022
2530cd5
Rename _newOwner to _pendingOwner
heldersepu Aug 17, 2022
de9beed
Remove unused const
heldersepu Aug 17, 2022
c06b773
Remove unused constants
heldersepu Aug 17, 2022
3279163
override _transferOwnership to delete _pendingOwner
heldersepu Aug 17, 2022
d995d7c
Update contracts/access/SafeOwnable.sol
heldersepu Aug 17, 2022
b12b1e7
Update contracts/access/SafeOwnable.sol
heldersepu Aug 17, 2022
8d62986
format code
heldersepu Aug 17, 2022
320aa21
Add test to forceTransferOwnership
heldersepu Aug 17, 2022
fbf41df
Add check on pendingOwner
heldersepu Aug 17, 2022
20aa36c
Merge branch 'master' into master
heldersepu Aug 22, 2022
2ba7878
Merge branch 'master' into master
heldersepu Aug 24, 2022
9bbeb47
Rename to Ownable2Step
heldersepu Aug 24, 2022
16d5c16
Rename mock to Ownable2StepMock
heldersepu Aug 24, 2022
baf063e
Update CHANGELOG.md
heldersepu Aug 24, 2022
adcf9aa
Update contracts/access/Ownable2Step.sol
heldersepu Aug 24, 2022
0b46f49
check the event arguments
heldersepu Aug 24, 2022
50ca6bc
Merge branch 'master' of https://github.com/heldersepu/openzeppelin-c…
heldersepu Aug 24, 2022
f22673d
format code
heldersepu Aug 24, 2022
ac7e601
Add check pending owner resets on force transfer
heldersepu Aug 24, 2022
b781427
Update Ownable2Step.test.js
heldersepu Aug 24, 2022
81c1f6c
Update contracts/access/Ownable2Step.sol
heldersepu Aug 29, 2022
a36cc8e
Update contracts/access/Ownable2Step.sol
heldersepu Aug 30, 2022
b0e363a
Update contracts/access/Ownable2Step.sol
heldersepu Aug 30, 2022
8ff0621
Add previousOwner to event OwnershipTransferStarted
heldersepu Aug 30, 2022
ff44765
Merge branch 'master' into master
heldersepu Aug 31, 2022
84f3b9f
Remove forceTransferOwnership
heldersepu Aug 31, 2022
fca0901
Merge branch 'master' into master
frangio Sep 1, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -14,6 +14,7 @@
* `SafeCast`: optimize downcasting of signed integers. ([#3565](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3565))
* `VestingWallet`: remove unused library `Math.sol`. ([#3605](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3605))
* `ECDSA`: Remove redundant check on the `v` value. ([#3591](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3591))
* `SafeOwnable`: add new contract SafeOwnable, makes the transfers a two step process `SafeOwnable.sol`. ([#3620](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3620))

### Compatibility Note

Expand Down
49 changes: 49 additions & 0 deletions contracts/access/SafeOwnable.sol
@@ -0,0 +1,49 @@
// 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 SafeOwnable is Ownable {
address private _pendingOwner;

event OwnershipTransferStarted(address indexed newOwner);

heldersepu marked this conversation as resolved.
Show resolved Hide resolved
/**
* @dev Starts the ownership transfer of the contract to a new account
* Can only be called by the current owner.
*/
function transferOwnership(address newOwner) public virtual override onlyOwner {
_pendingOwner = newOwner;
emit OwnershipTransferStarted(newOwner);
}

heldersepu marked this conversation as resolved.
Show resolved Hide resolved
/**
* @dev Transfers ownership of the contract to a new account (`newOwner`).
* 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 {
require(_pendingOwner == _msgSender(), "SafeOwnable: caller is not the new owner");
_transferOwnership(_msgSender());
}
}
7 changes: 7 additions & 0 deletions contracts/mocks/SafeOwnableMock.sol
@@ -0,0 +1,7 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "../access/SafeOwnable.sol";

contract SafeOwnableMock is SafeOwnable {}
36 changes: 36 additions & 0 deletions test/access/SafeOwnable.test.js
@@ -0,0 +1,36 @@
const { expectEvent, expectRevert } = require('@openzeppelin/test-helpers');

const { expect } = require('chai');

const SafeOwnable = artifacts.require('SafeOwnableMock');

contract('SafeOwnable', function (accounts) {
const [ owner, accountA, accountB ] = accounts;

beforeEach(async function () {
this.safeOwnable = await SafeOwnable.new({ from: owner });
});

describe('transfer ownership', function () {
it('starting a transfer does not change owner', async function () {
const receipt = await this.safeOwnable.transferOwnership(accountA, { from: owner });
expectEvent(receipt, 'OwnershipTransferStarted');
expect(await this.safeOwnable.owner()).to.equal(owner);
});

it('changes owner after transfer', async function () {
await this.safeOwnable.transferOwnership(accountA, { from: owner });
const receipt = await this.safeOwnable.acceptOwnership({ from: accountA });
expectEvent(receipt, 'OwnershipTransferred');
expect(await this.safeOwnable.owner()).to.equal(accountA);
});

it('guards transfer against invalid user', async function () {
await this.safeOwnable.transferOwnership(accountA, { from: owner });
await expectRevert(
this.safeOwnable.acceptOwnership({ from: accountB }),
'SafeOwnable: caller is not the new owner',
);
});
});
});