Standard L2 Genesis#12057
Conversation
| } | ||
|
|
||
| /// @notice | ||
| function encodeSetRemoteChainId(uint256 _chainId) internal pure returns (bytes memory) { |
There was a problem hiding this comment.
Named return arguments to functions must be appended with an underscore (_)
| } | ||
|
|
||
| /// @notice | ||
| function encodeSetAddress(address _address) internal pure returns (bytes memory) { |
There was a problem hiding this comment.
Named return arguments to functions must be appended with an underscore (_)
| } | ||
|
|
||
| /// @notice | ||
| function encodeSetFeeVaultConfig(address _recipient, uint256 _min, FeeVault.WithdrawalNetwork _network) internal pure returns (bytes memory) { |
There was a problem hiding this comment.
Named return arguments to functions must be appended with an underscore (_)
|
Semgrep found 1 Named return arguments to functions must be appended with an underscore ( |
73c3220 to
1eb9745
Compare
|
Semgrep found 2 No Semgrep found 6
Named return arguments to functions must be appended with an underscore ( Semgrep found 2 require() must include a reason string Ignore this finding from sol-style-require-reason.Semgrep found 12
_args parameter should be wrapped with DeployUtils.encodeConstructor Ignore this finding from sol-safety-deployutils-args. |
| @@ -0,0 +1,9 @@ | |||
| // SPDX-License-Identifier: MIT | |||
| pragma solidity 0.8.15; | |||
|
|
|||
There was a problem hiding this comment.
Consider refactoring the ProxyAdmin to remove the L1 specific features and make a L1ProxyAdmin
| vm.store(Predeploys.PROXY_ADMIN, _ownerSlot, bytes32(uint256(uint160(cfg.proxyAdminOwner())))); | ||
| // update the proxy to not be uninitialized (although not standard initialize pattern) | ||
| vm.store(impl, _ownerSlot, bytes32(uint256(uint160(cfg.proxyAdminOwner())))); | ||
| vm.store(impl, bytes32(0), bytes32(uint256(0xdead))); |
There was a problem hiding this comment.
todo: double check if this is necessary, this adds complexity to the genesis definition
There was a problem hiding this comment.
I can't think of why it would be necessary... is there something in storage at that slot on OP Mainnet?
|
Semgrep found 6
require() must include a reason string Ignore this finding from sol-style-require-reason. |
59db8a6 to
14d10ef
Compare
| checkStorageSlot(t, alloc, predeploys.ProxyAdminAddr, ownerSlot, addrAsSlot) | ||
| var defaultGovOwner common.Hash | ||
| defaultGovOwner.SetBytes(common.HexToAddress("0xDeaDDEaDDeAdDeAdDEAdDEaddeAddEAdDEAdDEad").Bytes()) | ||
| checkStorageSlot(t, alloc, predeploys.GovernanceTokenAddr, common.Hash{31: 0x0a}, defaultGovOwner) |
|
Going to close this PR due to it being replaced by another |
Description
Moves network specific values into L1Block.sol.
Fixes: #12297, #12349, #12298, #12299, #12300, #12301, #12303, #12304, #12305, #12655
Open items:
test_upgrade_correctEvent_succeedsfeeAdminto op-deployerSystemConfig.Rolesstruct should also contain theunsafeBlockSignerRolesstruct inOPCM. This is a bit confusing, can we disambiguate it?Cut from scope: