| Number | Details | Instances |
|---|---|---|
| [Medium-1] | Privileged functions can create points of failure | 6 |
| Number | Details | Instances |
|---|---|---|
| [Low-1] | Solidity version 0.8.20 won't work on all chains due to PUSH0 | 1 |
| [Low-2] | Usage of ecrecover is vulnerable to signature malleability | 2 |
| [Low-3] | Low level calls in solidity versions preceding 0.8.14 can result in an optimiser bug | 1 |
| [Low-4] | Missing zero address check in constructor | 2 |
| [Low-5] | Prefer skip over revert model in iteration | 2 |
| [Low-6] | Numbers downcast to addresses may result in collisions | 2 |
| [Low-7] | Return values not checked for OZ EnumerableSet add/remove functions | 1 |
| [Low-8] | Consider a uptime feed on L2 deployments to prevent issues caused by downtime | 3 |
| Number | Details | Instances |
|---|---|---|
| [NonCritical-1] | Using abi.encodePacked can result in hash collision when used in hashing functions | 1 |
| [NonCritical-2] | Default address(0) can be returned | 2 |
| [NonCritical-3] | Using zero as a parameter | 2 |
| [NonCritical-4] | Constructors missing validation | 3 |
| [NonCritical-5] | Floating pragma should be avoided | 1 |
| [NonCritical-6] | Events regarding state variable changes should emit the previous state variable value | 1 |
| [NonCritical-7] | In functions which accept an address as a parameter, there should be a zero address check to prevent bugs | 6 |
| [NonCritical-8] | Enum values should be used in place of constant array indexes | 1 |
| [NonCritical-9] | Contract lines should not be longer than 120 characters for readability | 2 |
| [NonCritical-10] | Specific imports should be used where possible so only used code is imported | 1 |
| [NonCritical-11] | Old Solidity version | 2 |
| [NonCritical-12] | Not all event definitions are utilizing indexed variables. | 1 |
| [NonCritical-13] | Contracts should have all public/external functions exposed by interfaces | 18 |
| [NonCritical-14] | Functions within contracts are not ordered according to the solidity style guide | 3 |
| [NonCritical-15] | Double type casts create complexity within the code | 2 |
| [NonCritical-16] | Constants should be on the left side of the comparison | 3 |
| [NonCritical-17] | Use of non-named numeric constants | 2 |
| [NonCritical-18] | Employ Explicit Casting to Bytes or Bytes32 for Enhanced Code Clarity and Meaning | 1 |
| [NonCritical-19] | Event emit should emit a parameter | 2 |
| [NonCritical-20] | Empty bytes check is missing | 1 |
| [NonCritical-21] | Assembly block creates dirty bits | 1 |
| [NonCritical-22] | Cyclomatic complexity in functions | 1 |
| [NonCritical-23] | Unused import | 2 |
| [NonCritical-24] | Use 'using' keyword when using specific imports rather than calling the specific import directly | 1 |
| [NonCritical-25] | Constructors should emit an event | 2 |
| [NonCritical-26] | For loop iterates on arrays not indexed | 2 |
| [NonCritical-27] | Events should have parameters | 2 |
| Number | Details | Instances | Gas |
|---|---|---|---|
| [Gas-1] | Default int values are manually set | 5 | 0.0 |
| [Gas-2] | There is a 32 byte length threshold for error strings, strings longer than this consume more gas | 7 | 686 |
| [Gas-3] | Public functions not used internally can be marked as external to save gas | 8 | 0.0 |
| [Gas-4] | Calldata should be used in place of memory function parameters when not mutated | 1 | 13 |
| [Gas-5] | Usage of smaller uint/int types causes overhead | 2 | 220 |
| [Gas-6] | Use != 0 instead of > 0 | 2 | 12 |
| [Gas-7] | Function calls within for loops | 4 | 0.0 |
| [Gas-8] | For loops in public or external functions should be avoided due to high gas costs and possible DOS | 3 | 0.0 |
| [Gas-9] | Shorten the array rather than copying to a new one | 1 | 0.0 |
| [Gas-10] | Lack of unchecked in loops | 12 | 10080 |
| [Gas-11] | Use assembly to validate msg.sender | 1 | 0.0 |
| [Gas-12] | Simple checks for zero uint can be done using assembly to save gas | 1 | 6 |
| [Gas-13] | Solidity versions 0.8.19 and above are more gas efficient | 2 | 4000 |
| [Gas-14] | Variable declared within iteration | 5 | 0.0 |
| [Gas-15] | Calling .length in a for loop wastes gas | 10 | 11640 |
| [Gas-16] | Internal functions only used once can be inlined to save gas | 5 | 750 |
| [Gas-17] | Constructors can be marked as payable to save deployment gas | 3 | 0.0 |
| [Gas-18] | There are comparisons to boolean literals (true and false), these can be simplified to save gas | 1 | 18 |
| [Gas-19] | Only emit event in setter function if the state variable was changed | 1 | 0.0 |
| [Gas-20] | It is a waste of GAS to emit variable literals | 1 | 8 |
| [Gas-21] | Use OZ Array.unsafeAccess() to avoid repeated array length checks | 9 | 170100 |
| [Gas-22] | Avoid emitting events in loops | 6 | 18000 |
| [Gas-23] | Public functions not called internally | 8 | 0.0 |
Ensure such accounts are protected and consider implementing multi sig to prevent a single point of failure
Num of instances: 4
Click to show findings
['84']
84: function pause() external {
85: _onlyDeputyGuardian(); // <= FOUND
86:
87: bytes memory data = abi.encodeCall(SUPERCHAIN_CONFIG.pause, ("Deputy Guardian"));
88: (bool success, bytes memory returnData) =
89: SAFE.execTransactionFromModuleReturnData(address(SUPERCHAIN_CONFIG), 0, data, Enum.Operation.Call);
90: if (!success) {
91: revert ExecutionFailed(string(returnData));
92: }
93: emit Paused("Deputy Guardian");
94: }['99']
99: function unpause() external {
100: _onlyDeputyGuardian(); // <= FOUND
101:
102: bytes memory data = abi.encodeCall(SUPERCHAIN_CONFIG.unpause, ());
103: (bool success, bytes memory returnData) =
104: SAFE.execTransactionFromModuleReturnData(address(SUPERCHAIN_CONFIG), 0, data, Enum.Operation.Call);
105: if (!success) {
106: revert ExecutionFailed(string(returnData));
107: }
108: emit Unpaused();
109: }['116']
116: function blacklistDisputeGame(OptimismPortal2 _portal, IDisputeGame _game) external {
117: _onlyDeputyGuardian(); // <= FOUND
118:
119: bytes memory data = abi.encodeCall(OptimismPortal2.blacklistDisputeGame, (_game));
120: (bool success, bytes memory returnData) =
121: SAFE.execTransactionFromModuleReturnData(address(_portal), 0, data, Enum.Operation.Call);
122: if (!success) {
123: revert ExecutionFailed(string(returnData));
124: }
125: emit DisputeGameBlacklisted(_game);
126: }['133']
133: function setRespectedGameType(OptimismPortal2 _portal, GameType _gameType) external {
134: _onlyDeputyGuardian(); // <= FOUND
135:
136: bytes memory data = abi.encodeCall(OptimismPortal2.setRespectedGameType, (_gameType));
137: (bool success, bytes memory returnData) =
138: SAFE.execTransactionFromModuleReturnData(address(_portal), 0, data, Enum.Operation.Call);
139: if (!success) {
140: revert ExecutionFailed(string(returnData));
141: }
142: emit RespectedGameTypeSet(_gameType);
143: }['69']
69: function checkTransaction(
70: address to,
71: uint256 value,
72: bytes memory data,
73: Enum.Operation operation,
74: uint256 safeTxGas,
75: uint256 baseGas,
76: uint256 gasPrice,
77: address gasToken,
78: address payable refundReceiver,
79: bytes memory signatures,
80: address msgSender
81: )
82: external
83: {
84: msgSender;
85: _requireOnlySafe(); // <= FOUND
86:
87:
88:
89: address[] memory owners = SAFE.getOwners();
90: for (uint256 i = 0; i < owners.length; i++) {
91: ownersBefore.add(owners[i]);
92: }
93:
94:
95:
96: bytes32 txHash = SAFE.getTransactionHash({
97: to: to,
98: value: value,
99: data: data,
100: operation: operation,
101: safeTxGas: safeTxGas,
102: baseGas: baseGas,
103: gasPrice: gasPrice,
104: gasToken: gasToken,
105: refundReceiver: refundReceiver,
106: _nonce: SAFE.nonce() - 1
107: });
108:
109: uint256 threshold = SAFE.getThreshold();
110: address[] memory signers =
111: SafeSigners.getNSigners({ dataHash: txHash, signatures: signatures, requiredSignatures: threshold });
112:
113: for (uint256 i = 0; i < signers.length; i++) {
114: lastLive[signers[i]] = block.timestamp;
115: emit OwnerRecorded(signers[i]);
116: }
117: }['125']
125: function checkAfterExecution(bytes32, bool) external {
126: _requireOnlySafe(); // <= FOUND
127:
128: address[] memory ownersAfter = SAFE.getOwners();
129:
130:
131: for (uint256 i = 0; i < ownersAfter.length; i++) {
132:
133: address ownerAfter = ownersAfter[i];
134: if (ownersBefore.remove(ownerAfter) == false) {
135:
136: lastLive[ownerAfter] = block.timestamp;
137: }
138: }
139:
140:
141:
142:
143: address[] memory ownersBeforeCache = ownersBefore.values();
144: for (uint256 i = 0; i < ownersBeforeCache.length; i++) {
145: address ownerBefore = ownersBeforeCache[i];
146: delete lastLive[ownerBefore];
147: ownersBefore.remove(ownerBefore);
148: }
149: }Solidity version 0.8.20 uses the new Shanghai EVM which introduces the PUSH0 opcode, this may not be implemented on all chains and L2 thus reducing the portability and compatability of the code. Consider using a earlier solidity version.
Num of instances: 1
The ecrecover function in Ethereum smart contracts is vulnerable to signature malleability due to the nature of Ethereum signatures and the ECDSA (Elliptic Curve Digital Signature Algorithm) used. Ethereum signatures include an additional field, v (the recovery identifier), which can have multiple valid values for the same signer and message. This field, which ranges from 27 to 30, helps to determine the correct public key associated with a signature. The vulnerability arises because there are multiple valid s values for any given r in a signature, meaning (r, s) and (r, n - s) can both be valid for the same message and signing key. The ecrecover function, which is used to verify signatures and recover addresses, must account for these different valid s values, leading to potential signature malleability issues.
Num of instances: 2
Click to show findings
['75']
75:
76:
77:
78: currentOwner =
79: ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), v - 4, r, s); // <= FOUND['80']
80:
81:
82: currentOwner = ecrecover(dataHash, v, r, s); // <= FOUNDIn Solidity versions 0.8.13 and 0.8.14, a known optimizer bug presents potential risks when a variable is used in a separate assembly block from the one in which it was stored. Specifically, the 'mstore' operation could be optimized out due to this bug, leading to the use of uninitialized memory. Although the current code does not exhibit this risky pattern of execution, it does utilize 'mstore' within assembly blocks, which introduces a vulnerability risk for future code modifications. As a preventative measure, it is advisable to avoid the usage of the afflicted Solidity versions, 0.8.13 and 0.8.14. Instead, consider utilizing a version that is not impacted by this optimizer bug to prevent potential memory initialization issues in your smart contract.
Num of instances: 1
Click to show findings
['25']
25: assembly { // <= FOUND
26: let signaturePos := mul(0x41, pos)
27: r := mload(add(signatures, add(signaturePos, 0x20)))
28: s := mload(add(signatures, add(signaturePos, 0x40)))
29:
30:
35: v := and(mload(add(signatures, add(signaturePos, 0x41))), 0xff)
36: }In Solidity, constructors often take address parameters to initialize important components of a contract, such as owner or linked contracts. However, without a check, there's a risk that an address parameter could be mistakenly set to the zero address (0x0). This could occur due to a mistake or oversight during contract deployment. A zero address in a crucial role can cause serious issues, as it cannot perform actions like a normal address, and any funds sent to it are irretrievable. Therefore, it's crucial to include a zero address check in constructors to prevent such potential problems. If a zero address is detected, the constructor should revert the transaction.
Num of instances: 2
Click to show findings
['50']
50: constructor(Safe _safe, SuperchainConfig _superchainConfig, address _deputyGuardian) { // <= FOUND
51: SAFE = _safe;
52: SUPERCHAIN_CONFIG = _superchainConfig;
53: DEPUTY_GUARDIAN = _deputyGuardian;
54: }['54']
54: constructor(
55: Safe _safe,
56: LivenessGuard _livenessGuard,
57: uint256 _livenessInterval,
58: uint256 _minOwners,
59: uint256 _thresholdPercentage,
60: address _fallbackOwner // <= FOUND
61: ) {
62: SAFE = _safe;
63: LIVENESS_GUARD = _livenessGuard;
64: LIVENESS_INTERVAL = _livenessInterval;
65: THRESHOLD_PERCENTAGE = _thresholdPercentage;
66: FALLBACK_OWNER = _fallbackOwner;
67: MIN_OWNERS = _minOwners;
68: address[] memory owners = _safe.getOwners();
69: require(_minOwners <= owners.length, "LivenessModule: minOwners must be less than the number of owners");
70: require(
71: _safe.getThreshold() >= getRequiredThreshold(owners.length),
72: "LivenessModule: Insufficient threshold for the number of owners"
73: );
74: require(_thresholdPercentage > 0, "LivenessModule: thresholdPercentage must be greater than 0");
75: require(_thresholdPercentage <= 100, "LivenessModule: thresholdPercentage must be less than or equal to 100");
76: }It is preferable to skip operations on an array index when a condition is not met rather than reverting the whole transaction as reverting can introduce the possiblity of malicous actors purposefully introducing array objects which fail conditional checks within for/while loops so group operations fail. As such it is recommended to simply skip such array indices over reverting unless there is a valid security or logic reason behind not doing so.
Num of instances: 2
Click to show findings
['140']
140: for (uint256 i = 0; i < _previousOwners.length; i++) { // <= FOUND
141:
142:
143:
144:
145: if (ownersCount >= MIN_OWNERS) {
146: require(canRemove(_ownersToRemove[i]), "LivenessModule: the owner to remove has signed recently"); // <= FOUND
147: }
148:
149:
150:
151: ownersCount--;
152:
153:
154: _removeOwner({
155: _prevOwner: _previousOwners[i],
156: _ownerToRemove: _ownersToRemove[i],
157: _newOwnersCount: ownersCount
158: });
159:
160:
161:
162:
163:
164: if (ownersCount == 0) {
165: break;
166: }
167: }['140']
140: for (uint256 i = 0; i < _previousOwners.length; i++) { // <= FOUND
141:
142:
143:
144:
145: if (ownersCount >= MIN_OWNERS) {
146: require(canRemove(_ownersToRemove[i]), "LivenessModule: the owner to remove has signed recently"); // <= FOUND
147: }
148:
149:
150:
151: ownersCount--;
152:
153:
154: _removeOwner({
155: _prevOwner: _previousOwners[i],
156: _ownerToRemove: _ownersToRemove[i],
157: _newOwnersCount: ownersCount
158: });
159:
160:
161:
162:
163:
164: if (ownersCount == 0) {
165: break;
166: }
167: }Downcasting numbers to addresses in blockchain contracts, particularly in Ethereum's Solidity, involves risks such as possible address collisions. A collision occurs when different inputs, when cast or hashed, generate the same output address, potentially compromising contract integrity and asset security. If an uint256, for instance, is downcast to an address (effectively an uint160) without ensuring it’s a legitimate, collision-free conversion, different uint256 inputs might yield the same address, creating vulnerabilities attackers might exploit. Implementing thorough checks and opting for secure practices, like avoiding downcasting in critical logic or utilizing mappings with original uint256 as keys, mitigates risks
Num of instances: 2
Click to show findings
['247']
247:
248: require(
249: address(LIVENESS_GUARD) == address(uint160(uint256(bytes32(SAFE.getStorageAt(GUARD_STORAGE_SLOT, 1))))), // <= FOUND
250: "LivenessModule: guard has been changed"
251: );['66']
66:
67:
68: currentOwner = address(uint160(uint256(r))); // <= FOUNDIn OpenZeppelin's EnumerableSet library, the add and remove functions return boolean values indicating success or failure. Not checking these return values can lead to unnoticed errors, especially in complex contract logic. It's a best practice to always check the return values of these functions to ensure that the intended modifications to the set were successful. Ignoring them could result in a false assumption of successful addition or removal, potentially leading to security flaws or logical errors in contract execution. Proper handling of these return values contributes to more robust and error-free smart contract code.
Num of instances: 1
Click to show findings
['21']
21: contract LivenessGuard is ISemver, BaseGuard { // <= FOUND
22: using EnumerableSet for EnumerableSet.AddressSet;
23:
26: event OwnerRecorded(address owner);
27:
91: ownersBefore.add(owners[i]); // <= FOUND
147: ownersBefore.remove(ownerBefore); // <= FOUND
In L2 deployments, incorporating an uptime feed is crucial to mitigate issues arising from sequencer downtime. Downtime can disrupt services, leading to transaction failures or incorrect data readings, affecting overall system reliability. By integrating an uptime feed, you gain insight into the operational status of the L2 network, enabling proactive measures like halting sensitive operations or alerting users. This approach ensures that your contract behaves predictably and securely during network outages, enhancing the robustness and reliability of your decentralized application, which is especially important in mission-critical or high-stakes environments.
Num of instances: 3
Click to show findings
['20']
20: contract DeputyGuardianModule is ISemver // <= FOUND['21']
21: contract LivenessGuard is ISemver, BaseGuard // <= FOUND['16']
16: contract LivenessModule is ISemver // <= FOUNDConsider using abi.encode as this pads data to 32 byte segments
Num of instances: 1
Click to show findings
['75']
75:
76:
77:
78: currentOwner =
79: ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), v - 4, r, s); // <= FOUNDAllowing a function in Solidity to return the default address (address(0)) can be problematic as it can represent uninitialized or invalid addresses. If such an address is utilized in transfer operations or other sensitive actions, it could lead to loss of funds or unpredicted behavior. It's prudent to include checks in your functions to prevent the return of the zero address, enhancing contract security.
Num of instances: 2
Click to show findings
['70']
70: function deputyGuardian() public view returns (address deputyGuardian_) {
71: deputyGuardian_ = DEPUTY_GUARDIAN;
72: }['116']
116: function fallbackOwner() public view returns (address fallbackOwner_) {
117: fallbackOwner_ = FALLBACK_OWNER;
118: }Taking 0 as a valid argument in Solidity without checks can lead to severe security issues. A historical example is the infamous 0x0 address bug where numerous tokens were lost. This happens because '0' can be interpreted as an uninitialized address, leading to transfers to the '0x0' address, effectively burning tokens. Moreover, 0 as a denominator in division operations would cause a runtime exception. It's also often indicative of a logical error in the caller's code. It's important to always validate input and handle edge cases like 0 appropriately. Use require() statements to enforce conditions and provide clear error messages to facilitate debugging and safer code.
Num of instances: 2
Click to show findings
['190']
190: function _swapToFallbackOwnerSafeCall(address _prevOwner, address _oldOwner) internal { // <= FOUND
191: require(
192: SAFE.execTransactionFromModule({
193: to: address(SAFE), // <= FOUND
194: value: 0,
195: operation: Enum.Operation.Call,
196: data: abi.encodeCall(OwnerManager.swapOwner, (_prevOwner, _oldOwner, FALLBACK_OWNER))
197: }),
198: "LivenessModule: failed to swap to fallback owner"
199: );
200: emit OwnershipTransferredToFallback();
201: }['207']
207: function _removeOwnerSafeCall(address _prevOwner, address _owner, uint256 _threshold) internal { // <= FOUND
208: require(
209: SAFE.execTransactionFromModule({
210: to: address(SAFE), // <= FOUND
211: value: 0,
212: operation: Enum.Operation.Call,
213: data: abi.encodeCall(OwnerManager.removeOwner, (_prevOwner, _owner, _threshold))
214: }),
215: "LivenessModule: failed to remove owner"
216: );
217: emit RemovedOwner(_owner);
218: }In Solidity, when values are being assigned in constructors to unsigned or integer variables, it's crucial to ensure the provided values adhere to the protocol's specific operational boundaries as laid out in the project specifications and documentation. If the constructors lack appropriate validation checks, there's a risk of setting state variables with values that could cause unexpected and potentially detrimental behavior within the contract's operations, violating the intended logic of the protocol. This can compromise the contract's security and impact the maintainability and reliability of the system. In order to avoid such issues, it is recommended to incorporate rigorous validation checks in constructors. These checks should align with the project's defined rules and constraints, making use of Solidity's built-in require function to enforce these conditions. If the validation checks fail, the require function will cause the transaction to revert, ensuring the integrity and adherence to the protocol's expected behavior.
Num of instances: 3
Click to show findings
['50']
50: constructor(Safe _safe, SuperchainConfig _superchainConfig, address _deputyGuardian) {
51: SAFE = _safe;
52: SUPERCHAIN_CONFIG = _superchainConfig;
53: DEPUTY_GUARDIAN = _deputyGuardian;
54: }['46']
46: constructor(Safe _safe) {
47: SAFE = _safe;
48: address[] memory owners = _safe.getOwners();
49: for (uint256 i = 0; i < owners.length; i++) {
50: address owner = owners[i];
51: lastLive[owner] = block.timestamp;
52: emit OwnerRecorded(owner);
53: }
54: }['54']
54: constructor(
55: Safe _safe,
56: LivenessGuard _livenessGuard,
57: uint256 _livenessInterval,
58: uint256 _minOwners,
59: uint256 _thresholdPercentage,
60: address _fallbackOwner
61: ) {
62: SAFE = _safe;
63: LIVENESS_GUARD = _livenessGuard;
64: LIVENESS_INTERVAL = _livenessInterval;
65: THRESHOLD_PERCENTAGE = _thresholdPercentage;
66: FALLBACK_OWNER = _fallbackOwner;
67: MIN_OWNERS = _minOwners;
68: address[] memory owners = _safe.getOwners();
69: require(_minOwners <= owners.length, "LivenessModule: minOwners must be less than the number of owners");
70: require(
71: _safe.getThreshold() >= getRequiredThreshold(owners.length),
72: "LivenessModule: Insufficient threshold for the number of owners"
73: );
74: require(_thresholdPercentage > 0, "LivenessModule: thresholdPercentage must be greater than 0");
75: require(_thresholdPercentage <= 100, "LivenessModule: thresholdPercentage must be less than or equal to 100");
76: }Num of instances: 1
[NonCritical-6] Events regarding state variable changes should emit the previous state variable value
Modify such events to contain the previous value of the state variable as demonstrated in the example below
Num of instances: 1
[NonCritical-7] In functions which accept an address as a parameter, there should be a zero address check to prevent bugs
In smart contract development, especially with Solidity, it's crucial to validate inputs to functions. When a function accepts an Ethereum address as a parameter, implementing a zero address check (i.e., ensuring the address is not 0x0) is a best practice to prevent potential bugs and vulnerabilities. The zero address (0x0) is a default value and generally indicates an uninitialized or invalid state. Passing the zero address to certain functions can lead to unintended behaviors, like funds getting locked permanently or transactions failing silently. By checking for and rejecting the zero address, developers can ensure that the function operates as intended and interacts only with valid Ethereum addresses. This check enhances the contract's robustness and security.
Num of instances: 6
Click to show findings
['69']
69: function checkTransaction(
70: address to,
71: uint256 value,
72: bytes memory data,
73: Enum.Operation operation,
74: uint256 safeTxGas,
75: uint256 baseGas,
76: uint256 gasPrice,
77: address gasToken,
78: address payable refundReceiver,
79: bytes memory signatures,
80: address msgSender
81: )
82: external
83: ['123']
123: function canRemove(address _owner) public view returns (bool canRemove_) ['133']
133: function removeOwners(address[] memory _previousOwners, address[] memory _ownersToRemove) external ['175']
175: function _removeOwner(address _prevOwner, address _ownerToRemove, uint256 _newOwnersCount) internal ['190']
190: function _swapToFallbackOwnerSafeCall(address _prevOwner, address _oldOwner) internal ['207']
207: function _removeOwnerSafeCall(address _prevOwner, address _owner, uint256 _threshold) internal Create a commented enum value to use in place of constant array indexes, this makes the code far easier to understand
Num of instances: 1
Click to show findings
['229']
229: require(owners[0] == FALLBACK_OWNER, "LivenessModule: must transfer ownership to fallback owner"); // <= FOUNDConsider spreading these lines over multiple lines to aid in readability and the support of VIM users everywhere.
Num of instances: 2
Click to show findings
['7']
7: /// https://github.com/safe-global/safe-contracts/blob/e870f514ad34cd9654c72174d6d4a839e3c6639f/contracts/common/SignatureDecoder.sol // <= FOUND['41']
41: /// https://github.com/safe-global/safe-contracts/blob/e870f514ad34cd9654c72174d6d4a839e3c6639f/contracts/Safe.sol#L274 // <= FOUNDIn many cases only some functionality is used from an import. In such cases it makes more sense to use {} to specify what to import and thus save gas whilst improving readability
Num of instances: 1
Using outdated Solidity versions can lead to security risks and inefficiencies. It's recommended to adopt newer versions, ideally the latest, which as of now is 0.8.24. This ensures access to the latest bug fixes, features, and optimizations, particularly crucial for Layer 2 deployments. Regular updates to versions like 0.8.19 or later, up to 0.8.24, enhance contract security and performance.
Num of instances: 2
Click to show findings
['2']
2: pragma solidity ^0.8.0; // <= FOUND['2']
2: pragma solidity 0.8.15;Try to index as much as three variables in event declarations as this is more gas efficient when done on value type variables (uint, address etc) however not for bytes and string variables
Num of instances: 1
Contracts should expose all public and external functions through interfaces. This practice ensures a clear and consistent definition of how the contract can be interacted with, promoting better transparency and integration.
Num of instances: 18
Click to show findings
['84']
84: function pause() external ['99']
99: function unpause() external ['116']
116: function blacklistDisputeGame(OptimismPortal2 _portal, IDisputeGame _game) external ['133']
133: function setRespectedGameType(OptimismPortal2 _portal, GameType _gameType) external ['69']
69: function checkTransaction(
70: address to,
71: uint256 value,
72: bytes memory data,
73: Enum.Operation operation,
74: uint256 safeTxGas,
75: uint256 baseGas,
76: uint256 gasPrice,
77: address gasToken,
78: address payable refundReceiver,
79: bytes memory signatures,
80: address msgSender
81: )
82: external
83: ['125']
125: function checkAfterExecution(bytes32, bool) external ['153']
153: function showLiveness() external ['133']
133: function removeOwners(address[] memory _previousOwners, address[] memory _ownersToRemove) external ['86']
86: function safe() public view returns (Safe safe_) ['64']
64: function superchainConfig() public view returns (SuperchainConfig superchainConfig_) ['70']
70: function deputyGuardian() public view returns (address deputyGuardian_) ['80']
80: function getRequiredThreshold(uint256 _numOwners) public view returns (uint256 threshold_) ['92']
92: function livenessGuard() public view returns (LivenessGuard livenessGuard_) ['98']
98: function livenessInterval() public view returns (uint256 livenessInterval_) ['104']
104: function minOwners() public view returns (uint256 minOwners_) ['110']
110: function thresholdPercentage() public view returns (uint256 thresholdPercentage_) ['116']
116: function fallbackOwner() public view returns (address fallbackOwner_) ['123']
123: function canRemove(address _owner) public view returns (bool canRemove_) The following order should be used within contracts
constructor
receive function (if exists)
fallback function (if exists)
external
public
internal
private
Rearrange the contract functions and contructors to fit this ordering
Num of instances: 3
Click to show findings
['20']
20: contract DeputyGuardianModule is ISemver // <= FOUND['21']
21: contract LivenessGuard is ISemver, BaseGuard // <= FOUND['16']
16: contract LivenessModule is ISemver // <= FOUNDDouble type casting should be avoided in Solidity contracts to prevent unintended consequences and ensure accurate data representation. Performing multiple type casts in succession can lead to unexpected truncation, rounding errors, or loss of precision, potentially compromising the contract's functionality and reliability. Furthermore, double type casting can make the code less readable and harder to maintain, increasing the likelihood of errors and misunderstandings during development and debugging. To ensure precise and consistent data handling, developers should use appropriate data types and avoid unnecessary or excessive type casting, promoting a more robust and dependable contract execution.
Num of instances: 2
Click to show findings
['247']
247:
248: require(
249: address(LIVENESS_GUARD) == address(uint160(uint256(bytes32(SAFE.getStorageAt(GUARD_STORAGE_SLOT, 1))))), // <= FOUND
250: "LivenessModule: guard has been changed"
251: );['66']
66:
67:
68: currentOwner = address(uint160(uint256(r))); // <= FOUNDPutting constants on the left side of a comparison operator like == or < is a best practice known as "Yoda conditions", which can help prevent accidental assignment instead of comparison. In some programming languages, if a variable is mistakenly put on the left with a single = instead of ==, it assigns the constant's value to the variable without any compiler error. However, doing this with the constant on the left would generate an error, as constants cannot be assigned values. Although Solidity's static typing system prevents accidental assignments within conditionals, adopting this practice enhances code readability and consistency, especially when developers are working across multiple languages that support this convention.
Num of instances: 3
Click to show findings
['164']
164: if (ownersCount == 0) // <= FOUND['228']
228: if (numOwners == 1) // <= FOUND['176']
176: if (_newOwnersCount > 0) // <= FOUNDMagic numbers should be avoided in Solidity code to enhance readability, maintainability, and reduce the likelihood of errors. Magic numbers are hard-coded values with no clear meaning or context, which can create confusion and make the code harder to understand for developers. Using well-defined constants or variables with descriptive names instead of magic numbers not only clarifies the purpose and significance of the value but also simplifies code updates and modifications.
Num of instances: 2
Click to show findings
['81']
81: threshold_ = (_numOwners * THRESHOLD_PERCENTAGE + 99) / 100; // <= FOUND['71']
71: } else if (v > 30) { // <= FOUNDSmart contracts are complex entities, and clarity in their operations is fundamental to ensure that they function as intended. Casting a single argument instead of utilizing 'abi.encodePacked()' improves the transparency of the operation. It elucidates the intent of the code, reducing ambiguity and making it easier for auditors and developers to understand the code’s purpose. Such practices promote readability and maintainability, thus reducing the likelihood of errors and misunderstandings. Therefore, it's recommended to employ explicit casts for single arguments where possible, to increase the contract's comprehensibility and ensure a smoother review process.
Num of instances: 1
Click to show findings
['45']
45: function getNSigners(
46: bytes32 dataHash,
47: bytes memory signatures,
48: uint256 requiredSignatures
49: )
50: internal
51: pure
52: returns (address[] memory _owners)
53: {
54: _owners = new address[](requiredSignatures);
55:
56: address currentOwner;
57: uint8 v;
58: bytes32 r;
59: bytes32 s;
60: uint256 i;
61: for (i = 0; i < requiredSignatures; i++) {
62: (v, r, s) = signatureSplit(signatures, i);
63: if (v == 0) {
64:
65:
66: currentOwner = address(uint160(uint256(r)));
67: } else if (v == 1) {
68:
69:
70: currentOwner = address(uint160(uint256(r)));
71: } else if (v > 30) {
72:
73:
74:
75: currentOwner =
76: ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), v - 4, r, s); // <= FOUND
77: } else {
78:
79:
80: currentOwner = ecrecover(dataHash, v, r, s);
81: }
82: _owners[i] = currentOwner;
83: }
84: }Events in Solidity offer valuable insight into the contract's execution as they log specific instances of state changes or value transfers. However, if events do not include any parameters, their usefulness can be significantly reduced. Events without parameters can provide limited information, mainly signaling that a specific operation occurred, but lacking the context of what exactly changed. It's generally recommended to include relevant parameters, such as state changes or value modifications, in the emitted events.
Num of instances: 2
Click to show findings
['108']
108: emit Unpaused(); // <= FOUND['200']
200: emit OwnershipTransferredToFallback(); // <= FOUNDWhen developing smart contracts in Solidity, it's crucial to validate the inputs of your functions. This includes ensuring that the bytes parameters are not empty, especially when they represent crucial data such as addresses, identifiers, or raw data that the contract needs to process.
Missing empty bytes checks can lead to unexpected behaviour in your contract. For instance, certain operations might fail, produce incorrect results, or consume unnecessary gas when performed with empty bytes. Moreover, missing input validation can potentially expose your contract to malicious activity, including exploitation of unhandled edge cases.
To mitigate these issues, always validate that bytes parameters are not empty when the logic of your contract requires it.
Num of instances: 1
Click to show findings
['45']
45: function getNSigners(
46: bytes32 dataHash,
47: bytes memory signatures,
48: uint256 requiredSignatures
49: )
50: internal
51: pure
52: returns (address[] memory _owners)
53: {
54: _owners = new address[](requiredSignatures);
55:
56: address currentOwner;
57: uint8 v;
58: bytes32 r;
59: bytes32 s;
60: uint256 i;
61: for (i = 0; i < requiredSignatures; i++) {
62: (v, r, s) = signatureSplit(signatures, i);
63: if (v == 0) {
64:
65:
66: currentOwner = address(uint160(uint256(r)));
67: } else if (v == 1) {
68:
69:
70: currentOwner = address(uint160(uint256(r)));
71: } else if (v > 30) {
72:
73:
74:
75: currentOwner =
76: ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), v - 4, r, s);
77: } else {
78:
79:
80: currentOwner = ecrecover(dataHash, v, r, s);
81: }
82: _owners[i] = currentOwner;
83: }
84: }Manipulating data directly at the free memory pointer location without subsequently adjusting the pointer can lead to unwanted data remnants, or "dirty bits", in that memory spot. This can cause challenges for the Solidity optimizer, making it difficult to determine if memory cleaning is required before reuse, potentially resulting in less efficient optimization. To mitigate this issue, it's advised to always update the free memory pointer following any data write operation. Furthermore, using the assembly ("memory-safe") { ... } annotation will clearly indicate to the optimizer the sections of your code that are memory-safe, improving code efficiency and reducing the potential for errors.
Num of instances: 1
Click to show findings
['25']
25: assembly {
26: let signaturePos := mul(0x41, pos)
27: r := mload(add(signatures, add(signaturePos, 0x20)))
28: s := mload(add(signatures, add(signaturePos, 0x40))) // <= FOUND
29:
30:
35: v := and(mload(add(signatures, add(signaturePos, 0x41))), 0xff)
36: }Cyclomatic complexity is a software metric used to measure the complexity of a program. It quantifies the number of linearly independent paths through a program's source code, giving an idea of how complex the control flow is. High cyclomatic complexity may indicate a higher risk of defects and can make the code harder to understand, test, and maintain. It often suggests that a function or method is trying to do too much, and a refactor might be needed. By breaking down complex functions into smaller, more focused pieces, you can improve readability, ease of testing, and overall maintainability.
Num of instances: 1
Click to show findings
[]
45: function getNSigners(
46: bytes32 dataHash,
47: bytes memory signatures,
48: uint256 requiredSignatures
49: )
50: internal
51: pure
52: returns (address[] memory _owners)
53: {
54: _owners = new address[](requiredSignatures);
55:
56: address currentOwner;
57: uint8 v;
58: bytes32 r;
59: bytes32 s;
60: uint256 i;
61: for (i = 0; i < requiredSignatures; i++) {
62: (v, r, s) = signatureSplit(signatures, i);
63: if (v == 0) {
64:
65:
66: currentOwner = address(uint160(uint256(r)));
67: } else if (v == 1) {
68:
69:
70: currentOwner = address(uint160(uint256(r)));
71: } else if (v > 30) {
72:
73:
74:
75: currentOwner =
76: ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), v - 4, r, s);
77: } else {
78:
79:
80: currentOwner = ecrecover(dataHash, v, r, s);
81: }
82: _owners[i] = currentOwner;
83: }
84: }If these serve no purpose, they should be safely removed
Num of instances: 2
Click to show findings
['5']
5: import { BaseGuard, GuardManager } from "safe-contracts/base/GuardManager.sol"; // <= FOUND['6']
6: import { ModuleManager } from "safe-contracts/base/ModuleManager.sol"; // <= FOUND[NonCritical-24] Use 'using' keyword when using specific imports rather than calling the specific import directly
In Solidity, the using keyword can streamline the use of library functions for specific types. Instead of calling library functions directly with their full import paths, you can declare a library once with using for a specific type. This approach makes your code more readable and concise. For example, instead of LibraryName.functionName(variable), you would first declare using LibraryName for TypeName; at the contract level. After this, you can call library functions directly on variables of TypeName like variable.functionName(). This method not only enhances code clarity but also promotes cleaner and more organized code, especially when multiple functions from the same library are used frequently.
Num of instances: 1
Click to show findings
['110']
110: address[] memory signers =
111: SafeSigners.getNSigners({ dataHash: txHash, signatures: signatures, requiredSignatures: threshold }); // <= FOUND 'SafeSigners.' // <= FOUND 'SafeSigners.'Emitting an event in a constructor of a smart contract provides transparency and traceability in blockchain applications. This event logs the contract’s creation, aiding in monitoring and verifying contract deployment. Although constructors are executed only once, the emitted event ensures the contract's initialization is recorded on the blockchain.
Num of instances: 2
Click to show findings
['50']
50: constructor(Safe _safe, SuperchainConfig _superchainConfig, address _deputyGuardian) { // <= FOUND
51: SAFE = _safe; // <= FOUND
52: SUPERCHAIN_CONFIG = _superchainConfig; // <= FOUND
53: DEPUTY_GUARDIAN = _deputyGuardian; // <= FOUND
54: }['54']
54: constructor( // <= FOUND
55: Safe _safe,
56: LivenessGuard _livenessGuard,
57: uint256 _livenessInterval,
58: uint256 _minOwners,
59: uint256 _thresholdPercentage,
60: address _fallbackOwner
61: ) {
62: SAFE = _safe; // <= FOUND
63: LIVENESS_GUARD = _livenessGuard; // <= FOUND
64: LIVENESS_INTERVAL = _livenessInterval; // <= FOUND
65: THRESHOLD_PERCENTAGE = _thresholdPercentage; // <= FOUND
66: FALLBACK_OWNER = _fallbackOwner; // <= FOUND
67: MIN_OWNERS = _minOwners; // <= FOUND
68: address[] memory owners = _safe.getOwners(); // <= FOUND
69: require(_minOwners <= owners.length, "LivenessModule: minOwners must be less than the number of owners"); // <= FOUND
70: require(
71: _safe.getThreshold() >= getRequiredThreshold(owners.length),
72: "LivenessModule: Insufficient threshold for the number of owners"
73: ); // <= FOUND
74: require(_thresholdPercentage > 0, "LivenessModule: thresholdPercentage must be greater than 0"); // <= FOUND
75: require(_thresholdPercentage <= 100, "LivenessModule: thresholdPercentage must be less than or equal to 100"); // <= FOUND
76: }Ensuring matching input array lengths in functions is crucial to prevent logical errors and inconsistencies in smart contracts. Mismatched array lengths can lead to incomplete operations or incorrect data handling, particularly if one array's length is assumed to reflect another's. For instance, iterating over a shorter array than intended could result in unprocessed elements from a longer array, potentially causing unintended consequences in contract execution. Validating that all input arrays have intended lengths before performing operations helps safeguard against such errors, ensuring that all elements are appropriately processed and the contract behaves as expected.
Num of instances: 2
Click to show findings
['140']
140: for (uint256 i = 0; i < _previousOwners.length; i++) { // <= FOUND
141:
142:
143:
144:
145: if (ownersCount >= MIN_OWNERS) {
146: require(canRemove(_ownersToRemove[i]), "LivenessModule: the owner to remove has signed recently"); // <= FOUND
147: }
148:
149:
150:
151: ownersCount--;
152:
153:
154: _removeOwner({
155: _prevOwner: _previousOwners[i],
156: _ownerToRemove: _ownersToRemove[i],
157: _newOwnersCount: ownersCount
158: });
159:
160:
161:
162:
163:
164: if (ownersCount == 0) {
165: break;
166: }
167: }['140']
140: for (uint256 i = 0; i < _previousOwners.length; i++) { // <= FOUND
141:
142:
143:
144:
145: if (ownersCount >= MIN_OWNERS) {
146: require(canRemove(_ownersToRemove[i]), "LivenessModule: the owner to remove has signed recently"); // <= FOUND
147: }
148:
149:
150:
151: ownersCount--;
152:
153:
154: _removeOwner({
155: _prevOwner: _previousOwners[i],
156: _ownerToRemove: _ownersToRemove[i],
157: _newOwnersCount: ownersCount
158: });
159:
160:
161:
162:
163:
164: if (ownersCount == 0) {
165: break;
166: }
167: }In Solidity, events are crucial for logging and informing external consumers about smart contract activities. Using parameters in events is essential for conveying detailed information about these activities. Parameters enable developers to specify what data is emitted, facilitating efficient and targeted data retrieval by front-end applications or off-chain monitoring tools. Well-defined event parameters improve transparency, enable better data analysis, and enhance the contract's interface with the external world, making interactions with the blockchain more informative and accessible.
Num of instances: 2
Click to show findings
['28']
28:
29: event Unpaused(); // <= FOUND['21']
21:
22: event OwnershipTransferredToFallback(); // <= FOUNDIn instances where a new variable is defined, there is no need to set it to it's default value.
Num of instances: 5
Click to show findings
['49']
49: for (uint256 i = 0; i < owners.length; i++) { // <= FOUND['113']
113: for (uint256 i = 0; i < signers.length; i++) { // <= FOUND['131']
131:
132: for (uint256 i = 0; i < ownersAfter.length; i++) { // <= FOUND['144']
144: for (uint256 i = 0; i < ownersBeforeCache.length; i++) { // <= FOUND['140']
140: for (uint256 i = 0; i < _previousOwners.length; i++) { // <= FOUND[Gas-2] There is a 32 byte length threshold for error strings, strings longer than this consume more gas
In require statements found which are longer than 32 characters, shorten these to 32 character or less
Num of instances: 7
Click to show findings
['63']
63: function _requireOnlySafe() internal view {
64: require(msg.sender == address(SAFE), "LivenessGuard: only Safe can call this function"); // <= FOUND
65: }['153']
153: function showLiveness() external {
154: require(SAFE.isOwner(msg.sender), "LivenessGuard: only Safe owners may demonstrate liveness"); // <= FOUND
155: lastLive[msg.sender] = block.timestamp;
156:
157: emit OwnerRecorded(msg.sender);
158: }['123']
123: function canRemove(address _owner) public view returns (bool canRemove_) {
124: require(SAFE.isOwner(_owner), "LivenessModule: the owner to remove must be an owner of the Safe"); // <= FOUND
125: canRemove_ = LIVENESS_GUARD.lastLive(_owner) + LIVENESS_INTERVAL < block.timestamp;
126: }['133']
133: function removeOwners(address[] memory _previousOwners, address[] memory _ownersToRemove) external {
134: require(_previousOwners.length == _ownersToRemove.length, "LivenessModule: arrays must be the same length"); // <= FOUND
135:
136:
137:
138:
139: uint256 ownersCount = SAFE.getOwners().length;
140: for (uint256 i = 0; i < _previousOwners.length; i++) {
141:
142:
143:
144:
145: if (ownersCount >= MIN_OWNERS) {
146: require(canRemove(_ownersToRemove[i]), "LivenessModule: the owner to remove has signed recently"); // <= FOUND
147: }
148:
149:
150:
151: ownersCount--;
152:
153:
154: _removeOwner({
155: _prevOwner: _previousOwners[i],
156: _ownerToRemove: _ownersToRemove[i],
157: _newOwnersCount: ownersCount
158: });
159:
160:
161:
162:
163:
164: if (ownersCount == 0) {
165: break;
166: }
167: }
168: _verifyFinalState();
169: }[]
190: function _swapToFallbackOwnerSafeCall(address _prevOwner, address _oldOwner) internal {
191: require(
192: SAFE.execTransactionFromModule({
193: to: address(SAFE),
194: value: 0,
195: operation: Enum.Operation.Call,
196: data: abi.encodeCall(OwnerManager.swapOwner, (_prevOwner, _oldOwner, FALLBACK_OWNER))
197: }),
198: "LivenessModule: failed to swap to fallback owner"
199: );
200: emit OwnershipTransferredToFallback();
201: }[]
207: function _removeOwnerSafeCall(address _prevOwner, address _owner, uint256 _threshold) internal {
208: require(
209: SAFE.execTransactionFromModule({
210: to: address(SAFE),
211: value: 0,
212: operation: Enum.Operation.Call,
213: data: abi.encodeCall(OwnerManager.removeOwner, (_prevOwner, _owner, _threshold))
214: }),
215: "LivenessModule: failed to remove owner"
216: );
217: emit RemovedOwner(_owner);
218: }['221']
221: function _verifyFinalState() internal view {
222: address[] memory owners = SAFE.getOwners();
223: uint256 numOwners = owners.length;
224:
225:
226:
227:
228: if (numOwners == 1) {
229: require(owners[0] == FALLBACK_OWNER, "LivenessModule: must transfer ownership to fallback owner"); // <= FOUND
230: } else {
231: require(
232: numOwners >= MIN_OWNERS,
233: "LivenessModule: must remove all owners and transfer to fallback owner if numOwners < minOwners"
234: );
235: }
236:
237:
238:
239:
240: uint256 threshold = SAFE.getThreshold();
241: require(
242: threshold == getRequiredThreshold(numOwners),
243: "LivenessModule: Insufficient threshold for the number of owners"
244: );
245:
246:
247: require(
248: address(LIVENESS_GUARD) == address(uint160(uint256(bytes32(SAFE.getStorageAt(GUARD_STORAGE_SLOT, 1))))),
249: "LivenessModule: guard has been changed"
250: );
251: }Public functions that aren't used internally in Solidity contracts should be made external to optimize gas usage and improve contract efficiency. External functions can only be called from outside the contract, and their arguments are directly read from the calldata, which is more gas-efficient than loading them into memory, as is the case for public functions. By using external visibility, developers can reduce gas consumption for external calls and ensure that the contract operates more cost-effectively for users. Moreover, setting the appropriate visibility level for functions also enhances code readability and maintainability, promoting a more secure and well-structured contract design.
Num of instances: 8
Click to show findings
['86']
86: function safe() public view returns (Safe safe_) ['64']
64: function superchainConfig() public view returns (SuperchainConfig superchainConfig_) ['70']
70: function deputyGuardian() public view returns (address deputyGuardian_) ['92']
92: function livenessGuard() public view returns (LivenessGuard livenessGuard_) ['98']
98: function livenessInterval() public view returns (uint256 livenessInterval_) ['104']
104: function minOwners() public view returns (uint256 minOwners_) ['110']
110: function thresholdPercentage() public view returns (uint256 thresholdPercentage_) ['116']
116: function fallbackOwner() public view returns (address fallbackOwner_) In Solidity, calldata should be used in place of memory for function parameters when the function is external. This is because calldata is a non-modifiable, non-persistent area where function arguments are stored, and it's cheaper in terms of gas than memory. It's especially efficient for arrays and complex data types. calldata provides a gas-efficient way to pass data in external function calls, whereas memory is a temporary space that's erased between external function calls. This distinction is crucial for optimizing smart contracts for gas usage and performance.
Num of instances: 1
Click to show findings
['69']
69: function checkTransaction(
70: address to,
71: uint256 value,
72: bytes memory data,
73: Enum.Operation operation,
74: uint256 safeTxGas,
75: uint256 baseGas,
76: uint256 gasPrice,
77: address gasToken,
78: address payable refundReceiver,
79: bytes memory signatures,
80: address msgSender
81: )
82: external
83: {
84: msgSender;
85: _requireOnlySafe();
86:
87:
88:
89: address[] memory owners = SAFE.getOwners();
90: for (uint256 i = 0; i < owners.length; i++) {
91: ownersBefore.add(owners[i]);
92: }
93:
94:
95:
96: bytes32 txHash = SAFE.getTransactionHash({
97: to: to,
98: value: value,
99: data: data,
100: operation: operation,
101: safeTxGas: safeTxGas,
102: baseGas: baseGas,
103: gasPrice: gasPrice,
104: gasToken: gasToken,
105: refundReceiver: refundReceiver,
106: _nonce: SAFE.nonce() - 1
107: });
108:
109: uint256 threshold = SAFE.getThreshold();
110: address[] memory signers =
111: SafeSigners.getNSigners({ dataHash: txHash, signatures: signatures, requiredSignatures: threshold });
112:
113: for (uint256 i = 0; i < signers.length; i++) {
114: lastLive[signers[i]] = block.timestamp;
115: emit OwnerRecorded(signers[i]);
116: }
117: }When using a smaller int/uint type it first needs to be converted to it's 258 bit counterpart to be operated, this increases the gass cost and thus should be avoided. However it does make sense to use smaller int/uint values within structs provided you pack the struct properly.
Num of instances: 2
Click to show findings
['17']
17: function signatureSplit(
18: bytes memory signatures,
19: uint256 pos
20: )
21: internal
22: pure
23: returns (uint8 v, bytes32 r, bytes32 s) // <= FOUND
24: {
25: assembly {
26: let signaturePos := mul(0x41, pos)
27: r := mload(add(signatures, add(signaturePos, 0x20)))
28: s := mload(add(signatures, add(signaturePos, 0x40)))
29:
30:
35: v := and(mload(add(signatures, add(signaturePos, 0x41))), 0xff)
36: }
37: }['45']
45: function getNSigners(
46: bytes32 dataHash,
47: bytes memory signatures,
48: uint256 requiredSignatures
49: )
50: internal
51: pure
52: returns (address[] memory _owners)
53: {
54: _owners = new address[](requiredSignatures);
55:
56: address currentOwner;
57: uint8 v; // <= FOUND
58: bytes32 r;
59: bytes32 s;
60: uint256 i;
61: for (i = 0; i < requiredSignatures; i++) {
62: (v, r, s) = signatureSplit(signatures, i);
63: if (v == 0) {
64:
65:
66: currentOwner = address(uint160(uint256(r)));
67: } else if (v == 1) {
68:
69:
70: currentOwner = address(uint160(uint256(r)));
71: } else if (v > 30) {
72:
73:
74:
75: currentOwner =
76: ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), v - 4, r, s);
77: } else {
78:
79:
80: currentOwner = ecrecover(dataHash, v, r, s);
81: }
82: _owners[i] = currentOwner;
83: }
84: }Replace spotted instances with != 0 for uints as this uses less gas
Num of instances: 2
Click to show findings
['74']
74: require(_thresholdPercentage > 0, "LivenessModule: thresholdPercentage must be greater than 0"); // <= FOUND['176']
176: if (_newOwnersCount > 0) { // <= FOUNDMaking function calls or external calls within loops in Solidity can lead to inefficient gas usage, potential bottlenecks, and increased vulnerability to attacks. Each function call or external call consumes gas, and when executed within a loop, the gas cost multiplies, potentially causing the transaction to run out of gas or exceed block gas limits. This can result in transaction failure or unpredictable behavior.
Num of instances: 4
Click to show findings
['140']
140: for (uint256 i = 0; i < _previousOwners.length; i++) {
141:
142:
143:
144:
145: if (ownersCount >= MIN_OWNERS) {
146: require(canRemove(_ownersToRemove[i]), "LivenessModule: the owner to remove has signed recently"); // <= FOUND
147: }
148:
149:
150:
151: ownersCount--;
152:
153:
154: _removeOwner({ // <= FOUND
155: _prevOwner: _previousOwners[i],
156: _ownerToRemove: _ownersToRemove[i],
157: _newOwnersCount: ownersCount
158: });
159:
160:
161:
162:
163:
164: if (ownersCount == 0) {
165: break;
166: }
167: }['140']
140: for (uint256 i = 0; i < _previousOwners.length; i++) {
141:
142:
143:
144:
145: if (ownersCount >= MIN_OWNERS) {
146: require(canRemove(_ownersToRemove[i]), "LivenessModule: the owner to remove has signed recently"); // <= FOUND
147: }
148:
149:
150:
151: ownersCount--;
152:
153:
154: _removeOwner({ // <= FOUND
155: _prevOwner: _previousOwners[i],
156: _ownerToRemove: _ownersToRemove[i],
157: _newOwnersCount: ownersCount
158: });
159:
160:
161:
162:
163:
164: if (ownersCount == 0) {
165: break;
166: }
167: }['61']
61: for (i = 0; i < requiredSignatures; i++) {
62: (v, r, s) = signatureSplit(signatures, i); // <= FOUND
63: if (v == 0) {
64:
65:
66: currentOwner = address(uint160(uint256(r)));
67: } else if (v == 1) {
68:
69:
70: currentOwner = address(uint160(uint256(r)));
71: } else if (v > 30) {
72:
73:
74:
75: currentOwner =
76: ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), v - 4, r, s);
77: } else {
78:
79:
80: currentOwner = ecrecover(dataHash, v, r, s);
81: }
82: _owners[i] = currentOwner;
83: }['61']
61: for (i = 0; i < requiredSignatures; i++) {
62: (v, r, s) = signatureSplit(signatures, i); // <= FOUND
63: if (v == 0) {
64:
65:
66: currentOwner = address(uint160(uint256(r)));
67: } else if (v == 1) {
68:
69:
70: currentOwner = address(uint160(uint256(r)));
71: } else if (v > 30) {
72:
73:
74:
75: currentOwner =
76: ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), v - 4, r, s);
77: } else {
78:
79:
80: currentOwner = ecrecover(dataHash, v, r, s);
81: }
82: _owners[i] = currentOwner;
83: }[Gas-8] For loops in public or external functions should be avoided due to high gas costs and possible DOS
In Solidity, for loops can potentially cause Denial of Service (DoS) attacks if not handled carefully. DoS attacks can occur when an attacker intentionally exploits the gas cost of a function, causing it to run out of gas or making it too expensive for other users to call. Below are some scenarios where for loops can lead to DoS attacks: Nested for loops can become exceptionally gas expensive and should be used sparingly
Num of instances: 3
Click to show findings
['69']
69: function checkTransaction(
70: address to,
71: uint256 value,
72: bytes memory data,
73: Enum.Operation operation,
74: uint256 safeTxGas,
75: uint256 baseGas,
76: uint256 gasPrice,
77: address gasToken,
78: address payable refundReceiver,
79: bytes memory signatures,
80: address msgSender
81: )
82: external
83: {
84: msgSender;
85: _requireOnlySafe();
86:
87:
88:
89: address[] memory owners = SAFE.getOwners();
90: for (uint256 i = 0; i < owners.length; i++) { // <= FOUND
91: ownersBefore.add(owners[i]);
92: }
93:
94:
95:
96: bytes32 txHash = SAFE.getTransactionHash({
97: to: to,
98: value: value,
99: data: data,
100: operation: operation,
101: safeTxGas: safeTxGas,
102: baseGas: baseGas,
103: gasPrice: gasPrice,
104: gasToken: gasToken,
105: refundReceiver: refundReceiver,
106: _nonce: SAFE.nonce() - 1
107: });
108:
109: uint256 threshold = SAFE.getThreshold();
110: address[] memory signers =
111: SafeSigners.getNSigners({ dataHash: txHash, signatures: signatures, requiredSignatures: threshold });
112:
113: for (uint256 i = 0; i < signers.length; i++) { // <= FOUND
114: lastLive[signers[i]] = block.timestamp;
115: emit OwnerRecorded(signers[i]);
116: }
117: }['125']
125: function checkAfterExecution(bytes32, bool) external {
126: _requireOnlySafe();
127:
128: address[] memory ownersAfter = SAFE.getOwners();
129:
130:
131: for (uint256 i = 0; i < ownersAfter.length; i++) { // <= FOUND
132:
133: address ownerAfter = ownersAfter[i];
134: if (ownersBefore.remove(ownerAfter) == false) {
135:
136: lastLive[ownerAfter] = block.timestamp;
137: }
138: }
139:
140:
141:
142:
143: address[] memory ownersBeforeCache = ownersBefore.values();
144: for (uint256 i = 0; i < ownersBeforeCache.length; i++) { // <= FOUND
145: address ownerBefore = ownersBeforeCache[i];
146: delete lastLive[ownerBefore];
147: ownersBefore.remove(ownerBefore);
148: }
149: }['133']
133: function removeOwners(address[] memory _previousOwners, address[] memory _ownersToRemove) external {
134: require(_previousOwners.length == _ownersToRemove.length, "LivenessModule: arrays must be the same length");
135:
136:
137:
138:
139: uint256 ownersCount = SAFE.getOwners().length;
140: for (uint256 i = 0; i < _previousOwners.length; i++) { // <= FOUND
141:
142:
143:
144:
145: if (ownersCount >= MIN_OWNERS) {
146: require(canRemove(_ownersToRemove[i]), "LivenessModule: the owner to remove has signed recently");
147: }
148:
149:
150:
151: ownersCount--;
152:
153:
154: _removeOwner({
155: _prevOwner: _previousOwners[i],
156: _ownerToRemove: _ownersToRemove[i],
157: _newOwnersCount: ownersCount
158: });
159:
160:
161:
162:
163:
164: if (ownersCount == 0) {
165: break;
166: }
167: }
168: _verifyFinalState();
169: }Leveraging inline assembly in Solidity provides the ability to directly manipulate the length slot of an array, thereby altering its length without the need to copy the elements to a new array of the desired size. This technique is more gas-efficient as it avoids the computational expense associated with array duplication. However, this method circumvents the type-checking and safety mechanisms of high-level Solidity and should be used judiciously. Always ensure that the array doesn't contain vital data beyond the revised length, as this data will become unreachable yet still consume storage space.
Num of instances: 1
In Solidity, the unchecked block allows arithmetic operations to not revert on overflow. Without using unchecked in loops, extra gas is consumed due to overflow checks. If it's certain that overflows won't occur within the loop, using unchecked can make the loop more gas-efficient by skipping unnecessary checks.
Num of instances: 12
Click to show findings
['49']
49: for (uint256 i = 0; i < owners.length; i++) {
50: address owner = owners[i];
51: lastLive[owner] = block.timestamp;
52: emit OwnerRecorded(owner);
53: }['90']
90: for (uint256 i = 0; i < owners.length; i++) {
91: ownersBefore.add(owners[i]);
92: }['131']
131: for (uint256 i = 0; i < ownersAfter.length; i++) {
132:
133: address ownerAfter = ownersAfter[i];
134: if (ownersBefore.remove(ownerAfter) == false) {
135:
136: lastLive[ownerAfter] = block.timestamp;
137: }
138: }['140']
140: for (uint256 i = 0; i < _previousOwners.length; i++) {
141:
142:
143:
144:
145: if (ownersCount >= MIN_OWNERS) {
146: require(canRemove(_ownersToRemove[i]), "LivenessModule: the owner to remove has signed recently");
147: }
148:
149:
150:
151: ownersCount--;
152:
153:
154: _removeOwner({
155: _prevOwner: _previousOwners[i],
156: _ownerToRemove: _ownersToRemove[i],
157: _newOwnersCount: ownersCount
158: });
159:
160:
161:
162:
163:
164: if (ownersCount == 0) {
165: break;
166: }
167: }['61']
61: for (i = 0; i < requiredSignatures; i++) {
62: (v, r, s) = signatureSplit(signatures, i);
63: if (v == 0) {
64:
65:
66: currentOwner = address(uint160(uint256(r)));
67: } else if (v == 1) {
68:
69:
70: currentOwner = address(uint160(uint256(r)));
71: } else if (v > 30) {
72:
73:
74:
75: currentOwner =
76: ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), v - 4, r, s);
77: } else {
78:
79:
80: currentOwner = ecrecover(dataHash, v, r, s);
81: }
82: _owners[i] = currentOwner;
83: }['49']
49: for (uint256 i = 0; i < owners.length; i++) {
50: address owner = owners[i];
51: lastLive[owner] = block.timestamp;
52: emit OwnerRecorded(owner);
53: }['90']
90: for (uint256 i = 0; i < owners.length; i++) {
91: ownersBefore.add(owners[i]);
92: }['113']
113: for (uint256 i = 0; i < signers.length; i++) {
114: lastLive[signers[i]] = block.timestamp;
115: emit OwnerRecorded(signers[i]);
116: }['131']
131: for (uint256 i = 0; i < ownersAfter.length; i++) {
132:
133: address ownerAfter = ownersAfter[i];
134: if (ownersBefore.remove(ownerAfter) == false) {
135:
136: lastLive[ownerAfter] = block.timestamp;
137: }
138: }['144']
144: for (uint256 i = 0; i < ownersBeforeCache.length; i++) {
145: address ownerBefore = ownersBeforeCache[i];
146: delete lastLive[ownerBefore];
147: ownersBefore.remove(ownerBefore);
148: }['140']
140: for (uint256 i = 0; i < _previousOwners.length; i++) {
141:
142:
143:
144:
145: if (ownersCount >= MIN_OWNERS) {
146: require(canRemove(_ownersToRemove[i]), "LivenessModule: the owner to remove has signed recently");
147: }
148:
149:
150:
151: ownersCount--;
152:
153:
154: _removeOwner({
155: _prevOwner: _previousOwners[i],
156: _ownerToRemove: _ownersToRemove[i],
157: _newOwnersCount: ownersCount
158: });
159:
160:
161:
162:
163:
164: if (ownersCount == 0) {
165: break;
166: }
167: }['61']
61: for (i = 0; i < requiredSignatures; i++) {
62: (v, r, s) = signatureSplit(signatures, i);
63: if (v == 0) {
64:
65:
66: currentOwner = address(uint160(uint256(r)));
67: } else if (v == 1) {
68:
69:
70: currentOwner = address(uint160(uint256(r)));
71: } else if (v > 30) {
72:
73:
74:
75: currentOwner =
76: ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), v - 4, r, s);
77: } else {
78:
79:
80: currentOwner = ecrecover(dataHash, v, r, s);
81: }
82: _owners[i] = currentOwner;
83: }Utilizing assembly for validating msg.sender can potentially save gas as it allows for more direct and efficient access to Ethereum’s EVM opcodes, bypassing some of the overhead introduced by Solidity’s higher-level abstractions. However, this practice requires deep expertise in EVM, as incorrect implementation can introduce critical vulnerabilities. It is a trade-off between gas efficiency and code safety.
Num of instances: 1
Click to show findings
['64']
64: require(msg.sender == address(SAFE), "LivenessGuard: only Safe can call this function"); // <= FOUNDUsing assembly for simple zero checks on unsigned integers can save gas due to lower-level, optimized operations.
Resolution: Implement inline assembly with Solidity's assembly block to perform zero checks. Ensure thorough testing and verification, as assembly lacks the safety checks of high-level Solidity, potentially introducing vulnerabilities if not used carefully.
Num of instances: 1
Click to show findings
['74']
74: require(_thresholdPercentage > 0, "LivenessModule: thresholdPercentage must be greater than 0"); // <= FOUNDSolidity version 0.8.19 introduced a array of gas optimizations which make contracts which use it more efficient. Provided compatability it may be beneficial to upgrade to this version
Num of instances: 2
Please elaborate and generalise the following with detail and feel free to use your own knowledge and lmit ur words to 100 words please:
Num of instances: 5
Click to show findings
['49']
49: for (uint256 i = 0; i < owners.length; i++) { // <= FOUND
50: address owner = owners[i]; // <= FOUND
51: lastLive[owner] = block.timestamp;
52: emit OwnerRecorded(owner);
53: }['49']
49: for (uint256 i = 0; i < owners.length; i++) { // <= FOUND
50: address owner = owners[i]; // <= FOUND
51: lastLive[owner] = block.timestamp;
52: emit OwnerRecorded(owner);
53: }['131']
131: for (uint256 i = 0; i < ownersAfter.length; i++) { // <= FOUND
132:
133: address ownerAfter = ownersAfter[i]; // <= FOUND
134: if (ownersBefore.remove(ownerAfter) == false) {
135:
136: lastLive[ownerAfter] = block.timestamp;
137: }
138: }['131']
131: for (uint256 i = 0; i < ownersAfter.length; i++) { // <= FOUND
132:
133: address ownerAfter = ownersAfter[i]; // <= FOUND
134: if (ownersBefore.remove(ownerAfter) == false) {
135:
136: lastLive[ownerAfter] = block.timestamp;
137: }
138: }['144']
144: for (uint256 i = 0; i < ownersBeforeCache.length; i++) { // <= FOUND
145: address ownerBefore = ownersBeforeCache[i]; // <= FOUND
146: delete lastLive[ownerBefore];
147: ownersBefore.remove(ownerBefore);
148: }Rather than calling .length for an array in a for loop declaration, it is far more gas efficient to cache this length before and use that instead. This will prevent the array length from being called every loop iteration
Num of instances: 10
Click to show findings
['49']
49: for (uint256 i = 0; i < owners.length; i++) // <= FOUND['49']
49: for (uint256 i = 0; i < owners.length; i++) // <= FOUND['131']
131: for (uint256 i = 0; i < ownersAfter.length; i++) // <= FOUND['140']
140: for (uint256 i = 0; i < _previousOwners.length; i++) // <= FOUND['49']
49: for (uint256 i = 0; i < owners.length; i++) // <= FOUND['49']
49: for (uint256 i = 0; i < owners.length; i++) // <= FOUND['113']
113: for (uint256 i = 0; i < signers.length; i++) // <= FOUND['131']
131: for (uint256 i = 0; i < ownersAfter.length; i++) // <= FOUND['144']
144: for (uint256 i = 0; i < ownersBeforeCache.length; i++) // <= FOUND['140']
140: for (uint256 i = 0; i < _previousOwners.length; i++) // <= FOUNDIf a internal function is only used once it doesn't make sense to modularise it unless the function which does call the function would be overly long and complex otherwise
Num of instances: 5
Click to show findings
['190']
190: function _swapToFallbackOwnerSafeCall(address _prevOwner, address _oldOwner) internal // <= FOUND['207']
207: function _removeOwnerSafeCall(address _prevOwner, address _owner, uint256 _threshold) internal // <= FOUND['221']
221: function _verifyFinalState() internal view // <= FOUND['17']
17: function signatureSplit( // <= FOUND
18: bytes memory signatures,
19: uint256 pos
20: )
21: internal
22: pure
23: returns (uint8 v, bytes32 r, bytes32 s)
24: ['45']
45: function getNSigners( // <= FOUND
46: bytes32 dataHash,
47: bytes memory signatures,
48: uint256 requiredSignatures
49: )
50: internal
51: pure
52: returns (address[] memory _owners)
53: Num of instances: 3
Click to show findings
['50']
50: constructor(Safe _safe, SuperchainConfig _superchainConfig, address _deputyGuardian) {
51: SAFE = _safe;
52: SUPERCHAIN_CONFIG = _superchainConfig;
53: DEPUTY_GUARDIAN = _deputyGuardian;
54: }['46']
46: constructor(Safe _safe) {
47: SAFE = _safe;
48: address[] memory owners = _safe.getOwners();
49: for (uint256 i = 0; i < owners.length; i++) {
50: address owner = owners[i];
51: lastLive[owner] = block.timestamp;
52: emit OwnerRecorded(owner);
53: }
54: }['54']
54: constructor(
55: Safe _safe,
56: LivenessGuard _livenessGuard,
57: uint256 _livenessInterval,
58: uint256 _minOwners,
59: uint256 _thresholdPercentage,
60: address _fallbackOwner
61: ) {
62: SAFE = _safe;
63: LIVENESS_GUARD = _livenessGuard;
64: LIVENESS_INTERVAL = _livenessInterval;
65: THRESHOLD_PERCENTAGE = _thresholdPercentage;
66: FALLBACK_OWNER = _fallbackOwner;
67: MIN_OWNERS = _minOwners;
68: address[] memory owners = _safe.getOwners();
69: require(_minOwners <= owners.length, "LivenessModule: minOwners must be less than the number of owners");
70: require(
71: _safe.getThreshold() >= getRequiredThreshold(owners.length),
72: "LivenessModule: Insufficient threshold for the number of owners"
73: );
74: require(_thresholdPercentage > 0, "LivenessModule: thresholdPercentage must be greater than 0");
75: require(_thresholdPercentage <= 100, "LivenessModule: thresholdPercentage must be less than or equal to 100");
76: }[Gas-18] There are comparisons to boolean literals (true and false), these can be simplified to save gas
In Solidity, gas optimization is crucial due to the cost associated with executing operations on the Ethereum network. Simplifying comparisons to boolean literals is one such optimization technique. Instead of explicitly comparing a boolean expression to true or false, you can use the expression directly or its negation. For example, replace if (x == true) with if (x) and if (x == false) with if (!x). This simplification reduces the bytecode size of the contract, thereby saving gas. It also enhances code readability and clarity.
Num of instances: 1
Emitting events in setter functions of smart contracts only when state variables change saves gas. This is because emitting events consumes gas, and unnecessary events, where no actual state change occurs, lead to wasteful consumption.
Num of instances: 1
Click to show findings
['133']
133: function setRespectedGameType(OptimismPortal2 _portal, GameType _gameType) external { // <= FOUND
134: _onlyDeputyGuardian();
135:
136: bytes memory data = abi.encodeCall(OptimismPortal2.setRespectedGameType, (_gameType));
137: (bool success, bytes memory returnData) =
138: SAFE.execTransactionFromModuleReturnData(address(_portal), 0, data, Enum.Operation.Call);
139: if (!success) {
140: revert ExecutionFailed(string(returnData));
141: }
142: emit RespectedGameTypeSet(_gameType); // <= FOUND
143: }Emitting variable literals (true, false, 'hello', 1 etc...) in events is inefficient, as it consumes extra gas without providing added value. These literals are fixed values that can be accessed or hardcoded elsewhere in the smart contract or application, making their inclusion in events redundant and an unnecessary drain on resources during transaction execution.
Num of instances: 1
The OpenZeppelin Array.unsafeAccess() method is a optimization strategy for Solidity, aimed at reducing gas costs by bypassing automatic length checks on storage array accesses. In Solidity, every access to an array element involves a hidden gas cost due to a length check, ensuring that accesses do not exceed the array bounds. However, if a developer has already verified the array's bounds earlier in the function or knows through logic that the access is safe, directly accessing the array elements without redundant length checks can save gas. This approach requires careful consideration to avoid out-of-bounds errors, as it trades off safety checks for efficiency.
Num of instances: 9
Click to show findings
['50']
50: address owner = owners[i]; // <= FOUND['91']
91: ownersBefore.add(owners[i]); // <= FOUND['114']
114: lastLive[signers[i]] = block.timestamp; // <= FOUND['115']
115: emit OwnerRecorded(signers[i]); // <= FOUND['133']
133:
134: address ownerAfter = ownersAfter[i]; // <= FOUND['145']
145: address ownerBefore = ownersBeforeCache[i]; // <= FOUND['146']
146: require(canRemove(_ownersToRemove[i]), "LivenessModule: the owner to remove has signed recently"); // <= FOUND['154']
154:
155: _removeOwner({
156: _prevOwner: _previousOwners[i], // <= FOUND
157: _ownerToRemove: _ownersToRemove[i], // <= FOUND
158: _newOwnersCount: ownersCount
159: });['82']
82: _owners[i] = currentOwner; // <= FOUNDEmitting events inside loops can significantly increase gas costs in Ethereum smart contracts, as each event emission consumes gas. This practice can quickly escalate transaction fees, especially with a high number of iterations. To optimize for efficiency and cost, it's advisable to minimize event emissions within loops, possibly aggregating data to emit a single event post-loop or reconsidering the design to reduce looped emissions. This approach helps maintain manageable transaction costs and enhances contract performance.
Num of instances: 6
Click to show findings
['49']
49: for (uint256 i = 0; i < owners.length; i++) {
50: address owner = owners[i];
51: lastLive[owner] = block.timestamp;
52: emit OwnerRecorded(owner); // <= FOUND
53: }['49']
49: for (uint256 i = 0; i < owners.length; i++) {
50: address owner = owners[i];
51: lastLive[owner] = block.timestamp;
52: emit OwnerRecorded(owner); // <= FOUND
53: }['113']
113: for (uint256 i = 0; i < signers.length; i++) {
114: lastLive[signers[i]] = block.timestamp;
115: emit OwnerRecorded(signers[i]); // <= FOUND
116: }['49']
49: for (uint256 i = 0; i < owners.length; i++) {
50: address owner = owners[i];
51: lastLive[owner] = block.timestamp;
52: emit OwnerRecorded(owner); // <= FOUND
53: }['49']
49: for (uint256 i = 0; i < owners.length; i++) {
50: address owner = owners[i];
51: lastLive[owner] = block.timestamp;
52: emit OwnerRecorded(owner); // <= FOUND
53: }['113']
113: for (uint256 i = 0; i < signers.length; i++) {
114: lastLive[signers[i]] = block.timestamp;
115: emit OwnerRecorded(signers[i]); // <= FOUND
116: }Public functions that aren't used internally in Solidity contracts should be made external to optimize gas usage and improve contract efficiency. External functions can only be called from outside the contract, and their arguments are directly read from the calldata, which is more gas-efficient than loading them into memory, as is the case for public functions. By using external visibility, developers can reduce gas consumption for external calls and ensure that the contract operates more cost-effectively for users. Moreover, setting the appropriate visibility level for functions also enhances code readability and maintainability, promoting a more secure and well-structured contract design.
Num of instances: 8
Click to show findings
['86']
86: function safe() public view returns (Safe safe_) ['64']
64: function superchainConfig() public view returns (SuperchainConfig superchainConfig_) ['70']
70: function deputyGuardian() public view returns (address deputyGuardian_) ['92']
92: function livenessGuard() public view returns (LivenessGuard livenessGuard_) ['98']
98: function livenessInterval() public view returns (uint256 livenessInterval_) ['104']
104: function minOwners() public view returns (uint256 minOwners_) ['110']
110: function thresholdPercentage() public view returns (uint256 thresholdPercentage_) ['116']
116: function fallbackOwner() public view returns (address fallbackOwner_)