diff --git a/target_chains/ethereum/contracts/contracts/entropy/EntropyGovernance.sol b/target_chains/ethereum/contracts/contracts/entropy/EntropyGovernance.sol index 3d5d2725..d2a11760 100644 --- a/target_chains/ethereum/contracts/contracts/entropy/EntropyGovernance.sol +++ b/target_chains/ethereum/contracts/contracts/entropy/EntropyGovernance.sol @@ -9,31 +9,49 @@ import "./EntropyState.sol"; * @dev `Governance` defines a means to enacting changes to the Entropy contract. */ abstract contract EntropyGovernance is EntropyState { - event AdminSet(address oldAdmin, address newAdmin); event PythFeeSet(uint oldPythFee, uint newPythFee); event DefaultProviderSet( address oldDefaultProvider, address newDefaultProvider ); - function getAdmin() external view returns (address) { - return _state.admin; + event NewAdminProposed(address oldAdmin, address newAdmin); + event NewAdminAccepted(address oldAdmin, address newAdmin); + + /** + * @dev Returns the address of the proposed admin. + */ + function proposedAdmin() public view virtual returns (address) { + return _state.proposedAdmin; } /** - * @dev Set the admin of the contract. - * - * Calls {_authoriseAdminAction}. - * - * Emits an {AdminSet} event. + * @dev Proposes a new admin of the contract. Replaces the proposed admin if there is one. + * Can only be called by either admin or owner. */ - function setAdmin(address newAdmin) external { + function proposeAdmin(address newAdmin) public virtual { _authoriseAdminAction(); - address oldAdmin = _state.admin; - _state.admin = newAdmin; + _state.proposedAdmin = newAdmin; + emit NewAdminProposed(_state.admin, newAdmin); + } - emit AdminSet(oldAdmin, newAdmin); + /** + * @dev The proposed admin accepts the admin transfer. + */ + function acceptAdmin() external { + if (msg.sender != _state.proposedAdmin) + revert EntropyErrors.Unauthorized(); + + address oldAdmin = _state.admin; + _state.admin = msg.sender; + + _state.proposedAdmin = address(0); + emit NewAdminAccepted(oldAdmin, msg.sender); + } + + function getAdmin() external view returns (address) { + return _state.admin; } /** diff --git a/target_chains/ethereum/contracts/contracts/entropy/EntropyState.sol b/target_chains/ethereum/contracts/contracts/entropy/EntropyState.sol index 6855b674..94b6e0ec 100644 --- a/target_chains/ethereum/contracts/contracts/entropy/EntropyState.sol +++ b/target_chains/ethereum/contracts/contracts/entropy/EntropyState.sol @@ -34,6 +34,9 @@ contract EntropyInternalStructs { mapping(bytes32 => EntropyStructs.Request) requestsOverflow; // Mapping from randomness providers to information about each them. mapping(address => EntropyStructs.ProviderInfo) providers; + // proposedAdmin is the new admin's account address proposed by either the owner or the current admin. + // If there is no pending transfer request, this value will hold `address(0)`. + address proposedAdmin; } } diff --git a/target_chains/ethereum/contracts/contracts/entropy/EntropyUpgradable.sol b/target_chains/ethereum/contracts/contracts/entropy/EntropyUpgradable.sol index 71eea6f1..f008e42a 100644 --- a/target_chains/ethereum/contracts/contracts/entropy/EntropyUpgradable.sol +++ b/target_chains/ethereum/contracts/contracts/entropy/EntropyUpgradable.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.0; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol"; -import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; +import "@openzeppelin/contracts-upgradeable/access/Ownable2StepUpgradeable.sol"; import "@pythnetwork/entropy-sdk-solidity/EntropyErrors.sol"; import "./EntropyGovernance.sol"; @@ -11,7 +11,7 @@ import "./Entropy.sol"; contract EntropyUpgradable is Initializable, - OwnableUpgradeable, + Ownable2StepUpgradeable, UUPSUpgradeable, Entropy, EntropyGovernance @@ -42,7 +42,7 @@ contract EntropyUpgradable is ); // We need to transfer the ownership from deployer to the new owner - transferOwnership(owner); + _transferOwnership(owner); } /// Ensures the contract cannot be uninitialized and taken over. diff --git a/target_chains/ethereum/contracts/forge-test/EntropyAuthorized.t.sol b/target_chains/ethereum/contracts/forge-test/EntropyAuthorized.t.sol index ee682994..3ab06b7b 100644 --- a/target_chains/ethereum/contracts/forge-test/EntropyAuthorized.t.sol +++ b/target_chains/ethereum/contracts/forge-test/EntropyAuthorized.t.sol @@ -24,6 +24,8 @@ contract EntropyAuthorized is Test, EntropyTestUtils { address public provider1 = address(4); address public provider2 = address(5); + address public owner2 = address(6); + uint128 pythFeeInWei = 7; function setUp() public { @@ -39,24 +41,6 @@ contract EntropyAuthorized is Test, EntropyTestUtils { random.initialize(owner, admin, pythFeeInWei, provider1, false); } - function testSetAdminByAdmin() public { - vm.prank(admin); - random.setAdmin(admin2); - assertEq(random.getAdmin(), admin2); - } - - function testSetAdminByOwner() public { - vm.prank(owner); - random.setAdmin(admin2); - assertEq(random.getAdmin(), admin2); - } - - function testExpectRevertSetAdminByUnauthorized() public { - vm.expectRevert(EntropyErrors.Unauthorized.selector); - vm.prank(admin2); - random.setAdmin(admin); - } - function testSetPythFeeByAdmin() public { vm.prank(admin); random.setPythFee(10); @@ -120,4 +104,74 @@ contract EntropyAuthorized is Test, EntropyTestUtils { vm.prank(owner); random.upgradeTo(address(randomDifferentMagic)); } + + function testExpectRevertRequestOwnershipTransferByUnauthorized() public { + vm.expectRevert("Ownable: caller is not the owner"); + vm.prank(provider1); + random.transferOwnership(owner2); + } + + function testExpectRevertRequestOwnershipTransferByAdmin() public { + vm.expectRevert("Ownable: caller is not the owner"); + vm.prank(admin); + random.transferOwnership(owner2); + } + + function testRequestAndAcceptOwnershipTransfer() public { + vm.prank(owner); + random.transferOwnership(owner2); + assertEq(random.pendingOwner(), owner2); + + vm.prank(owner2); + random.acceptOwnership(); + assertEq(random.owner(), owner2); + } + + function testRequestAndAcceptOwnershipTransferUnauthorizedAccept() public { + vm.prank(owner); + random.transferOwnership(owner2); + assertEq(random.pendingOwner(), owner2); + + vm.prank(admin); + vm.expectRevert("Ownable2Step: caller is not the new owner"); + random.acceptOwnership(); + } + + function testProposeAdminByOwner() public { + vm.prank(owner); + random.proposeAdmin(admin2); + + assertEq(random.proposedAdmin(), admin2); + } + + function testProposeAdminByAdmin() public { + vm.prank(admin); + random.proposeAdmin(admin2); + + assertEq(random.proposedAdmin(), admin2); + } + + function testProposeAdminByUnauthorized() public { + vm.expectRevert(EntropyErrors.Unauthorized.selector); + random.proposeAdmin(admin2); + } + + function testAcceptAdminByPropsed() public { + vm.prank(owner); + random.proposeAdmin(admin2); + + vm.prank(admin2); + random.acceptAdmin(); + + assertEq(random.getAdmin(), admin2); + } + + function testAcceptAdminByUnauthorized() public { + vm.prank(owner); + random.proposeAdmin(admin2); + + vm.prank(provider1); + vm.expectRevert(EntropyErrors.Unauthorized.selector); + random.acceptAdmin(); + } }