Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions .semgrep/rules/sol-rules.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -388,3 +388,13 @@ rules:
# LegacyMintableERC20 and the corresponding interface use legacy naming conventions.
- packages/contracts-bedrock/src/legacy/LegacyMintableERC20.sol
- packages/contracts-bedrock/interfaces/legacy/ILegacyMintableERC20Full.sol

- id: sol-safety-opcmv2-use-internal-version
languages: [generic]
severity: ERROR
message: Use _version() instead of version() in OPContractsManagerV2. Direct calls to version() are not properly mockable/testable due to DELEGATECALL context.
patterns:
- pattern-regex: (?<!\.)(?<!function\s)(?<!_)version\(\)
paths:
include:
- packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
contract SemgrepTest__sol_safety_opcmv2_use_internal_version {
function badUsage() public view returns (string memory) {
// ruleid: sol-safety-opcmv2-use-internal-version
return version();
}

// ok: sol-safety-opcmv2-use-internal-version
function version() public pure returns (string memory) {
return "1.0.0";
}

function _version() internal view returns (string memory) {
// ok: sol-safety-opcmv2-use-internal-version
return thisOPCM.version();
}

function goodUsage() public view returns (string memory) {
// ok: sol-safety-opcmv2-use-internal-version
return _version();
}

function externalCall() public view returns (string memory) {
// ok: sol-safety-opcmv2-use-internal-version
return someContract.version();
}
}
2 changes: 1 addition & 1 deletion packages/contracts-bedrock/checks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ phases:
command: forge fmt --check
- name: semgrep
description: Run semgrep security linter
command: cd ../../ && semgrep scan --config .semgrep/rules/ ./packages/contracts-bedrock
command: cd ../../ && semgrep scan --error --config .semgrep/rules/ ./packages/contracts-bedrock
- name: semgrep-test-validity
description: Check semgrep tests are valid
command: forge fmt ../../.semgrep/tests/sol-rules.t.sol --check
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ interface IOPContractsManagerV2 {
error OPContractsManagerV2_UnsupportedGameType();
error OPContractsManagerV2_InvalidUpgradeInstruction(string _key);
error OPContractsManagerV2_CannotUpgradeToCustomGasToken();
error OPContractsManagerV2_InvalidUpgradeSequence(string _lastVersion, string _thisVersion);
error IdentityPrecompileCallFailed();
error ReservedBitsSet();
error BytesArrayTooLong();
Expand Down Expand Up @@ -148,4 +149,7 @@ interface IOPContractsManagerV2 {

/// @notice Returns whether a development feature is enabled.
function isDevFeatureEnabled(bytes32 _feature) external view returns (bool);

/// @notice Checks if the upgrade sequence from the last used OPCM to this OPCM is permitted.
function isPermittedUpgradeSequence(ISystemConfig _systemConfig) external view returns (bool);
}
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,25 @@
"stateMutability": "view",
"type": "function"
},
{
"inputs": [
{
"internalType": "contract ISystemConfig",
"name": "_systemConfig",
"type": "address"
}
],
"name": "isPermittedUpgradeSequence",
"outputs": [
{
"internalType": "bool",
"name": "",
"type": "bool"
}
],
"stateMutability": "view",
"type": "function"
},
{
"inputs": [],
"name": "standardValidator",
Expand Down Expand Up @@ -657,7 +676,7 @@
"type": "string"
}
],
"stateMutability": "view",
"stateMutability": "pure",
"type": "function"
},
{
Expand Down Expand Up @@ -711,6 +730,22 @@
"name": "OPContractsManagerV2_InvalidUpgradeInstruction",
"type": "error"
},
{
"inputs": [
{
"internalType": "string",
"name": "_lastVersion",
"type": "string"
},
{
"internalType": "string",
"name": "_thisVersion",
"type": "string"
}
],
"name": "OPContractsManagerV2_InvalidUpgradeSequence",
"type": "error"
},
{
"inputs": [],
"name": "OPContractsManagerV2_SuperchainConfigNeedsUpgrade",
Expand Down
4 changes: 2 additions & 2 deletions packages/contracts-bedrock/snapshots/semver-lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@
"sourceCodeHash": "0xb3184aa5d95a82109e7134d1f61941b30e25f655b9849a0e303d04bbce0cde0b"
},
"src/L1/opcm/OPContractsManagerV2.sol:OPContractsManagerV2": {
"initCodeHash": "0x08ce57f50f66685b62b09597a30b5080a140bb3c3cb4f282524200f96c321996",
"sourceCodeHash": "0x0a49fca3800abd9da52ca7fccb018af451c7f988328c826f9a0acb80395bff15"
"initCodeHash": "0xa2e3af90e9ff372b737f7eba83d20e44ceb22ed990d290d98b32d2ecb1316101",
"sourceCodeHash": "0x3333dd6ca2d7a0575ca9a7c10b79b0a38de8ea0aa550f98b99c11ba1f6b93c84"
},
"src/L2/BaseFeeVault.sol:BaseFeeVault": {
"initCodeHash": "0x838bbd7f381e84e21887f72bd1da605bfc4588b3c39aed96cbce67c09335b3ee",
Expand Down
68 changes: 65 additions & 3 deletions packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,9 @@ contract OPContractsManagerV2 is ISemver, OPContractsManagerUtilsCaller {
/// @notice Thrown when a chain attempts to upgrade to custom gas token after initial deployment.
error OPContractsManagerV2_CannotUpgradeToCustomGasToken();

/// @notice Thrown when an invalid upgrade sequence is provided.
error OPContractsManagerV2_InvalidUpgradeSequence(string _lastVersion, string _thisVersion);

/// @notice Container of blueprint and implementation contract addresses.
IOPContractsManagerContainer public immutable contractsContainer;

Expand All @@ -166,8 +169,10 @@ contract OPContractsManagerV2 is ISemver, OPContractsManagerUtilsCaller {
/// - Major bump: New required sequential upgrade
/// - Minor bump: Replacement OPCM for same upgrade
/// - Patch bump: Development changes (expected for normal dev work)
/// @custom:semver 6.0.6
string public constant version = "6.0.6";
/// @custom:semver 6.0.7
function version() public pure returns (string memory) {
return "6.0.7";
}

/// @param _contractsContainer The container of blueprint and implementation contract addresses.
/// @param _standardValidator The standard validator for this OPCM release.
Expand Down Expand Up @@ -299,7 +304,7 @@ contract OPContractsManagerV2 is ISemver, OPContractsManagerUtilsCaller {
// developers start working on the next release this will automatically become false so
// even if the code is somehow forgotten it will not actually apply to the deployment. Make
// sure to REMOVE the allowance once the upgrade is complete.
if (SemverComp.lt(version, "7.0.0")) {
if (SemverComp.lt(_version(), "7.0.0")) {
// Unified DelayedWETH is being deployed for the first time.
// TODO:(#18382): Remove this allowance after unified DelayedWETH is deployed.
if (_isMatchingInstruction(_instruction, Constants.PERMITTED_PROXY_DEPLOYMENT_KEY, "DelayedWETH")) {
Expand Down Expand Up @@ -698,6 +703,15 @@ contract OPContractsManagerV2 is ISemver, OPContractsManagerUtilsCaller {
revert OPContractsManagerV2_SuperchainConfigNeedsUpgrade();
}

// Chains prior to OPCMv2 don't yet have a functional lastUsedOPCMVersion function on the
// SystemConfig contract. The first deployment of OPCMv2 will make this function available
// and subsequent deployments will then use this function to verify the system is
// progressing from one OPCM to the next. We only care about this for upgrades, you can
// perform an initial deployment from any OPCM.
if (!_isInitialDeployment && !isPermittedUpgradeSequence(_cts.systemConfig)) {
revert OPContractsManagerV2_InvalidUpgradeSequence(_cts.systemConfig.lastUsedOPCMVersion(), _version());
}

// Update the SystemConfig.
// SystemConfig initializer is the only one large enough to require a separate function to
// avoid stack-too-deep errors.
Expand Down Expand Up @@ -978,6 +992,42 @@ contract OPContractsManagerV2 is ISemver, OPContractsManagerUtilsCaller {
// PUBLIC UTILITY FUNCTIONS //
///////////////////////////////////////////////////////////////////////////

/// @notice Checks if the upgrade sequence from the last used OPCM to this OPCM is permitted.
/// This function is public to allow tests to verify upgrade sequence logic directly
/// by mocking the OPCM version and calling this function, rather than running the full
/// upgrade flow. This avoids the need for special testing flags that bypass validation.
/// @param _systemConfig The SystemConfig contract to check the upgrade sequence for.
/// @return True if the upgrade sequence is permitted, false otherwise.
function isPermittedUpgradeSequence(ISystemConfig _systemConfig) public view returns (bool) {
// If the SystemConfig is not initialized, this is an initial deployment, which is always
// permitted. Initial deployments can use any OPCM version.
if (address(_systemConfig) == address(0)) {
return true;
}

// Chains prior to OPCMv2 (version 7.0.0) don't have a functional lastUsedOPCM function on
// the SystemConfig contract. The first deployment of OPCMv2 makes this function available,
// so we skip this check for versions below 7.0.0.
if (SemverComp.lt(_version(), "7.0.0")) {
return true;
}

ISemver lastUsedOPCM = ISemver(address(_systemConfig.lastUsedOPCM()));
SemverComp.Semver memory lastUsedSemver = SemverComp.parse(lastUsedOPCM.version());
SemverComp.Semver memory thisSemver = SemverComp.parse(_version());

// We have three permitted cases:
// 1. Address of the last used OPCM is identical to the address of this OPCM (re-running).
// 2. This OPCM version is the same major version but a greater minor version (patch).
// 3. This OPCM version is the next major version (sequential upgrade).
bool isSameOPCM = address(lastUsedOPCM) == address(thisOPCM);
bool isNextMajor = thisSemver.major == lastUsedSemver.major + 1;
bool isSameMajorHigherMinor =
thisSemver.major == lastUsedSemver.major && thisSemver.minor > lastUsedSemver.minor;

return isSameOPCM || isSameMajorHigherMinor || isNextMajor;
}

/// @notice Returns the blueprint contract addresses.
function blueprints() public view returns (IOPContractsManagerContainer.Blueprints memory) {
return contractsContainer.blueprints();
Expand All @@ -994,4 +1044,16 @@ contract OPContractsManagerV2 is ISemver, OPContractsManagerUtilsCaller {
function isDevFeatureEnabled(bytes32 _feature) public view returns (bool) {
return contractsContainer.isDevFeatureEnabled(_feature);
}

///////////////////////////////////////////////////////////////////////////
// INTERNAL UTILITY FUNCTIONS //
///////////////////////////////////////////////////////////////////////////

/// @notice Helper for retrieving the version of the OPCM contract.
/// @dev We use thisOPCM.version() because it allows us to properly mock the version function
/// in tests without running into issues because this contract is being DELEGATECALLed.
/// @return The version of the OPCM contract.
function _version() internal view returns (string memory) {
return thisOPCM.version();
}
}
Loading