Commit 7562e4c9 authored by Delweng's avatar Delweng Committed by GitHub

feat(contracts): add more require revert error message (#12702)

* ct/scripts/ChainAssertions: add require revert message
Signed-off-by: default avatarjsvisa <delweng@gmail.com>

* ct/scripts/deploy/Deploy: add require revert message
Signed-off-by: default avatarjsvisa <delweng@gmail.com>

* ct/scripts/deploy/DeployOwnership: add require revert message
Signed-off-by: default avatarjsvisa <delweng@gmail.com>

* ct/scripts/deploy/DeployPeriphery: add require revert message
Signed-off-by: default avatarjsvisa <delweng@gmail.com>

* feat(semgrep): enable for all solidity files, except WETH9
Signed-off-by: default avatarjsvisa <delweng@gmail.com>

* fix require issues
Signed-off-by: default avatarjsvisa <delweng@gmail.com>

* fix(semgrep): run just semgrep to fix the require rules
Signed-off-by: default avatarjsvisa <delweng@gmail.com>

---------
Signed-off-by: default avatarjsvisa <delweng@gmail.com>
parent d5439094
...@@ -178,7 +178,7 @@ rules: ...@@ -178,7 +178,7 @@ rules:
- pattern-not: require($ERR, $MSG); - pattern-not: require($ERR, $MSG);
paths: paths:
exclude: exclude:
- packages/contracts-bedrock/ - packages/contracts-bedrock/src/universal/WETH98.sol
- id: sol-style-no-bare-imports - id: sol-style-no-bare-imports
languages: [solidity] languages: [solidity]
......
...@@ -44,7 +44,7 @@ library ChainAssertions { ...@@ -44,7 +44,7 @@ library ChainAssertions {
console.log("Running post-deploy assertions"); console.log("Running post-deploy assertions");
IResourceMetering.ResourceConfig memory rcfg = ISystemConfig(_prox.SystemConfig).resourceConfig(); IResourceMetering.ResourceConfig memory rcfg = ISystemConfig(_prox.SystemConfig).resourceConfig();
IResourceMetering.ResourceConfig memory dflt = Constants.DEFAULT_RESOURCE_CONFIG(); IResourceMetering.ResourceConfig memory dflt = Constants.DEFAULT_RESOURCE_CONFIG();
require(keccak256(abi.encode(rcfg)) == keccak256(abi.encode(dflt))); require(keccak256(abi.encode(rcfg)) == keccak256(abi.encode(dflt)), "CHECK-RCFG-10");
checkSystemConfig({ _contracts: _prox, _cfg: _cfg, _isProxy: true }); checkSystemConfig({ _contracts: _prox, _cfg: _cfg, _isProxy: true });
checkL1CrossDomainMessenger({ _contracts: _prox, _vm: _vm, _isProxy: true }); checkL1CrossDomainMessenger({ _contracts: _prox, _vm: _vm, _isProxy: true });
......
...@@ -450,7 +450,7 @@ contract Deploy is Deployer { ...@@ -450,7 +450,7 @@ contract Deploy is Deployer {
_args: DeployUtils.encodeConstructor(abi.encodeCall(IProxy.__constructor__, (_proxyOwner))) _args: DeployUtils.encodeConstructor(abi.encodeCall(IProxy.__constructor__, (_proxyOwner)))
}) })
); );
require(EIP1967Helper.getAdmin(address(proxy)) == _proxyOwner); require(EIP1967Helper.getAdmin(address(proxy)) == _proxyOwner, "Deploy: EIP1967Proxy admin not set");
addr_ = address(proxy); addr_ = address(proxy);
} }
...@@ -466,7 +466,9 @@ contract Deploy is Deployer { ...@@ -466,7 +466,9 @@ contract Deploy is Deployer {
_args: DeployUtils.encodeConstructor(abi.encodeCall(IProxy.__constructor__, (proxyAdmin))) _args: DeployUtils.encodeConstructor(abi.encodeCall(IProxy.__constructor__, (proxyAdmin)))
}) })
); );
require(EIP1967Helper.getAdmin(address(proxy)) == proxyAdmin); require(
EIP1967Helper.getAdmin(address(proxy)) == proxyAdmin, "Deploy: DataAvailabilityChallengeProxy admin not set"
);
addr_ = address(proxy); addr_ = address(proxy);
} }
...@@ -565,11 +567,16 @@ contract Deploy is Deployer { ...@@ -565,11 +567,16 @@ contract Deploy is Deployer {
string memory version = dac.version(); string memory version = dac.version();
console.log("DataAvailabilityChallenge version: %s", version); console.log("DataAvailabilityChallenge version: %s", version);
require(dac.owner() == finalSystemOwner); require(dac.owner() == finalSystemOwner, "Deploy: DataAvailabilityChallenge owner not set");
require(dac.challengeWindow() == daChallengeWindow); require(
require(dac.resolveWindow() == daResolveWindow); dac.challengeWindow() == daChallengeWindow, "Deploy: DataAvailabilityChallenge challenge window not set"
require(dac.bondSize() == daBondSize); );
require(dac.resolverRefundPercentage() == daResolverRefundPercentage); require(dac.resolveWindow() == daResolveWindow, "Deploy: DataAvailabilityChallenge resolve window not set");
require(dac.bondSize() == daBondSize, "Deploy: DataAvailabilityChallenge bond size not set");
require(
dac.resolverRefundPercentage() == daResolverRefundPercentage,
"Deploy: DataAvailabilityChallenge resolver refund percentage not set"
);
} }
//////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////
......
...@@ -241,7 +241,7 @@ contract DeployOPCM is Script { ...@@ -241,7 +241,7 @@ contract DeployOPCM is Script {
OPContractsManager impl = OPContractsManager(address(_doo.opcm())); OPContractsManager impl = OPContractsManager(address(_doo.opcm()));
require(address(impl.superchainConfig()) == address(_doi.superchainConfig()), "OPCMI-10"); require(address(impl.superchainConfig()) == address(_doi.superchainConfig()), "OPCMI-10");
require(address(impl.protocolVersions()) == address(_doi.protocolVersions()), "OPCMI-20"); require(address(impl.protocolVersions()) == address(_doi.protocolVersions()), "OPCMI-20");
require(LibString.eq(impl.l1ContractsRelease(), _doi.l1ContractsRelease())); require(LibString.eq(impl.l1ContractsRelease(), _doi.l1ContractsRelease()), "OPCMI-30");
OPContractsManager.Blueprints memory blueprints = impl.blueprints(); OPContractsManager.Blueprints memory blueprints = impl.blueprints();
require(blueprints.addressManager == _doi.addressManagerBlueprint(), "OPCMI-40"); require(blueprints.addressManager == _doi.addressManagerBlueprint(), "OPCMI-40");
......
...@@ -631,7 +631,7 @@ contract DeployOPChain is Script { ...@@ -631,7 +631,7 @@ contract DeployOPChain is Script {
// This slot is the custom gas token _balance and this check ensures // This slot is the custom gas token _balance and this check ensures
// that it stays unset for forwards compatibility with custom gas token. // that it stays unset for forwards compatibility with custom gas token.
require(vm.load(address(portal), bytes32(uint256(61))) == bytes32(0)); require(vm.load(address(portal), bytes32(uint256(61))) == bytes32(0), "PORTAL-70");
} }
function assertValidDisputeGameFactory(DeployOPChainInput _doi, DeployOPChainOutput _doo) internal { function assertValidDisputeGameFactory(DeployOPChainInput _doi, DeployOPChainOutput _doo) internal {
......
...@@ -327,9 +327,9 @@ contract DeployOwnership is Deploy { ...@@ -327,9 +327,9 @@ contract DeployOwnership is Deploy {
}) })
); );
require(superchainConfig.guardian() == address(0)); require(superchainConfig.guardian() == address(0), "SuperchainConfig: guardian must be address(0)");
bytes32 initialized = vm.load(address(superchainConfig), bytes32(0)); bytes32 initialized = vm.load(address(superchainConfig), bytes32(0));
require(initialized != 0); require(initialized != 0, "SuperchainConfig: must be initialized");
} }
/// @notice Configure the Guardian Safe with the DeputyGuardianModule. /// @notice Configure the Guardian Safe with the DeputyGuardianModule.
......
...@@ -86,7 +86,7 @@ contract DeployPeriphery is Script, Artifacts { ...@@ -86,7 +86,7 @@ contract DeployPeriphery is Script, Artifacts {
}); });
ProxyAdmin admin = ProxyAdmin(addr_); ProxyAdmin admin = ProxyAdmin(addr_);
require(admin.owner() == msg.sender); require(admin.owner() == msg.sender, "DeployPeriphery: ProxyAdmin owner mismatch");
} }
/// @notice Deploy FaucetProxy. /// @notice Deploy FaucetProxy.
...@@ -98,7 +98,10 @@ contract DeployPeriphery is Script, Artifacts { ...@@ -98,7 +98,10 @@ contract DeployPeriphery is Script, Artifacts {
}); });
Proxy proxy = Proxy(payable(addr_)); Proxy proxy = Proxy(payable(addr_));
require(EIP1967Helper.getAdmin(address(proxy)) == mustGetAddress("ProxyAdmin")); require(
EIP1967Helper.getAdmin(address(proxy)) == mustGetAddress("ProxyAdmin"),
"DeployPeriphery: FaucetProxy admin mismatch"
);
} }
/// @notice Deploy the Faucet contract. /// @notice Deploy the Faucet contract.
...@@ -110,7 +113,7 @@ contract DeployPeriphery is Script, Artifacts { ...@@ -110,7 +113,7 @@ contract DeployPeriphery is Script, Artifacts {
}); });
Faucet faucet = Faucet(payable(addr_)); Faucet faucet = Faucet(payable(addr_));
require(faucet.ADMIN() == cfg.faucetAdmin()); require(faucet.ADMIN() == cfg.faucetAdmin(), "DeployPeriphery: Faucet admin mismatch");
} }
/// @notice Deploy the Drippie contract. /// @notice Deploy the Drippie contract.
...@@ -122,7 +125,7 @@ contract DeployPeriphery is Script, Artifacts { ...@@ -122,7 +125,7 @@ contract DeployPeriphery is Script, Artifacts {
}); });
Drippie drippie = Drippie(payable(addr_)); Drippie drippie = Drippie(payable(addr_));
require(drippie.owner() == cfg.faucetDrippieOwner()); require(drippie.owner() == cfg.faucetDrippieOwner(), "DeployPeriphery: FaucetDrippie owner mismatch");
} }
/// @notice Deploy the Drippie contract for standard operations. /// @notice Deploy the Drippie contract for standard operations.
...@@ -134,7 +137,7 @@ contract DeployPeriphery is Script, Artifacts { ...@@ -134,7 +137,7 @@ contract DeployPeriphery is Script, Artifacts {
}); });
Drippie drippie = Drippie(payable(addr_)); Drippie drippie = Drippie(payable(addr_));
require(drippie.owner() == cfg.operationsDrippieOwner()); require(drippie.owner() == cfg.operationsDrippieOwner(), "DeployPeriphery: OperationsDrippie owner mismatch");
} }
/// @notice Deploy On-Chain Authentication Module. /// @notice Deploy On-Chain Authentication Module.
...@@ -146,7 +149,9 @@ contract DeployPeriphery is Script, Artifacts { ...@@ -146,7 +149,9 @@ contract DeployPeriphery is Script, Artifacts {
}); });
AdminFaucetAuthModule module = AdminFaucetAuthModule(addr_); AdminFaucetAuthModule module = AdminFaucetAuthModule(addr_);
require(module.ADMIN() == cfg.faucetOnchainAuthModuleAdmin()); require(
module.ADMIN() == cfg.faucetOnchainAuthModuleAdmin(), "DeployPeriphery: OnChainAuthModule admin mismatch"
);
} }
/// @notice Deploy Off-Chain Authentication Module. /// @notice Deploy Off-Chain Authentication Module.
...@@ -158,7 +163,9 @@ contract DeployPeriphery is Script, Artifacts { ...@@ -158,7 +163,9 @@ contract DeployPeriphery is Script, Artifacts {
}); });
AdminFaucetAuthModule module = AdminFaucetAuthModule(addr_); AdminFaucetAuthModule module = AdminFaucetAuthModule(addr_);
require(module.ADMIN() == cfg.faucetOffchainAuthModuleAdmin()); require(
module.ADMIN() == cfg.faucetOffchainAuthModuleAdmin(), "DeployPeriphery: OffChainAuthModule admin mismatch"
);
} }
/// @notice Deploy CheckTrue contract. /// @notice Deploy CheckTrue contract.
...@@ -209,7 +216,10 @@ contract DeployPeriphery is Script, Artifacts { ...@@ -209,7 +216,10 @@ contract DeployPeriphery is Script, Artifacts {
proxyAdmin.upgrade({ _proxy: payable(faucetProxy), _implementation: faucet }); proxyAdmin.upgrade({ _proxy: payable(faucetProxy), _implementation: faucet });
} }
require(Faucet(payable(faucetProxy)).ADMIN() == Faucet(payable(faucet)).ADMIN()); require(
Faucet(payable(faucetProxy)).ADMIN() == Faucet(payable(faucet)).ADMIN(),
"DeployPeriphery: Faucet admin mismatch"
);
} }
/// @notice Installs the OnChain AuthModule on the Faucet contract. /// @notice Installs the OnChain AuthModule on the Faucet contract.
......
...@@ -146,7 +146,7 @@ contract LivenessModule_GetRequiredThreshold_Test is LivenessModule_TestInit { ...@@ -146,7 +146,7 @@ contract LivenessModule_GetRequiredThreshold_Test is LivenessModule_TestInit {
pure pure
returns (uint256) returns (uint256)
{ {
require(_percentage > 0 && _percentage <= 100); require(_percentage > 0 && _percentage <= 100, "LivenessModule: _percentage must be between 1 and 100");
uint256 toAdd; uint256 toAdd;
// If the total multiplied by the percentage is not divisible by 100, we need to add 1 to the result to // If the total multiplied by the percentage is not divisible by 100, we need to add 1 to the result to
......
...@@ -197,7 +197,7 @@ contract CommonTest is Test, Setup, Events { ...@@ -197,7 +197,7 @@ contract CommonTest is Test, Setup, Events {
if (!(alice == address(0) && bob == address(0))) { if (!(alice == address(0) && bob == address(0))) {
revert("CommonTest: Cannot enable custom gas token after deployment. Consider overriding `setUp`."); revert("CommonTest: Cannot enable custom gas token after deployment. Consider overriding `setUp`.");
} }
require(_token != Constants.ETHER); require(_token != Constants.ETHER, "CommonTest: Cannot set gas token to ETHER");
customGasToken = _token; customGasToken = _token;
} }
......
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