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.

Sign up for GitHub

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

Jump to bottom

OPCM: AddressManager and ProxyAdmin assertions. #12370

Merged
merged 10 commits into from
Oct 8, 2024
Merged

OPCM: AddressManager and ProxyAdmin assertions. #12370

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8b0cf5a
OPCM: AddressManager and ProxyAdmin assertions.
blmalone Oct 8, 2024
f0d6e16
fix: Added logic for proxy types to check implementations.
blmalone Oct 8, 2024
2a3412d
fix: touch ups for RDP impl checking.
blmalone Oct 8, 2024
2f86c1a
fix: fully fledged proxies added to tests because of new assertions.
blmalone Oct 8, 2024
60b6542
fix: removing console2 lib.
blmalone Oct 8, 2024
0d813ff
fix: semgrep complaining.
blmalone Oct 8, 2024
3a51884
fix: comment cleanup and logical split to avoid stack too deep error.
blmalone Oct 8, 2024
6dfc444
fix: adding comment back in.
blmalone Oct 8, 2024
c621036
fix: added natspec comments for new DeployUtils functions.
blmalone Oct 8, 2024
e6e644b
fix: removed unused imports.
blmalone Oct 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/contracts-bedrock/scripts/DeployImplementations.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ contract DeployImplementationsOutput is BaseDeployIO {

function opcmProxy() public returns (OPContractsManager) {
DeployUtils.assertValidContractAddress(address(_opcmProxy));
DeployUtils.assertImplementationSet(address(_opcmProxy));
DeployUtils.assertERC1967ImplementationSet(address(_opcmProxy));
return _opcmProxy;
}

Expand Down
91 changes: 79 additions & 12 deletions packages/contracts-bedrock/scripts/DeployOPChain.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { IProxyAdmin } from "src/universal/interfaces/IProxyAdmin.sol";
import { IProxy } from "src/universal/interfaces/IProxy.sol";

import { IAddressManager } from "src/legacy/interfaces/IAddressManager.sol";
import { IL1ChugSplashProxy } from "src/legacy/interfaces/IL1ChugSplashProxy.sol";
mds1 marked this conversation as resolved.
Show resolved Hide resolved
import { IDelayedWETH } from "src/dispute/interfaces/IDelayedWETH.sol";
import { IDisputeGameFactory } from "src/dispute/interfaces/IDisputeGameFactory.sol";
import { IAnchorStateRegistry } from "src/dispute/interfaces/IAnchorStateRegistry.sol";
Expand Down Expand Up @@ -171,7 +172,7 @@ contract DeployOPChainInput is BaseDeployIO {
function opcmProxy() public returns (OPContractsManager) {
require(address(_opcmProxy) != address(0), "DeployOPChainInput: not set");
DeployUtils.assertValidContractAddress(address(_opcmProxy));
DeployUtils.assertImplementationSet(address(_opcmProxy));
DeployUtils.assertERC1967ImplementationSet(address(_opcmProxy));
return _opcmProxy;
}

Expand Down Expand Up @@ -286,43 +287,51 @@ contract DeployOPChainOutput is BaseDeployIO {
return _addressManager;
}

function l1ERC721BridgeProxy() public view returns (IL1ERC721Bridge) {
function l1ERC721BridgeProxy() public returns (IL1ERC721Bridge) {
DeployUtils.assertValidContractAddress(address(_l1ERC721BridgeProxy));
DeployUtils.assertERC1967ImplementationSet(address(_l1ERC721BridgeProxy));
return _l1ERC721BridgeProxy;
}

function systemConfigProxy() public view returns (ISystemConfig) {
function systemConfigProxy() public returns (ISystemConfig) {
DeployUtils.assertValidContractAddress(address(_systemConfigProxy));
DeployUtils.assertERC1967ImplementationSet(address(_systemConfigProxy));
return _systemConfigProxy;
}

function optimismMintableERC20FactoryProxy() public view returns (IOptimismMintableERC20Factory) {
function optimismMintableERC20FactoryProxy() public returns (IOptimismMintableERC20Factory) {
DeployUtils.assertValidContractAddress(address(_optimismMintableERC20FactoryProxy));
DeployUtils.assertERC1967ImplementationSet(address(_optimismMintableERC20FactoryProxy));
return _optimismMintableERC20FactoryProxy;
}

function l1StandardBridgeProxy() public view returns (IL1StandardBridge) {
function l1StandardBridgeProxy() public returns (IL1StandardBridge) {
DeployUtils.assertValidContractAddress(address(_l1StandardBridgeProxy));
DeployUtils.assertL1ChugSplashImplementationSet(address(_l1StandardBridgeProxy));
return _l1StandardBridgeProxy;
}

function l1CrossDomainMessengerProxy() public view returns (IL1CrossDomainMessenger) {
DeployUtils.assertValidContractAddress(address(_l1CrossDomainMessengerProxy));
DeployUtils.assertResolvedDelegateProxyImplementationSet("OVM_L1CrossDomainMessenger", addressManager());
return _l1CrossDomainMessengerProxy;
}

function optimismPortalProxy() public view returns (IOptimismPortal2) {
function optimismPortalProxy() public returns (IOptimismPortal2) {
DeployUtils.assertValidContractAddress(address(_optimismPortalProxy));
DeployUtils.assertERC1967ImplementationSet(address(_optimismPortalProxy));
return _optimismPortalProxy;
}

function disputeGameFactoryProxy() public view returns (IDisputeGameFactory) {
function disputeGameFactoryProxy() public returns (IDisputeGameFactory) {
DeployUtils.assertValidContractAddress(address(_disputeGameFactoryProxy));
DeployUtils.assertERC1967ImplementationSet(address(_disputeGameFactoryProxy));
return _disputeGameFactoryProxy;
}

function anchorStateRegistryProxy() public view returns (IAnchorStateRegistry) {
function anchorStateRegistryProxy() public returns (IAnchorStateRegistry) {
DeployUtils.assertValidContractAddress(address(_anchorStateRegistryProxy));
DeployUtils.assertERC1967ImplementationSet(address(_anchorStateRegistryProxy));
return _anchorStateRegistryProxy;
}

Expand All @@ -341,8 +350,9 @@ contract DeployOPChainOutput is BaseDeployIO {
return _permissionedDisputeGame;
}

function delayedWETHPermissionedGameProxy() public view returns (IDelayedWETH) {
function delayedWETHPermissionedGameProxy() public returns (IDelayedWETH) {
DeployUtils.assertValidContractAddress(address(_delayedWETHPermissionedGameProxy));
DeployUtils.assertERC1967ImplementationSet(address(_delayedWETHPermissionedGameProxy));
return _delayedWETHPermissionedGameProxy;
}

Expand All @@ -366,6 +376,8 @@ contract DeployOPChainOutput is BaseDeployIO {
assertValidOptimismPortal(_doi);
assertValidPermissionedDisputeGame(_doi);
assertValidSystemConfig(_doi);
assertValidAddressManager(_doi);
assertValidOPChainProxyAdmin(_doi);
}

function assertValidPermissionedDisputeGame(DeployOPChainInput _doi) internal {
Expand Down Expand Up @@ -416,7 +428,7 @@ contract DeployOPChainOutput is BaseDeployIO {
require(Hash.unwrap(actualRoot) == expectedRoot, "ANCHORP-40");
}

function assertValidAnchorStateRegistryImpl(DeployOPChainInput) internal view {
function assertValidAnchorStateRegistryImpl(DeployOPChainInput) internal {
IAnchorStateRegistry registry = anchorStateRegistryImpl();

DeployUtils.assertInitialized({ _contractAddress: address(registry), _slot: 0, _offset: 0 });
Expand Down Expand Up @@ -492,7 +504,7 @@ contract DeployOPChainOutput is BaseDeployIO {
require(address(bridge.superchainConfig()) == address(_doi.opcmProxy().superchainConfig()), "L1SB-50");
}

function assertValidOptimismMintableERC20Factory(DeployOPChainInput) internal view {
function assertValidOptimismMintableERC20Factory(DeployOPChainInput) internal {
IOptimismMintableERC20Factory factory = optimismMintableERC20FactoryProxy();

DeployUtils.assertInitialized({ _contractAddress: address(factory), _slot: 0, _offset: 0 });
Expand Down Expand Up @@ -530,7 +542,7 @@ contract DeployOPChainOutput is BaseDeployIO {
require(vm.load(address(portal), bytes32(uint256(61))) == bytes32(0));
}

function assertValidDisputeGameFactory(DeployOPChainInput _doi) internal view {
function assertValidDisputeGameFactory(DeployOPChainInput _doi) internal {
IDisputeGameFactory factory = disputeGameFactoryProxy();

DeployUtils.assertInitialized({ _contractAddress: address(factory), _slot: 0, _offset: 0 });
Expand All @@ -551,6 +563,61 @@ contract DeployOPChainOutput is BaseDeployIO {
address admin = proxy.admin();
require(admin == address(opChainProxyAdmin()), "DWETH-20");
}

function assertValidAddressManager(DeployOPChainInput) internal view {
require(addressManager().owner() == address(opChainProxyAdmin()), "AM-10");
}

function assertValidOPChainProxyAdmin(DeployOPChainInput _doi) internal {
IProxyAdmin admin = opChainProxyAdmin();
require(admin.owner() == _doi.opChainProxyAdminOwner(), "OPCPA-10");
require(
admin.getProxyImplementation(address(l1CrossDomainMessengerProxy()))
== DeployUtils.assertResolvedDelegateProxyImplementationSet("OVM_L1CrossDomainMessenger", addressManager()),
"OPCPA-20"
);
require(address(admin.addressManager()) == address(addressManager()), "OPCPA-30");
require(
admin.getProxyImplementation(address(l1StandardBridgeProxy()))
== DeployUtils.assertL1ChugSplashImplementationSet(address(l1StandardBridgeProxy())),
"OPCPA-40"
);
require(
admin.getProxyImplementation(address(l1ERC721BridgeProxy()))
== DeployUtils.assertERC1967ImplementationSet(address(l1ERC721BridgeProxy())),
"OPCPA-50"
);
require(
admin.getProxyImplementation(address(optimismPortalProxy()))
== DeployUtils.assertERC1967ImplementationSet(address(optimismPortalProxy())),
"OPCPA-60"
);
require(
admin.getProxyImplementation(address(systemConfigProxy()))
== DeployUtils.assertERC1967ImplementationSet(address(systemConfigProxy())),
"OPCPA-70"
);
require(
admin.getProxyImplementation(address(optimismMintableERC20FactoryProxy()))
== DeployUtils.assertERC1967ImplementationSet(address(optimismMintableERC20FactoryProxy())),
"OPCPA-80"
);
require(
admin.getProxyImplementation(address(disputeGameFactoryProxy()))
== DeployUtils.assertERC1967ImplementationSet(address(disputeGameFactoryProxy())),
"OPCPA-90"
);
require(
admin.getProxyImplementation(address(delayedWETHPermissionedGameProxy()))
== DeployUtils.assertERC1967ImplementationSet(address(delayedWETHPermissionedGameProxy())),
"OPCPA-100"
);
require(
admin.getProxyImplementation(address(anchorStateRegistryProxy()))
== DeployUtils.assertERC1967ImplementationSet(address(anchorStateRegistryProxy())),
"OPCPA-110"
);
}
}

contract DeployOPChain is Script {
Expand Down
26 changes: 23 additions & 3 deletions packages/contracts-bedrock/scripts/libraries/DeployUtils.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import { Bytes } from "src/libraries/Bytes.sol";

// Interfaces
import { IProxy } from "src/universal/interfaces/IProxy.sol";
import { IAddressManager } from "src/legacy/interfaces/IAddressManager.sol";
import { IStaticL1ChugSplashProxy } from "src/legacy/interfaces/IL1ChugSplashProxy.sol";

library DeployUtils {
Vm internal constant vm = Vm(address(uint160(uint256(keccak256("hevm cheat code")))));
Expand Down Expand Up @@ -217,12 +219,30 @@ library DeployUtils {

/// @notice Asserts that the given proxy has an implementation set.
/// @param _proxy Proxy to check.
function assertImplementationSet(address _proxy) internal {
function assertERC1967ImplementationSet(address _proxy) internal returns (address implementation_) {
// We prank as the zero address due to the Proxy's `proxyCallIfNotAdmin` modifier.
// Pranking inside this function also means it can no longer be considered `view`.
vm.prank(address(0));
address implementation = IProxy(payable(_proxy)).implementation();
assertValidContractAddress(implementation);
implementation_ = IProxy(payable(_proxy)).implementation();
assertValidContractAddress(implementation_);
}

function assertL1ChugSplashImplementationSet(address _proxy) internal returns (address implementation_) {
vm.prank(address(0));
implementation_ = IStaticL1ChugSplashProxy(_proxy).getImplementation();
assertValidContractAddress(implementation_);
}

function assertResolvedDelegateProxyImplementationSet(
string memory _implementationName,
IAddressManager _addressManager
)
internal
view
returns (address implementation_)
{
implementation_ = _addressManager.getAddress(_implementationName);
assertValidContractAddress(implementation_);
}

/// @notice Asserts that the given addresses are valid contract addresses.
Expand Down
6 changes: 3 additions & 3 deletions packages/contracts-bedrock/test/opcm/DeployOPChain.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,9 +558,9 @@ contract DeployOPChain_Test is DeployOPChain_TestBase {
assertEq(doo.permissionedDisputeGame().splitDepth(), 30, "3400");
assertEq(doo.permissionedDisputeGame().maxGameDepth(), 73, "3500");

// Most architecture assertions are handled within the OP Contracts Manager itself and therefore
// we only assert on the things that are not visible onchain.
// TODO add these assertions: AddressManager, Proxy, ProxyAdmin, etc.
assertEq(address(doo.opChainProxyAdmin().addressManager().owner()), address(doo.opChainProxyAdmin()), "3600");
assertEq(address(doo.opChainProxyAdmin().addressManager()), address(doo.addressManager()), "3700");
assertEq(address(doo.opChainProxyAdmin().owner()), opChainProxyAdminOwner, "3800");
}
}

Expand Down