Skip to content

N
nebula
Commit 52079cc1 authored by Kelvin Fichter's avatar Kelvin Fichter
Browse files
Options

feat(ctb): use Ownable for ProxyAdmin 

Has ProxyAdmin use Ownable instead of Owned to make sure we're only
using one library for ownership.
parent f045c541
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 124 additions and 96 deletions
.changeset/forty-pigs-draw.md 0 → 100644
View file @ 52079cc1
---
'@eth-optimism/contracts-bedrock': patch
---
Has ProxyAdmin use Ownable instead of Owned
op-bindings/bindings/governancetoken_more.go
View file @ 52079cc1
......@@ -9,7 +9,7 @@ import (
"github.com/ethereum-optimism/optimism/op-bindings/solc"
)
const GovernanceTokenStorageLayoutJSON = "{\"storage\":[{\"astId\":1000,\"contract\":\"contracts/L2/GovernanceToken.sol:GovernanceToken\",\"label\":\"_balances\",\"offset\":0,\"slot\":\"0\",\"type\":\"t_mapping(t_address,t_uint256)\"},{\"astId\":1001,\"contract\":\"contracts/L2/GovernanceToken.sol:GovernanceToken\",\"label\":\"_allowances\",\"offset\":0,\"slot\":\"1\",\"type\":\"t_mapping(t_address,t_mapping(t_address,t_uint256))\"},{\"astId\":1002,\"contract\":\"contracts/L2/GovernanceToken.sol:GovernanceToken\",\"label\":\"_totalSupply\",\"offset\":0,\"slot\":\"2\",\"type\":\"t_uint256\"},{\"astId\":1003,\"contract\":\"contracts/L2/GovernanceToken.sol:GovernanceToken\",\"label\":\"_name\",\"offset\":0,\"slot\":\"3\",\"type\":\"t_string_storage\"},{\"astId\":1004,\"contract\":\"contracts/L2/GovernanceToken.sol:GovernanceToken\",\"label\":\"_symbol\",\"offset\":0,\"slot\":\"4\",\"type\":\"t_string_storage\"},{\"astId\":1005,\"contract\":\"contracts/L2/GovernanceToken.sol:GovernanceToken\",\"label\":\"_nonces\",\"offset\":0,\"slot\":\"5\",\"type\":\"t_mapping(t_address,t_struct(Counter)33677_storage)\"},{\"astId\":1006,\"contract\":\"contracts/L2/GovernanceToken.sol:GovernanceToken\",\"label\":\"_PERMIT_TYPEHASH_DEPRECATED_SLOT\",\"offset\":0,\"slot\":\"6\",\"type\":\"t_bytes32\"},{\"astId\":1007,\"contract\":\"contracts/L2/GovernanceToken.sol:GovernanceToken\",\"label\":\"_delegates\",\"offset\":0,\"slot\":\"7\",\"type\":\"t_mapping(t_address,t_address)\"},{\"astId\":1008,\"contract\":\"contracts/L2/GovernanceToken.sol:GovernanceToken\",\"label\":\"_checkpoints\",\"offset\":0,\"slot\":\"8\",\"type\":\"t_mapping(t_address,t_array(t_struct(Checkpoint)30823_storage)dyn_storage)\"},{\"astId\":1009,\"contract\":\"contracts/L2/GovernanceToken.sol:GovernanceToken\",\"label\":\"_totalSupplyCheckpoints\",\"offset\":0,\"slot\":\"9\",\"type\":\"t_array(t_struct(Checkpoint)30823_storage)dyn_storage\"},{\"astId\":1010,\"contract\":\"contracts/L2/GovernanceToken.sol:GovernanceToken\",\"label\":\"_owner\",\"offset\":0,\"slot\":\"10\",\"type\":\"t_address\"}],\"types\":{\"t_address\":{\"encoding\":\"inplace\",\"label\":\"address\",\"numberOfBytes\":\"20\"},\"t_array(t_struct(Checkpoint)30823_storage)dyn_storage\":{\"encoding\":\"dynamic_array\",\"label\":\"struct ERC20Votes.Checkpoint[]\",\"numberOfBytes\":\"32\"},\"t_bytes32\":{\"encoding\":\"inplace\",\"label\":\"bytes32\",\"numberOfBytes\":\"32\"},\"t_mapping(t_address,t_address)\":{\"encoding\":\"mapping\",\"label\":\"mapping(address =\u003e address)\",\"numberOfBytes\":\"32\",\"key\":\"t_address\",\"value\":\"t_address\"},\"t_mapping(t_address,t_array(t_struct(Checkpoint)30823_storage)dyn_storage)\":{\"encoding\":\"mapping\",\"label\":\"mapping(address =\u003e struct ERC20Votes.Checkpoint[])\",\"numberOfBytes\":\"32\",\"key\":\"t_address\",\"value\":\"t_array(t_struct(Checkpoint)30823_storage)dyn_storage\"},\"t_mapping(t_address,t_mapping(t_address,t_uint256))\":{\"encoding\":\"mapping\",\"label\":\"mapping(address =\u003e mapping(address =\u003e uint256))\",\"numberOfBytes\":\"32\",\"key\":\"t_address\",\"value\":\"t_mapping(t_address,t_uint256)\"},\"t_mapping(t_address,t_struct(Counter)33677_storage)\":{\"encoding\":\"mapping\",\"label\":\"mapping(address =\u003e struct Counters.Counter)\",\"numberOfBytes\":\"32\",\"key\":\"t_address\",\"value\":\"t_struct(Counter)33677_storage\"},\"t_mapping(t_address,t_uint256)\":{\"encoding\":\"mapping\",\"label\":\"mapping(address =\u003e uint256)\",\"numberOfBytes\":\"32\",\"key\":\"t_address\",\"value\":\"t_uint256\"},\"t_string_storage\":{\"encoding\":\"bytes\",\"label\":\"string\",\"numberOfBytes\":\"32\"},\"t_struct(Checkpoint)30823_storage\":{\"encoding\":\"inplace\",\"label\":\"struct ERC20Votes.Checkpoint\",\"numberOfBytes\":\"32\"},\"t_struct(Counter)33677_storage\":{\"encoding\":\"inplace\",\"label\":\"struct Counters.Counter\",\"numberOfBytes\":\"32\"},\"t_uint224\":{\"encoding\":\"inplace\",\"label\":\"uint224\",\"numberOfBytes\":\"28\"},\"t_uint256\":{\"encoding\":\"inplace\",\"label\":\"uint256\",\"numberOfBytes\":\"32\"},\"t_uint32\":{\"encoding\":\"inplace\",\"label\":\"uint32\",\"numberOfBytes\":\"4\"}}}"
const GovernanceTokenStorageLayoutJSON = "{\"storage\":[{\"astId\":1000,\"contract\":\"contracts/L2/GovernanceToken.sol:GovernanceToken\",\"label\":\"_balances\",\"offset\":0,\"slot\":\"0\",\"type\":\"t_mapping(t_address,t_uint256)\"},{\"astId\":1001,\"contract\":\"contracts/L2/GovernanceToken.sol:GovernanceToken\",\"label\":\"_allowances\",\"offset\":0,\"slot\":\"1\",\"type\":\"t_mapping(t_address,t_mapping(t_address,t_uint256))\"},{\"astId\":1002,\"contract\":\"contracts/L2/GovernanceToken.sol:GovernanceToken\",\"label\":\"_totalSupply\",\"offset\":0,\"slot\":\"2\",\"type\":\"t_uint256\"},{\"astId\":1003,\"contract\":\"contracts/L2/GovernanceToken.sol:GovernanceToken\",\"label\":\"_name\",\"offset\":0,\"slot\":\"3\",\"type\":\"t_string_storage\"},{\"astId\":1004,\"contract\":\"contracts/L2/GovernanceToken.sol:GovernanceToken\",\"label\":\"_symbol\",\"offset\":0,\"slot\":\"4\",\"type\":\"t_string_storage\"},{\"astId\":1005,\"contract\":\"contracts/L2/GovernanceToken.sol:GovernanceToken\",\"label\":\"_nonces\",\"offset\":0,\"slot\":\"5\",\"type\":\"t_mapping(t_address,t_struct(Counter)33680_storage)\"},{\"astId\":1006,\"contract\":\"contracts/L2/GovernanceToken.sol:GovernanceToken\",\"label\":\"_PERMIT_TYPEHASH_DEPRECATED_SLOT\",\"offset\":0,\"slot\":\"6\",\"type\":\"t_bytes32\"},{\"astId\":1007,\"contract\":\"contracts/L2/GovernanceToken.sol:GovernanceToken\",\"label\":\"_delegates\",\"offset\":0,\"slot\":\"7\",\"type\":\"t_mapping(t_address,t_address)\"},{\"astId\":1008,\"contract\":\"contracts/L2/GovernanceToken.sol:GovernanceToken\",\"label\":\"_checkpoints\",\"offset\":0,\"slot\":\"8\",\"type\":\"t_mapping(t_address,t_array(t_struct(Checkpoint)30826_storage)dyn_storage)\"},{\"astId\":1009,\"contract\":\"contracts/L2/GovernanceToken.sol:GovernanceToken\",\"label\":\"_totalSupplyCheckpoints\",\"offset\":0,\"slot\":\"9\",\"type\":\"t_array(t_struct(Checkpoint)30826_storage)dyn_storage\"},{\"astId\":1010,\"contract\":\"contracts/L2/GovernanceToken.sol:GovernanceToken\",\"label\":\"_owner\",\"offset\":0,\"slot\":\"10\",\"type\":\"t_address\"}],\"types\":{\"t_address\":{\"encoding\":\"inplace\",\"label\":\"address\",\"numberOfBytes\":\"20\"},\"t_array(t_struct(Checkpoint)30826_storage)dyn_storage\":{\"encoding\":\"dynamic_array\",\"label\":\"struct ERC20Votes.Checkpoint[]\",\"numberOfBytes\":\"32\"},\"t_bytes32\":{\"encoding\":\"inplace\",\"label\":\"bytes32\",\"numberOfBytes\":\"32\"},\"t_mapping(t_address,t_address)\":{\"encoding\":\"mapping\",\"label\":\"mapping(address =\u003e address)\",\"numberOfBytes\":\"32\",\"key\":\"t_address\",\"value\":\"t_address\"},\"t_mapping(t_address,t_array(t_struct(Checkpoint)30826_storage)dyn_storage)\":{\"encoding\":\"mapping\",\"label\":\"mapping(address =\u003e struct ERC20Votes.Checkpoint[])\",\"numberOfBytes\":\"32\",\"key\":\"t_address\",\"value\":\"t_array(t_struct(Checkpoint)30826_storage)dyn_storage\"},\"t_mapping(t_address,t_mapping(t_address,t_uint256))\":{\"encoding\":\"mapping\",\"label\":\"mapping(address =\u003e mapping(address =\u003e uint256))\",\"numberOfBytes\":\"32\",\"key\":\"t_address\",\"value\":\"t_mapping(t_address,t_uint256)\"},\"t_mapping(t_address,t_struct(Counter)33680_storage)\":{\"encoding\":\"mapping\",\"label\":\"mapping(address =\u003e struct Counters.Counter)\",\"numberOfBytes\":\"32\",\"key\":\"t_address\",\"value\":\"t_struct(Counter)33680_storage\"},\"t_mapping(t_address,t_uint256)\":{\"encoding\":\"mapping\",\"label\":\"mapping(address =\u003e uint256)\",\"numberOfBytes\":\"32\",\"key\":\"t_address\",\"value\":\"t_uint256\"},\"t_string_storage\":{\"encoding\":\"bytes\",\"label\":\"string\",\"numberOfBytes\":\"32\"},\"t_struct(Checkpoint)30826_storage\":{\"encoding\":\"inplace\",\"label\":\"struct ERC20Votes.Checkpoint\",\"numberOfBytes\":\"32\"},\"t_struct(Counter)33680_storage\":{\"encoding\":\"inplace\",\"label\":\"struct Counters.Counter\",\"numberOfBytes\":\"32\"},\"t_uint224\":{\"encoding\":\"inplace\",\"label\":\"uint224\",\"numberOfBytes\":\"28\"},\"t_uint256\":{\"encoding\":\"inplace\",\"label\":\"uint256\",\"numberOfBytes\":\"32\"},\"t_uint32\":{\"encoding\":\"inplace\",\"label\":\"uint32\",\"numberOfBytes\":\"4\"}}}"
var GovernanceTokenStorageLayout = new(solc.StorageLayout)
......
op-bindings/bindings/proxyadmin.go
View file @ 52079cc1
This diff is collapsed.
op-bindings/bindings/proxyadmin_more.go
View file @ 52079cc1
This diff is collapsed.
op-chain-ops/genesis/config.go
View file @ 52079cc1
......@@ -200,7 +200,7 @@ func NewL2StorageConfig(config *DeployConfig, block *types.Block, l2Addrs *L2Add
"_owner": common.Address{},
}
storage["ProxyAdmin"] = state.StorageValues{
"owner": l2Addrs.ProxyAdminOwner,
"_owner": l2Addrs.ProxyAdminOwner,
}
return storage, nil
}
packages/contracts-bedrock/.gas-snapshot
View file @ 52079cc1
......@@ -192,29 +192,29 @@ Proxy_Test:test_payableUpgradeToAndCall() (gas: 53865)
Proxy_Test:test_revertUpgradeToAndCall() (gas: 104727)
Proxy_Test:test_upgradeToAndCall() (gas: 125305)
Proxy_Test:test_zeroAddressCaller() (gas: 14825)
ProxyAdmin_Test:test_chugsplashChangeProxyAdmin() (gas: 35642)
ProxyAdmin_Test:test_chugsplashChangeProxyAdmin() (gas: 35644)
ProxyAdmin_Test:test_chugsplashGetProxyAdmin() (gas: 15683)
ProxyAdmin_Test:test_chugsplashGetProxyImplementation() (gas: 51149)
ProxyAdmin_Test:test_chugsplashUpgrade() (gas: 48960)
ProxyAdmin_Test:test_chugsplashUpgradeAndCall() (gas: 82330)
ProxyAdmin_Test:test_delegateResolvedChangeProxyAdmin() (gas: 34006)
ProxyAdmin_Test:test_chugsplashGetProxyImplementation() (gas: 51152)
ProxyAdmin_Test:test_chugsplashUpgrade() (gas: 48985)
ProxyAdmin_Test:test_chugsplashUpgradeAndCall() (gas: 82312)
ProxyAdmin_Test:test_delegateResolvedChangeProxyAdmin() (gas: 34008)
ProxyAdmin_Test:test_delegateResolvedGetProxyAdmin() (gas: 17699)
ProxyAdmin_Test:test_delegateResolvedGetProxyImplementation() (gas: 62115)
ProxyAdmin_Test:test_delegateResolvedUpgrade() (gas: 58520)
ProxyAdmin_Test:test_delegateResolvedUpgradeAndCall() (gas: 97989)
ProxyAdmin_Test:test_erc1967ChangeProxyAdmin() (gas: 33870)
ProxyAdmin_Test:test_delegateResolvedGetProxyImplementation() (gas: 62118)
ProxyAdmin_Test:test_delegateResolvedUpgrade() (gas: 58545)
ProxyAdmin_Test:test_delegateResolvedUpgradeAndCall() (gas: 98015)
ProxyAdmin_Test:test_erc1967ChangeProxyAdmin() (gas: 33872)
ProxyAdmin_Test:test_erc1967GetProxyAdmin() (gas: 15669)
ProxyAdmin_Test:test_erc1967GetProxyImplementation() (gas: 52093)
ProxyAdmin_Test:test_erc1967Upgrade() (gas: 50030)
ProxyAdmin_Test:test_erc1967UpgradeAndCall() (gas: 79013)
ProxyAdmin_Test:test_isUpgrading() (gas: 19575)
ProxyAdmin_Test:test_onlyOwner() (gas: 22715)
ProxyAdmin_Test:test_onlyOwnerSetAddressManager() (gas: 10600)
ProxyAdmin_Test:test_onlyOwnerSetImplementationName() (gas: 11113)
ProxyAdmin_Test:test_onlyOwnerSetProxyType() (gas: 10751)
ProxyAdmin_Test:test_owner() (gas: 9818)
ProxyAdmin_Test:test_proxyType() (gas: 20622)
ProxyAdmin_Test:test_setImplementationName() (gas: 38968)
ProxyAdmin_Test:test_erc1967GetProxyImplementation() (gas: 52096)
ProxyAdmin_Test:test_erc1967Upgrade() (gas: 50055)
ProxyAdmin_Test:test_erc1967UpgradeAndCall() (gas: 79015)
ProxyAdmin_Test:test_isUpgrading() (gas: 19555)
ProxyAdmin_Test:test_onlyOwner() (gas: 22801)
ProxyAdmin_Test:test_onlyOwnerSetAddressManager() (gas: 10632)
ProxyAdmin_Test:test_onlyOwnerSetImplementationName() (gas: 11134)
ProxyAdmin_Test:test_onlyOwnerSetProxyType() (gas: 10772)
ProxyAdmin_Test:test_owner() (gas: 9812)
ProxyAdmin_Test:test_proxyType() (gas: 20556)
ProxyAdmin_Test:test_setImplementationName() (gas: 39059)
RLPReader_Test:test_readBytes_bytestring00() (gas: 1855)
RLPReader_Test:test_readBytes_bytestring01() (gas: 1901)
RLPReader_Test:test_readBytes_bytestring7f() (gas: 1900)
......
packages/contracts-bedrock/.storage-layout
View file @ 52079cc1
......@@ -320,7 +320,7 @@
+--------------------+-----------------------------------------------+------+--------+-------+-----------------------------------------------+
| Name | Type | Slot | Offset | Bytes | Contract |
+============================================================================================================================================+
| owner | address | 0 | 0 | 20 | contracts/universal/ProxyAdmin.sol:ProxyAdmin |
| _owner | address | 0 | 0 | 20 | contracts/universal/ProxyAdmin.sol:ProxyAdmin |
|--------------------+-----------------------------------------------+------+--------+-------+-----------------------------------------------|
| proxyType | mapping(address => enum ProxyAdmin.ProxyType) | 1 | 0 | 32 | contracts/universal/ProxyAdmin.sol:ProxyAdmin |
|--------------------+-----------------------------------------------+------+--------+-------+-----------------------------------------------|
......
packages/contracts-bedrock/contracts/deployment/FreshSystemDictator.sol
View file @ 52079cc1
......@@ -74,6 +74,6 @@ contract FreshSystemDictator is BaseSystemDictator {
*/
function step2() external onlyOwner step(2) {
// Transfer ownership of the ProxyAdmin to the final owner.
config.globalConfig.proxyAdmin.setOwner(config.globalConfig.finalOwner);
config.globalConfig.proxyAdmin.transferOwnership(config.globalConfig.finalOwner);
}
}
packages/contracts-bedrock/contracts/deployment/MigrationSystemDictator.sol
View file @ 52079cc1
......@@ -172,6 +172,6 @@ contract MigrationSystemDictator is BaseSystemDictator {
.transferOwnership(config.globalConfig.finalOwner);
// Transfer ownership of the ProxyAdmin to the final owner.
config.globalConfig.proxyAdmin.setOwner(config.globalConfig.finalOwner);
config.globalConfig.proxyAdmin.transferOwnership(config.globalConfig.finalOwner);
}
}
packages/contracts-bedrock/contracts/test/ProxyAdmin.t.sol
View file @ 52079cc1
......@@ -66,17 +66,17 @@ contract ProxyAdmin_Test is Test {
}
function test_onlyOwnerSetAddressManager() external {
vm.expectRevert("UNAUTHORIZED");
vm.expectRevert("Ownable: caller is not the owner");
admin.setAddressManager(AddressManager((address(0))));
}
function test_onlyOwnerSetImplementationName() external {
vm.expectRevert("UNAUTHORIZED");
vm.expectRevert("Ownable: caller is not the owner");
admin.setImplementationName(address(0), "foo");
}
function test_onlyOwnerSetProxyType() external {
vm.expectRevert("UNAUTHORIZED");
vm.expectRevert("Ownable: caller is not the owner");
admin.setProxyType(address(0), ProxyAdmin.ProxyType.CHUGSPLASH);
}
......@@ -236,13 +236,13 @@ contract ProxyAdmin_Test is Test {
}
function test_onlyOwner() external {
vm.expectRevert("UNAUTHORIZED");
vm.expectRevert("Ownable: caller is not the owner");
admin.changeProxyAdmin(payable(proxy), address(0));
vm.expectRevert("UNAUTHORIZED");
vm.expectRevert("Ownable: caller is not the owner");
admin.upgrade(payable(proxy), address(implementation));
vm.expectRevert("UNAUTHORIZED");
vm.expectRevert("Ownable: caller is not the owner");
admin.upgradeAndCall(payable(proxy), address(implementation), hex"");
}
......
packages/contracts-bedrock/contracts/universal/ProxyAdmin.sol
View file @ 52079cc1
// SPDX-License-Identifier: MIT
pragma solidity 0.8.15;
import { Owned } from "@rari-capital/solmate/src/auth/Owned.sol";
import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
import { Proxy } from "./Proxy.sol";
import { AddressManager } from "../legacy/AddressManager.sol";
import { L1ChugSplashProxy } from "../legacy/L1ChugSplashProxy.sol";
......@@ -32,7 +32,7 @@ interface IStaticL1ChugSplashProxy {
* based on the OpenZeppelin implementation. It has backwards compatibility logic to work
* with the various types of proxies that have been deployed by Optimism in the past.
*/
contract ProxyAdmin is Owned {
contract ProxyAdmin is Ownable {
/**
* @notice The proxy types that the ProxyAdmin can manage.
*
......@@ -76,7 +76,9 @@ contract ProxyAdmin is Owned {
/**
* @param _owner Address of the initial owner of this contract.
*/
constructor(address _owner) Owned(_owner) {}
constructor(address _owner) Ownable() {
_transferOwnership(_owner);
}
/**
* @notice Sets the proxy type for a given address. Only required for non-standard (legacy)
......
packages/contracts-bedrock/deploy/015-FreshSystemDictatorSteps.ts
View file @ 52079cc1
......@@ -26,7 +26,7 @@ const deployFn: DeployFunction = async (hre) => {
if ((await ProxyAdmin.owner()) !== FreshSystemDictator.address) {
console.log(`Transferring proxy admin ownership to the FreshSystemDictator`)
await ProxyAdmin.setOwner(FreshSystemDictator.address)
await ProxyAdmin.transferOwnership(FreshSystemDictator.address)
} else {
console.log(`Proxy admin already owned by the FreshSystemDictator`)
}
......
packages/contracts-bedrock/deploy/017-MigrationSystemDictatorSteps.ts
View file @ 52079cc1
......@@ -45,7 +45,7 @@ const deployFn: DeployFunction = async (hre) => {
console.log(
`Transferring proxy admin ownership to the MigrationSystemDictator`
)
await ProxyAdmin.setOwner(MigrationSystemDictator.address)
await ProxyAdmin.transferOwnership(MigrationSystemDictator.address)
} else {
console.log(`Proxy admin already owned by the MigrationSystemDictator`)
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment