[entropy] governance (#1154)

* governance

* correct comment

* add upgradability

* remove unused lib

* update governance and update logic

* change specifiers

* fix tests

* add tests

* rebase bug fixes

* change error name

* fix tests

* no need of UUPSProxy

* minimise the amount of code

* separate file for authorized tests

* add comment

* correct comment

target_chains/ethereum/contracts/contracts/entropy/Entropy.sol

@@ -73,13 +73,14 @@ import "./EntropyState.sol";
 // protocol prior to the random number being revealed (i.e., prior to step (6) above). Integrators should ensure that
 // the user is always incentivized to reveal their random number, and that the protocol has an escape hatch for
 // cases where the user chooses not to reveal.
-contract Entropy is IEntropy, EntropyState {
+abstract contract Entropy is IEntropy, EntropyState {
-    // TODO: Use an upgradeable proxy
+    function _initialize(
-    constructor(
+        address admin,
         uint128 pythFeeInWei,
         address defaultProvider,
         bool prefillRequestStorage
-    ) {
+    ) internal {
+        _state.admin = admin;
         _state.accruedPythFeesInWei = 0;
         _state.pythFeeInWei = pythFeeInWei;
         _state.defaultProvider = defaultProvider;
@@ -305,6 +306,10 @@ contract Entropy is IEntropy, EntropyState {
         return _state.providers[provider].feeInWei + _state.pythFeeInWei;
     }
 
+    function getPythFee() public view returns (uint128 feeAmount) {
+        return _state.pythFeeInWei;
+    }
+
     function getAccruedPythFees()
         public
         view

target_chains/ethereum/contracts/contracts/entropy/EntropyGovernance.sol

@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: Apache 2
+pragma solidity ^0.8.0;
+
+import "@pythnetwork/entropy-sdk-solidity/EntropyErrors.sol";
+
+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;
+    }
+
+    /**
+     * @dev Set the admin of the contract.
+     *
+     * Calls {_authoriseAdminAction}.
+     *
+     * Emits an {AdminSet} event.
+     */
+    function setAdmin(address newAdmin) external {
+        _authoriseAdminAction();
+
+        address oldAdmin = _state.admin;
+        _state.admin = newAdmin;
+
+        emit AdminSet(oldAdmin, newAdmin);
+    }
+
+    /**
+     * @dev Set the Pyth fee in Wei
+     *
+     * Calls {_authoriseAdminAction}.
+     *
+     * Emits an {PythFeeSet} event.
+     */
+    function setPythFee(uint128 newPythFee) external {
+        _authoriseAdminAction();
+
+        uint oldPythFee = _state.pythFeeInWei;
+        _state.pythFeeInWei = newPythFee;
+
+        emit PythFeeSet(oldPythFee, newPythFee);
+    }
+
+    /**
+     * @dev Set the default provider of the contract
+     *
+     * Calls {_authoriseAdminAction}.
+     *
+     * Emits an {DefaultProviderSet} event.
+     */
+    function setDefaultProvider(address newDefaultProvider) external {
+        _authoriseAdminAction();
+
+        address oldDefaultProvider = _state.defaultProvider;
+        _state.defaultProvider = newDefaultProvider;
+
+        emit DefaultProviderSet(oldDefaultProvider, newDefaultProvider);
+    }
+
+    function _authoriseAdminAction() internal virtual;
+}

target_chains/ethereum/contracts/contracts/entropy/EntropyState.sol

@@ -6,6 +6,8 @@ import "@pythnetwork/entropy-sdk-solidity/EntropyStructs.sol";
 
 contract EntropyInternalStructs {
     struct State {
+        // Admin has the rights to update pyth configs
+        address admin;
         // Fee charged by the pyth protocol in wei.
         uint128 pythFeeInWei;
         // Total quantity of fees (in wei) earned by the pyth protocol that are currently stored in the contract.

target_chains/ethereum/contracts/contracts/entropy/EntropyUpgradable.sol

@@ -0,0 +1,99 @@
+// SPDX-License-Identifier: Apache 2
+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 "@pythnetwork/entropy-sdk-solidity/EntropyErrors.sol";
+
+import "./EntropyGovernance.sol";
+import "./Entropy.sol";
+
+contract EntropyUpgradable is
+    Initializable,
+    OwnableUpgradeable,
+    UUPSUpgradeable,
+    Entropy,
+    EntropyGovernance
+{
+    event ContractUpgraded(
+        address oldImplementation,
+        address newImplementation
+    );
+
+    // The contract will have an owner and an admin
+    // The owner will have all the power over it.
+    // The admin can set some config parameters only.
+    function initialize(
+        address owner,
+        address admin,
+        uint128 pythFeeInWei,
+        address defaultProvider,
+        bool prefillRequestStorage
+    ) public initializer {
+        __Ownable_init();
+        __UUPSUpgradeable_init();
+
+        Entropy._initialize(
+            admin,
+            pythFeeInWei,
+            defaultProvider,
+            prefillRequestStorage
+        );
+
+        // We need to transfer the ownership from deployer to the new owner
+        transferOwnership(owner);
+    }
+
+    /// Ensures the contract cannot be uninitialized and taken over.
+    /// @custom:oz-upgrades-unsafe-allow constructor
+    constructor() initializer {}
+
+    // Only allow the owner to upgrade the proxy to a new implementation.
+    function _authorizeUpgrade(address) internal override onlyOwner {}
+
+    // There are some actions which both and admin and owner can perform
+    function _authoriseAdminAction() internal view override {
+        if (msg.sender != owner() && msg.sender != _state.admin)
+            revert EntropyErrors.Unauthorized();
+    }
+
+    // We have not overridden these methods in Pyth contracts implementation.
+    // But we are overriding them here because there was no owner before and
+    // `_authorizeUpgrade` would cause a revert for these. Now we have an owner, and
+    // because we want to test for the magic. We are overriding these methods.
+    function upgradeTo(address newImplementation) external override onlyProxy {
+        address oldImplementation = _getImplementation();
+        _authorizeUpgrade(newImplementation);
+        _upgradeToAndCallUUPS(newImplementation, new bytes(0), false);
+
+        magicCheck();
+
+        emit ContractUpgraded(oldImplementation, _getImplementation());
+    }
+
+    function upgradeToAndCall(
+        address newImplementation,
+        bytes memory data
+    ) external payable override onlyProxy {
+        address oldImplementation = _getImplementation();
+        _authorizeUpgrade(newImplementation);
+        _upgradeToAndCallUUPS(newImplementation, data, true);
+
+        magicCheck();
+
+        emit ContractUpgraded(oldImplementation, _getImplementation());
+    }
+
+    function magicCheck() internal view {
+        // Calling a method using `this.<method>` will cause a contract call that will use
+        // the new contract. This call will fail if the method does not exists or the magic
+        // is different.
+        if (this.entropyUpgradableMagic() != 0x66697265)
+            revert EntropyErrors.InvalidUpgradeMagic();
+    }
+
+    function entropyUpgradableMagic() public pure returns (uint32) {
+        return 0x66697265;
+    }
+}

target_chains/ethereum/contracts/forge-test/Entropy.t.sol

@@ -4,13 +4,15 @@ pragma solidity ^0.8.0;
 
 import "forge-std/Test.sol";
 import "@pythnetwork/entropy-sdk-solidity/EntropyStructs.sol";
-import "../contracts/entropy/Entropy.sol";
+import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
 import "./utils/EntropyTestUtils.t.sol";
+import "../contracts/entropy/EntropyUpgradable.sol";
 
 // TODO
 // - fuzz test?
 contract EntropyTest is Test, EntropyTestUtils {
-    Entropy public random;
+    ERC1967Proxy public proxy;
+    EntropyUpgradable public random;
 
     uint128 pythFeeInWei = 7;
 
@@ -33,8 +35,18 @@ contract EntropyTest is Test, EntropyTestUtils {
     uint128 MAX_UINT128 = 2 ** 128 - 1;
     bytes32 ALL_ZEROS = bytes32(uint256(0));
 
+    address public owner = address(8);
+    address public admin = address(9);
+    address public admin2 = address(10);
+
     function setUp() public {
-        random = new Entropy(pythFeeInWei, provider1, false);
+        EntropyUpgradable _random = new EntropyUpgradable();
+        // deploy proxy contract and point it to implementation
+        proxy = new ERC1967Proxy(address(_random), "");
+        // wrap in ABI to support easier calls
+        random = EntropyUpgradable(address(proxy));
+
+        random.initialize(owner, admin, pythFeeInWei, provider1, false);
 
         bytes32[] memory hashChain1 = generateHashChain(
             provider1,

target_chains/ethereum/contracts/forge-test/EntropyAuthorized.t.sol

@@ -0,0 +1,123 @@
+// SPDX-License-Identifier: Apache 2
+
+pragma solidity ^0.8.0;
+
+import "./utils/EntropyTestUtils.t.sol";
+import "../contracts/entropy/EntropyUpgradable.sol";
+import "./utils/EntropyTestContracts/EntropyDifferentMagic.t.sol";
+import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
+import "@pythnetwork/entropy-sdk-solidity/EntropyErrors.sol";
+
+contract EntropyAuthorized is Test, EntropyTestUtils {
+    ERC1967Proxy public proxy;
+    EntropyUpgradable public random;
+    EntropyUpgradable public random2;
+    EntropyDifferentMagic public randomDifferentMagic;
+
+    address public owner = address(1);
+    address public admin = address(2);
+    address public admin2 = address(3);
+
+    // We don't need to register providers for these tests
+    // We are just checking for the default provider, which
+    // only required an address.
+    address public provider1 = address(4);
+    address public provider2 = address(5);
+
+    uint128 pythFeeInWei = 7;
+
+    function setUp() public {
+        EntropyUpgradable _random = new EntropyUpgradable();
+        // deploy proxy contract and point it to implementation
+        proxy = new ERC1967Proxy(address(_random), "");
+        // wrap in ABI to support easier calls
+        random = EntropyUpgradable(address(proxy));
+        // to test for upgrade
+        random2 = new EntropyUpgradable();
+        randomDifferentMagic = new EntropyDifferentMagic();
+
+        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);
+        assertEq(random.getPythFee(), 10);
+    }
+
+    function testSetPythFeeByOwner() public {
+        vm.prank(owner);
+        random.setPythFee(10);
+        assertEq(random.getPythFee(), 10);
+    }
+
+    function testExpectRevertSetPythFeeByUnauthorized() public {
+        vm.expectRevert(EntropyErrors.Unauthorized.selector);
+        vm.prank(admin2);
+        random.setPythFee(10);
+    }
+
+    function testSetDefaultProviderByOwner() public {
+        vm.prank(owner);
+        random.setDefaultProvider(provider2);
+        assertEq(random.getDefaultProvider(), provider2);
+    }
+
+    function testSetDefaultProviderByAdmin() public {
+        vm.prank(admin);
+        random.setDefaultProvider(provider2);
+        assertEq(random.getDefaultProvider(), provider2);
+    }
+
+    function testExpectRevertSetDefaultProviderByUnauthorized() public {
+        vm.expectRevert(EntropyErrors.Unauthorized.selector);
+        vm.prank(admin2);
+        random.setDefaultProvider(provider2);
+    }
+
+    function testUpgradeByOwner() public {
+        vm.prank(owner);
+        random.upgradeTo(address(random2));
+    }
+
+    function testExpectRevertUpgradeByAdmin() public {
+        // The message is returned by openzepplin upgrade contracts
+        vm.expectRevert("Ownable: caller is not the owner");
+        vm.prank(admin);
+        random.upgradeTo(address(random2));
+    }
+
+    function testExpectRevertUpgradeByUnauthorized() public {
+        // The message is returned by openzepplin upgrade contracts
+        vm.expectRevert("Ownable: caller is not the owner");
+        vm.prank(provider1);
+        random.upgradeTo(address(random2));
+    }
+
+    // There can be another case that the magic function doesn't
+    // exist but it's fine. (It will revert with no reason)
+    // The randomDifferentMagic contract do have a magic in this case
+    function testExpectRevertDifferentMagicContractUpgrade() public {
+        vm.expectRevert(EntropyErrors.InvalidUpgradeMagic.selector);
+        vm.prank(owner);
+        random.upgradeTo(address(randomDifferentMagic));
+    }
+}

target_chains/ethereum/contracts/forge-test/EntropyGasBenchmark.t.sol

@@ -4,14 +4,17 @@ pragma solidity ^0.8.0;
 
 import "forge-std/Test.sol";
 import "@pythnetwork/entropy-sdk-solidity/EntropyStructs.sol";
-import "../contracts/entropy/Entropy.sol";
+import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
+
 import "./utils/EntropyTestUtils.t.sol";
+import "../contracts/entropy/EntropyUpgradable.sol";
 
 // TODO
 // - what's the impact of # of in-flight requests on gas usage? More requests => more hashes to
 //   verify the provider's value.
 contract EntropyGasBenchmark is Test, EntropyTestUtils {
-    Entropy public random;
+    ERC1967Proxy public proxy;
+    EntropyUpgradable public random;
 
     uint128 pythFeeInWei = 7;
 
@@ -23,7 +26,19 @@ contract EntropyGasBenchmark is Test, EntropyTestUtils {
     address public user1 = address(3);
 
     function setUp() public {
-        random = new Entropy(pythFeeInWei, provider1, true);
+        EntropyUpgradable _random = new EntropyUpgradable();
+        // deploy proxy contract and point it to implementation
+        proxy = new ERC1967Proxy(address(_random), "");
+        // wrap in ABI to support easier calls
+        random = EntropyUpgradable(address(proxy));
+
+        random.initialize(
+            address(4),
+            address(5),
+            pythFeeInWei,
+            provider1,
+            false
+        );
 
         bytes32[] memory hashChain1 = generateHashChain(
             provider1,

target_chains/ethereum/contracts/forge-test/utils/EntropyTestContracts/EntropyDifferentMagic.t.sol

@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: Apache 2
+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 "@pythnetwork/entropy-sdk-solidity/EntropyErrors.sol";
+
+contract EntropyDifferentMagic is
+    Initializable,
+    OwnableUpgradeable,
+    UUPSUpgradeable
+{
+    function initialize() public initializer {
+        __Ownable_init();
+        __UUPSUpgradeable_init();
+    }
+
+    // /// Ensures the contract cannot be uninitialized and taken over.
+    // /// @custom:oz-upgrades-unsafe-allow constructor
+    constructor() initializer {}
+
+    // // Only allow the owner to upgrade the proxy to a new implementation.
+    function _authorizeUpgrade(address) internal override onlyOwner {}
+
+    function magicCheck() internal view {
+        // Calling a method using `this.<method>` will cause a contract call that will use
+        // the new contract. This call will fail if the method does not exists or the magic
+        // is different.
+        if (this.entropyUpgradableMagic() != 0x666972)
+            revert EntropyErrors.InvalidUpgradeMagic();
+    }
+
+    function entropyUpgradableMagic() public pure returns (uint32) {
+        return 0x666972;
+    }
+}

target_chains/ethereum/entropy_sdk/solidity/EntropyErrors.sol

@@ -18,4 +18,8 @@ library EntropyErrors {
     error InsufficientFee();
     // Either the user's or the provider's revealed random values did not match their commitment.
     error IncorrectRevelation();
+    // Governance message is invalid (e.g., deserialization error).
+    error InvalidUpgradeMagic();
+    // Unauthorized (e.g., invalid admin or owner authorisation).
+    error Unauthorized();
 }