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

fix(ct): correct input and return argument formatting #12131

Merged
merged 1 commit into from
Sep 25, 2024
Merged

fix(ct): correct input and return argument formatting #12131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 17 additions & 1 deletion .semgrepignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,25 @@ tests/
.semgrep_logs/

op-chain-ops/script/testdata
op-chain-ops/script/testdata/scripts/ScriptExample.s.sol

packages/*/node_modules
packages/*/test

# Autogenerated solidity library
# TODO: Define these exclusions inside of the semgrep rules once those rules
# are all defined locally in the repository instead of the semgrep app.

# Contracts: autogenerated solidity library
packages/contracts-bedrock/scripts/libraries/Solarray.sol

# Contracts: vendor interfaces
packages/contracts-bedrock/scripts/interfaces/IGnosisSafe.sol
packages/contracts-bedrock/src/EAS/

# Contracts: deliberate exclusions
packages/contracts-bedrock/src/universal/WETH98.sol
packages/contracts-bedrock/src/universal/interfaces/IWETH.sol
packages/contracts-bedrock/src/L2/SuperchainWETH.sol
packages/contracts-bedrock/src/L2/interfaces/ISuperchainWETH.sol
packages/contracts-bedrock/src/governance/GovernanceToken.sol
packages/contracts-bedrock/src/governance/interfaces/IGovernanceToken.sol
4 changes: 2 additions & 2 deletions packages/contracts-bedrock/scripts/DeployAuthSystem.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ contract DeployAuthSystemInput is CommonBase {
contract DeployAuthSystemOutput is CommonBase {
Safe internal _safe;

function set(bytes4 sel, address _address) public {
if (sel == this.safe.selector) _safe = Safe(payable(_address));
function set(bytes4 _sel, address _address) public {
if (_sel == this.safe.selector) _safe = Safe(payable(_address));
else revert("DeployAuthSystemOutput: unknown selector");
}

Expand Down
54 changes: 27 additions & 27 deletions packages/contracts-bedrock/scripts/DeployImplementations.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,41 +64,41 @@ contract DeployImplementationsInput is BaseDeployIO {

string internal _standardVersionsToml;

function set(bytes4 sel, uint256 _value) public {
function set(bytes4 _sel, uint256 _value) public {
require(_value != 0, "DeployImplementationsInput: cannot set zero value");

if (sel == this.withdrawalDelaySeconds.selector) {
if (_sel == this.withdrawalDelaySeconds.selector) {
_withdrawalDelaySeconds = _value;
} else if (sel == this.minProposalSizeBytes.selector) {
} else if (_sel == this.minProposalSizeBytes.selector) {
_minProposalSizeBytes = _value;
} else if (sel == this.challengePeriodSeconds.selector) {
} else if (_sel == this.challengePeriodSeconds.selector) {
require(_value <= type(uint64).max, "DeployImplementationsInput: challengePeriodSeconds too large");
_challengePeriodSeconds = _value;
} else if (sel == this.proofMaturityDelaySeconds.selector) {
} else if (_sel == this.proofMaturityDelaySeconds.selector) {
_proofMaturityDelaySeconds = _value;
} else if (sel == this.disputeGameFinalityDelaySeconds.selector) {
} else if (_sel == this.disputeGameFinalityDelaySeconds.selector) {
_disputeGameFinalityDelaySeconds = _value;
} else {
revert("DeployImplementationsInput: unknown selector");
}
}

function set(bytes4 sel, string memory _value) public {
function set(bytes4 _sel, string memory _value) public {
require(!LibString.eq(_value, ""), "DeployImplementationsInput: cannot set empty string");
if (sel == this.release.selector) _release = _value;
else if (sel == this.standardVersionsToml.selector) _standardVersionsToml = _value;
if (_sel == this.release.selector) _release = _value;
else if (_sel == this.standardVersionsToml.selector) _standardVersionsToml = _value;
else revert("DeployImplementationsInput: unknown selector");
}

function set(bytes4 sel, address _addr) public {
function set(bytes4 _sel, address _addr) public {
require(_addr != address(0), "DeployImplementationsInput: cannot set zero address");
if (sel == this.superchainConfigProxy.selector) _superchainConfigProxy = SuperchainConfig(_addr);
else if (sel == this.protocolVersionsProxy.selector) _protocolVersionsProxy = ProtocolVersions(_addr);
if (_sel == this.superchainConfigProxy.selector) _superchainConfigProxy = SuperchainConfig(_addr);
else if (_sel == this.protocolVersionsProxy.selector) _protocolVersionsProxy = ProtocolVersions(_addr);
else revert("DeployImplementationsInput: unknown selector");
}

function set(bytes4 sel, bytes32 _value) public {
if (sel == this.salt.selector) _salt = _value;
function set(bytes4 _sel, bytes32 _value) public {
if (_sel == this.salt.selector) _salt = _value;
else revert("DeployImplementationsInput: unknown selector");
}

Expand Down Expand Up @@ -179,22 +179,22 @@ contract DeployImplementationsOutput is BaseDeployIO {
OptimismMintableERC20Factory internal _optimismMintableERC20FactoryImpl;
DisputeGameFactory internal _disputeGameFactoryImpl;

function set(bytes4 sel, address _addr) public {
function set(bytes4 _sel, address _addr) public {
require(_addr != address(0), "DeployImplementationsOutput: cannot set zero address");

// forgefmt: disable-start
if (sel == this.opcmProxy.selector) _opcmProxy = OPContractsManager(payable(_addr));
else if (sel == this.opcmImpl.selector) _opcmImpl = OPContractsManager(payable(_addr));
else if (sel == this.optimismPortalImpl.selector) _optimismPortalImpl = OptimismPortal2(payable(_addr));
else if (sel == this.delayedWETHImpl.selector) _delayedWETHImpl = DelayedWETH(payable(_addr));
else if (sel == this.preimageOracleSingleton.selector) _preimageOracleSingleton = PreimageOracle(_addr);
else if (sel == this.mipsSingleton.selector) _mipsSingleton = MIPS(_addr);
else if (sel == this.systemConfigImpl.selector) _systemConfigImpl = SystemConfig(_addr);
else if (sel == this.l1CrossDomainMessengerImpl.selector) _l1CrossDomainMessengerImpl = L1CrossDomainMessenger(_addr);
else if (sel == this.l1ERC721BridgeImpl.selector) _l1ERC721BridgeImpl = L1ERC721Bridge(_addr);
else if (sel == this.l1StandardBridgeImpl.selector) _l1StandardBridgeImpl = L1StandardBridge(payable(_addr));
else if (sel == this.optimismMintableERC20FactoryImpl.selector) _optimismMintableERC20FactoryImpl = OptimismMintableERC20Factory(_addr);
else if (sel == this.disputeGameFactoryImpl.selector) _disputeGameFactoryImpl = DisputeGameFactory(_addr);
if (_sel == this.opcmProxy.selector) _opcmProxy = OPContractsManager(payable(_addr));
else if (_sel == this.opcmImpl.selector) _opcmImpl = OPContractsManager(payable(_addr));
else if (_sel == this.optimismPortalImpl.selector) _optimismPortalImpl = OptimismPortal2(payable(_addr));
else if (_sel == this.delayedWETHImpl.selector) _delayedWETHImpl = DelayedWETH(payable(_addr));
else if (_sel == this.preimageOracleSingleton.selector) _preimageOracleSingleton = PreimageOracle(_addr);
else if (_sel == this.mipsSingleton.selector) _mipsSingleton = MIPS(_addr);
else if (_sel == this.systemConfigImpl.selector) _systemConfigImpl = SystemConfig(_addr);
else if (_sel == this.l1CrossDomainMessengerImpl.selector) _l1CrossDomainMessengerImpl = L1CrossDomainMessenger(_addr);
else if (_sel == this.l1ERC721BridgeImpl.selector) _l1ERC721BridgeImpl = L1ERC721Bridge(_addr);
else if (_sel == this.l1StandardBridgeImpl.selector) _l1StandardBridgeImpl = L1StandardBridge(payable(_addr));
else if (_sel == this.optimismMintableERC20FactoryImpl.selector) _optimismMintableERC20FactoryImpl = OptimismMintableERC20Factory(_addr);
else if (_sel == this.disputeGameFactoryImpl.selector) _disputeGameFactoryImpl = DisputeGameFactory(_addr);
else revert("DeployImplementationsOutput: unknown selector");
// forgefmt: disable-end
}
Expand Down
32 changes: 16 additions & 16 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 @@ -164,24 +164,24 @@ contract DeployOPChainOutput is BaseDeployIO {
DelayedWETH internal _delayedWETHPermissionedGameProxy;
DelayedWETH internal _delayedWETHPermissionlessGameProxy;

function set(bytes4 sel, address _addr) public {
function set(bytes4 _sel, address _addr) public {
require(_addr != address(0), "DeployOPChainOutput: cannot set zero address");
// forgefmt: disable-start
if (sel == this.opChainProxyAdmin.selector) _opChainProxyAdmin = ProxyAdmin(_addr) ;
else if (sel == this.addressManager.selector) _addressManager = AddressManager(_addr) ;
else if (sel == this.l1ERC721BridgeProxy.selector) _l1ERC721BridgeProxy = L1ERC721Bridge(_addr) ;
else if (sel == this.systemConfigProxy.selector) _systemConfigProxy = SystemConfig(_addr) ;
else if (sel == this.optimismMintableERC20FactoryProxy.selector) _optimismMintableERC20FactoryProxy = OptimismMintableERC20Factory(_addr) ;
else if (sel == this.l1StandardBridgeProxy.selector) _l1StandardBridgeProxy = L1StandardBridge(payable(_addr)) ;
else if (sel == this.l1CrossDomainMessengerProxy.selector) _l1CrossDomainMessengerProxy = L1CrossDomainMessenger(_addr) ;
else if (sel == this.optimismPortalProxy.selector) _optimismPortalProxy = OptimismPortal2(payable(_addr)) ;
else if (sel == this.disputeGameFactoryProxy.selector) _disputeGameFactoryProxy = DisputeGameFactory(_addr) ;
else if (sel == this.anchorStateRegistryProxy.selector) _anchorStateRegistryProxy = AnchorStateRegistry(_addr) ;
else if (sel == this.anchorStateRegistryImpl.selector) _anchorStateRegistryImpl = AnchorStateRegistry(_addr) ;
else if (sel == this.faultDisputeGame.selector) _faultDisputeGame = FaultDisputeGame(_addr) ;
else if (sel == this.permissionedDisputeGame.selector) _permissionedDisputeGame = PermissionedDisputeGame(_addr) ;
else if (sel == this.delayedWETHPermissionedGameProxy.selector) _delayedWETHPermissionedGameProxy = DelayedWETH(payable(_addr)) ;
else if (sel == this.delayedWETHPermissionlessGameProxy.selector) _delayedWETHPermissionlessGameProxy = DelayedWETH(payable(_addr)) ;
if (_sel == this.opChainProxyAdmin.selector) _opChainProxyAdmin = ProxyAdmin(_addr) ;
else if (_sel == this.addressManager.selector) _addressManager = AddressManager(_addr) ;
else if (_sel == this.l1ERC721BridgeProxy.selector) _l1ERC721BridgeProxy = L1ERC721Bridge(_addr) ;
else if (_sel == this.systemConfigProxy.selector) _systemConfigProxy = SystemConfig(_addr) ;
else if (_sel == this.optimismMintableERC20FactoryProxy.selector) _optimismMintableERC20FactoryProxy = OptimismMintableERC20Factory(_addr) ;
else if (_sel == this.l1StandardBridgeProxy.selector) _l1StandardBridgeProxy = L1StandardBridge(payable(_addr)) ;
else if (_sel == this.l1CrossDomainMessengerProxy.selector) _l1CrossDomainMessengerProxy = L1CrossDomainMessenger(_addr) ;
else if (_sel == this.optimismPortalProxy.selector) _optimismPortalProxy = OptimismPortal2(payable(_addr)) ;
else if (_sel == this.disputeGameFactoryProxy.selector) _disputeGameFactoryProxy = DisputeGameFactory(_addr) ;
else if (_sel == this.anchorStateRegistryProxy.selector) _anchorStateRegistryProxy = AnchorStateRegistry(_addr) ;
else if (_sel == this.anchorStateRegistryImpl.selector) _anchorStateRegistryImpl = AnchorStateRegistry(_addr) ;
else if (_sel == this.faultDisputeGame.selector) _faultDisputeGame = FaultDisputeGame(_addr) ;
else if (_sel == this.permissionedDisputeGame.selector) _permissionedDisputeGame = PermissionedDisputeGame(_addr) ;
else if (_sel == this.delayedWETHPermissionedGameProxy.selector) _delayedWETHPermissionedGameProxy = DelayedWETH(payable(_addr)) ;
else if (_sel == this.delayedWETHPermissionlessGameProxy.selector) _delayedWETHPermissionlessGameProxy = DelayedWETH(payable(_addr)) ;
else revert("DeployOPChainOutput: unknown selector");
// forgefmt: disable-end
}
Expand Down
12 changes: 6 additions & 6 deletions packages/contracts-bedrock/scripts/DeploySuperchain.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,13 +164,13 @@ contract DeploySuperchainOutput is BaseDeployIO {

// This method lets each field be set individually. The selector of an output's getter method
// is used to determine which field to set.
function set(bytes4 sel, address _address) public {
function set(bytes4 _sel, address _address) public {
require(_address != address(0), "DeploySuperchainOutput: cannot set zero address");
if (sel == this.superchainProxyAdmin.selector) _superchainProxyAdmin = ProxyAdmin(_address);
else if (sel == this.superchainConfigImpl.selector) _superchainConfigImpl = SuperchainConfig(_address);
else if (sel == this.superchainConfigProxy.selector) _superchainConfigProxy = SuperchainConfig(_address);
else if (sel == this.protocolVersionsImpl.selector) _protocolVersionsImpl = ProtocolVersions(_address);
else if (sel == this.protocolVersionsProxy.selector) _protocolVersionsProxy = ProtocolVersions(_address);
if (_sel == this.superchainProxyAdmin.selector) _superchainProxyAdmin = ProxyAdmin(_address);
else if (_sel == this.superchainConfigImpl.selector) _superchainConfigImpl = SuperchainConfig(_address);
else if (_sel == this.superchainConfigProxy.selector) _superchainConfigProxy = SuperchainConfig(_address);
else if (_sel == this.protocolVersionsImpl.selector) _protocolVersionsImpl = ProtocolVersions(_address);
else if (_sel == this.protocolVersionsProxy.selector) _protocolVersionsProxy = ProtocolVersions(_address);
else revert("DeploySuperchainOutput: unknown selector");
}

Expand Down
7 changes: 5 additions & 2 deletions packages/contracts-bedrock/scripts/checks/check-semver-diff.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ temp_dir=$(mktemp -d)
trap 'rm -rf "$temp_dir"' EXIT

# Exit early if semver-lock.json has not changed.
if ! git diff origin/develop...HEAD --name-only | grep -q "$SEMVER_LOCK"; then
if ! { git diff origin/develop...HEAD --name-only; git diff --name-only; git diff --cached --name-only; } | grep -q "$SEMVER_LOCK"; then
echo "No changes detected in semver-lock.json"
exit 0
fi
Expand Down Expand Up @@ -71,9 +71,12 @@ for contract in $changed_contracts; do
has_errors=true
fi

# TODO: Use an existing semver comparison function since this will only
# check if the version has changed at all and not that the version has
# increased properly.
# Check if the version changed.
if [ "$old_version" = "$new_version" ]; then
echo "❌ Error: src/$contract has changes in semver-lock.json but no version change"
echo "❌ Error: $contract has changes in semver-lock.json but no version change"
echo " Old version: $old_version"
echo " New version: $new_version"
has_errors=true
Expand Down
6 changes: 3 additions & 3 deletions packages/contracts-bedrock/scripts/deploy/Deploy.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,11 +281,11 @@ contract Deploy is Deployer {
/// @notice Deploy a new OP Chain using an existing SuperchainConfig and ProtocolVersions
/// @param _superchainConfigProxy Address of the existing SuperchainConfig proxy
/// @param _protocolVersionsProxy Address of the existing ProtocolVersions proxy
/// @param includeDump Whether to include a state dump after deployment
/// @param _includeDump Whether to include a state dump after deployment
function runWithSuperchain(
address payable _superchainConfigProxy,
address payable _protocolVersionsProxy,
bool includeDump
bool _includeDump
)
public
{
Expand All @@ -306,7 +306,7 @@ contract Deploy is Deployer {

_run(false);

if (includeDump) {
if (_includeDump) {
vm.dumpState(Config.stateDumpPath(""));
}
}
Expand Down
56 changes: 36 additions & 20 deletions packages/contracts-bedrock/scripts/deploy/DeployConfig.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,8 @@ contract DeployConfig is Script {

function read(string memory _path) public {
console.log("DeployConfig: reading file %s", _path);
try vm.readFile(_path) returns (string memory data) {
_json = data;
try vm.readFile(_path) returns (string memory data_) {
_json = data_;
} catch {
require(false, string.concat("Cannot find deploy config file at ", _path));
}
Expand Down Expand Up @@ -191,14 +191,14 @@ contract DeployConfig is Script {
}

function l1StartingBlockTag() public returns (bytes32) {
try vm.parseJsonBytes32(_json, "$.l1StartingBlockTag") returns (bytes32 tag) {
return tag;
try vm.parseJsonBytes32(_json, "$.l1StartingBlockTag") returns (bytes32 tag_) {
return tag_;
} catch {
try vm.parseJsonString(_json, "$.l1StartingBlockTag") returns (string memory tag) {
return _getBlockByTag(tag);
try vm.parseJsonString(_json, "$.l1StartingBlockTag") returns (string memory tag_) {
return _getBlockByTag(tag_);
} catch {
try vm.parseJsonUint(_json, "$.l1StartingBlockTag") returns (uint256 tag) {
return _getBlockByTag(vm.toString(tag));
try vm.parseJsonUint(_json, "$.l1StartingBlockTag") returns (uint256 tag_) {
return _getBlockByTag(vm.toString(tag_));
} catch { }
}
}
Expand Down Expand Up @@ -266,32 +266,48 @@ contract DeployConfig is Script {
return abi.decode(res, (bytes32));
}

function _readOr(string memory json, string memory key, bool defaultValue) internal view returns (bool) {
return vm.keyExistsJson(json, key) ? json.readBool(key) : defaultValue;
function _readOr(string memory _jsonInp, string memory _key, bool _defaultValue) internal view returns (bool) {
return vm.keyExistsJson(_jsonInp, _key) ? _jsonInp.readBool(_key) : _defaultValue;
}

function _readOr(string memory json, string memory key, uint256 defaultValue) internal view returns (uint256) {
return (vm.keyExistsJson(json, key) && !_isNull(json, key)) ? json.readUint(key) : defaultValue;
function _readOr(
string memory _jsonInp,
string memory _key,
uint256 _defaultValue
)
internal
view
returns (uint256)
{
return (vm.keyExistsJson(_jsonInp, _key) && !_isNull(_json, _key)) ? _jsonInp.readUint(_key) : _defaultValue;
}

function _readOr(string memory json, string memory key, address defaultValue) internal view returns (address) {
return vm.keyExistsJson(json, key) ? json.readAddress(key) : defaultValue;
function _readOr(
string memory _jsonInp,
string memory _key,
address _defaultValue
)
internal
view
returns (address)
{
return vm.keyExistsJson(_jsonInp, _key) ? _jsonInp.readAddress(_key) : _defaultValue;
}

function _isNull(string memory json, string memory key) internal pure returns (bool) {
string memory value = json.readString(key);
function _isNull(string memory _jsonInp, string memory _key) internal pure returns (bool) {
string memory value = _jsonInp.readString(_key);
return (keccak256(bytes(value)) == keccak256(bytes("null")));
}

function _readOr(
string memory json,
string memory key,
string memory defaultValue
string memory _jsonInp,
string memory _key,
string memory _defaultValue
)
internal
view
returns (string memory)
{
return vm.keyExists(json, key) ? json.readString(key) : defaultValue;
return vm.keyExists(_jsonInp, _key) ? _jsonInp.readString(_key) : _defaultValue;
}
}
Loading