| Number | Details | Instances |
|---|---|---|
| [Medium-1] | Privileged functions can create points of failure | 1 |
| Number | Details | Instances |
|---|---|---|
| [Low-1] | Require statements should have error string | 1 |
| [Low-2] | Use SafeCast to safely downcast variables | 1 |
| [Low-3] | For loops in public or external functions should be avoided due to high gas costs and possible DOS | 2 |
| [Low-4] | Initializer function can be front run | 1 |
| [Low-5] | Critical functions should be a two step procedure | 1 |
| [Low-6] | Missing zero address check in initializer | 1 |
| [Low-7] | Critical functions should have a timelock | 1 |
| [Low-8] | Consider implementing two-step procedure for updating protocol addresses | 1 |
| [Low-9] | Prefer skip over revert model in iteration | 2 |
| [Low-10] | Use of abi.encodePacked with dynamic types inside keccak256 | 2 |
| [Low-11] | No license selected for project | 1 |
| [Low-12] | Constructors missing validation | 1 |
| [Low-13] | Contract contains payable functions but no withdraw/sweep function | 1 |
| [Low-14] | int/uint passed into abi.encodePacked without casting to a string. | 2 |
| Number | Details | Instances |
|---|---|---|
| [NonCritical-1] | Functions which set address state variables should have zero address checks | 1 |
| [NonCritical-2] | It is standard for all external and public functions to be override from an interface | 9 |
| [NonCritical-3] | Using abi.encodePacked can result in hash collision when used in hashing functions | 2 |
| [NonCritical-4] | Non constant/immutable state variables are missing a setter post deployment | 3 |
| [NonCritical-5] | Consider adding emergency-stop functionality | 1 |
| [NonCritical-6] | Employ Explicit Casting to Bytes or Bytes32 for Enhanced Code Clarity and Meaning | 2 |
| [NonCritical-7] | Consider making private state variables internal to increase flexibility | 1 |
| [NonCritical-8] | Using Low-Level Call for Transfers | 1 |
| [NonCritical-9] | External call recipient may consume all transaction gas | 1 |
| [NonCritical-10] | Library has public/external functions | 1 |
| [NonCritical-11] | NatSpec: @notice tags missing from contract/abstract/library/interface/function/modifier/constructor/receive/fallback | 6 |
| [NonCritical-12] | Natspec @author is missing from contract/interface/library | 1 |
| [NonCritical-13] | Natspec @title is missing from contract/interface/library | 1 |
| [NonCritical-14] | NatSpec: @dev tags missing from contract/abstract/library/interface/function/modifier/constructor/receive/fallback | 29 |
| [NonCritical-15] | NatSpec: @param tags missing from function/modifier/constructor | 6 |
| [NonCritical-16] | NatSpec: @return tags missing from function/modifier/constructor | 2 |
| [NonCritical-17] | Inconsistent comment spacing | 4 |
| [NonCritical-18] | Floating pragma should be avoided | 1 |
| [NonCritical-19] | Open TODO in comments | 1 |
| [NonCritical-20] | In functions which accept an address as a parameter, there should be a zero address check to prevent bugs | 3 |
| [NonCritical-21] | Functions which are either private or internal should have a preceding _ in their name | 10 |
| [NonCritical-22] | Contract lines should not be longer than 120 characters for readability | 22 |
| [NonCritical-23] | Setters should prevent re-setting of the same value | 1 |
| [NonCritical-24] | Specific imports should be used where possible so only used code is imported | 3 |
| [NonCritical-25] | Use newer solidity versions | 1 |
| [NonCritical-26] | Function names should differ to make the code more readable | 2 |
| [NonCritical-27] | Functions within contracts are not ordered according to the solidity style guide | 1 |
| [NonCritical-28] | Emits without msg.sender parameter | 1 |
| [NonCritical-29] | Interface imports should be declared first | 3 |
| [NonCritical-30] | SPDX identifier should be the in the first line of a solidity file | 1 |
| [NonCritical-31] | Use allowlist/denylist rather than whitelist/blacklist | 3 |
| [NonCritical-32] | Multiple mappings can be replaced with a single struct mapping | 1 |
| [NonCritical-33] | Unused state variables present | 3 |
| [NonCritical-34] | Initialize functions do not emit an event | 1 |
| [NonCritical-35] | Consider using named mappings | 3 |
| [NonCritical-36] | Use a single contract or library for system wide constants | 1 |
| [NonCritical-37] | Consider using modifiers for address control | 2 |
| [NonCritical-38] | Variables should be used in place of magic numbers to improve readability | 1 |
| [NonCritical-39] | Inconsistent usage of int/uint with int256/uint256 in contract/abstract/library/interface | 2 |
| [NonCritical-40] | Unused structs present | 1 |
| [NonCritical-41] | Empty bytes check is missing | 2 |
| [NonCritical-42] | Consider using SMTChecker | 7 |
| [NonCritical-43] | Top level declarations should be separated by two blank lines | 1 |
| [NonCritical-44] | Contracts should have full test coverage | 2 |
| [NonCritical-45] | Consider using SafeTransferLib.safeTransferETH() or Address.sendValue() for clearer semantic meaning | 1 |
| [NonCritical-46] | Whitespace in expressions | 1 |
| [NonCritical-47] | Public state variables should include natspec comments | 1 |
| [NonCritical-48] | Lack Of Brace Spacing | 18 |
| [NonCritical-49] | Consider adding formal verification proofs | 2 |
| [NonCritical-50] | Unused import | 1 |
| [NonCritical-51] | Consider bounding input array length | 2 |
| [NonCritical-52] | Missing events in sensitive functions | 1 |
| [NonCritical-53] | Don't assume specific ETH balance | 1 |
| [NonCritical-54] | It is best practice to use linear inheritance | 1 |
| [NonCritical-55] | A event should be emitted if a non immutable state variable is set in a constructor | 1 |
| [NonCritical-56] | Superfluous parameter can only be one value | 2 |
| [NonCritical-57] | Unnecessary msg.sender conditional check | 1 |
| [NonCritical-58] | Use the Modern Upgradeable Contract Paradigm | 2 |
| [NonCritical-59] | Use transfer libraries instead of low level calls | 1 |
| [NonCritical-60] | Use a struct to encapsulate multiple function parameters | 1 |
| [NonCritical-61] | Contracts inherits pausable without utilising whenNotPaused | 1 |
| [NonCritical-62] | Consider using a format prettier or forge fmt | 8 |
| [NonCritical-63] | Avoid defining a function in a single line including it's contents | 6 |
| [NonCritical-64] | Use 'using' keyword when using specific imports rather than calling the specific import directly | 34 |
| [NonCritical-65] | All verbatim blocks are considered identical by deduplicator and can incorrectly be unified | 1 |
| [NonCritical-66] | Constructors should emit an event | 2 |
| [NonCritical-67] | Avoid single line non empty object declarations | 17 |
| [NonCritical-68] | Contract and Abstract files should have a fixed compiler version | 3 |
| [NonCritical-69] | Consider using 'using-for' syntax when using libraries | 12 |
| [NonCritical-70] | Consider validating all user inputs | 5 |
| [NonCritical-71] | Consider providing a ranged getter for array state variables | 1 |
| [NonCritical-72] | Consider using named returns | 2 |
| [NonCritical-73] | Avoid external calls in modifiers | 1 |
| Number | Details | Instances | Gas |
|---|---|---|---|
| [Gas-1] | Only emit event in setter function if the state variable was changed | 1 | 0.0 |
| [Gas-2] | Some error strings are not descriptive | 1 | 0.0 |
| [Gas-3] | State variables which are not modified within functions should be set as constants or immutable for values set at deployment | 2 | 0.0 |
| [Gas-4] | Low level call can be optimized with assembly | 3 | 2232 |
| [Gas-5] | There is a 32 byte length threshold for error strings, strings longer than this consume more gas | 5 | 350 |
| [Gas-6] | Public functions not used internally can be marked as external to save gas | 1 | 0.0 |
| [Gas-7] | Calldata should be used in place of memory function parameters when not mutated | 1 | 13 |
| [Gas-8] | Usage of smaller uint/int types causes overhead | 9 | 4455 |
| [Gas-9] | Integer increments by one can be unchecked to save on gas fees | 2 | 480 |
| [Gas-10] | Default bool values are manually reset | 1 | 0.0 |
| [Gas-11] | Default int values are manually reset | 3 | 0.0 |
| [Gas-12] | Use assembly to check for the zero address | 2 | 0.0 |
| [Gas-13] | Redundant state variable getters | 1 | 0.0 |
| [Gas-14] | Consider activating via-ir for deploying | 7 | 12250 |
| [Gas-15] | Use bitmap to save gas | 3 | 630 |
| [Gas-16] | Use assembly to emit events | 2 | 152 |
| [Gas-17] | Counting down in for statements is more gas efficient | 2 | 0.0 |
| [Gas-18] | Using private rather than public for constants and immutables, saves gas | 3 | 0.0 |
| [Gas-19] | Mark Functions That Revert For Normal Users As payable | 1 | 25 |
| [Gas-20] | Function names can be optimized | 2 | 512 |
| [Gas-21] | Lack of unchecked in loops | 2 | 240 |
| [Gas-22] | Consider migrating require statements to custom errors | 20 | 5600 |
| [Gas-23] | Consider not using libraries when implementing simple functionality. | 2 | 4000 |
| [Gas-24] | Use assembly to validate msg.sender | 5 | 0.0 |
| [Gas-25] | Optimize Deployment Size by Fine-tuning IPFS Hash | 2 | 42400 |
| [Gas-26] | Avoid Unnecessary Public Variables | 9 | 1782000 |
| [Gas-27] | Stack variable cost less than state variables while used in emiting event | 1 | 9 |
| [Gas-28] | Inline modifiers used only once | 1 | 0.0 |
| [Gas-29] | ++X costs slightly less gas than X++ (same with --) | 2 | 20 |
| [Gas-30] | Solidity versions 0.8.19 and above are more gas efficient | 1 | 1000 |
| [Gas-31] | Variable declared within iteration | 1 | 0.0 |
| [Gas-32] | Calling .length in a for loop wastes gas | 2 | 388 |
| [Gas-33] | Internal functions only used once can be inlined so save gas | 5 | 750 |
| [Gas-34] | Constructors can be marked as payable to save deployment gas | 2 | 0.0 |
| [Gas-35] | Use assembly scratch space to build calldata for external calls | 15 | 49500 |
| [Gas-36] | Use assembly scratch space to build calldata for event emits | 2 | 880 |
| [Gas-37] | Consider using solady's "FixedPointMathLib" | 1 | 0.0 |
| [Gas-38] | Internal functions never used once can be removed | 2 | 0.0 |
| [Gas-39] | Use uint256(1)/uint256(2) instead of true/false to save gas for changes | 2 | 34200 |
| [Gas-40] | Enable IR-based code generation | 2 | 0.0 |
| [Gas-41] | Call msg.sender directly rather than caching the value to save gas | 1 | 0.0 |
| [Gas-42] | Write direct outcome, instead of performing mathematical operations for constant state variables | 1 | 0.0 |
Ensure such accounts are protected and consider implementing multi sig to prevent a single point of failure
Num of instances: 1
Click to show findings
['126']
126: function setBatchConfirmer(address _batchConfirmer) external onlyOwner() // <= FOUND
Adding error strings to require statements in Solidity contracts, although not mandatory, enhances error handling, debugging, and overall contract maintainability. Providing a descriptive error message with each require statement helps identify the specific reason for a transaction failure, making it easier for developers to troubleshoot issues and for users to understand the cause of a revert. Including error strings improves code readability and fosters transparency, as the logic and conditions behind each requirement are clearly communicated
Num of instances: 1
Downcasting int/uints in Solidity can be unsafe due to the potential for data loss and unintended behavior. When downcasting a larger integer type to a smaller one (e.g., uint256 to uint128), the value may exceed the range of the target type, leading to truncation and loss of significant digits. This data loss can result in unexpected state changes, incorrect calculations, or other contract vulnerabilities, ultimately compromising the contracts functionality and reliability. To prevent these risks, developers should carefully consider the range of values their variables may hold and ensure that proper checks are in place to prevent out-of-range values before performing downcasting. Also consider using OZ SafeCast functionality.
Num of instances: 1
Click to show findings
['56']
32: function verifyBlob(
33: IEigenDAServiceManager.BlobHeader calldata blobHeader,
34: IEigenDAServiceManager eigenDAServiceManager,
35: BlobVerificationProof calldata blobVerificationProof
36: ) external view {
37: require(
38: EigenDAHasher.hashBatchMetadata(blobVerificationProof.batchMetadata)
39: == eigenDAServiceManager.batchIdToBatchMetadataHash(blobVerificationProof.batchId),
40: "EigenDARollupUtils.verifyBlob: batchMetadata does not match stored metadata"
41: );
42:
43: require(
44: Merkle.verifyInclusionKeccak(
45: blobVerificationProof.inclusionProof,
46: blobVerificationProof.batchMetadata.batchHeader.blobHeadersRoot,
47: keccak256(abi.encodePacked(EigenDAHasher.hashBlobHeader(blobHeader))),
48: blobVerificationProof.blobIndex
49: ),
50: "EigenDARollupUtils.verifyBlob: inclusion proof is invalid"
51: );
52:
53:
54: for (uint i = 0; i < blobHeader.quorumBlobParams.length; i++) {
55:
56: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumNumbers[uint8(blobVerificationProof.quorumThresholdIndexes[i])]) == blobHeader.quorumBlobParams[i].quorumNumber, // <= FOUND
57: "EigenDARollupUtils.verifyBlob: quorumNumber does not match"
58: );
59:
60:
61: require(blobHeader.quorumBlobParams[i].adversaryThresholdPercentage
62: < blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
63: "EigenDARollupUtils.verifyBlob: adversaryThresholdPercentage is not valid"
64: );
65:
66:
67: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumThresholdPercentages[uint8(blobVerificationProof.quorumThresholdIndexes[i])]) // <= FOUND
68: >= blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
69: "EigenDARollupUtils.verifyBlob: quorumThresholdPercentage is not met"
70: );
71:
72: }
73: }
[Low-3] 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: 2
Click to show findings
['54']
32: function verifyBlob(
33: IEigenDAServiceManager.BlobHeader calldata blobHeader,
34: IEigenDAServiceManager eigenDAServiceManager,
35: BlobVerificationProof calldata blobVerificationProof
36: ) external view {
37: require(
38: EigenDAHasher.hashBatchMetadata(blobVerificationProof.batchMetadata)
39: == eigenDAServiceManager.batchIdToBatchMetadataHash(blobVerificationProof.batchId),
40: "EigenDARollupUtils.verifyBlob: batchMetadata does not match stored metadata"
41: );
42:
43: require(
44: Merkle.verifyInclusionKeccak(
45: blobVerificationProof.inclusionProof,
46: blobVerificationProof.batchMetadata.batchHeader.blobHeadersRoot,
47: keccak256(abi.encodePacked(EigenDAHasher.hashBlobHeader(blobHeader))),
48: blobVerificationProof.blobIndex
49: ),
50: "EigenDARollupUtils.verifyBlob: inclusion proof is invalid"
51: );
52:
53:
54: for (uint i = 0; i < blobHeader.quorumBlobParams.length; i++) { // <= FOUND
55:
56: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumNumbers[uint8(blobVerificationProof.quorumThresholdIndexes[i])]) == blobHeader.quorumBlobParams[i].quorumNumber,
57: "EigenDARollupUtils.verifyBlob: quorumNumber does not match"
58: );
59:
60:
61: require(blobHeader.quorumBlobParams[i].adversaryThresholdPercentage
62: < blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
63: "EigenDARollupUtils.verifyBlob: adversaryThresholdPercentage is not valid"
64: );
65:
66:
67: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumThresholdPercentages[uint8(blobVerificationProof.quorumThresholdIndexes[i])])
68: >= blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
69: "EigenDARollupUtils.verifyBlob: quorumThresholdPercentage is not met"
70: );
71:
72: }
73: }
['104']
67: function confirmBatch(
68: BatchHeader calldata batchHeader,
69: NonSignerStakesAndSignature memory nonSignerStakesAndSignature
70: ) external onlyWhenNotPaused(PAUSED_CONFIRM_BATCH) onlyBatchConfirmer() {
71:
72: require(tx.origin == msg.sender, "EigenDAServiceManager.confirmBatch: header and nonsigner data must be in calldata");
73:
74: require(
75: batchHeader.referenceBlockNumber <= block.number, "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is in future"
76: );
77:
78: require(
79: (batchHeader.referenceBlockNumber + BLOCK_STALE_MEASURE) >= uint32(block.number),
80: "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is too far in past"
81: );
82:
83:
84: require(
85: batchHeader.quorumNumbers.length == batchHeader.quorumThresholdPercentages.length,
86: "EigenDAServiceManager.confirmBatch: quorumNumbers and quorumThresholdPercentages must be of the same length"
87: );
88:
89:
90: bytes32 reducedBatchHeaderHash = batchHeader.hashBatchHeaderToReducedBatchHeader();
91:
92:
93: (
94: QuorumStakeTotals memory quorumStakeTotals,
95: bytes32 signatoryRecordHash
96: ) = checkSignatures(
97: reducedBatchHeaderHash,
98: batchHeader.quorumNumbers,
99: batchHeader.referenceBlockNumber,
100: nonSignerStakesAndSignature
101: );
102:
103:
104: for (uint i = 0; i < batchHeader.quorumThresholdPercentages.length; i++) { // <= FOUND
105:
106:
107: require(
108: quorumStakeTotals.signedStakeForQuorum[i] * THRESHOLD_DENOMINATOR >=
109: quorumStakeTotals.totalStakeForQuorum[i] * uint8(batchHeader.quorumThresholdPercentages[i]),
110: "EigenDAServiceManager.confirmBatch: signatories do not own at least threshold percentage of a quorum"
111: );
112: }
113:
114:
115: uint32 batchIdMemory = batchId;
116: bytes32 batchHeaderHash = batchHeader.hashBatchHeader();
117: batchIdToBatchMetadataHash[batchIdMemory] = EigenDAHasher.hashBatchHashedMetadata(batchHeaderHash, signatoryRecordHash, uint32(block.number));
118:
119: emit BatchConfirmed(reducedBatchHeaderHash, batchIdMemory);
120:
121:
122: batchId = batchIdMemory + 1;
123: }
In Solidity contract deployment, not making the initialize() function call atomic with the contract creation can leave a vulnerability window. A malicious actor could exploit this time gap and call initialize() before the intended initialization. This action could disrupt the contract's setup, potentially necessitating a full contract re-deployment to ensure proper initialization. To mitigate such risks, it's advised to use a factory contract. This factory contract can be programmed to deploy and initialize a new contract in a single atomic transaction, closing the window of vulnerability and ensuring correct and secure contract initialization.
Num of instances: 1
Click to show findings
[]
47: function initialize(
48: IPauserRegistry _pauserRegistry,
49: uint256 _initialPausedStatus,
50: address _initialOwner,
51: address _batchConfirmer
52: )
53: public
54: initializer
55:
Critical functions in Solidity contracts should follow a two-step procedure to enhance security, minimize human error, and ensure proper access control. By dividing sensitive operations into distinct phases, such as initiation and confirmation, developers can introduce a safeguard against unintended actions or unauthorized access.
Num of instances: 1
Click to show findings
['126']
126: function setBatchConfirmer(address _batchConfirmer) external onlyOwner() // <= FOUND
Initializer functions in contracts often set important parameters or addresses. Failing to check for the zero address (0x0000000000000000000000000000000000000000) in initializers can lead to unintended behavior, as this address typically signifies an unset or default value. Transfers to or interactions with the zero address can result in permanent loss of assets or broken functionality. It's crucial to add checks using require(targetAddress != address(0), "Address cannot be zero") in initializers to prevent accidentally setting important state variables or parameters to this address, ensuring the system's integrity and user asset safety.
Num of instances: 1
Click to show findings
['47']
47: function initialize(
48: IPauserRegistry _pauserRegistry,
49: uint256 _initialPausedStatus,
50: address _initialOwner,
51: address _batchConfirmer
52: )
53: public
54: initializer
55: {
56: _initializePauser(_pauserRegistry, _initialPausedStatus);
57: _transferOwnership(_initialOwner);
58: _setBatchConfirmer(_batchConfirmer);
59: }
Critical functions, especially those affecting protocol parameters or user funds, are potential points of failure or exploitation. To mitigate risks, incorporating a timelock on such functions can be beneficial. A timelock requires a waiting period between the time an action is initiated and when it's executed, giving stakeholders time to react, potentially vetoing malicious or erroneous changes. To implement, integrate a smart contract like OpenZeppelin's TimelockController or build a custom mechanism. This ensures governance decisions or administrative changes are transparent and allows for community or multi-signature interventions, enhancing protocol security and trustworthiness.
Num of instances: 1
Click to show findings
['126']
126: function setBatchConfirmer(address _batchConfirmer) external onlyOwner() // <= FOUND
Implementing a two-step procedure for updating protocol addresses adds an extra layer of security. In such a system, the first step initiates the change, and the second step, after a predefined delay, confirms and finalizes it. This delay allows stakeholders or monitoring tools to observe and react to unintended or malicious changes. If an unauthorized change is detected, corrective actions can be taken before the change is finalized. To achieve this, introduce a "proposed address" state variable and a "delay period". Upon an update request, set the "proposed address". After the delay, if not contested, the main protocol address can be updated.
Num of instances: 1
Click to show findings
['126']
126: function setBatchConfirmer(address _batchConfirmer) external onlyOwner() { // <= FOUND
127: _setBatchConfirmer(_batchConfirmer);
128: }
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
['54']
54: for (uint i = 0; i < blobHeader.quorumBlobParams.length; i++) { // <= FOUND
55:
56: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumNumbers[uint8(blobVerificationProof.quorumThresholdIndexes[i])]) == blobHeader.quorumBlobParams[i].quorumNumber,
57: "EigenDARollupUtils.verifyBlob: quorumNumber does not match"
58: );
59:
60:
61: require(blobHeader.quorumBlobParams[i].adversaryThresholdPercentage
62: < blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
63: "EigenDARollupUtils.verifyBlob: adversaryThresholdPercentage is not valid"
64: );
65:
66:
67: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumThresholdPercentages[uint8(blobVerificationProof.quorumThresholdIndexes[i])])
68: >= blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
69: "EigenDARollupUtils.verifyBlob: quorumThresholdPercentage is not met"
70: );
71:
72: }
['104']
104: for (uint i = 0; i < batchHeader.quorumThresholdPercentages.length; i++) { // <= FOUND
105:
106:
107: require(
108: quorumStakeTotals.signedStakeForQuorum[i] * THRESHOLD_DENOMINATOR >=
109: quorumStakeTotals.totalStakeForQuorum[i] * uint8(batchHeader.quorumThresholdPercentages[i]),
110: "EigenDAServiceManager.confirmBatch: signatories do not own at least threshold percentage of a quorum"
111: );
112: }
Using abi.encodePacked with dynamic types for hashing functions like keccak256 can be risky due to the potential for hash collisions. This function concatenates arguments tightly, without padding, which might lead to different inputs producing the same hash. This is especially problematic with dynamic types, where the boundaries between inputs can blur. To mitigate this, use abi.encode instead. abi.encode pads its arguments to 32 bytes, creating clear distinctions between different inputs and significantly reducing the chance of hash collisions. This approach ensures more reliable and collision-resistant hashing, crucial for maintaining data integrity and security in smart contracts.
Num of instances: 2
Click to show findings
['24']
24: return keccak256(abi.encodePacked(batchHeaderHash, signatoryRecordHash, blockNumber)); // <= FOUND
['38']
38: return keccak256(abi.encodePacked(batchHeaderHash, confirmationData, blockNumber)); // <= FOUND
Not selecting a license for a software project, particularly in the context of blockchain and smart contracts, poses significant risks and uncertainties. A license explicitly defines how others can use, modify, distribute, and contribute to your code. Without it, the legal status of a project remains unclear, potentially deterring usage and contributions, and raising concerns about intellectual property rights. To mitigate these risks, it is advisable to choose an appropriate open-source license. This fosters a clear understanding of how your project can be utilized and encourages collaboration and adoption within the community, aligning with the open and collaborative nature of blockchain technology.
Num of instances: 1
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: 1
Click to show findings
['35']
35: constructor(IEigenDAServiceManager _eigenDAServiceManager, BN254.G1Point memory _tau, uint256 _illegalValue, uint256 _stakeRequired) {
36: eigenDAServiceManager = _eigenDAServiceManager; // <= FOUND
37: tau = _tau; // <= FOUND
38: illegalValue = _illegalValue; // <= FOUND
39: stakeRequired = _stakeRequired; // <= FOUND
40: }
In smart contract development, particularly for Ethereum, having payable functions without a corresponding withdraw or sweep function can lead to potential issues. Payable functions allow the contract to receive Ether, but without a mechanism to withdraw these funds, the Ether can become locked within the contract indefinitely. This situation might be intentional in some cases (like a burn function), but generally, it’s a design oversight. A withdraw or sweep function is necessary to transfer Ether out of the contract to a specific address, typically the owner's or a designated recipient. Without this, the contract lacks flexibility in managing its funds, potentially leading to lost or inaccessible Ether.
Num of instances: 1
Not casting an integer as a string before passing it into abi.encode can result in unintended behaviour. lets say '1' being encoded as '1' it will be encoded as char(1) which is the 'start of heading' control character or id of 60 would be encoded as '<'. This is may not be intended. To rectify this, simply cast the Id as a string with string(id) or ideally use solmate's libString library (toString)
Num of instances: 2
Click to show findings
['19']
19: function hashBatchHashedMetadata(
20: bytes32 batchHeaderHash,
21: bytes32 signatoryRecordHash,
22: uint32 blockNumber
23: ) internal pure returns(bytes32) {
24: return keccak256(abi.encodePacked(batchHeaderHash, signatoryRecordHash, blockNumber)); // <= FOUND
25: }
['33']
33: function hashBatchHashedMetadata(
34: bytes32 batchHeaderHash,
35: bytes memory confirmationData,
36: uint32 blockNumber
37: ) internal pure returns(bytes32) {
38: return keccak256(abi.encodePacked(batchHeaderHash, confirmationData, blockNumber)); // <= FOUND
39: }
Num of instances: 1
Click to show findings
['126']
126: function setBatchConfirmer(address _batchConfirmer) external onlyOwner() {
127: _setBatchConfirmer(_batchConfirmer);
128: }
[NonCritical-2] It is standard for all external and public functions to be override from an interface
This is to ensure the whole API is extracted in a interface
Num of instances: 9
Click to show findings
['32']
32: function verifyBlob(
33: IEigenDAServiceManager.BlobHeader calldata blobHeader,
34: IEigenDAServiceManager eigenDAServiceManager,
35: BlobVerificationProof calldata blobVerificationProof
36: ) external view
['67']
67: function confirmBatch(
68: BatchHeader calldata batchHeader,
69: NonSignerStakesAndSignature memory nonSignerStakesAndSignature
70: ) external onlyWhenNotPaused(PAUSED_CONFIRM_BATCH) onlyBatchConfirmer()
['126']
126: function setBatchConfirmer(address _batchConfirmer) external onlyOwner()
['138']
138: function taskNumber() external view returns (uint32)
['143']
143: function latestServeUntilBlock() external view returns (uint32)
['43']
43: function registerValidator() external payable
['55']
55: function postCommitment(
56: IEigenDAServiceManager.BlobHeader memory blobHeader,
57: EigenDARollupUtils.BlobVerificationProof memory blobVerificationProof
58: ) external
['74']
74: function challengeCommitment(uint256 timestamp, uint256 point, BN254.G2Point memory proof) external
['47']
47: function initialize(
48: IPauserRegistry _pauserRegistry,
49: uint256 _initialPausedStatus,
50: address _initialOwner,
51: address _batchConfirmer
52: )
53: public
54: initializer
55:
Consider using abi.encode as this pads data to 32 byte segments
Num of instances: 2
Click to show findings
['24']
24: return keccak256(abi.encodePacked(batchHeaderHash, signatoryRecordHash, blockNumber)); // <= FOUND
['38']
38: return keccak256(abi.encodePacked(batchHeaderHash, confirmationData, blockNumber)); // <= FOUND
Non-constant or non-immutable state variables lacking a setter function can create inflexibility in contract operations. If there's no way to update these variables post-deployment, the contract might not adapt to changing conditions or requirements, which can be a significant drawback, especially in upgradable or long-lived contracts. To resolve this, implement setter functions guarded by appropriate access controls, like onlyOwner or similar modifiers, so that these variables can be updated as required while maintaining security. This enables smoother contract maintenance and feature upgrades.
Num of instances: 3
Click to show findings
['25']
25: uint256 public illegalValue;
['26']
26: uint256 public stakeRequired;
['44']
44: uint256[47] private __GAP;
In the event of a security breach or any unforeseen emergency, swiftly suspending all protocol operations becomes crucial. Having a mechanism in place to halt all functions collectively, instead of pausing individual contracts separately, substantially enhances the efficiency of mitigating ongoing attacks or vulnerabilities. This not only quickens the response time to potential threats but also reduces operational stress during these critical periods. Therefore, consider integrating a 'circuit breaker' or 'emergency stop' function into the smart contract system architecture. Such a feature would provide the capability to suspend the entire protocol instantly, which could prove invaluable during a time-sensitive crisis management situation.
Num of instances: 1
Smart 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: 2
Click to show findings
['24']
19: function hashBatchHashedMetadata(
20: bytes32 batchHeaderHash,
21: bytes32 signatoryRecordHash,
22: uint32 blockNumber
23: ) internal pure returns(bytes32) {
24: return keccak256(abi.encodePacked(batchHeaderHash, signatoryRecordHash, blockNumber)); // <= FOUND
25: }
['38']
33: function hashBatchHashedMetadata(
34: bytes32 batchHeaderHash,
35: bytes memory confirmationData,
36: uint32 blockNumber
37: ) internal pure returns(bytes32) {
38: return keccak256(abi.encodePacked(batchHeaderHash, confirmationData, blockNumber)); // <= FOUND
39: }
In Solidity, private state variables are strictly confined to the contract they are defined in and can't be accessed or modified by its derived contracts. While this offers strong encapsulation, it can limit contract extensibility and modification in inheritance chains. On the other hand, internal variables can be accessed and potentially overridden by child contracts, granting more flexibility in contract development and upgrades. Therefore, it's recommended to use private only when you explicitly want to prevent child contract access. Otherwise, prefer internal to maintain a balance between encapsulation and the flexibility offered by inheritance patterns in Solidity.
Num of instances: 1
Utilizing low-level calls like .call{value: value} for Ether transfers in Ethereum can be risky, as it can inadvertently allow malicious contract executions through fallback functions. To mitigate these risks and ensure safer Ether transfers, it is recommended to adopt more secure and explicit methods provided by reputable libraries such as OpenZeppelin. Functions like Address.sendValue() from OpenZeppelin provide a clearer and safer alternative for sending Ether, as they encapsulate necessary checks and error handling, ensuring that Ether is transferred securely and any errors are appropriately dealt with. This not only enhances the security of your smart contract but also improves code readability and maintainability, aligning with modern Solidity development practices.
Num of instances: 1
Click to show findings
['89']
89:
90:
91: (bool success, ) = msg.sender.call{value: 1 ether}(""); // <= FOUND
When making external calls, the called contract can intentionally or unintentionally consume all provided gas, leading to unintended transaction reversion. To mitigate this risk, it's crucial to specify a gas limit when making the call. By using addr.call{gas: <amount>}(""), you allocate a specific amount of gas to the external call, ensuring the parent transaction has gas left for post-call operations. This approach safeguards against malevolent contracts aiming to exhaust gas and provides greater control over transaction execution.
Num of instances: 1
Click to show findings
['89']
89:
90:
91: (bool success, ) = msg.sender.call{value: 1 ether}(""); // <= FOUND
Libraries in Solidity are meant to be static collections of functions. They are deployed once and their code is reused by contracts through DELEGATECALL, which executes the library's code in the context of the calling contract. Public or external functions in libraries can be misleading because libraries are not supposed to maintain state or have external interactions in the way contracts do. Designing libraries with these kinds of functions goes against their intended purpose and can lead to confusion or misuse. For state management or external interactions, using contracts instead of libraries is more appropriate. Libraries should focus on utility functions that can operate on the data passed to them without side effects, enhancing code reuse and gas efficiency.
Num of instances: 1
Click to show findings
['14']
14: library EigenDARollupUtils { // <= FOUND
15: using BN254 for BN254.G1Point;
16:
18: struct BlobVerificationProof {
19: uint32 batchId;
20: uint8 blobIndex;
21: IEigenDAServiceManager.BatchMetadata batchMetadata;
22: bytes inclusionProof;
23: bytes quorumThresholdIndexes;
24: }
25:
32: function verifyBlob(
33: IEigenDAServiceManager.BlobHeader calldata blobHeader,
34: IEigenDAServiceManager eigenDAServiceManager,
35: BlobVerificationProof calldata blobVerificationProof
36: ) external view {
37: require(
38: EigenDAHasher.hashBatchMetadata(blobVerificationProof.batchMetadata)
39: == eigenDAServiceManager.batchIdToBatchMetadataHash(blobVerificationProof.batchId),
40: "EigenDARollupUtils.verifyBlob: batchMetadata does not match stored metadata"
41: );
42:
43: require(
44: Merkle.verifyInclusionKeccak(
45: blobVerificationProof.inclusionProof,
46: blobVerificationProof.batchMetadata.batchHeader.blobHeadersRoot,
47: keccak256(abi.encodePacked(EigenDAHasher.hashBlobHeader(blobHeader))),
48: blobVerificationProof.blobIndex
49: ),
50: "EigenDARollupUtils.verifyBlob: inclusion proof is invalid"
51: );
52:
53:
54: for (uint i = 0; i < blobHeader.quorumBlobParams.length; i++) {
55:
56: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumNumbers[uint8(blobVerificationProof.quorumThresholdIndexes[i])]) == blobHeader.quorumBlobParams[i].quorumNumber,
57: "EigenDARollupUtils.verifyBlob: quorumNumber does not match"
58: );
59:
60:
61: require(blobHeader.quorumBlobParams[i].adversaryThresholdPercentage
62: < blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
63: "EigenDARollupUtils.verifyBlob: adversaryThresholdPercentage is not valid"
64: );
65:
66:
67: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumThresholdPercentages[uint8(blobVerificationProof.quorumThresholdIndexes[i])])
68: >= blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
69: "EigenDARollupUtils.verifyBlob: quorumThresholdPercentage is not met"
70: );
[NonCritical-11] NatSpec: @notice tags missing from contract/abstract/library/interface/function/modifier/constructor/receive/fallback
Num of instances: 6
Click to show findings
['8']
8: interface IEigenDAServiceManager is IServiceManager
['11']
11: library EigenDAHasher
['14']
14: library EigenDARollupUtils
['47']
47: function initialize(
48: IPauserRegistry _pauserRegistry,
49: uint256 _initialPausedStatus,
50: address _initialOwner,
51: address _batchConfirmer
52: )
53: public
54: initializer
55:
['36']
36: constructor(
37: IAVSDirectory __avsDirectory,
38: IRegistryCoordinator __registryCoordinator,
39: IStakeRegistry __stakeRegistry
40: )
41: BLSSignatureChecker(__registryCoordinator)
42: ServiceManagerBase(__avsDirectory, __registryCoordinator, __stakeRegistry)
43:
['35']
35: constructor(IEigenDAServiceManager _eigenDAServiceManager, BN254.G1Point memory _tau, uint256 _illegalValue, uint256 _stakeRequired)
Num of instances: 1
Click to show findings
['8']
8: interface IEigenDAServiceManager is IServiceManager {
9: // EVENTS
10:
11: /**
12: * @notice Emitted when a Batch is confirmed.
13: * @param batchHeaderHash The hash of the batch header
14: * @param batchId The ID for the Batch inside of the specified duration (i.e. *not* the globalBatchId)
15: */
16: event BatchConfirmed(bytes32 indexed batchHeaderHash, uint32 batchId);
17:
18: /**
19: * @notice Emitted when the batch confirmer is changed.
20: * @param previousAddress The address of the previous batch confirmer
21: * @param newAddress The address of the new batch confirmer
22: */
23: event BatchConfirmerChanged(address previousAddress, address newAddress);
24:
25: // STRUCTS
26:
27: struct QuorumBlobParam {
28: uint8 quorumNumber;
29: uint8 adversaryThresholdPercentage;
30: uint8 quorumThresholdPercentage;
31: uint32 chunkLength; // the length of the chunks in the quorum
32: }
33:
34: struct BlobHeader {
35: BN254.G1Point commitment; // the kzg commitment to the blob
36: uint32 dataLength; // the length of the blob in coefficients of the polynomial
37: QuorumBlobParam[] quorumBlobParams; // the quorumBlobParams for each quorum
38: }
39:
40: struct ReducedBatchHeader {
41: bytes32 blobHeadersRoot;
42: uint32 referenceBlockNumber;
43: }
44:
45: struct BatchHeader {
46: bytes32 blobHeadersRoot;
47: bytes quorumNumbers; // each byte is a different quorum number
48: bytes quorumThresholdPercentages; // every bytes is an amount less than 100 specifying the percentage of stake
49: // the must have signed in the corresponding quorum in `quorumNumbers`
50: uint32 referenceBlockNumber;
51: }
52:
53: // Relevant metadata for a given datastore
54: struct BatchMetadata {
55: BatchHeader batchHeader; // the header of the data store
56: bytes32 signatoryRecordHash; // the hash of the signatory record
57: uint96 fee; // the amount of paymentToken paid for the datastore
58: uint32 confirmationBlockNumber; // the block number at which the batch was confirmed
59: }
60:
61: // Relevant metadata for a given datastore
62: struct BatchMetadataWithSignatoryRecord {
63: bytes32 batchHeaderHash; // the header hash of the data store
64: uint32 referenceBlockNumber; // the block number at which stakes
65: bytes32[] nonSignerPubkeyHashes; // the pubkeyHashes of all of the nonSigners
66: uint96 fee; // the amount of paymentToken paid for the datastore
67: uint32 blockNumber; // the block number at which the datastore was confirmed
68: }
69:
70: // FUNCTIONS
71:
72: /// @notice mapping between the batchId to the hash of the metadata of the corresponding Batch
73: function batchIdToBatchMetadataHash(uint32 batchId) external view returns(bytes32);
74:
75: /**
76: * @notice This function is used for
77: * - submitting data availabilty certificates,
78: * - check that the aggregate signature is valid,
79: * - and check whether quorum has been achieved or not.
80: */
81: function confirmBatch(
82: BatchHeader calldata batchHeader,
83: BLSSignatureChecker.NonSignerStakesAndSignature memory nonSignerStakesAndSignature
84: ) external;
85:
86: /// @notice This function is used for changing the batch confirmer
87: function setBatchConfirmer(address _batchConfirmer) external;
88:
89: /// @notice Returns the current batchId
90: function taskNumber() external view returns (uint32);
91:
92: /// @notice Returns the block until which operators must serve.
93: function latestServeUntilBlock() external view returns (uint32);
94:
95: /// @notice The maximum amount of blocks in the past that the service will consider stake amounts to still be 'valid'.
96: function BLOCK_STALE_MEASURE() external view returns (uint32);
97: }
Num of instances: 1
[NonCritical-14] NatSpec: @dev tags missing from contract/abstract/library/interface/function/modifier/constructor/receive/fallback
Num of instances: 29
Click to show findings
['24']
24: contract EigenDAServiceManager is EigenDAServiceManagerStorage, ServiceManagerBase, BLSSignatureChecker, Pausable
['21']
21: contract MockRollup
['8']
8: interface IEigenDAServiceManager is IServiceManager
['11']
11: library EigenDAHasher
['14']
14: library EigenDARollupUtils
['11']
11: abstract contract EigenDAServiceManagerStorage is IEigenDAServiceManager
['31']
31: modifier onlyBatchConfirmer()
['19']
19: function hashBatchHashedMetadata(
20: bytes32 batchHeaderHash,
21: bytes32 signatoryRecordHash,
22: uint32 blockNumber
23: ) internal pure returns(bytes32)
['33']
33: function hashBatchHashedMetadata(
34: bytes32 batchHeaderHash,
35: bytes memory confirmationData,
36: uint32 blockNumber
37: ) internal pure returns(bytes32)
['46']
46: function hashBatchMetadata(
47: IEigenDAServiceManager.BatchMetadata memory batchMetadata
48: ) internal pure returns(bytes32)
['60']
60: function hashBatchHeaderMemory(IEigenDAServiceManager.BatchHeader memory batchHeader) internal pure returns(bytes32)
['68']
68: function hashBatchHeader(IEigenDAServiceManager.BatchHeader calldata batchHeader) internal pure returns(bytes32)
['76']
76: function hashReducedBatchHeader(IEigenDAServiceManager.ReducedBatchHeader memory reducedBatchHeader) internal pure returns(bytes32)
['84']
84: function hashBlobHeader(IEigenDAServiceManager.BlobHeader memory blobHeader) internal pure returns(bytes32)
['92']
92: function convertBatchHeaderToReducedBatchHeader(IEigenDAServiceManager.BatchHeader memory batchHeader) internal pure returns(IEigenDAServiceManager.ReducedBatchHeader memory)
['103']
103: function hashBatchHeaderToReducedBatchHeader(IEigenDAServiceManager.BatchHeader memory batchHeader) internal pure returns(bytes32)
['32']
32: function verifyBlob(
33: IEigenDAServiceManager.BlobHeader calldata blobHeader,
34: IEigenDAServiceManager eigenDAServiceManager,
35: BlobVerificationProof calldata blobVerificationProof
36: ) external view
['83']
83: function openCommitment(
84: uint256 point,
85: uint256 evaluation,
86: BN254.G1Point memory tau,
87: BN254.G1Point memory commitment,
88: BN254.G2Point memory proof
89: ) internal view returns(bool)
['47']
47: function initialize(
48: IPauserRegistry _pauserRegistry,
49: uint256 _initialPausedStatus,
50: address _initialOwner,
51: address _batchConfirmer
52: )
53: public
54: initializer
55:
['67']
67: function confirmBatch(
68: BatchHeader calldata batchHeader,
69: NonSignerStakesAndSignature memory nonSignerStakesAndSignature
70: ) external onlyWhenNotPaused(PAUSED_CONFIRM_BATCH) onlyBatchConfirmer()
['126']
126: function setBatchConfirmer(address _batchConfirmer) external onlyOwner()
['131']
131: function _setBatchConfirmer(address _batchConfirmer) internal
['138']
138: function taskNumber() external view returns (uint32)
['143']
143: function latestServeUntilBlock() external view returns (uint32)
['43']
43: function registerValidator() external payable
['55']
55: function postCommitment(
56: IEigenDAServiceManager.BlobHeader memory blobHeader,
57: EigenDARollupUtils.BlobVerificationProof memory blobVerificationProof
58: ) external
['74']
74: function challengeCommitment(uint256 timestamp, uint256 point, BN254.G2Point memory proof) external
['36']
36: constructor(
37: IAVSDirectory __avsDirectory,
38: IRegistryCoordinator __registryCoordinator,
39: IStakeRegistry __stakeRegistry
40: )
41: BLSSignatureChecker(__registryCoordinator)
42: ServiceManagerBase(__avsDirectory, __registryCoordinator, __stakeRegistry)
43:
['35']
35: constructor(IEigenDAServiceManager _eigenDAServiceManager, BN254.G1Point memory _tau, uint256 _illegalValue, uint256 _stakeRequired)
Num of instances: 6
Click to show findings
['47']
47: function initialize(
48: IPauserRegistry _pauserRegistry,
49: uint256 _initialPausedStatus,
50: address _initialOwner,
51: address _batchConfirmer
52: )
53: public
54: initializer
55:
['67']
67: function confirmBatch(
68: BatchHeader calldata batchHeader,
69: NonSignerStakesAndSignature memory nonSignerStakesAndSignature
70: ) external onlyWhenNotPaused(PAUSED_CONFIRM_BATCH) onlyBatchConfirmer()
['126']
126: function setBatchConfirmer(address _batchConfirmer) external onlyOwner()
['131']
131: function _setBatchConfirmer(address _batchConfirmer) internal
['36']
36: constructor(
37: IAVSDirectory __avsDirectory,
38: IRegistryCoordinator __registryCoordinator,
39: IStakeRegistry __stakeRegistry
40: )
41: BLSSignatureChecker(__registryCoordinator)
42: ServiceManagerBase(__avsDirectory, __registryCoordinator, __stakeRegistry)
43:
['35']
35: constructor(IEigenDAServiceManager _eigenDAServiceManager, BN254.G1Point memory _tau, uint256 _illegalValue, uint256 _stakeRequired)
Num of instances: 2
Click to show findings
['138']
138: function taskNumber() external view returns (uint32)
['143']
143: function latestServeUntilBlock() external view returns (uint32)
Some comments use // X and others //X Amend comments to use only use // X or //X consistently
Num of instances: 4
Click to show findings
['92']
92: //e([s]_1 - w[1]_1, [pi(x)]_2) = e([p(x)]_1 - p(w)[1]_1, [1]_2)
['83']
83: //make sure that the quorumNumbers and quorumThresholdPercentages are of the same length
['15']
15: //TODO: mechanism to change any of these values?
['24']
24: //power of tau
Num of instances: 1
Production code should not have open TODOs as this makes code appear incomplete at production deployment
Num of instances: 1
[NonCritical-20] 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: 3
Click to show findings
['47']
47: function initialize(
48: IPauserRegistry _pauserRegistry,
49: uint256 _initialPausedStatus,
50: address _initialOwner,
51: address _batchConfirmer
52: )
53: public
54: initializer
55:
['126']
126: function setBatchConfirmer(address _batchConfirmer) external onlyOwner()
['131']
131: function _setBatchConfirmer(address _batchConfirmer) internal
[NonCritical-21] Functions which are either private or internal should have a preceding _ in their name
Add a preceding underscore to the function name, take care to refactor where there functions are called
Num of instances: 10
Click to show findings
['19']
19: function hashBatchHashedMetadata(
20: bytes32 batchHeaderHash,
21: bytes32 signatoryRecordHash,
22: uint32 blockNumber
23: ) internal pure returns(bytes32)
['33']
33: function hashBatchHashedMetadata(
34: bytes32 batchHeaderHash,
35: bytes memory confirmationData,
36: uint32 blockNumber
37: ) internal pure returns(bytes32)
['46']
46: function hashBatchMetadata(
47: IEigenDAServiceManager.BatchMetadata memory batchMetadata
48: ) internal pure returns(bytes32)
['60']
60: function hashBatchHeaderMemory(IEigenDAServiceManager.BatchHeader memory batchHeader) internal pure returns(bytes32)
['68']
68: function hashBatchHeader(IEigenDAServiceManager.BatchHeader calldata batchHeader) internal pure returns(bytes32)
['76']
76: function hashReducedBatchHeader(IEigenDAServiceManager.ReducedBatchHeader memory reducedBatchHeader) internal pure returns(bytes32)
['84']
84: function hashBlobHeader(IEigenDAServiceManager.BlobHeader memory blobHeader) internal pure returns(bytes32)
['92']
92: function convertBatchHeaderToReducedBatchHeader(IEigenDAServiceManager.BatchHeader memory batchHeader) internal pure returns(IEigenDAServiceManager.ReducedBatchHeader memory)
['103']
103: function hashBatchHeaderToReducedBatchHeader(IEigenDAServiceManager.BatchHeader memory batchHeader) internal pure returns(bytes32)
['83']
83: function openCommitment(
84: uint256 point,
85: uint256 evaluation,
86: BN254.G1Point memory tau,
87: BN254.G1Point memory commitment,
88: BN254.G2Point memory proof
89: ) internal view returns(bool)
Consider spreading these lines over multiple lines to aid in readability and the support of VIM users everywhere.
Num of instances: 22
Click to show findings
['95']
95: /// @notice The maximum amount of blocks in the past that the service will consider stake amounts to still be 'valid'. // <= FOUND
['24']
24: * @notice The maximum amount of blocks in the past that the service will consider stake amounts to still be 'valid'. // <= FOUND
['25']
25: * @dev To clarify edge cases, the middleware can look `BLOCK_STALE_MEASURE` blocks into the past, i.e. it may trust stakes from the interval // <= FOUND
['26']
26: * [block.number - BLOCK_STALE_MEASURE, block.number] (specifically, *inclusive* of the block that is `BLOCK_STALE_MEASURE` before the current one) // <= FOUND
['27']
27: * @dev BLOCK_STALE_MEASURE should be greater than the number of blocks till finalization, but not too much greater, as it is the amount of // <= FOUND
['28']
28: * time that nodes can be active after they have deregistered. The larger it is, the farther back stakes can be used, but the longer operators // <= FOUND
['60']
60: function hashBatchHeaderMemory(IEigenDAServiceManager.BatchHeader memory batchHeader) internal pure returns(bytes32) { // <= FOUND
['76']
76: function hashReducedBatchHeader(IEigenDAServiceManager.ReducedBatchHeader memory reducedBatchHeader) internal pure returns(bytes32) { // <= FOUND
['92']
92: function convertBatchHeaderToReducedBatchHeader(IEigenDAServiceManager.BatchHeader memory batchHeader) internal pure returns(IEigenDAServiceManager.ReducedBatchHeader memory) { // <= FOUND
['103']
103: function hashBatchHeaderToReducedBatchHeader(IEigenDAServiceManager.BatchHeader memory batchHeader) internal pure returns(bytes32) { // <= FOUND
['35']
35: constructor(IEigenDAServiceManager _eigenDAServiceManager, BN254.G1Point memory _tau, uint256 _illegalValue, uint256 _stakeRequired) { // <= FOUND
['60']
60: require(commitments[block.timestamp].validator == address(0), "MockRollup.postCommitment: Commitment already posted"); // <= FOUND
['82']
82: require(EigenDARollupUtils.openCommitment(point, illegalValue, tau, commitment.polynomialCommitment, proof), "MockRollup.challengeCommitment: Does not evaluate to illegal value"); // <= FOUND
['72']
72: require(tx.origin == msg.sender, "EigenDAServiceManager.confirmBatch: header and nonsigner data must be in calldata"); // <= FOUND
['75']
75: batchHeader.referenceBlockNumber <= block.number, "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is in future" // <= FOUND
['86']
86: "EigenDAServiceManager.confirmBatch: quorumNumbers and quorumThresholdPercentages must be of the same length" // <= FOUND
['105']
105: // we don't check that the quorumThresholdPercentages are not >100 because a greater value would trivially fail the check, implying // <= FOUND
['117']
117: batchIdToBatchMetadataHash[batchIdMemory] = EigenDAHasher.hashBatchHashedMetadata(batchHeaderHash, signatoryRecordHash, uint32(block.number)); // <= FOUND
['27']
27: * @notice Verifies the inclusion of a blob within a batch confirmed in `eigenDAServiceManager` and its trust assumptions // <= FOUND
['30']
30: * @param blobVerificationProof the relevant data needed to prove inclusion of the blob and that the trust assumptions were as expected // <= FOUND
['56']
56: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumNumbers[uint8(blobVerificationProof.quorumThresholdIndexes[i])]) == blobHeader.quorumBlobParams[i].quorumNumber, // <= FOUND
['67']
67: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumThresholdPercentages[uint8(blobVerificationProof.quorumThresholdIndexes[i])]) // <= FOUND
In Solidity, manipulating contract storage comes with significant gas costs. One can optimize gas usage by preventing unnecessary storage updates when the new value is the same as the existing one. If an existing value is the same as the new one, not reassigning it to the storage could potentially save substantial amounts of gas, notably 2900 gas for a 'Gsreset'. This saving may come at the expense of a cold storage load operation ('Gcoldsload'), which costs 2100 gas, or a warm storage access operation ('Gwarmaccess'), which costs 100 gas. Therefore, the gas efficiency of your contract can be significantly improved by adding a check that compares the new value with the current one before any storage update operation. If the values are the same, you can bypass the storage operation, thereby saving gas.
Num of instances: 1
Click to show findings
['126']
126: function setBatchConfirmer(address _batchConfirmer) external onlyOwner() {
127: _setBatchConfirmer(_batchConfirmer);
128: }
In 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: 3
Click to show findings
['3']
3: import "eigenlayer-middleware/OperatorStateRetriever.sol";
['4']
4: import "eigenlayer-middleware/BLSApkRegistry.sol";
['5']
5: import "eigenlayer-middleware/RegistryCoordinator.sol";
Newer solidity versions have new functionality and are generally more gas efficient too (0.8.19) as such it makes sense to use them provided it is safe to do so
Num of instances: 1
Click to show findings
[]
2: pragma solidity ^0.8.9;
In Solidity, while function overriding allows for functions with the same name to coexist, it is advisable to avoid this practice to enhance code readability and maintainability. Having multiple functions with the same name, even with different parameters or in inherited contracts, can cause confusion and increase the likelihood of errors during development, testing, and debugging. Using distinct and descriptive function names not only clarifies the purpose and behavior of each function, but also helps prevent unintended function calls or incorrect overriding. By adopting a clear and consistent naming convention, developers can create more comprehensible and maintainable smart contracts.
Num of instances: 2
Click to show findings
['19']
19: function hashBatchHashedMetadata( // <= FOUND
20: bytes32 batchHeaderHash,
21: bytes32 signatoryRecordHash,
22: uint32 blockNumber
23: ) internal pure returns(bytes32)
['33']
33: function hashBatchHashedMetadata( // <= FOUND
34: bytes32 batchHeaderHash,
35: bytes memory confirmationData,
36: uint32 blockNumber
37: ) internal pure returns(bytes32)
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: 1
Click to show findings
['24']
24: contract EigenDAServiceManager is EigenDAServiceManagerStorage, ServiceManagerBase, BLSSignatureChecker, Pausable // <= FOUND
In Solidity, when msg.sender plays a crucial role in a function's logic, it's important for transparency and auditability that any events emitted by this function include msg.sender as a parameter. This practice enhances the traceability and accountability of transactions, allowing users and external observers to easily track who initiated a particular action. Including msg.sender in event logs helps in creating a clear and verifiable record of interactions with the contract, thereby increasing user trust and facilitating easier debugging and analysis of contract behavior. It's a key aspect of writing clear, transparent, and user-friendly smart contracts.
Num of instances: 1
Click to show findings
['72']
67: function confirmBatch(
68: BatchHeader calldata batchHeader,
69: NonSignerStakesAndSignature memory nonSignerStakesAndSignature
70: ) external onlyWhenNotPaused(PAUSED_CONFIRM_BATCH) onlyBatchConfirmer() {
71:
72: require(tx.origin == msg.sender, "EigenDAServiceManager.confirmBatch: header and nonsigner data must be in calldata"); // <= FOUND
73:
74: require(
75: batchHeader.referenceBlockNumber <= block.number, "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is in future"
76: );
77:
78: require(
79: (batchHeader.referenceBlockNumber + BLOCK_STALE_MEASURE) >= uint32(block.number),
80: "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is too far in past"
81: );
82:
83:
84: require(
85: batchHeader.quorumNumbers.length == batchHeader.quorumThresholdPercentages.length,
86: "EigenDAServiceManager.confirmBatch: quorumNumbers and quorumThresholdPercentages must be of the same length"
87: );
88:
89:
90: bytes32 reducedBatchHeaderHash = batchHeader.hashBatchHeaderToReducedBatchHeader();
91:
92:
93: (
94: QuorumStakeTotals memory quorumStakeTotals,
95: bytes32 signatoryRecordHash
96: ) = checkSignatures(
97: reducedBatchHeaderHash,
98: batchHeader.quorumNumbers,
99: batchHeader.referenceBlockNumber,
100: nonSignerStakesAndSignature
101: );
102:
103:
104: for (uint i = 0; i < batchHeader.quorumThresholdPercentages.length; i++) {
105:
106:
107: require(
108: quorumStakeTotals.signedStakeForQuorum[i] * THRESHOLD_DENOMINATOR >=
109: quorumStakeTotals.totalStakeForQuorum[i] * uint8(batchHeader.quorumThresholdPercentages[i]),
110: "EigenDAServiceManager.confirmBatch: signatories do not own at least threshold percentage of a quorum"
111: );
112: }
113:
114:
115: uint32 batchIdMemory = batchId;
116: bytes32 batchHeaderHash = batchHeader.hashBatchHeader();
117: batchIdToBatchMetadataHash[batchIdMemory] = EigenDAHasher.hashBatchHashedMetadata(batchHeaderHash, signatoryRecordHash, uint32(block.number));
118:
119: emit BatchConfirmed(reducedBatchHeaderHash, batchIdMemory); // <= FOUND
120:
121:
122: batchId = batchIdMemory + 1;
123: }
Amend the ordering of imports to import interfaces first followed by other imports
Num of instances: 3
Click to show findings
['7']
3:
5: pragma solidity ^0.8.9;
6:
7: import {EigenDARollupUtils} from "../libraries/EigenDARollupUtils.sol"; // <= FOUND
8: import {EigenDAServiceManager} from "../core/EigenDAServiceManager.sol"; // <= FOUND
9: import {IEigenDAServiceManager} from "../interfaces/IEigenDAServiceManager.sol"; // <= FOUND
10: import {BN254} from "eigenlayer-middleware/libraries/BN254.sol"; // <= FOUND
11:
12: struct Commitment {
13: address validator;
14: uint32 dataLength;
15: BN254.G1Point polynomialCommitment;
16: }
17:
23: contract MockRollup {
24:
25: IEigenDAServiceManager public eigenDAServiceManager;
26: BN254.G1Point public tau;
27: uint256 public illegalValue;
28: uint256 public stakeRequired;
29:
31: mapping(address => bool) public validators;
32:
33: mapping(address => bool) public blacklist;
34:
35: mapping(uint256 => Commitment) public commitments;
36:
37: constructor(IEigenDAServiceManager _eigenDAServiceManager, BN254.G1Point memory _tau, uint256 _illegalValue, uint256 _stakeRequired) {
38: eigenDAServiceManager = _eigenDAServiceManager;
39: tau = _tau;
40: illegalValue = _illegalValue;
41: stakeRequired = _stakeRequired;
42: }
43:
45: function registerValidator() external payable {
46: require(msg.value == stakeRequired, "MockRollup.registerValidator: Must send stake required to register");
47: require(!validators[msg.sender], "MockRollup.registerValidator: Validator already registered");
48: require(!blacklist[msg.sender], "MockRollup.registerValidator: Validator blacklisted");
49: validators[msg.sender] = true;
50: }
51:
57: function postCommitment(
58: IEigenDAServiceManager.BlobHeader memory blobHeader,
59: EigenDARollupUtils.BlobVerificationProof memory blobVerificationProof
60: ) external {
61: require(validators[msg.sender], "MockRollup.postCommitment: Validator not registered");
62: require(commitments[block.timestamp].validator == address(0), "MockRollup.postCommitment: Commitment already posted");
63:
64:
65: EigenDARollupUtils.verifyBlob(blobHeader, eigenDAServiceManager, blobVerificationProof);
66:
67: commitments[block.timestamp] = Commitment(msg.sender, blobHeader.dataLength, blobHeader.commitment);
68: }
69:
76: function challengeCommitment(uint256 timestamp, uint256 point, BN254.G2Point memory proof) external {
77: Commitment memory commitment = commitments[timestamp];
78: require(commitment.validator != address(0), "MockRollup.challengeCommitment: Commitment not posted");
['5']
2:
3: pragma solidity ^0.8.9;
4:
5: import {Pausable} from "eigenlayer-core/contracts/permissions/Pausable.sol"; // <= FOUND
6: import {IAVSDirectory} from "eigenlayer-core/contracts/interfaces/IAVSDirectory.sol"; // <= FOUND
7: import {IPauserRegistry} from "eigenlayer-core/contracts/interfaces/IPauserRegistry.sol"; // <= FOUND
8:
9: import {ServiceManagerBase} from "eigenlayer-middleware/ServiceManagerBase.sol"; // <= FOUND
10: import {BLSSignatureChecker} from "eigenlayer-middleware/BLSSignatureChecker.sol"; // <= FOUND
11: import {IRegistryCoordinator} from "eigenlayer-middleware/interfaces/IRegistryCoordinator.sol"; // <= FOUND
12: import {IStakeRegistry} from "eigenlayer-middleware/interfaces/IStakeRegistry.sol"; // <= FOUND
13:
14: import {EigenDAServiceManagerStorage} from "./EigenDAServiceManagerStorage.sol"; // <= FOUND
15: import {EigenDAHasher} from "../libraries/EigenDAHasher.sol"; // <= FOUND
16:
25: contract EigenDAServiceManager is EigenDAServiceManagerStorage, ServiceManagerBase, BLSSignatureChecker, Pausable {
26: using EigenDAHasher for BatchHeader;
27: using EigenDAHasher for ReducedBatchHeader;
28:
29: uint8 internal constant PAUSED_CONFIRM_BATCH = 0;
30:
32: modifier onlyBatchConfirmer() {
33: require(msg.sender == batchConfirmer, "onlyBatchConfirmer: not from batch confirmer");
34: _;
35: }
36:
37: constructor(
38: IAVSDirectory __avsDirectory,
39: IRegistryCoordinator __registryCoordinator,
40: IStakeRegistry __stakeRegistry
41: )
42: BLSSignatureChecker(__registryCoordinator)
43: ServiceManagerBase(__avsDirectory, __registryCoordinator, __stakeRegistry)
44: {
45: _disableInitializers();
46: }
47:
48: function initialize(
49: IPauserRegistry _pauserRegistry,
50: uint256 _initialPausedStatus,
51: address _initialOwner,
52: address _batchConfirmer
53: )
54: public
55: initializer
56: {
57: _initializePauser(_pauserRegistry, _initialPausedStatus);
58: _transferOwnership(_initialOwner);
59: _setBatchConfirmer(_batchConfirmer);
60: }
61:
68: function confirmBatch(
69: BatchHeader calldata batchHeader,
70: NonSignerStakesAndSignature memory nonSignerStakesAndSignature
71: ) external onlyWhenNotPaused(PAUSED_CONFIRM_BATCH) onlyBatchConfirmer() {
72:
73: require(tx.origin == msg.sender, "EigenDAServiceManager.confirmBatch: header and nonsigner data must be in calldata");
74:
75: require(
76: batchHeader.referenceBlockNumber <= block.number, "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is in future"
77: );
78:
79: require(
['7']
3:
5: pragma solidity ^0.8.9;
6:
7: import {Merkle} from "eigenlayer-core/contracts/libraries/Merkle.sol"; // <= FOUND
8: import {BN254} from "eigenlayer-middleware/libraries/BN254.sol"; // <= FOUND
9: import {EigenDAHasher} from "./EigenDAHasher.sol"; // <= FOUND
10: import {IEigenDAServiceManager} from "../interfaces/IEigenDAServiceManager.sol"; // <= FOUND
11:
16: library EigenDARollupUtils {
17: using BN254 for BN254.G1Point;
18:
20: struct BlobVerificationProof {
21: uint32 batchId;
22: uint8 blobIndex;
23: IEigenDAServiceManager.BatchMetadata batchMetadata;
24: bytes inclusionProof;
25: bytes quorumThresholdIndexes;
26: }
27:
34: function verifyBlob(
35: IEigenDAServiceManager.BlobHeader calldata blobHeader,
36: IEigenDAServiceManager eigenDAServiceManager,
37: BlobVerificationProof calldata blobVerificationProof
38: ) external view {
39: require(
40: EigenDAHasher.hashBatchMetadata(blobVerificationProof.batchMetadata)
41: == eigenDAServiceManager.batchIdToBatchMetadataHash(blobVerificationProof.batchId),
42: "EigenDARollupUtils.verifyBlob: batchMetadata does not match stored metadata"
43: );
44:
45: require(
46: Merkle.verifyInclusionKeccak(
47: blobVerificationProof.inclusionProof,
48: blobVerificationProof.batchMetadata.batchHeader.blobHeadersRoot,
49: keccak256(abi.encodePacked(EigenDAHasher.hashBlobHeader(blobHeader))),
50: blobVerificationProof.blobIndex
51: ),
52: "EigenDARollupUtils.verifyBlob: inclusion proof is invalid"
53: );
54:
55:
56: for (uint i = 0; i < blobHeader.quorumBlobParams.length; i++) {
57:
58: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumNumbers[uint8(blobVerificationProof.quorumThresholdIndexes[i])]) == blobHeader.quorumBlobParams[i].quorumNumber,
59: "EigenDARollupUtils.verifyBlob: quorumNumber does not match"
60: );
61:
62:
63: require(blobHeader.quorumBlobParams[i].adversaryThresholdPercentage
64: < blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
65: "EigenDARollupUtils.verifyBlob: adversaryThresholdPercentage is not valid"
66: );
67:
68:
69: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumThresholdPercentages[uint8(blobVerificationProof.quorumThresholdIndexes[i])])
70: >= blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
71: "EigenDARollupUtils.verifyBlob: quorumThresholdPercentage is not met"
Ensure the SPDX identifier is defined in the top line of the solidity file
Num of instances: 1
Click to show findings
['1']
1: // Imports used for compiling for bindings for clients // <= FOUND
2:
3: import "eigenlayer-middleware/OperatorStateRetriever.sol";
4: import "eigenlayer-middleware/BLSApkRegistry.sol";
5: import "eigenlayer-middleware/RegistryCoordinator.sol";
6:
Num of instances: 3
Click to show findings
['32']
31:
32: mapping(address => bool) public blacklist; // <= FOUND
['46']
46: require(!blacklist[msg.sender], "MockRollup.registerValidator: Validator blacklisted"); // <= FOUND
['86']
86: blacklist[commitment.validator] = true; // <= FOUND
Using a single struct mapping in place of multiple defined mappings in a Solidity contract can lead to improved code organization, better readability, and easier maintainability. By consolidating related data into a single struct, developers can create a more cohesive data structure that logically groups together relevant pieces of information, thus reducing redundancy and clutter. This approach simplifies the codebase, making it easier to understand, navigate, and modify. Additionally, it can result in more efficient gas usage when accessing or updating multiple related data points simultaneously.
Num of instances: 1
Click to show findings
['29']
21: contract MockRollup {
22:
23: IEigenDAServiceManager public eigenDAServiceManager;
24: BN254.G1Point public tau;
25: uint256 public illegalValue;
26: uint256 public stakeRequired;
27:
29: mapping(address => bool) public validators; // <= FOUND
30:
31: mapping(address => bool) public blacklist; // <= FOUND
32:
33: mapping(uint256 => Commitment) public commitments; // <= FOUND
34:
55: }
If these serve no purpose, they should be safely removed
Num of instances: 3
Click to show findings
['20']
20: uint32 internal constant MIN_STORE_SIZE = 32; // <= FOUND
['22']
22: uint32 internal constant MAX_STORE_SIZE = 4e9; // <= FOUND
['44']
44: uint256[47] private __GAP; // <= FOUND
Emitting an event within initializer functions in Solidity is a best practice for providing transparency and traceability. Initializer functions set the initial state and values of an upgradeable contract. Emitting an event during initialization allows anyone to verify and audit the initial state of the contract via the transaction logs. This can be particularly useful for verifying the parameters set during initialization, tracking the contract's deployment, and troubleshooting or debugging. Therefore, developers should include an event emission in their initializer functions, providing a clear record of the contract's initialization and enhancing the contract's transparency and security.
Num of instances: 1
Click to show findings
['47']
47: function initialize( // <= FOUND
48: IPauserRegistry _pauserRegistry,
49: uint256 _initialPausedStatus,
50: address _initialOwner,
51: address _batchConfirmer
52: )
53: public
54: initializer
55: {
56: _initializePauser(_pauserRegistry, _initialPausedStatus);
57: _transferOwnership(_initialOwner);
58: _setBatchConfirmer(_batchConfirmer);
59: }
In Solidity version 0.8.18 and beyond mapping parameters can be named. This makes the purpose and function of a given mapping far clearer which in turn improves readability.
Num of instances: 3
Click to show findings
['29']
29: mapping(address => bool) public validators; // <= FOUND
['31']
31: mapping(address => bool) public blacklist; // <= FOUND
['37']
37: mapping(uint32 => bytes32) public batchIdToBatchMetadataHash; // <= FOUND
Num of instances: 1
Click to show findings
['28']
24: contract EigenDAServiceManager is EigenDAServiceManagerStorage, ServiceManagerBase, BLSSignatureChecker, Pausable {
25: using EigenDAHasher for BatchHeader;
26: using EigenDAHasher for ReducedBatchHeader;
27:
28: uint8 internal constant PAUSED_CONFIRM_BATCH = 0; // <= FOUND
29:
57: }
Modifiers in Solidity can improve code readability and modularity by encapsulating repetitive checks, such as address validity checks, into a reusable construct. For example, an onlyOwner modifier can be used to replace repetitive require(msg.sender == owner) checks across several functions, reducing code redundancy and enhancing maintainability. To implement, define a modifier with the check, then apply the modifier to relevant functions.
Num of instances: 2
Click to show findings
['32']
32: require(msg.sender == batchConfirmer, "onlyBatchConfirmer: not from batch confirmer"); // <= FOUND
['72']
72:
73: require(tx.origin == msg.sender, "EigenDAServiceManager.confirmBatch: header and nonsigner data must be in calldata"); // <= FOUND
Magic 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: 1
Click to show findings
['17']
17:
20: uint32 public constant STORE_DURATION_BLOCKS = 2 weeks / 12 seconds; // <= FOUND
[NonCritical-39] Inconsistent usage of int/uint with int256/uint256 in contract/abstract/library/interface
Use uint256/int256 consistently in place of uint/int to prevent ambiguity rather than using both within the same contract/abstract/library/interface
Num of instances: 2
Click to show findings
['24']
24: contract EigenDAServiceManager is EigenDAServiceManagerStorage, ServiceManagerBase, BLSSignatureChecker, Pausable {
25: using EigenDAHasher for BatchHeader;
49: uint256 _initialPausedStatus, // <= FOUND
104: for (uint i = 0; i < batchHeader.quorumThresholdPercentages.length; i++) { // <= FOUND
['14']
14: library EigenDARollupUtils {
15: using BN254 for BN254.G1Point;
54: for (uint i = 0; i < blobHeader.quorumBlobParams.length; i++) { // <= FOUND
84: uint256 point, // <= FOUND
85: uint256 evaluation, // <= FOUND
If these serve no purpose, they should be safely removed
Num of instances: 1
Click to show findings
['62']
62: struct BatchMetadataWithSignatoryRecord { // <= FOUND
63: bytes32 batchHeaderHash;
64: uint32 referenceBlockNumber;
65: bytes32[] nonSignerPubkeyHashes;
66: uint96 fee;
67: uint32 blockNumber;
68: }
When 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: 2
Click to show findings
['19']
19: function hashBatchHashedMetadata(
20: bytes32 batchHeaderHash,
21: bytes32 signatoryRecordHash,
22: uint32 blockNumber
23: ) internal pure returns(bytes32) {
24: return keccak256(abi.encodePacked(batchHeaderHash, signatoryRecordHash, blockNumber));
25: }
['33']
33: function hashBatchHashedMetadata(
34: bytes32 batchHeaderHash,
35: bytes memory confirmationData,
36: uint32 blockNumber
37: ) internal pure returns(bytes32) {
38: return keccak256(abi.encodePacked(batchHeaderHash, confirmationData, blockNumber));
39: }
The SMTChecker is a valuable tool for Solidity developers as it helps detect potential vulnerabilities and logical errors in the contract's code. By utilizing Satisfiability Modulo Theories (SMT) solvers, it can reason about the potential states a contract can be in, and therefore, identify conditions that could lead to undesirable behavior. This automatic formal verification can catch issues that might otherwise be missed in manual code reviews or standard testing, enhancing the overall contract's security and reliability.
Num of instances: 7
Click to show findings
['1']
1: // SPDX-License-Identifier: UNLICENSED
2: pragma solidity ^0.8.9;
3:
4: import {IServiceManager} from "eigenlayer-middleware/interfaces/IServiceManager.sol";
5: import {BLSSignatureChecker} from "eigenlayer-middleware/BLSSignatureChecker.sol";
6: import {BN254} from "eigenlayer-middleware/libraries/BN254.sol";
7:
8: interface IEigenDAServiceManager is IServiceManager {
9: // EVENTS
10:
11: /**
12: * @notice Emitted when a Batch is confirmed.
13: * @param batchHeaderHash The hash of the batch header
14: * @param batchId The ID for the Batch inside of the specified duration (i.e. *not* the globalBatchId)
15: */
16: event BatchConfirmed(bytes32 indexed batchHeaderHash, uint32 batchId);
17:
18: /**
19: * @notice Emitted when the batch confirmer is changed.
20: * @param previousAddress The address of the previous batch confirmer
21: * @param newAddress The address of the new batch confirmer
22: */
23: event BatchConfirmerChanged(address previousAddress, address newAddress);
24:
25: // STRUCTS
26:
27: struct QuorumBlobParam {
28: uint8 quorumNumber;
29: uint8 adversaryThresholdPercentage;
30: uint8 quorumThresholdPercentage;
31: uint32 chunkLength; // the length of the chunks in the quorum
32: }
33:
34: struct BlobHeader {
35: BN254.G1Point commitment; // the kzg commitment to the blob
36: uint32 dataLength; // the length of the blob in coefficients of the polynomial
37: QuorumBlobParam[] quorumBlobParams; // the quorumBlobParams for each quorum
38: }
39:
40: struct ReducedBatchHeader {
41: bytes32 blobHeadersRoot;
42: uint32 referenceBlockNumber;
43: }
44:
45: struct BatchHeader {
46: bytes32 blobHeadersRoot;
47: bytes quorumNumbers; // each byte is a different quorum number
48: bytes quorumThresholdPercentages; // every bytes is an amount less than 100 specifying the percentage of stake
49: // the must have signed in the corresponding quorum in `quorumNumbers`
50: uint32 referenceBlockNumber;
51: }
52:
53: // Relevant metadata for a given datastore
54: struct BatchMetadata {
55: BatchHeader batchHeader; // the header of the data store
56: bytes32 signatoryRecordHash; // the hash of the signatory record
57: uint96 fee; // the amount of paymentToken paid for the datastore
58: uint32 confirmationBlockNumber; // the block number at which the batch was confirmed
59: }
60:
61: // Relevant metadata for a given datastore
62: struct BatchMetadataWithSignatoryRecord {
63: bytes32 batchHeaderHash; // the header hash of the data store
64: uint32 referenceBlockNumber; // the block number at which stakes
65: bytes32[] nonSignerPubkeyHashes; // the pubkeyHashes of all of the nonSigners
66: uint96 fee; // the amount of paymentToken paid for the datastore
67: uint32 blockNumber; // the block number at which the datastore was confirmed
68: }
69:
70: // FUNCTIONS
71:
72: /// @notice mapping between the batchId to the hash of the metadata of the corresponding Batch
73: function batchIdToBatchMetadataHash(uint32 batchId) external view returns(bytes32);
74:
75: /**
76: * @notice This function is used for
77: * - submitting data availabilty certificates,
78: * - check that the aggregate signature is valid,
79: * - and check whether quorum has been achieved or not.
80: */
81: function confirmBatch(
82: BatchHeader calldata batchHeader,
83: BLSSignatureChecker.NonSignerStakesAndSignature memory nonSignerStakesAndSignature
84: ) external;
85:
86: /// @notice This function is used for changing the batch confirmer
87: function setBatchConfirmer(address _batchConfirmer) external;
88:
89: /// @notice Returns the current batchId
90: function taskNumber() external view returns (uint32);
91:
92: /// @notice Returns the block until which operators must serve.
93: function latestServeUntilBlock() external view returns (uint32);
94:
95: /// @notice The maximum amount of blocks in the past that the service will consider stake amounts to still be 'valid'.
96: function BLOCK_STALE_MEASURE() external view returns (uint32);
97: }
98:
['1']
1: // SPDX-License-Identifier: UNLICENSED
2: pragma solidity ^0.8.9;
3:
4: import {IEigenDAServiceManager} from "../interfaces/IEigenDAServiceManager.sol";
5:
6: /**
7: * @title Storage variables for the `EigenDAServiceManager` contract.
8: * @author Layr Labs, Inc.
9: * @notice This storage contract is separate from the logic to simplify the upgrade process.
10: */
11: abstract contract EigenDAServiceManagerStorage is IEigenDAServiceManager {
12: // CONSTANTS
13: uint256 public constant THRESHOLD_DENOMINATOR = 100;
14:
15: //TODO: mechanism to change any of these values?
16: /// @notice Unit of measure (in blocks) for which data will be stored for after confirmation.
17: uint32 public constant STORE_DURATION_BLOCKS = 2 weeks / 12 seconds;
18:
19: /// @notice Minimum Batch size, in bytes.
20: uint32 internal constant MIN_STORE_SIZE = 32;
21: /// @notice Maximum Batch size, in bytes.
22: uint32 internal constant MAX_STORE_SIZE = 4e9;
23: /**
24: * @notice The maximum amount of blocks in the past that the service will consider stake amounts to still be 'valid'.
25: * @dev To clarify edge cases, the middleware can look `BLOCK_STALE_MEASURE` blocks into the past, i.e. it may trust stakes from the interval
26: * [block.number - BLOCK_STALE_MEASURE, block.number] (specifically, *inclusive* of the block that is `BLOCK_STALE_MEASURE` before the current one)
27: * @dev BLOCK_STALE_MEASURE should be greater than the number of blocks till finalization, but not too much greater, as it is the amount of
28: * time that nodes can be active after they have deregistered. The larger it is, the farther back stakes can be used, but the longer operators
29: * have to serve after they've deregistered.
30: */
31: uint32 public constant BLOCK_STALE_MEASURE = 150;
32:
33: /// @notice The current batchId
34: uint32 public batchId;
35:
36: /// @notice mapping between the batchId to the hash of the metadata of the corresponding Batch
37: mapping(uint32 => bytes32) public batchIdToBatchMetadataHash;
38:
39: /// @notice address that is permissioned to confirm batches
40: address public batchConfirmer;
41:
42: // storage gap for upgradeability
43: // slither-disable-next-line shadowing-state
44: uint256[47] private __GAP;
45:
46: }
47:
['1']
1: // SPDX-License-Identifier: UNLICENSED
2:
3: pragma solidity ^0.8.9;
4:
5: import {IEigenDAServiceManager} from "../interfaces/IEigenDAServiceManager.sol";
6:
7: /**
8: * @title Library of functions for hashing various EigenDA structs.
9: * @author Layr Labs, Inc.
10: */
11: library EigenDAHasher {
12:
13: /**
14: * @notice hashes the given metdata into the commitment that will be stored in the contract
15: * @param batchHeaderHash the hash of the batchHeader
16: * @param signatoryRecordHash the hash of the signatory record
17: * @param blockNumber the block number at which the batch was confirmed
18: */
19: function hashBatchHashedMetadata(
20: bytes32 batchHeaderHash,
21: bytes32 signatoryRecordHash,
22: uint32 blockNumber
23: ) internal pure returns(bytes32) {
24: return keccak256(abi.encodePacked(batchHeaderHash, signatoryRecordHash, blockNumber));
25: }
26:
27: /**
28: * @notice hashes the given metdata into the commitment that will be stored in the contract
29: * @param batchHeaderHash the hash of the batchHeader
30: * @param confirmationData the confirmation data of the batch
31: * @param blockNumber the block number at which the batch was confirmed
32: */
33: function hashBatchHashedMetadata(
34: bytes32 batchHeaderHash,
35: bytes memory confirmationData,
36: uint32 blockNumber
37: ) internal pure returns(bytes32) {
38: return keccak256(abi.encodePacked(batchHeaderHash, confirmationData, blockNumber));
39: }
40:
41: /**
42: * @notice given the a batchHeader in the provided metdata, calculates the hash of the batchMetadata
43: * @param batchMetadata the metadata of the batch
44: * @return the hash of the batchMetadata
45: */
46: function hashBatchMetadata(
47: IEigenDAServiceManager.BatchMetadata memory batchMetadata
48: ) internal pure returns(bytes32) {
49: return hashBatchHashedMetadata(
50: keccak256(abi.encode(batchMetadata.batchHeader)),
51: batchMetadata.signatoryRecordHash,
52: batchMetadata.confirmationBlockNumber
53: );
54: }
55:
56: /**
57: * @notice hashes the given batch header
58: * @param batchHeader the batch header to hash
59: */
60: function hashBatchHeaderMemory(IEigenDAServiceManager.BatchHeader memory batchHeader) internal pure returns(bytes32) {
61: return keccak256(abi.encode(batchHeader));
62: }
63:
64: /**
65: * @notice hashes the given batch header
66: * @param batchHeader the batch header to hash
67: */
68: function hashBatchHeader(IEigenDAServiceManager.BatchHeader calldata batchHeader) internal pure returns(bytes32) {
69: return keccak256(abi.encode(batchHeader));
70: }
71:
72: /**
73: * @notice hashes the given reduced batch header
74: * @param reducedBatchHeader the reduced batch header to hash
75: */
76: function hashReducedBatchHeader(IEigenDAServiceManager.ReducedBatchHeader memory reducedBatchHeader) internal pure returns(bytes32) {
77: return keccak256(abi.encode(reducedBatchHeader));
78: }
79:
80: /**
81: * @notice hashes the given blob header
82: * @param blobHeader the blob header to hash
83: */
84: function hashBlobHeader(IEigenDAServiceManager.BlobHeader memory blobHeader) internal pure returns(bytes32) {
85: return keccak256(abi.encode(blobHeader));
86: }
87:
88: /**
89: * @notice converts a batch header to a reduced batch header
90: * @param batchHeader the batch header to convert
91: */
92: function convertBatchHeaderToReducedBatchHeader(IEigenDAServiceManager.BatchHeader memory batchHeader) internal pure returns(IEigenDAServiceManager.ReducedBatchHeader memory) {
93: return IEigenDAServiceManager.ReducedBatchHeader({
94: blobHeadersRoot: batchHeader.blobHeadersRoot,
95: referenceBlockNumber: batchHeader.referenceBlockNumber
96: });
97: }
98:
99: /**
100: * @notice converts the given batch header to a reduced batch header and then hashes it
101: * @param batchHeader the batch header to hash
102: */
103: function hashBatchHeaderToReducedBatchHeader(IEigenDAServiceManager.BatchHeader memory batchHeader) internal pure returns(bytes32) {
104: return keccak256(abi.encode(convertBatchHeaderToReducedBatchHeader(batchHeader)));
105: }
106: }
107:
['1']
1: // SPDX-License-Identifier: UNLICENSED
2:
3: pragma solidity ^0.8.9;
4:
5: import {EigenDARollupUtils} from "../libraries/EigenDARollupUtils.sol";
6: import {EigenDAServiceManager} from "../core/EigenDAServiceManager.sol";
7: import {IEigenDAServiceManager} from "../interfaces/IEigenDAServiceManager.sol";
8: import {BN254} from "eigenlayer-middleware/libraries/BN254.sol";
9:
10: struct Commitment {
11: address validator; // validator who posted the commitment
12: uint32 dataLength; // length of the data
13: BN254.G1Point polynomialCommitment; // commitment to the polynomial
14: }
15:
16: /**
17: * @title MockRollup
18: * @author Layr Labs, Inc.
19: * @notice This contract is used to emulate a rollup contract for the purpose of testing the rollup interface.
20: */
21: contract MockRollup {
22:
23: IEigenDAServiceManager public eigenDAServiceManager; // EigenDASM contract
24: BN254.G1Point public tau; //power of tau
25: uint256 public illegalValue; // special "illegal" value that should not be included in blob
26: uint256 public stakeRequired; // amount of stake required to register as a validator
27:
28: ///@notice mapping of validators who have registered
29: mapping(address => bool) public validators;
30: ///@notice mapping of validators who have been blacklisted
31: mapping(address => bool) public blacklist;
32: ///@notice mapping of timestamps to commitments
33: mapping(uint256 => Commitment) public commitments;
34:
35: constructor(IEigenDAServiceManager _eigenDAServiceManager, BN254.G1Point memory _tau, uint256 _illegalValue, uint256 _stakeRequired) {
36: eigenDAServiceManager = _eigenDAServiceManager;
37: tau = _tau;
38: illegalValue = _illegalValue;
39: stakeRequired = _stakeRequired;
40: }
41:
42: ///@notice registers msg.sender as validator by putting up 1 ether of stake
43: function registerValidator() external payable {
44: require(msg.value == stakeRequired, "MockRollup.registerValidator: Must send stake required to register");
45: require(!validators[msg.sender], "MockRollup.registerValidator: Validator already registered");
46: require(!blacklist[msg.sender], "MockRollup.registerValidator: Validator blacklisted");
47: validators[msg.sender] = true;
48: }
49:
50: /**
51: * @notice a function for validators to post a commitment to a blob on behalf of the rollup
52: * @param blobHeader the blob header
53: * @param blobVerificationProof the blob verification proof
54: */
55: function postCommitment(
56: IEigenDAServiceManager.BlobHeader memory blobHeader,
57: EigenDARollupUtils.BlobVerificationProof memory blobVerificationProof
58: ) external {
59: require(validators[msg.sender], "MockRollup.postCommitment: Validator not registered");
60: require(commitments[block.timestamp].validator == address(0), "MockRollup.postCommitment: Commitment already posted");
61:
62: // verify that the blob was included in the batch
63: EigenDARollupUtils.verifyBlob(blobHeader, eigenDAServiceManager, blobVerificationProof);
64:
65: commitments[block.timestamp] = Commitment(msg.sender, blobHeader.dataLength, blobHeader.commitment);
66: }
67:
68: /**
69: * @notice a function for users to challenge a commitment that contains the illegal value
70: * @param timestamp the timestamp of the commitment being challenged
71: * @param point the point on the polynomial to evaluate
72: * @param proof revelvant KZG proof
73: */
74: function challengeCommitment(uint256 timestamp, uint256 point, BN254.G2Point memory proof) external {
75: Commitment memory commitment = commitments[timestamp];
76: require(commitment.validator != address(0), "MockRollup.challengeCommitment: Commitment not posted");
77:
78: // point on the polynomial must be less than the length of the data stored
79: require(point < commitment.dataLength, "MockRollup.challengeCommitment: Point must be less than data length");
80:
81: // verify that the commitment contains the illegal value
82: require(EigenDARollupUtils.openCommitment(point, illegalValue, tau, commitment.polynomialCommitment, proof), "MockRollup.challengeCommitment: Does not evaluate to illegal value");
83:
84: // blacklist the validator
85: validators[commitment.validator] = false;
86: blacklist[commitment.validator] = true;
87:
88: // send validators stake to the user who challenged the commitment
89: (bool success, ) = msg.sender.call{value: 1 ether}("");
90: require(success);
91:
92: }
93:
94: }
['1']
1: // SPDX-License-Identifier: UNLICENSED
2: pragma solidity ^0.8.9;
3:
4: import {Pausable} from "eigenlayer-core/contracts/permissions/Pausable.sol";
5: import {IAVSDirectory} from "eigenlayer-core/contracts/interfaces/IAVSDirectory.sol";
6: import {IPauserRegistry} from "eigenlayer-core/contracts/interfaces/IPauserRegistry.sol";
7:
8: import {ServiceManagerBase} from "eigenlayer-middleware/ServiceManagerBase.sol";
9: import {BLSSignatureChecker} from "eigenlayer-middleware/BLSSignatureChecker.sol";
10: import {IRegistryCoordinator} from "eigenlayer-middleware/interfaces/IRegistryCoordinator.sol";
11: import {IStakeRegistry} from "eigenlayer-middleware/interfaces/IStakeRegistry.sol";
12:
13: import {EigenDAServiceManagerStorage} from "./EigenDAServiceManagerStorage.sol";
14: import {EigenDAHasher} from "../libraries/EigenDAHasher.sol";
15:
16: /**
17: * @title Primary entrypoint for procuring services from EigenDA.
18: * @author Layr Labs, Inc.
19: * @notice This contract is used for:
20: * - initializing the data store by the disperser
21: * - confirming the data store by the disperser with inferred aggregated signatures of the quorum
22: * - freezing operators as the result of various "challenges"
23: */
24: contract EigenDAServiceManager is EigenDAServiceManagerStorage, ServiceManagerBase, BLSSignatureChecker, Pausable {
25: using EigenDAHasher for BatchHeader;
26: using EigenDAHasher for ReducedBatchHeader;
27:
28: uint8 internal constant PAUSED_CONFIRM_BATCH = 0;
29:
30: /// @notice when applied to a function, ensures that the function is only callable by the `batchConfirmer`.
31: modifier onlyBatchConfirmer() {
32: require(msg.sender == batchConfirmer, "onlyBatchConfirmer: not from batch confirmer");
33: _;
34: }
35:
36: constructor(
37: IAVSDirectory __avsDirectory,
38: IRegistryCoordinator __registryCoordinator,
39: IStakeRegistry __stakeRegistry
40: )
41: BLSSignatureChecker(__registryCoordinator)
42: ServiceManagerBase(__avsDirectory, __registryCoordinator, __stakeRegistry)
43: {
44: _disableInitializers();
45: }
46:
47: function initialize(
48: IPauserRegistry _pauserRegistry,
49: uint256 _initialPausedStatus,
50: address _initialOwner,
51: address _batchConfirmer
52: )
53: public
54: initializer
55: {
56: _initializePauser(_pauserRegistry, _initialPausedStatus);
57: _transferOwnership(_initialOwner);
58: _setBatchConfirmer(_batchConfirmer);
59: }
60:
61: /**
62: * @notice This function is used for
63: * - submitting data availabilty certificates,
64: * - check that the aggregate signature is valid,
65: * - and check whether quorum has been achieved or not.
66: */
67: function confirmBatch(
68: BatchHeader calldata batchHeader,
69: NonSignerStakesAndSignature memory nonSignerStakesAndSignature
70: ) external onlyWhenNotPaused(PAUSED_CONFIRM_BATCH) onlyBatchConfirmer() {
71: // make sure the information needed to derive the non-signers and batch is in calldata to avoid emitting events
72: require(tx.origin == msg.sender, "EigenDAServiceManager.confirmBatch: header and nonsigner data must be in calldata");
73: // make sure the stakes against which the Batch is being confirmed are not stale
74: require(
75: batchHeader.referenceBlockNumber <= block.number, "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is in future"
76: );
77:
78: require(
79: (batchHeader.referenceBlockNumber + BLOCK_STALE_MEASURE) >= uint32(block.number),
80: "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is too far in past"
81: );
82:
83: //make sure that the quorumNumbers and quorumThresholdPercentages are of the same length
84: require(
85: batchHeader.quorumNumbers.length == batchHeader.quorumThresholdPercentages.length,
86: "EigenDAServiceManager.confirmBatch: quorumNumbers and quorumThresholdPercentages must be of the same length"
87: );
88:
89: // calculate reducedBatchHeaderHash which nodes signed
90: bytes32 reducedBatchHeaderHash = batchHeader.hashBatchHeaderToReducedBatchHeader();
91:
92: // check the signature
93: (
94: QuorumStakeTotals memory quorumStakeTotals,
95: bytes32 signatoryRecordHash
96: ) = checkSignatures(
97: reducedBatchHeaderHash,
98: batchHeader.quorumNumbers, // use list of uint8s instead of uint256 bitmap to not iterate 256 times
99: batchHeader.referenceBlockNumber,
100: nonSignerStakesAndSignature
101: );
102:
103: // check that signatories own at least a threshold percentage of each quourm
104: for (uint i = 0; i < batchHeader.quorumThresholdPercentages.length; i++) {
105: // we don't check that the quorumThresholdPercentages are not >100 because a greater value would trivially fail the check, implying
106: // signed stake > total stake
107: require(
108: quorumStakeTotals.signedStakeForQuorum[i] * THRESHOLD_DENOMINATOR >=
109: quorumStakeTotals.totalStakeForQuorum[i] * uint8(batchHeader.quorumThresholdPercentages[i]),
110: "EigenDAServiceManager.confirmBatch: signatories do not own at least threshold percentage of a quorum"
111: );
112: }
113:
114: // store the metadata hash
115: uint32 batchIdMemory = batchId;
116: bytes32 batchHeaderHash = batchHeader.hashBatchHeader();
117: batchIdToBatchMetadataHash[batchIdMemory] = EigenDAHasher.hashBatchHashedMetadata(batchHeaderHash, signatoryRecordHash, uint32(block.number));
118:
119: emit BatchConfirmed(reducedBatchHeaderHash, batchIdMemory);
120:
121: // increment the batchId
122: batchId = batchIdMemory + 1;
123: }
124:
125: /// @notice This function is used for changing the batch confirmer
126: function setBatchConfirmer(address _batchConfirmer) external onlyOwner() {
127: _setBatchConfirmer(_batchConfirmer);
128: }
129:
130: /// @notice changes the batch confirmer
131: function _setBatchConfirmer(address _batchConfirmer) internal {
132: address previousBatchConfirmer = batchConfirmer;
133: batchConfirmer = _batchConfirmer;
134: emit BatchConfirmerChanged(previousBatchConfirmer, batchConfirmer);
135: }
136:
137: /// @notice Returns the current batchId
138: function taskNumber() external view returns (uint32) {
139: return batchId;
140: }
141:
142: /// @notice Returns the block until which operators must serve.
143: function latestServeUntilBlock() external view returns (uint32) {
144: return uint32(block.number) + STORE_DURATION_BLOCKS + BLOCK_STALE_MEASURE;
145: }
146:
147: }
['1']
1: // SPDX-License-Identifier: MIT
2:
3: pragma solidity ^0.8.9;
4:
5: import {Merkle} from "eigenlayer-core/contracts/libraries/Merkle.sol";
6: import {BN254} from "eigenlayer-middleware/libraries/BN254.sol";
7: import {EigenDAHasher} from "./EigenDAHasher.sol";
8: import {IEigenDAServiceManager} from "../interfaces/IEigenDAServiceManager.sol";
9:
10: /**
11: * @title Library of functions to be used by smart contracts wanting to prove blobs on EigenDA and open KZG commitments.
12: * @author Layr Labs, Inc.
13: */
14: library EigenDARollupUtils {
15: using BN254 for BN254.G1Point;
16:
17: // STRUCTS
18: struct BlobVerificationProof {
19: uint32 batchId;
20: uint8 blobIndex;
21: IEigenDAServiceManager.BatchMetadata batchMetadata;
22: bytes inclusionProof;
23: bytes quorumThresholdIndexes;
24: }
25:
26: /**
27: * @notice Verifies the inclusion of a blob within a batch confirmed in `eigenDAServiceManager` and its trust assumptions
28: * @param blobHeader the header of the blob containing relevant attributes of the blob
29: * @param eigenDAServiceManager the contract in which the batch was confirmed
30: * @param blobVerificationProof the relevant data needed to prove inclusion of the blob and that the trust assumptions were as expected
31: */
32: function verifyBlob(
33: IEigenDAServiceManager.BlobHeader calldata blobHeader,
34: IEigenDAServiceManager eigenDAServiceManager,
35: BlobVerificationProof calldata blobVerificationProof
36: ) external view {
37: require(
38: EigenDAHasher.hashBatchMetadata(blobVerificationProof.batchMetadata)
39: == eigenDAServiceManager.batchIdToBatchMetadataHash(blobVerificationProof.batchId),
40: "EigenDARollupUtils.verifyBlob: batchMetadata does not match stored metadata"
41: );
42:
43: require(
44: Merkle.verifyInclusionKeccak(
45: blobVerificationProof.inclusionProof,
46: blobVerificationProof.batchMetadata.batchHeader.blobHeadersRoot,
47: keccak256(abi.encodePacked(EigenDAHasher.hashBlobHeader(blobHeader))),
48: blobVerificationProof.blobIndex
49: ),
50: "EigenDARollupUtils.verifyBlob: inclusion proof is invalid"
51: );
52:
53: // require that the security param in each blob is met
54: for (uint i = 0; i < blobHeader.quorumBlobParams.length; i++) {
55: // make sure that the quorumIndex matches the given quorumNumber
56: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumNumbers[uint8(blobVerificationProof.quorumThresholdIndexes[i])]) == blobHeader.quorumBlobParams[i].quorumNumber,
57: "EigenDARollupUtils.verifyBlob: quorumNumber does not match"
58: );
59:
60: // make sure that the adversaryThresholdPercentage is less than the given quorumThresholdPercentage
61: require(blobHeader.quorumBlobParams[i].adversaryThresholdPercentage
62: < blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
63: "EigenDARollupUtils.verifyBlob: adversaryThresholdPercentage is not valid"
64: );
65:
66: // make sure that the stake signed for is greater than the given quorumThresholdPercentage
67: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumThresholdPercentages[uint8(blobVerificationProof.quorumThresholdIndexes[i])])
68: >= blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
69: "EigenDARollupUtils.verifyBlob: quorumThresholdPercentage is not met"
70: );
71:
72: }
73: }
74:
75: /**
76: * @notice opens the KZG commitment at a point
77: * @param point the point to evaluate the polynomial at
78: * @param evaluation the evaluation of the polynomial at the point
79: * @param tau the power of tau
80: * @param commitment the commitment to the polynomial
81: * @param proof the proof of the commitment
82: */
83: function openCommitment(
84: uint256 point,
85: uint256 evaluation,
86: BN254.G1Point memory tau,
87: BN254.G1Point memory commitment,
88: BN254.G2Point memory proof
89: ) internal view returns(bool) {
90: BN254.G1Point memory negGeneratorG1 = BN254.generatorG1().negate();
91:
92: //e([s]_1 - w[1]_1, [pi(x)]_2) = e([p(x)]_1 - p(w)[1]_1, [1]_2)
93: return BN254.pairing(
94: tau.plus(negGeneratorG1.scalar_mul(point)),
95: proof,
96: commitment.plus(negGeneratorG1.scalar_mul(evaluation)),
97: BN254.negGeneratorG2()
98: );
99: }
100: }
101:
['1']
1: // Imports used for compiling for bindings for clients
2:
3: import "eigenlayer-middleware/OperatorStateRetriever.sol";
4: import "eigenlayer-middleware/BLSApkRegistry.sol";
5: import "eigenlayer-middleware/RegistryCoordinator.sol";
6:
Num of instances: 1
Attaining 100% code coverage is not an assurance of a bug-free codebase, but it significantly improves the likelihood of identifying simple bugs and aids in maintaining a stable codebase by preventing regressions during code modifications. Additionally, to achieve complete coverage, code writers usually have to structure their code more modularly, which implies testing each component independently. This reduces the complex interdependencies between modules and layers, creating a more understandable and auditable codebase. Consequently, this practice aids in enhancing code maintainability and reduces the risk of introducing bugs during future changes.
Num of instances: 2
Click to show findings
['24']
24: contract EigenDAServiceManager is EigenDAServiceManagerStorage, ServiceManagerBase, BLSSignatureChecker, Pausable // <= FOUND
['21']
21: contract MockRollup // <= FOUND
[NonCritical-45] Consider using SafeTransferLib.safeTransferETH() or Address.sendValue() for clearer semantic meaning
For improved code readability and better semantic understanding, it's recommended to use OpenZeppelin's SafeTransferLib.safeTransferETH() or Address.sendValue(). These functions explicitly describe their operation with their naming convention, increasing the comprehensibility of the code. Their usage over lower-level calls enhances the maintainability of your smart contract code by clearly indicating the purpose of the function.
Num of instances: 1
Click to show findings
['74']
74: function challengeCommitment(uint256 timestamp, uint256 point, BN254.G2Point memory proof) external { // <= FOUND
75: Commitment memory commitment = commitments[timestamp];
76: require(commitment.validator != address(0), "MockRollup.challengeCommitment: Commitment not posted");
77:
78:
79: require(point < commitment.dataLength, "MockRollup.challengeCommitment: Point must be less than data length");
80:
81:
82: require(EigenDARollupUtils.openCommitment(point, illegalValue, tau, commitment.polynomialCommitment, proof), "MockRollup.challengeCommitment: Does not evaluate to illegal value");
83:
84:
85: validators[commitment.validator] = false;
86: blacklist[commitment.validator] = true;
87:
88:
89: (bool success, ) = msg.sender.call{value: 1 ether}("");
90: require(success);
91:
92: }
Avoid unnecessary whitespace in contract lines such as ' ;' and ', )'
Num of instances: 1
Click to show findings
['91']
89:
90:
91: (bool success, ) = msg.sender.call{value: 1 ether}(""); // <= FOUND
State variables in Solidity contracts are essential for defining the state of the contract. Including comments for these variables improves code readability and maintainability by providing context and explaining their purpose. This practice aids future developers or auditors in understanding the code, thus reducing the likelihood of misinterpretation or errors.
Num of instances: 1
Click to show findings
[]
abstract contract EigenDAServiceManagerStorage is IEigenDAServiceManager {
// CONSTANTS
uint256 public constant THRESHOLD_DENOMINATOR = 100; // <= FOUND
Lack of brace spacing in coding refers to the absence of spaces around braces, which can hinder code readability. In Solidity, as in many programming languages, spacing can enhance the visual distinction between different parts of the code, making it easier to follow. A lack of spacing can lead to a dense, confusing appearance. The resolution to this issue is to follow a consistent style guide that defines rules for brace spacing. By including spaces around braces, such as { statement } instead of {statement}, developers can ensure that the code is more legible and maintainable, especially in larger codebases.
Num of instances: 18
Click to show findings
['4']
4:
5: import {IEigenDAServiceManager} from "../interfaces/IEigenDAServiceManager.sol"; // <= FOUND
['5']
5:
6: import {Merkle} from "eigenlayer-core/contracts/libraries/Merkle.sol"; // <= FOUND
['6']
6: import {BN254} from "eigenlayer-middleware/libraries/BN254.sol"; // <= FOUND
['7']
7: import {EigenDAHasher} from "./EigenDAHasher.sol"; // <= FOUND
['4']
4: import {IEigenDAServiceManager} from "../interfaces/IEigenDAServiceManager.sol"; // <= FOUND
['4']
4:
5: import {Pausable} from "eigenlayer-core/contracts/permissions/Pausable.sol"; // <= FOUND
['5']
5: import {IAVSDirectory} from "eigenlayer-core/contracts/interfaces/IAVSDirectory.sol"; // <= FOUND
['6']
6: import {IPauserRegistry} from "eigenlayer-core/contracts/interfaces/IPauserRegistry.sol"; // <= FOUND
['8']
8:
9: import {ServiceManagerBase} from "eigenlayer-middleware/ServiceManagerBase.sol"; // <= FOUND
['5']
5: import {BLSSignatureChecker} from "eigenlayer-middleware/BLSSignatureChecker.sol"; // <= FOUND
['10']
10: import {IRegistryCoordinator} from "eigenlayer-middleware/interfaces/IRegistryCoordinator.sol"; // <= FOUND
['11']
11: import {IStakeRegistry} from "eigenlayer-middleware/interfaces/IStakeRegistry.sol"; // <= FOUND
['13']
13:
14: import {EigenDAServiceManagerStorage} from "./EigenDAServiceManagerStorage.sol"; // <= FOUND
['14']
14: import {EigenDAHasher} from "../libraries/EigenDAHasher.sol"; // <= FOUND
['4']
4:
5: import {IServiceManager} from "eigenlayer-middleware/interfaces/IServiceManager.sol"; // <= FOUND
['5']
5:
6: import {EigenDARollupUtils} from "../libraries/EigenDARollupUtils.sol"; // <= FOUND
['6']
6: import {EigenDAServiceManager} from "../core/EigenDAServiceManager.sol"; // <= FOUND
['89']
89:
90:
91: (bool success, ) = msg.sender.call{value: 1 ether}(""); // <= FOUND
Num of instances: 2
Click to show findings
['24']
24: contract EigenDAServiceManager is EigenDAServiceManagerStorage, ServiceManagerBase, BLSSignatureChecker, Pausable // <= FOUND
['21']
21: contract MockRollup // <= FOUND
If these serve no purpose, they should be safely removed
Num of instances: 1
Click to show findings
['4']
4: import {IServiceManager} from "eigenlayer-middleware/interfaces/IServiceManager.sol"; // <= FOUND
Unbounded array inputs in functions can lead to unintentional excessive gas consumption, potentially causing a transaction to revert after expending substantial gas. To enhance user experience and prevent such scenarios, consider implementing a require() statement that limits the array length to a defined maximum. This constraint ensures that transactions won't proceed if they're likely to hit gas limits due to array size, saving users from unnecessary gas costs and offering a more predictable interaction with the contract.
Num of instances: 2
Click to show findings
['54']
54: for (uint i = 0; i < blobHeader.quorumBlobParams.length; i++) { // <= FOUND
55:
56: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumNumbers[uint8(blobVerificationProof.quorumThresholdIndexes[i])]) == blobHeader.quorumBlobParams[i].quorumNumber,
57: "EigenDARollupUtils.verifyBlob: quorumNumber does not match"
58: );
59:
60:
61: require(blobHeader.quorumBlobParams[i].adversaryThresholdPercentage
62: < blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
63: "EigenDARollupUtils.verifyBlob: adversaryThresholdPercentage is not valid"
64: );
65:
66:
67: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumThresholdPercentages[uint8(blobVerificationProof.quorumThresholdIndexes[i])])
68: >= blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
69: "EigenDARollupUtils.verifyBlob: quorumThresholdPercentage is not met"
70: );
71:
72: }
['104']
104: for (uint i = 0; i < batchHeader.quorumThresholdPercentages.length; i++) { // <= FOUND
105:
106:
107: require(
108: quorumStakeTotals.signedStakeForQuorum[i] * THRESHOLD_DENOMINATOR >=
109: quorumStakeTotals.totalStakeForQuorum[i] * uint8(batchHeader.quorumThresholdPercentages[i]),
110: "EigenDAServiceManager.confirmBatch: signatories do not own at least threshold percentage of a quorum"
111: );
112: }
Sensitive setter functions in smart contracts often alter critical state variables. Without events emitted in these functions, external observers or dApps cannot easily track or react to these state changes. Missing events can obscure contract activity, hampering transparency and making integration more challenging. To resolve this, incorporate appropriate event emissions within these functions. Events offer an efficient way to log crucial changes, aiding in real-time tracking and post-transaction verification.
Num of instances: 1
Click to show findings
['126']
126: function setBatchConfirmer(address _batchConfirmer) external onlyOwner() { // <= FOUND
127: _setBatchConfirmer(_batchConfirmer);
128: }
ETH balances can change due to gas costs, miner reordering, or unexpected transactions. Directly comparing an expected balance with == or != can lead to unpredictable results. Instead, use range checks or require a minimum balance. For instance, instead of requiring an exact balance, check if an account's balance is within an acceptable range or above a certain threshold. This provides flexibility and accounts for slight variations that might occur due to the dynamic nature of blockchain transactions and fees. By avoiding exact balance checks, smart contracts can avoid unintentional failures and enhance robustness.
Num of instances: 1
Click to show findings
['44']
44: require(msg.value == stakeRequired, "MockRollup.registerValidator: Must send stake required to register"); // <= FOUND
In Solidity, complex inheritance structures can obfuscate code understanding, introducing potential security risks. Multiple inheritance, especially with overlapping function names or state variables, can cause unintentional overrides or ambiguous behavior. Resolution: Strive for linear and simple inheritance chains. Avoid diamond or circular inheritance patterns. Clearly document the purpose and relationships of base contracts, ensuring that overrides are intentional. Tools like Remix or Hardhat can visualize inheritance chains, assisting in verification. Keeping inheritance streamlined aids in better code readability, reduces potential errors, and ensures smoother audits and upgrades.
Num of instances: 1
Click to show findings
['24']
24: contract EigenDAServiceManager is EigenDAServiceManagerStorage, ServiceManagerBase, BLSSignatureChecker, Pausable // <= FOUND
[NonCritical-55] A event should be emitted if a non immutable state variable is set in a constructor
Num of instances: 1
Click to show findings
['35']
35: constructor(IEigenDAServiceManager _eigenDAServiceManager, BN254.G1Point memory _tau, uint256 _illegalValue, uint256 _stakeRequired) {
36: eigenDAServiceManager = _eigenDAServiceManager; // <= FOUND
37: tau = _tau; // <= FOUND
38: illegalValue = _illegalValue; // <= FOUND
39: stakeRequired = _stakeRequired; // <= FOUND
40: }
Using redundant parameters in smart contracts can lead to unnecessary complexity and potential vulnerabilities. When a function parameter is always constrained to a specific value due to an if or require statement, it renders the parameter superfluous. Including such parameters can be misleading to developers or users, suggesting a flexibility that doesn't exist in reality. Additionally, unnecessary parameters increase the gas cost for transactions. Resolution: Analyze the contract to identify parameters that are rendered static by conditional checks. Remove these parameters from the function signature and update the function logic accordingly. This simplifies the code, reduces gas costs, and enhances clarity and security.
Num of instances: 2
Click to show findings
['32']
32: function verifyBlob(
33: IEigenDAServiceManager.BlobHeader calldata blobHeader,
34: IEigenDAServiceManager eigenDAServiceManager,
35: BlobVerificationProof calldata blobVerificationProof
36: ) external view {
37: require(
38: EigenDAHasher.hashBatchMetadata(blobVerificationProof.batchMetadata)
39: == eigenDAServiceManager.batchIdToBatchMetadataHash(blobVerificationProof.batchId),
40: "EigenDARollupUtils.verifyBlob: batchMetadata does not match stored metadata"
41: );
42:
43: require(
44: Merkle.verifyInclusionKeccak(
45: blobVerificationProof.inclusionProof,
46: blobVerificationProof.batchMetadata.batchHeader.blobHeadersRoot,
47: keccak256(abi.encodePacked(EigenDAHasher.hashBlobHeader(blobHeader))),
48: blobVerificationProof.blobIndex
49: ),
50: "EigenDARollupUtils.verifyBlob: inclusion proof is invalid"
51: );
52:
53:
54: for (uint i = 0; i < blobHeader.quorumBlobParams.length; i++) {
55:
56: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumNumbers[uint8(blobVerificationProof.quorumThresholdIndexes[i])]) == blobHeader.quorumBlobParams[i].quorumNumber,
57: "EigenDARollupUtils.verifyBlob: quorumNumber does not match"
58: );
59:
60:
61: require(blobHeader.quorumBlobParams[i].adversaryThresholdPercentage
62: < blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
63: "EigenDARollupUtils.verifyBlob: adversaryThresholdPercentage is not valid"
64: );
65:
66:
67: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumThresholdPercentages[uint8(blobVerificationProof.quorumThresholdIndexes[i])])
68: >= blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
69: "EigenDARollupUtils.verifyBlob: quorumThresholdPercentage is not met"
70: );
71:
72: }
73: }
['67']
67: function confirmBatch(
68: BatchHeader calldata batchHeader,
69: NonSignerStakesAndSignature memory nonSignerStakesAndSignature
70: ) external onlyWhenNotPaused(PAUSED_CONFIRM_BATCH) onlyBatchConfirmer() {
71:
72: require(tx.origin == msg.sender, "EigenDAServiceManager.confirmBatch: header and nonsigner data must be in calldata");
73:
74: require(
75: batchHeader.referenceBlockNumber <= block.number, "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is in future"
76: );
77:
78: require(
79: (batchHeader.referenceBlockNumber + BLOCK_STALE_MEASURE) >= uint32(block.number),
80: "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is too far in past"
81: );
82:
83:
84: require(
85: batchHeader.quorumNumbers.length == batchHeader.quorumThresholdPercentages.length,
86: "EigenDAServiceManager.confirmBatch: quorumNumbers and quorumThresholdPercentages must be of the same length"
87: );
88:
89:
90: bytes32 reducedBatchHeaderHash = batchHeader.hashBatchHeaderToReducedBatchHeader();
91:
92:
93: (
94: QuorumStakeTotals memory quorumStakeTotals,
95: bytes32 signatoryRecordHash
96: ) = checkSignatures(
97: reducedBatchHeaderHash,
98: batchHeader.quorumNumbers,
99: batchHeader.referenceBlockNumber,
100: nonSignerStakesAndSignature
101: );
102:
103:
104: for (uint i = 0; i < batchHeader.quorumThresholdPercentages.length; i++) {
105:
106:
107: require(
108: quorumStakeTotals.signedStakeForQuorum[i] * THRESHOLD_DENOMINATOR >=
109: quorumStakeTotals.totalStakeForQuorum[i] * uint8(batchHeader.quorumThresholdPercentages[i]),
110: "EigenDAServiceManager.confirmBatch: signatories do not own at least threshold percentage of a quorum"
111: );
112: }
113:
114:
115: uint32 batchIdMemory = batchId;
116: bytes32 batchHeaderHash = batchHeader.hashBatchHeader();
117: batchIdToBatchMetadataHash[batchIdMemory] = EigenDAHasher.hashBatchHashedMetadata(batchHeaderHash, signatoryRecordHash, uint32(block.number));
118:
119: emit BatchConfirmed(reducedBatchHeaderHash, batchIdMemory);
120:
121:
122: batchId = batchIdMemory + 1;
123: }
The following function already as address conditional checks in the modifier it uses thus having more checks against msg.sender within the function is not necessary
Num of instances: 1
Click to show findings
['67']
67: function confirmBatch(
68: BatchHeader calldata batchHeader,
69: NonSignerStakesAndSignature memory nonSignerStakesAndSignature
70: ) external onlyWhenNotPaused(PAUSED_CONFIRM_BATCH) onlyBatchConfirmer() { // <= FOUND
71:
72: require(tx.origin == msg.sender, "EigenDAServiceManager.confirmBatch: header and nonsigner data must be in calldata"); // <= FOUND
73:
74: require(
75: batchHeader.referenceBlockNumber <= block.number, "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is in future"
76: );
77:
78: require(
79: (batchHeader.referenceBlockNumber + BLOCK_STALE_MEASURE) >= uint32(block.number),
80: "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is too far in past"
81: );
82:
83:
84: require(
85: batchHeader.quorumNumbers.length == batchHeader.quorumThresholdPercentages.length,
86: "EigenDAServiceManager.confirmBatch: quorumNumbers and quorumThresholdPercentages must be of the same length"
87: );
88:
89:
90: bytes32 reducedBatchHeaderHash = batchHeader.hashBatchHeaderToReducedBatchHeader();
91:
92:
93: (
94: QuorumStakeTotals memory quorumStakeTotals,
95: bytes32 signatoryRecordHash
96: ) = checkSignatures(
97: reducedBatchHeaderHash,
98: batchHeader.quorumNumbers,
99: batchHeader.referenceBlockNumber,
100: nonSignerStakesAndSignature
101: );
102:
103:
104: for (uint i = 0; i < batchHeader.quorumThresholdPercentages.length; i++) {
105:
106:
107: require(
108: quorumStakeTotals.signedStakeForQuorum[i] * THRESHOLD_DENOMINATOR >=
109: quorumStakeTotals.totalStakeForQuorum[i] * uint8(batchHeader.quorumThresholdPercentages[i]),
110: "EigenDAServiceManager.confirmBatch: signatories do not own at least threshold percentage of a quorum"
111: );
112: }
113:
114:
115: uint32 batchIdMemory = batchId;
116: bytes32 batchHeaderHash = batchHeader.hashBatchHeader();
117: batchIdToBatchMetadataHash[batchIdMemory] = EigenDAHasher.hashBatchHashedMetadata(batchHeaderHash, signatoryRecordHash, uint32(block.number));
118:
119: emit BatchConfirmed(reducedBatchHeaderHash, batchIdMemory);
120:
121:
Modern smart contract development often employs upgradeable contract structures, utilizing proxy patterns like OpenZeppelin’s Upgradeable Contracts. This paradigm separates logic and state, allowing developers to amend and enhance the contract's functionality without altering its state or the deployed contract address. Transitioning to this approach enhances long-term maintainability.
Resolution: Adopt a well-established proxy pattern for upgradeability, ensuring proper initialization and employing transparent proxies to mitigate potential risks. Embrace comprehensive testing and audit practices, particularly when updating contract logic, to ensure state consistency and security are preserved across upgrades. This ensures your contract remains robust and adaptable to future requirements.
Num of instances: 2
Click to show findings
['24']
24: contract EigenDAServiceManager is EigenDAServiceManagerStorage, ServiceManagerBase, BLSSignatureChecker, Pausable
['21']
21: contract MockRollup
Using transfer libraries like OpenZeppelin's Address.sendValue is preferred over low-level calls for transferring Ether in Solidity. These libraries provide clearer, more semantically meaningful methods compared to low-level call() functions. They encapsulate best practices for error handling and gas management, enhancing the security and readability of your code. Low-level calls lack these built-in safety checks and can be more error-prone, especially when dealing with Ether transfers.
Num of instances: 1
Click to show findings
['89']
89:
90:
91: (bool success, ) = msg.sender.call{value: 1 ether}(""); // <= FOUND
Using a struct to encapsulate multiple parameters in Solidity functions can significantly enhance code readability and maintainability. Instead of passing a long list of arguments, which can be error-prone and hard to manage, a struct allows grouping related data into a single, coherent entity. This approach simplifies function signatures and makes the code more organized. It also enhances code clarity, as developers can easily understand the relationship between the parameters. Moreover, it aids in future code modifications and expansions, as adding or modifying a parameter only requires changes in the struct definition, rather than in every function that uses these parameters.
Num of instances: 1
Click to show findings
['84']
83: function openCommitment(
84: uint256 point, // <= FOUND
85: uint256 evaluation, // <= FOUND
86: BN254.G1Point memory tau, // <= FOUND
87: BN254.G1Point memory commitment, // <= FOUND
88: BN254.G2Point memory proof
89: ) internal view returns(bool)
In Solidity, inheriting a Pausable contract without utilizing its whenNotPaused modifier can be an oversight. The Pausable pattern is designed to add an emergency stop mechanism, typically controlled by an admin role. By not using whenNotPaused, the contract fails to leverage this safety feature. To rectify this, functions that should be restricted during the pause state must include the whenNotPaused modifier. This ensures that critical functions are inaccessible when the contract is paused, enhancing security and control. It's important to judiciously apply this modifier to sensitive functions, balancing security with functionality.
Num of instances: 1
Click to show findings
['24']
24: contract EigenDAServiceManager is EigenDAServiceManagerStorage, ServiceManagerBase, BLSSignatureChecker, Pausable
Some comments use // X and others //X Amend comments to use only use // X or //X consistently such style inconsistencies can be resolved by running the project through a format prettier or by using forge fmt.
Num of instances: 8
Click to show findings
['92']
92: //e([s]_1 - w[1]_1, [pi(x)]_2) = e([p(x)]_1 - p(w)[1]_1, [1]_2)
['83']
83: //make sure that the quorumNumbers and quorumThresholdPercentages are of the same length
['15']
15: //TODO: mechanism to change any of these values?
['24']
24: //power of tau
['28']
28: ///@notice mapping of validators who have registered // <= FOUND
['30']
30: ///@notice mapping of validators who have been blacklisted // <= FOUND
['32']
32: ///@notice mapping of timestamps to commitments // <= FOUND
['42']
42: ///@notice registers msg.sender as validator by putting up 1 ether of stake // <= FOUND
Num of instances: 6
Click to show findings
['73']
73:
77: function batchIdToBatchMetadataHash(uint32 batchId) external view returns(bytes32); // <= FOUND
['81']
81:
88: function confirmBatch( // <= FOUND
89: BatchHeader calldata batchHeader,
90: BLSSignatureChecker.NonSignerStakesAndSignature memory nonSignerStakesAndSignature
91: ) external; // <= FOUND
['87']
87:
89: function setBatchConfirmer(address _batchConfirmer) external; // <= FOUND
['90']
90:
92: function taskNumber() external view returns (uint32); // <= FOUND
['93']
93:
95: function latestServeUntilBlock() external view returns (uint32); // <= FOUND
['96']
96:
98: function BLOCK_STALE_MEASURE() external view returns (uint32); // <= FOUND
[NonCritical-64] 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: 34
Click to show findings
['46']
46:
52: function hashBatchMetadata(
53: IEigenDAServiceManager.BatchMetadata memory batchMetadata // <= FOUND 'IEigenDAServiceManager.'
54: ) internal pure returns(bytes32) {
['60']
60:
65: function hashBatchHeaderMemory(IEigenDAServiceManager.BatchHeader memory batchHeader) internal pure returns(bytes32) { // <= FOUND 'IEigenDAServiceManager.'
['68']
68:
73: function hashBatchHeader(IEigenDAServiceManager.BatchHeader calldata batchHeader) internal pure returns(bytes32) { // <= FOUND 'IEigenDAServiceManager.'
['76']
76:
81: function hashReducedBatchHeader(IEigenDAServiceManager.ReducedBatchHeader memory reducedBatchHeader) internal pure returns(bytes32) { // <= FOUND 'IEigenDAServiceManager.'
['84']
84:
89: function hashBlobHeader(IEigenDAServiceManager.BlobHeader memory blobHeader) internal pure returns(bytes32) { // <= FOUND 'IEigenDAServiceManager.'
['92']
92:
97: function convertBatchHeaderToReducedBatchHeader(IEigenDAServiceManager.BatchHeader memory batchHeader) internal pure returns(IEigenDAServiceManager.ReducedBatchHeader memory) { // <= FOUND 'IEigenDAServiceManager.'
['93']
93: return IEigenDAServiceManager.ReducedBatchHeader({ // <= FOUND 'IEigenDAServiceManager.'
['103']
103:
108: function hashBatchHeaderToReducedBatchHeader(IEigenDAServiceManager.BatchHeader memory batchHeader) internal pure returns(bytes32) { // <= FOUND 'IEigenDAServiceManager.'
['21']
21: IEigenDAServiceManager.BatchMetadata batchMetadata; // <= FOUND 'IEigenDAServiceManager.'
['32']
32:
39: function verifyBlob(
40: IEigenDAServiceManager.BlobHeader calldata blobHeader, // <= FOUND 'IEigenDAServiceManager.'
41: IEigenDAServiceManager eigenDAServiceManager,
42: BlobVerificationProof calldata blobVerificationProof
43: ) external view {
['55']
55:
61: function postCommitment(
62: IEigenDAServiceManager.BlobHeader memory blobHeader, // <= FOUND 'IEigenDAServiceManager.'
63: EigenDARollupUtils.BlobVerificationProof memory blobVerificationProof // <= FOUND 'EigenDARollupUtils.'
64: ) external {
['43']
43:
44: require(
45: Merkle.verifyInclusionKeccak( // <= FOUND 'Merkle.'
46: blobVerificationProof.inclusionProof,
47: blobVerificationProof.batchMetadata.batchHeader.blobHeadersRoot,
48: keccak256(abi.encodePacked(EigenDAHasher.hashBlobHeader(blobHeader))), // <= FOUND 'EigenDAHasher.'
49: blobVerificationProof.blobIndex
50: ),
51: "EigenDARollupUtils.verifyBlob: inclusion proof is invalid" // <= FOUND 'EigenDARollupUtils.'
52: );
['15']
15: using BN254 for BN254.G1Point; // <= FOUND 'BN254.'
['83']
83:
92: function openCommitment(
93: uint256 point,
94: uint256 evaluation,
95: BN254.G1Point memory tau, // <= FOUND 'BN254.'
96: BN254.G1Point memory commitment, // <= FOUND 'BN254.'
97: BN254.G2Point memory proof // <= FOUND 'BN254.'
98: ) internal view returns(bool) {
['90']
90: BN254.G1Point memory negGeneratorG1 = BN254.generatorG1().negate(); // <= FOUND 'BN254.'
['93']
93:
94:
95: return BN254.pairing( // <= FOUND 'BN254.'
96: tau.plus(negGeneratorG1.scalar_mul(point)),
97: proof,
98: commitment.plus(negGeneratorG1.scalar_mul(evaluation)),
99: BN254.negGeneratorG2() // <= FOUND 'BN254.'
100: );
['35']
35: BN254.G1Point commitment; // <= FOUND 'BN254.'
['13']
13: BN254.G1Point polynomialCommitment; // <= FOUND 'BN254.'
['24']
24: BN254.G1Point public tau; // <= FOUND 'BN254.'
['35']
35:
36: constructor(IEigenDAServiceManager _eigenDAServiceManager, BN254.G1Point memory _tau, uint256 _illegalValue, uint256 _stakeRequired) { // <= FOUND 'BN254.'
['74']
74:
81: function challengeCommitment(uint256 timestamp, uint256 point, BN254.G2Point memory proof) external { // <= FOUND 'BN254.'
['37']
37: require(
38: EigenDAHasher.hashBatchMetadata(blobVerificationProof.batchMetadata) // <= FOUND 'EigenDAHasher.'
39: == eigenDAServiceManager.batchIdToBatchMetadataHash(blobVerificationProof.batchId),
40: "EigenDARollupUtils.verifyBlob: batchMetadata does not match stored metadata" // <= FOUND 'EigenDARollupUtils.'
41: );
['117']
117: batchIdToBatchMetadataHash[batchIdMemory] = EigenDAHasher.hashBatchHashedMetadata(batchHeaderHash, signatoryRecordHash, uint32(block.number)); // <= FOUND 'EigenDAHasher.'
['81']
81:
88: function confirmBatch(
89: BatchHeader calldata batchHeader,
90: BLSSignatureChecker.NonSignerStakesAndSignature memory nonSignerStakesAndSignature // <= FOUND 'BLSSignatureChecker.'
91: ) external;
['56']
56:
57: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumNumbers[uint8(blobVerificationProof.quorumThresholdIndexes[i])]) == blobHeader.quorumBlobParams[i].quorumNumber,
58: "EigenDARollupUtils.verifyBlob: quorumNumber does not match" // <= FOUND 'EigenDARollupUtils.'
59: );
['61']
61:
62:
63: require(blobHeader.quorumBlobParams[i].adversaryThresholdPercentage
64: < blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
65: "EigenDARollupUtils.verifyBlob: adversaryThresholdPercentage is not valid" // <= FOUND 'EigenDARollupUtils.'
66: );
['67']
67:
68:
69: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumThresholdPercentages[uint8(blobVerificationProof.quorumThresholdIndexes[i])])
70: >= blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
71: "EigenDARollupUtils.verifyBlob: quorumThresholdPercentage is not met" // <= FOUND 'EigenDARollupUtils.'
72: );
['63']
63:
64:
65: EigenDARollupUtils.verifyBlob(blobHeader, eigenDAServiceManager, blobVerificationProof); // <= FOUND 'EigenDARollupUtils.'
['82']
82:
83:
84: require(EigenDARollupUtils.openCommitment(point, illegalValue, tau, commitment.polynomialCommitment, proof), "MockRollup.challengeCommitment: Does not evaluate to illegal value"); // <= FOUND 'EigenDARollupUtils.'
['72']
72:
73: require(tx.origin == msg.sender, "EigenDAServiceManager.confirmBatch: header and nonsigner data must be in calldata"); // <= FOUND 'EigenDAServiceManager.'
['74']
74:
75: require(
76: batchHeader.referenceBlockNumber <= block.number, "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is in future" // <= FOUND 'EigenDAServiceManager.'
77: );
['78']
78:
79: require(
80: (batchHeader.referenceBlockNumber + BLOCK_STALE_MEASURE) >= uint32(block.number),
81: "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is too far in past" // <= FOUND 'EigenDAServiceManager.'
82: );
['84']
84:
85:
86: require(
87: batchHeader.quorumNumbers.length == batchHeader.quorumThresholdPercentages.length,
88: "EigenDAServiceManager.confirmBatch: quorumNumbers and quorumThresholdPercentages must be of the same length" // <= FOUND 'EigenDAServiceManager.'
89: );
['107']
107:
108:
109: require(
110: quorumStakeTotals.signedStakeForQuorum[i] * THRESHOLD_DENOMINATOR >=
111: quorumStakeTotals.totalStakeForQuorum[i] * uint8(batchHeader.quorumThresholdPercentages[i]),
112: "EigenDAServiceManager.confirmBatch: signatories do not own at least threshold percentage of a quorum" // <= FOUND 'EigenDAServiceManager.'
113: );
[NonCritical-65] All verbatim blocks are considered identical by deduplicator and can incorrectly be unified
The Solidity Team reported a bug on October 24, 2023, affecting Yul code using the verbatim builtin, specifically in the Block Deduplicator optimizer step. This bug, present since Solidity version 0.8.5, caused incorrect deduplication of verbatim assembly items surrounded by identical opcodes, considering them identical regardless of their data. The bug was confined to pure Yul compilation with optimization enabled and was unlikely to be exploited as an attack vector. The conditions triggering the bug were very specific, and its occurrence was deemed to have a low likelihood. The bug was rated with an overall low score due to these factors.
Num of instances: 1
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
['36']
36: constructor( // <= FOUND
37: IAVSDirectory __avsDirectory,
38: IRegistryCoordinator __registryCoordinator,
39: IStakeRegistry __stakeRegistry
40: )
41: BLSSignatureChecker(__registryCoordinator)
42: ServiceManagerBase(__avsDirectory, __registryCoordinator, __stakeRegistry)
43: {
44: _disableInitializers();
45: }
['35']
35: constructor(IEigenDAServiceManager _eigenDAServiceManager, BN254.G1Point memory _tau, uint256 _illegalValue, uint256 _stakeRequired) { // <= FOUND
36: eigenDAServiceManager = _eigenDAServiceManager;
37: tau = _tau;
38: illegalValue = _illegalValue;
39: stakeRequired = _stakeRequired;
40: }
In programming, avoiding single-line, non-empty object declarations enhances readability and maintainability. When properties are declared in a single line, it becomes challenging to scan and differentiate between them, especially as the object grows. Multi-line declarations improve clarity, making it easier to add, remove, or modify properties and to track changes in version control systems.
Num of instances: 17
Click to show findings
['4']
4: import {IEigenDAServiceManager} from "../interfaces/IEigenDAServiceManager.sol"; // <= FOUND
['5']
5: import {Merkle} from "eigenlayer-core/contracts/libraries/Merkle.sol"; // <= FOUND
['6']
6: import {BN254} from "eigenlayer-middleware/libraries/BN254.sol"; // <= FOUND
['7']
7: import {EigenDAHasher} from "./EigenDAHasher.sol"; // <= FOUND
['4']
4: import {Pausable} from "eigenlayer-core/contracts/permissions/Pausable.sol"; // <= FOUND
['5']
5: import {IAVSDirectory} from "eigenlayer-core/contracts/interfaces/IAVSDirectory.sol"; // <= FOUND
['6']
6: import {IPauserRegistry} from "eigenlayer-core/contracts/interfaces/IPauserRegistry.sol"; // <= FOUND
['8']
8: import {ServiceManagerBase} from "eigenlayer-middleware/ServiceManagerBase.sol"; // <= FOUND
['5']
5: import {BLSSignatureChecker} from "eigenlayer-middleware/BLSSignatureChecker.sol"; // <= FOUND
['10']
10: import {IRegistryCoordinator} from "eigenlayer-middleware/interfaces/IRegistryCoordinator.sol"; // <= FOUND
['11']
11: import {IStakeRegistry} from "eigenlayer-middleware/interfaces/IStakeRegistry.sol"; // <= FOUND
['13']
13: import {EigenDAServiceManagerStorage} from "./EigenDAServiceManagerStorage.sol"; // <= FOUND
['14']
14: import {EigenDAHasher} from "../libraries/EigenDAHasher.sol"; // <= FOUND
['4']
4: import {IServiceManager} from "eigenlayer-middleware/interfaces/IServiceManager.sol"; // <= FOUND
['5']
5: import {EigenDARollupUtils} from "../libraries/EigenDARollupUtils.sol"; // <= FOUND
['6']
6: import {EigenDAServiceManager} from "../core/EigenDAServiceManager.sol"; // <= FOUND
['89']
89: (bool success, ) = msg.sender.call{value: 1 ether}(""); // <= FOUND
Using a fixed compiler version in Solidity contract and abstract files ensures consistency and predictability in smart contract behavior. A fixed version prevents unforeseen issues arising from compiler updates, which might introduce breaking changes or new bugs. It guarantees that the contract's behavior remains stable and consistent with the version used during its development and testing.
Num of instances: 3
Click to show findings
[]
24: contract EigenDAServiceManager is EigenDAServiceManagerStorage, ServiceManagerBase, BLSSignatureChecker, Pausable
[]
21: contract MockRollup
[]
11: abstract contract EigenDAServiceManagerStorage is IEigenDAServiceManager
The using-for directive in Solidity allows smart contracts to utilize library functions as if they were methods of the type being passed. This syntactical feature streamlines code by enabling type-specific function applications without the need for explicit function calls. When a contract uses a library with the using-for declaration, it implicitly attaches the library's functions to a given type, enhancing code readability and efficiency. This approach is particularly beneficial for common operations on data types, such as array manipulation or complex mathematical computations, making it a valuable tool for developing more maintainable and intuitive smart contract code.
Num of instances: 12
Click to show findings
['7']
7: import {EigenDAHasher} from "./EigenDAHasher.sol"; // <= FOUND
['38']
37: require(
38: EigenDAHasher.hashBatchMetadata(blobVerificationProof.batchMetadata) // <= FOUND
39: == eigenDAServiceManager.batchIdToBatchMetadataHash(blobVerificationProof.batchId),
40: "EigenDARollupUtils.verifyBlob: batchMetadata does not match stored metadata" // <= FOUND
41: );
['48']
43:
44: require(
45: Merkle.verifyInclusionKeccak(
46: blobVerificationProof.inclusionProof,
47: blobVerificationProof.batchMetadata.batchHeader.blobHeadersRoot,
48: keccak256(abi.encodePacked(EigenDAHasher.hashBlobHeader(blobHeader))), // <= FOUND
49: blobVerificationProof.blobIndex
50: ),
51: "EigenDARollupUtils.verifyBlob: inclusion proof is invalid" // <= FOUND
52: );
['14']
14: import {EigenDAHasher} from "../libraries/EigenDAHasher.sol"; // <= FOUND
['117']
117: batchIdToBatchMetadataHash[batchIdMemory] = EigenDAHasher.hashBatchHashedMetadata(batchHeaderHash, signatoryRecordHash, uint32(block.number)); // <= FOUND
['58']
56:
57: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumNumbers[uint8(blobVerificationProof.quorumThresholdIndexes[i])]) == blobHeader.quorumBlobParams[i].quorumNumber,
58: "EigenDARollupUtils.verifyBlob: quorumNumber does not match" // <= FOUND
59: );
['65']
61:
62:
63: require(blobHeader.quorumBlobParams[i].adversaryThresholdPercentage
64: < blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
65: "EigenDARollupUtils.verifyBlob: adversaryThresholdPercentage is not valid" // <= FOUND
66: );
['71']
67:
68:
69: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumThresholdPercentages[uint8(blobVerificationProof.quorumThresholdIndexes[i])])
70: >= blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
71: "EigenDARollupUtils.verifyBlob: quorumThresholdPercentage is not met" // <= FOUND
72: );
['6']
5:
6: import {EigenDARollupUtils} from "../libraries/EigenDARollupUtils.sol"; // <= FOUND
['63']
55:
61: function postCommitment(
62: IEigenDAServiceManager.BlobHeader memory blobHeader,
63: EigenDARollupUtils.BlobVerificationProof memory blobVerificationProof // <= FOUND
64: ) external {
['65']
63:
64:
65: EigenDARollupUtils.verifyBlob(blobHeader, eigenDAServiceManager, blobVerificationProof); // <= FOUND
['84']
82:
83:
84: require(EigenDARollupUtils.openCommitment(point, illegalValue, tau, commitment.polynomialCommitment, proof), "MockRollup.challengeCommitment: Does not evaluate to illegal value"); // <= FOUND
Validating user inputs in external and public functions is a critical security practice in smart contract development. This approach prevents malicious or erroneous data from compromising contract integrity or behavior. By checking inputs against expected formats, ranges, or conditions before processing, contracts can avoid common vulnerabilities like reentrancy, overflow/underflow, and unauthorized access.
Num of instances: 5
Click to show findings
['32']
32: function verifyBlob(
33: IEigenDAServiceManager.BlobHeader calldata blobHeader, // <= FOUND
34: IEigenDAServiceManager eigenDAServiceManager,
35: BlobVerificationProof calldata blobVerificationProof // <= FOUND
36: ) external view {
37: require(
38: EigenDAHasher.hashBatchMetadata(blobVerificationProof.batchMetadata) // <= FOUND
39: == eigenDAServiceManager.batchIdToBatchMetadataHash(blobVerificationProof.batchId), // <= FOUND
40: "EigenDARollupUtils.verifyBlob: batchMetadata does not match stored metadata" // <= FOUND
41: );
42:
43: require(
44: Merkle.verifyInclusionKeccak(
45: blobVerificationProof.inclusionProof, // <= FOUND
46: blobVerificationProof.batchMetadata.batchHeader.blobHeadersRoot, // <= FOUND
47: keccak256(abi.encodePacked(EigenDAHasher.hashBlobHeader(blobHeader))), // <= FOUND
48: blobVerificationProof.blobIndex // <= FOUND
49: ),
50: "EigenDARollupUtils.verifyBlob: inclusion proof is invalid" // <= FOUND
51: );
52:
53:
54: for (uint i = 0; i < blobHeader.quorumBlobParams.length; i++) { // <= FOUND
55:
56: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumNumbers[uint8(blobVerificationProof.quorumThresholdIndexes[i])]) == blobHeader.quorumBlobParams[i].quorumNumber, // <= FOUND
57: "EigenDARollupUtils.verifyBlob: quorumNumber does not match"
58: );
59:
60:
61: require(blobHeader.quorumBlobParams[i].adversaryThresholdPercentage // <= FOUND
62: < blobHeader.quorumBlobParams[i].quorumThresholdPercentage, // <= FOUND
63: "EigenDARollupUtils.verifyBlob: adversaryThresholdPercentage is not valid"
64: );
65:
66:
67: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumThresholdPercentages[uint8(blobVerificationProof.quorumThresholdIndexes[i])]) // <= FOUND
68: >= blobHeader.quorumBlobParams[i].quorumThresholdPercentage, // <= FOUND
69: "EigenDARollupUtils.verifyBlob: quorumThresholdPercentage is not met"
70: );
71:
72: }
73: }
['47']
47: function initialize(
48: IPauserRegistry _pauserRegistry, // <= FOUND
49: uint256 _initialPausedStatus, // <= FOUND
50: address _initialOwner,
51: address _batchConfirmer // <= FOUND
52: )
53: public
54: initializer
55: {
56: _initializePauser(_pauserRegistry, _initialPausedStatus); // <= FOUND
57: _transferOwnership(_initialOwner);
58: _setBatchConfirmer(_batchConfirmer); // <= FOUND
59: }
['67']
67: function confirmBatch(
68: BatchHeader calldata batchHeader, // <= FOUND
69: NonSignerStakesAndSignature memory nonSignerStakesAndSignature // <= FOUND
70: ) external onlyWhenNotPaused(PAUSED_CONFIRM_BATCH) onlyBatchConfirmer() {
71:
72: require(tx.origin == msg.sender, "EigenDAServiceManager.confirmBatch: header and nonsigner data must be in calldata");
73:
74: require(
75: batchHeader.referenceBlockNumber <= block.number, "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is in future" // <= FOUND
76: );
77:
78: require(
79: (batchHeader.referenceBlockNumber + BLOCK_STALE_MEASURE) >= uint32(block.number), // <= FOUND
80: "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is too far in past"
81: );
82:
83:
84: require(
85: batchHeader.quorumNumbers.length == batchHeader.quorumThresholdPercentages.length, // <= FOUND
86: "EigenDAServiceManager.confirmBatch: quorumNumbers and quorumThresholdPercentages must be of the same length"
87: );
88:
89:
90: bytes32 reducedBatchHeaderHash = batchHeader.hashBatchHeaderToReducedBatchHeader(); // <= FOUND
91:
92:
93: (
94: QuorumStakeTotals memory quorumStakeTotals,
95: bytes32 signatoryRecordHash // <= FOUND
96: ) = checkSignatures(
97: reducedBatchHeaderHash,
98: batchHeader.quorumNumbers, // <= FOUND
99: batchHeader.referenceBlockNumber, // <= FOUND
100: nonSignerStakesAndSignature // <= FOUND
101: );
102:
103:
104: for (uint i = 0; i < batchHeader.quorumThresholdPercentages.length; i++) { // <= FOUND
105:
106:
107: require(
108: quorumStakeTotals.signedStakeForQuorum[i] * THRESHOLD_DENOMINATOR >=
109: quorumStakeTotals.totalStakeForQuorum[i] * uint8(batchHeader.quorumThresholdPercentages[i]), // <= FOUND
110: "EigenDAServiceManager.confirmBatch: signatories do not own at least threshold percentage of a quorum"
111: );
112: }
113:
114:
115: uint32 batchIdMemory = batchId;
116: bytes32 batchHeaderHash = batchHeader.hashBatchHeader(); // <= FOUND
117: batchIdToBatchMetadataHash[batchIdMemory] = EigenDAHasher.hashBatchHashedMetadata(batchHeaderHash, signatoryRecordHash, uint32(block.number)); // <= FOUND
118:
119: emit BatchConfirmed(reducedBatchHeaderHash, batchIdMemory);
120:
121:
122: batchId = batchIdMemory + 1;
123: }
['55']
55: function postCommitment(
56: IEigenDAServiceManager.BlobHeader memory blobHeader, // <= FOUND
57: EigenDARollupUtils.BlobVerificationProof memory blobVerificationProof // <= FOUND
58: ) external {
59: require(validators[msg.sender], "MockRollup.postCommitment: Validator not registered");
60: require(commitments[block.timestamp].validator == address(0), "MockRollup.postCommitment: Commitment already posted"); // <= FOUND
61:
62:
63: EigenDARollupUtils.verifyBlob(blobHeader, eigenDAServiceManager, blobVerificationProof); // <= FOUND
64:
65: commitments[block.timestamp] = Commitment(msg.sender, blobHeader.dataLength, blobHeader.commitment); // <= FOUND
66: }
['74']
74: function challengeCommitment(uint256 timestamp, uint256 point, BN254.G2Point memory proof) external { // <= FOUND
75: Commitment memory commitment = commitments[timestamp]; // <= FOUND
76: require(commitment.validator != address(0), "MockRollup.challengeCommitment: Commitment not posted"); // <= FOUND
77:
78:
79: require(point < commitment.dataLength, "MockRollup.challengeCommitment: Point must be less than data length"); // <= FOUND
80:
81:
82: require(EigenDARollupUtils.openCommitment(point, illegalValue, tau, commitment.polynomialCommitment, proof), "MockRollup.challengeCommitment: Does not evaluate to illegal value"); // <= FOUND
83:
84:
85: validators[commitment.validator] = false; // <= FOUND
86: blacklist[commitment.validator] = true; // <= FOUND
87:
88:
89: (bool success, ) = msg.sender.call{value: 1 ether}("");
90: require(success);
91:
92: }
Implementing a ranged getter for array state variables enhances data accessibility in smart contracts. The Solidity compiler auto-generates getters for individual elements of public arrays but lacks direct support for retrieving entire arrays or specific ranges. Adding a custom function to fetch array slices can significantly improve efficiency and flexibility, reducing the need for multiple calls to access several elements. This is particularly useful for contracts without multicall capabilities, streamlining data retrieval and minimizing gas costs associated with separate calls for each element.
Num of instances: 1
Using named returns in Solidity functions enhances code readability and clarity. By explicitly naming return variables in the function signature, developers can document what each return value represents, making the code easier to understand and maintain. This practice can also slightly optimize gas usage by avoiding extra variable declarations.
Num of instances: 2
Click to show findings
['138']
138: function taskNumber() external view returns (uint32) // <= FOUND
['143']
143: function latestServeUntilBlock() external view returns (uint32) // <= FOUND
Including external calls in modifiers is generally discouraged in Solidity programming because modifiers are primarily intended for checks and assertions. External calls increase complexity and can obscure the flow of transactions, making it harder for reviewers to identify potentially risky interactions. To maintain clarity and security, it's advisable to encapsulate external calls within internal functions. These functions can then be invoked within the main function body, rather than in modifiers, ensuring that external interactions are explicit and easier to audit, thus enhancing contract safety and readability.
Num of instances: 1
Click to show findings
['31']
31: modifier onlyBatchConfirmer() { // <= FOUND
32: require(msg.sender == batchConfirmer, "onlyBatchConfirmer: not from batch confirmer");
33: _;
34: }
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
['131']
131: function _setBatchConfirmer(address _batchConfirmer) internal { // <= FOUND
132: address previousBatchConfirmer = batchConfirmer;
133: batchConfirmer = _batchConfirmer;
134: emit BatchConfirmerChanged(previousBatchConfirmer, batchConfirmer); // <= FOUND
135: }
Consider adding more detail to these error strings
Num of instances: 1
Click to show findings
['90']
74: function challengeCommitment(uint256 timestamp, uint256 point, BN254.G2Point memory proof) external {
75: Commitment memory commitment = commitments[timestamp];
76: require(commitment.validator != address(0), "MockRollup.challengeCommitment: Commitment not posted");
77:
78:
79: require(point < commitment.dataLength, "MockRollup.challengeCommitment: Point must be less than data length");
80:
81:
82: require(EigenDARollupUtils.openCommitment(point, illegalValue, tau, commitment.polynomialCommitment, proof), "MockRollup.challengeCommitment: Does not evaluate to illegal value");
83:
84:
85: validators[commitment.validator] = false;
86: blacklist[commitment.validator] = true;
87:
88:
89: (bool success, ) = msg.sender.call{value: 1 ether}("");
90: require(success); // <= FOUND
91:
92: }
[Gas-3] State variables which are not modified within functions should be set as constants or immutable for values set at deployment
Set state variables listed below as constant or immutable for values set at deployment. Ensure it is safe to do so
Num of instances: 2
Click to show findings
['25']
25: uint256 public illegalValue; // <= FOUND
['26']
26: uint256 public stakeRequired; // <= FOUND
Optimizing low-level calls using assembly in Solidity can be beneficial, particularly when dealing with function return data. Typically, even if return data from a low-level call is not used, Solidity still allocates memory to store it, which incurs gas costs. By using assembly, developers can bypass the automatic memory allocation for unused return data. This manual optimization involves handling the call at the assembly level and deliberately choosing not to store the return data in memory when it's not needed.
Num of instances: 3
Click to show findings
['91']
89:
90:
91: (bool success, ) = msg.sender.call{value: 1 ether}(""); // <= FOUND
['89']
89:
90:
91: (bool success, ) = msg.sender.call{value: 1 ether}(""); // <= FOUND
['89']
89:
90:
91: (bool success, ) = msg.sender.call{value: 1 ether}(""); // <= FOUND
[Gas-5] 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: 5
Click to show findings
[]
32: function verifyBlob(
33: IEigenDAServiceManager.BlobHeader calldata blobHeader,
34: IEigenDAServiceManager eigenDAServiceManager,
35: BlobVerificationProof calldata blobVerificationProof
36: ) external view {
37: require(
38: EigenDAHasher.hashBatchMetadata(blobVerificationProof.batchMetadata)
39: == eigenDAServiceManager.batchIdToBatchMetadataHash(blobVerificationProof.batchId),
40: "EigenDARollupUtils.verifyBlob: batchMetadata does not match stored metadata"
41: );
42:
43: require(
44: Merkle.verifyInclusionKeccak(
45: blobVerificationProof.inclusionProof,
46: blobVerificationProof.batchMetadata.batchHeader.blobHeadersRoot,
47: keccak256(abi.encodePacked(EigenDAHasher.hashBlobHeader(blobHeader))),
48: blobVerificationProof.blobIndex
49: ),
50: "EigenDARollupUtils.verifyBlob: inclusion proof is invalid"
51: );
52:
53:
54: for (uint i = 0; i < blobHeader.quorumBlobParams.length; i++) {
55:
56: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumNumbers[uint8(blobVerificationProof.quorumThresholdIndexes[i])]) == blobHeader.quorumBlobParams[i].quorumNumber,
57: "EigenDARollupUtils.verifyBlob: quorumNumber does not match"
58: );
59:
60:
61: require(blobHeader.quorumBlobParams[i].adversaryThresholdPercentage
62: < blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
63: "EigenDARollupUtils.verifyBlob: adversaryThresholdPercentage is not valid"
64: );
65:
66:
67: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumThresholdPercentages[uint8(blobVerificationProof.quorumThresholdIndexes[i])])
68: >= blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
69: "EigenDARollupUtils.verifyBlob: quorumThresholdPercentage is not met"
70: );
71:
72: }
73: }
['72']
67: function confirmBatch(
68: BatchHeader calldata batchHeader,
69: NonSignerStakesAndSignature memory nonSignerStakesAndSignature
70: ) external onlyWhenNotPaused(PAUSED_CONFIRM_BATCH) onlyBatchConfirmer() {
71:
72: require(tx.origin == msg.sender, "EigenDAServiceManager.confirmBatch: header and nonsigner data must be in calldata"); // <= FOUND
73:
74: require(
75: batchHeader.referenceBlockNumber <= block.number, "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is in future"
76: );
77:
78: require(
79: (batchHeader.referenceBlockNumber + BLOCK_STALE_MEASURE) >= uint32(block.number),
80: "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is too far in past"
81: );
82:
83:
84: require(
85: batchHeader.quorumNumbers.length == batchHeader.quorumThresholdPercentages.length,
86: "EigenDAServiceManager.confirmBatch: quorumNumbers and quorumThresholdPercentages must be of the same length"
87: );
88:
89:
90: bytes32 reducedBatchHeaderHash = batchHeader.hashBatchHeaderToReducedBatchHeader();
91:
92:
93: (
94: QuorumStakeTotals memory quorumStakeTotals,
95: bytes32 signatoryRecordHash
96: ) = checkSignatures(
97: reducedBatchHeaderHash,
98: batchHeader.quorumNumbers,
99: batchHeader.referenceBlockNumber,
100: nonSignerStakesAndSignature
101: );
102:
103:
104: for (uint i = 0; i < batchHeader.quorumThresholdPercentages.length; i++) {
105:
106:
107: require(
108: quorumStakeTotals.signedStakeForQuorum[i] * THRESHOLD_DENOMINATOR >=
109: quorumStakeTotals.totalStakeForQuorum[i] * uint8(batchHeader.quorumThresholdPercentages[i]),
110: "EigenDAServiceManager.confirmBatch: signatories do not own at least threshold percentage of a quorum"
111: );
112: }
113:
114:
115: uint32 batchIdMemory = batchId;
116: bytes32 batchHeaderHash = batchHeader.hashBatchHeader();
117: batchIdToBatchMetadataHash[batchIdMemory] = EigenDAHasher.hashBatchHashedMetadata(batchHeaderHash, signatoryRecordHash, uint32(block.number));
118:
119: emit BatchConfirmed(reducedBatchHeaderHash, batchIdMemory);
120:
121:
['44']
43: function registerValidator() external payable {
44: require(msg.value == stakeRequired, "MockRollup.registerValidator: Must send stake required to register"); // <= FOUND
45: require(!validators[msg.sender], "MockRollup.registerValidator: Validator already registered"); // <= FOUND
46: require(!blacklist[msg.sender], "MockRollup.registerValidator: Validator blacklisted"); // <= FOUND
47: validators[msg.sender] = true;
48: }
['59']
55: function postCommitment(
56: IEigenDAServiceManager.BlobHeader memory blobHeader,
57: EigenDARollupUtils.BlobVerificationProof memory blobVerificationProof
58: ) external {
59: require(validators[msg.sender], "MockRollup.postCommitment: Validator not registered"); // <= FOUND
60: require(commitments[block.timestamp].validator == address(0), "MockRollup.postCommitment: Commitment already posted"); // <= FOUND
61:
62:
63: EigenDARollupUtils.verifyBlob(blobHeader, eigenDAServiceManager, blobVerificationProof);
64:
65: commitments[block.timestamp] = Commitment(msg.sender, blobHeader.dataLength, blobHeader.commitment);
66: }
['76']
74: function challengeCommitment(uint256 timestamp, uint256 point, BN254.G2Point memory proof) external {
75: Commitment memory commitment = commitments[timestamp];
76: require(commitment.validator != address(0), "MockRollup.challengeCommitment: Commitment not posted"); // <= FOUND
77:
78:
79: require(point < commitment.dataLength, "MockRollup.challengeCommitment: Point must be less than data length"); // <= FOUND
80:
81:
82: require(EigenDARollupUtils.openCommitment(point, illegalValue, tau, commitment.polynomialCommitment, proof), "MockRollup.challengeCommitment: Does not evaluate to illegal value"); // <= FOUND
83:
84:
85: validators[commitment.validator] = false;
86: blacklist[commitment.validator] = true;
87:
88:
89: (bool success, ) = msg.sender.call{value: 1 ether}("");
90: require(success);
91:
92: }
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: 1
Click to show findings
['47']
47: function initialize(
48: IPauserRegistry _pauserRegistry,
49: uint256 _initialPausedStatus,
50: address _initialOwner,
51: address _batchConfirmer
52: )
53: public
54: initializer
55:
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
['67']
67: function confirmBatch(
68: BatchHeader calldata batchHeader, // <= FOUND
69: NonSignerStakesAndSignature memory nonSignerStakesAndSignature // <= FOUND
70: ) external onlyWhenNotPaused(PAUSED_CONFIRM_BATCH) onlyBatchConfirmer() {
71:
72: require(tx.origin == msg.sender, "EigenDAServiceManager.confirmBatch: header and nonsigner data must be in calldata");
73:
74: require(
75: batchHeader.referenceBlockNumber <= block.number, "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is in future" // <= FOUND
76: );
77:
78: require(
79: (batchHeader.referenceBlockNumber + BLOCK_STALE_MEASURE) >= uint32(block.number), // <= FOUND
80: "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is too far in past"
81: );
82:
83:
84: require(
85: batchHeader.quorumNumbers.length == batchHeader.quorumThresholdPercentages.length, // <= FOUND
86: "EigenDAServiceManager.confirmBatch: quorumNumbers and quorumThresholdPercentages must be of the same length"
87: );
88:
89:
90: bytes32 reducedBatchHeaderHash = batchHeader.hashBatchHeaderToReducedBatchHeader(); // <= FOUND
91:
92:
93: (
94: QuorumStakeTotals memory quorumStakeTotals,
95: bytes32 signatoryRecordHash
96: ) = checkSignatures(
97: reducedBatchHeaderHash,
98: batchHeader.quorumNumbers, // <= FOUND
99: batchHeader.referenceBlockNumber, // <= FOUND
100: nonSignerStakesAndSignature // <= FOUND
101: );
102:
103:
104: for (uint i = 0; i < batchHeader.quorumThresholdPercentages.length; i++) { // <= FOUND
105:
106:
107: require(
108: quorumStakeTotals.signedStakeForQuorum[i] * THRESHOLD_DENOMINATOR >=
109: quorumStakeTotals.totalStakeForQuorum[i] * uint8(batchHeader.quorumThresholdPercentages[i]), // <= FOUND
110: "EigenDAServiceManager.confirmBatch: signatories do not own at least threshold percentage of a quorum"
111: );
112: }
113:
114:
115: uint32 batchIdMemory = batchId;
116: bytes32 batchHeaderHash = batchHeader.hashBatchHeader(); // <= FOUND
117: batchIdToBatchMetadataHash[batchIdMemory] = EigenDAHasher.hashBatchHashedMetadata(batchHeaderHash, signatoryRecordHash, uint32(block.number)); // <= FOUND
118:
119: emit BatchConfirmed(reducedBatchHeaderHash, batchIdMemory);
120:
121:
122: batchId = batchIdMemory + 1;
123: }
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: 9
Click to show findings
['28']
28: uint8 internal constant PAUSED_CONFIRM_BATCH = 0; // <= FOUND
['22']
19: function hashBatchHashedMetadata(
20: bytes32 batchHeaderHash,
21: bytes32 signatoryRecordHash,
22: uint32 blockNumber // <= FOUND
23: ) internal pure returns(bytes32) {
24: return keccak256(abi.encodePacked(batchHeaderHash, signatoryRecordHash, blockNumber));
25: }
['36']
33: function hashBatchHashedMetadata(
34: bytes32 batchHeaderHash,
35: bytes memory confirmationData,
36: uint32 blockNumber // <= FOUND
37: ) internal pure returns(bytes32) {
38: return keccak256(abi.encodePacked(batchHeaderHash, confirmationData, blockNumber));
39: }
['115']
67: function confirmBatch(
68: BatchHeader calldata batchHeader,
69: NonSignerStakesAndSignature memory nonSignerStakesAndSignature
70: ) external onlyWhenNotPaused(PAUSED_CONFIRM_BATCH) onlyBatchConfirmer() {
71:
72: require(tx.origin == msg.sender, "EigenDAServiceManager.confirmBatch: header and nonsigner data must be in calldata");
73:
74: require(
75: batchHeader.referenceBlockNumber <= block.number, "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is in future"
76: );
77:
78: require(
79: (batchHeader.referenceBlockNumber + BLOCK_STALE_MEASURE) >= uint32(block.number),
80: "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is too far in past"
81: );
82:
83:
84: require(
85: batchHeader.quorumNumbers.length == batchHeader.quorumThresholdPercentages.length,
86: "EigenDAServiceManager.confirmBatch: quorumNumbers and quorumThresholdPercentages must be of the same length"
87: );
88:
89:
90: bytes32 reducedBatchHeaderHash = batchHeader.hashBatchHeaderToReducedBatchHeader();
91:
92:
93: (
94: QuorumStakeTotals memory quorumStakeTotals,
95: bytes32 signatoryRecordHash
96: ) = checkSignatures(
97: reducedBatchHeaderHash,
98: batchHeader.quorumNumbers,
99: batchHeader.referenceBlockNumber,
100: nonSignerStakesAndSignature
101: );
102:
103:
104: for (uint i = 0; i < batchHeader.quorumThresholdPercentages.length; i++) {
105:
106:
107: require(
108: quorumStakeTotals.signedStakeForQuorum[i] * THRESHOLD_DENOMINATOR >=
109: quorumStakeTotals.totalStakeForQuorum[i] * uint8(batchHeader.quorumThresholdPercentages[i]),
110: "EigenDAServiceManager.confirmBatch: signatories do not own at least threshold percentage of a quorum"
111: );
112: }
113:
114:
115: uint32 batchIdMemory = batchId; // <= FOUND
116: bytes32 batchHeaderHash = batchHeader.hashBatchHeader();
117: batchIdToBatchMetadataHash[batchIdMemory] = EigenDAHasher.hashBatchHashedMetadata(batchHeaderHash, signatoryRecordHash, uint32(block.number));
118:
119: emit BatchConfirmed(reducedBatchHeaderHash, batchIdMemory);
120:
121:
122: batchId = batchIdMemory + 1;
123: }
['17']
17: uint32 public constant STORE_DURATION_BLOCKS = 2 weeks / 12 seconds; // <= FOUND
['20']
20: uint32 internal constant MIN_STORE_SIZE = 32; // <= FOUND
['22']
22: uint32 internal constant MAX_STORE_SIZE = 4e9; // <= FOUND
['31']
31: uint32 public constant BLOCK_STALE_MEASURE = 150; // <= FOUND
['34']
34: uint32 public batchId; // <= FOUND
Using unchecked increments in Solidity can save on gas fees by bypassing built-in overflow checks, thus optimizing gas usage, but requires careful assessment of potential risks and edge cases to avoid unintended consequences.
Num of instances: 2
Click to show findings
['54']
32: function verifyBlob(
33: IEigenDAServiceManager.BlobHeader calldata blobHeader,
34: IEigenDAServiceManager eigenDAServiceManager,
35: BlobVerificationProof calldata blobVerificationProof
36: ) external view {
37: require(
38: EigenDAHasher.hashBatchMetadata(blobVerificationProof.batchMetadata)
39: == eigenDAServiceManager.batchIdToBatchMetadataHash(blobVerificationProof.batchId),
40: "EigenDARollupUtils.verifyBlob: batchMetadata does not match stored metadata"
41: );
42:
43: require(
44: Merkle.verifyInclusionKeccak(
45: blobVerificationProof.inclusionProof,
46: blobVerificationProof.batchMetadata.batchHeader.blobHeadersRoot,
47: keccak256(abi.encodePacked(EigenDAHasher.hashBlobHeader(blobHeader))),
48: blobVerificationProof.blobIndex
49: ),
50: "EigenDARollupUtils.verifyBlob: inclusion proof is invalid"
51: );
52:
53:
54: for (uint i = 0; i < blobHeader.quorumBlobParams.length; i++) { // <= FOUND
55:
56: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumNumbers[uint8(blobVerificationProof.quorumThresholdIndexes[i])]) == blobHeader.quorumBlobParams[i].quorumNumber,
57: "EigenDARollupUtils.verifyBlob: quorumNumber does not match"
58: );
59:
60:
61: require(blobHeader.quorumBlobParams[i].adversaryThresholdPercentage
62: < blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
63: "EigenDARollupUtils.verifyBlob: adversaryThresholdPercentage is not valid"
64: );
65:
66:
67: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumThresholdPercentages[uint8(blobVerificationProof.quorumThresholdIndexes[i])])
68: >= blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
69: "EigenDARollupUtils.verifyBlob: quorumThresholdPercentage is not met"
70: );
71:
72: }
73: }
['104']
67: function confirmBatch(
68: BatchHeader calldata batchHeader,
69: NonSignerStakesAndSignature memory nonSignerStakesAndSignature
70: ) external onlyWhenNotPaused(PAUSED_CONFIRM_BATCH) onlyBatchConfirmer() {
71:
72: require(tx.origin == msg.sender, "EigenDAServiceManager.confirmBatch: header and nonsigner data must be in calldata");
73:
74: require(
75: batchHeader.referenceBlockNumber <= block.number, "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is in future"
76: );
77:
78: require(
79: (batchHeader.referenceBlockNumber + BLOCK_STALE_MEASURE) >= uint32(block.number),
80: "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is too far in past"
81: );
82:
83:
84: require(
85: batchHeader.quorumNumbers.length == batchHeader.quorumThresholdPercentages.length,
86: "EigenDAServiceManager.confirmBatch: quorumNumbers and quorumThresholdPercentages must be of the same length"
87: );
88:
89:
90: bytes32 reducedBatchHeaderHash = batchHeader.hashBatchHeaderToReducedBatchHeader();
91:
92:
93: (
94: QuorumStakeTotals memory quorumStakeTotals,
95: bytes32 signatoryRecordHash
96: ) = checkSignatures(
97: reducedBatchHeaderHash,
98: batchHeader.quorumNumbers,
99: batchHeader.referenceBlockNumber,
100: nonSignerStakesAndSignature
101: );
102:
103:
104: for (uint i = 0; i < batchHeader.quorumThresholdPercentages.length; i++) { // <= FOUND
105:
106:
107: require(
108: quorumStakeTotals.signedStakeForQuorum[i] * THRESHOLD_DENOMINATOR >=
109: quorumStakeTotals.totalStakeForQuorum[i] * uint8(batchHeader.quorumThresholdPercentages[i]),
110: "EigenDAServiceManager.confirmBatch: signatories do not own at least threshold percentage of a quorum"
111: );
112: }
113:
114:
115: uint32 batchIdMemory = batchId;
116: bytes32 batchHeaderHash = batchHeader.hashBatchHeader();
117: batchIdToBatchMetadataHash[batchIdMemory] = EigenDAHasher.hashBatchHashedMetadata(batchHeaderHash, signatoryRecordHash, uint32(block.number));
118:
119: emit BatchConfirmed(reducedBatchHeaderHash, batchIdMemory);
120:
121:
122: batchId = batchIdMemory + 1;
123: }
Using .delete is better than resetting a Solidity variable to its default value manually because it frees up storage space on the Ethereum blockchain, resulting in gas cost savings.
Num of instances: 1
Click to show findings
['74']
74: function challengeCommitment(uint256 timestamp, uint256 point, BN254.G2Point memory proof) external { // <= FOUND
75: Commitment memory commitment = commitments[timestamp];
76: require(commitment.validator != address(0), "MockRollup.challengeCommitment: Commitment not posted");
77:
78:
79: require(point < commitment.dataLength, "MockRollup.challengeCommitment: Point must be less than data length");
80:
81:
82: require(EigenDARollupUtils.openCommitment(point, illegalValue, tau, commitment.polynomialCommitment, proof), "MockRollup.challengeCommitment: Does not evaluate to illegal value");
83:
84:
85: validators[commitment.validator] = false;
86: blacklist[commitment.validator] = true;
87:
88:
89: (bool success, ) = msg.sender.call{value: 1 ether}("");
90: require(success);
91:
92: }
Using .delete is better than resetting a Solidity variable to its default value manually because it frees up storage space on the Ethereum blockchain, resulting in gas cost savings.
Num of instances: 3
Click to show findings
['28']
28:
29: uint8 internal constant PAUSED_CONFIRM_BATCH = 0; // <= FOUND
['54']
54:
55:
56: for (uint i = 0; i < blobHeader.quorumBlobParams.length; i++) { // <= FOUND
['104']
104:
105:
106: for (uint i = 0; i < batchHeader.quorumThresholdPercentages.length; i++) { // <= FOUND
Using assembly for address comparisons in Solidity can save gas because it allows for more direct access to the Ethereum Virtual Machine (EVM), reducing the overhead of higher-level operations. Solidity's high-level abstraction simplifies coding but can introduce additional gas costs. Using assembly for simple operations like address comparisons can be more gas-efficient.
Num of instances: 2
Click to show findings
['55']
55: function postCommitment(
56: IEigenDAServiceManager.BlobHeader memory blobHeader,
57: EigenDARollupUtils.BlobVerificationProof memory blobVerificationProof
58: ) external {
59: require(validators[msg.sender], "MockRollup.postCommitment: Validator not registered");
60: require(commitments[block.timestamp].validator == address(0), "MockRollup.postCommitment: Commitment already posted"); // <= FOUND
61:
62:
63: EigenDARollupUtils.verifyBlob(blobHeader, eigenDAServiceManager, blobVerificationProof);
64:
65: commitments[block.timestamp] = Commitment(msg.sender, blobHeader.dataLength, blobHeader.commitment);
66: }
['74']
74: function challengeCommitment(uint256 timestamp, uint256 point, BN254.G2Point memory proof) external {
75: Commitment memory commitment = commitments[timestamp];
76: require(commitment.validator != address(0), "MockRollup.challengeCommitment: Commitment not posted"); // <= FOUND
77:
78:
79: require(point < commitment.dataLength, "MockRollup.challengeCommitment: Point must be less than data length");
80:
81:
82: require(EigenDARollupUtils.openCommitment(point, illegalValue, tau, commitment.polynomialCommitment, proof), "MockRollup.challengeCommitment: Does not evaluate to illegal value");
83:
84:
85: validators[commitment.validator] = false;
86: blacklist[commitment.validator] = true;
87:
88:
89: (bool success, ) = msg.sender.call{value: 1 ether}("");
90: require(success);
91:
92: }
Getters for public state variables are automatically generated so there is no need to code them manually and lose gas
Num of instances: 1
Click to show findings
['139']
138: function taskNumber() external view returns (uint32) {
139: return batchId; // <= FOUND
140: }
The Solidity compiler's Intermediate Representation (IR) based code generator, which can be activated using --via-ir on the command line or {"viaIR": true} in the options, serves a dual purpose. Firstly, it boosts the transparency and audibility of code generation, which enhances developers' comprehension and control over the contract's final bytecode. Secondly, it enables more sophisticated optimization passes that span multiple functions, thereby potentially leading to more efficient bytecode.
It's important to note that using the IR-based code generator may lengthen compile times due to the extra optimization steps. Therefore, it's advised to test your contract with and without this option enabled to measure the performance and gas cost implications. If the IR-based code generator significantly enhances your contract's performance or reduces gas costs, consider using the --via-ir flag during deployment. This way, you can leverage more advanced compiler optimizations without hindering your development workflow.
Num of instances: 7
Click to show findings
['65']
65: all
['65']
65: all
['65']
65: all
['65']
65: all
['65']
65: all
['65']
65: all
['65']
65: all
Bitmaps in Solidity are essentially a way of representing a set of boolean values within an integer type variable such as uint256. Each bit in the integer represents a true or false value (1 or 0), thus allowing efficient storage of multiple boolean values.
Bitmaps can save gas in the Ethereum network because they condense a lot of information into a small amount of storage. In Ethereum, storage is one of the most significant costs in terms of gas usage. By reducing the amount of storage space needed, you can potentially save on gas fees.
Here's a quick comparison:
If you were to represent 256 different boolean values in the traditional way, you would have to declare 256 different bool variables. Given that each bool occupies a storage slot and each storage slot costs 20,000 gas to initialize, you would end up paying a considerable amount of gas.
On the other hand, if you were to use a bitmap, you could store these 256 boolean values within a single uint256 variable. In other words, you'd only pay for a single storage slot, resulting in significant gas savings.
However, it's important to note that while bitmaps can provide gas efficiencies, they do add complexity to the code, making it harder to read and maintain. Also, using bitmaps is efficient only when dealing with a large number of boolean variables that are frequently changed or accessed together.
In contrast, the straightforward counterpart to bitmaps would be using arrays or mappings to store boolean values, with each bool value occupying its own storage slot. This approach is simpler and more readable but could potentially be more expensive in terms of gas usage.
Num of instances: 3
Click to show findings
['47']
47: validators[msg.sender] = true; // <= FOUND
['86']
86: blacklist[commitment.validator] = true; // <= FOUND
['87']
85:
86:
87: validators[commitment.validator] = false; // <= FOUND
With the use of inline assembly in Solidity, we can take advantage of low-level features like scratch space and the free memory pointer, offering more gas-efficient ways of emitting events. The scratch space is a certain area of memory where we can temporarily store data, and the free memory pointer indicates the next available memory slot. Using these, we can efficiently assemble event data without incurring additional memory expansion costs. However, safety is paramount: to avoid overwriting or leakage, we must cache the free memory pointer before use and restore it afterward, ensuring that it points to the correct memory location post-operation.
Num of instances: 2
Click to show findings
['120']
119:
120: emit BatchConfirmed(reducedBatchHeaderHash, batchIdMemory); // <= FOUND
['134']
134: emit BatchConfirmerChanged(previousBatchConfirmer, batchConfirmer); // <= FOUND
Looping downwards in Solidity is more gas efficient due to how the EVM compares variables. In a 'for' loop that counts down, the end condition is usually to compare with zero, which is cheaper than comparing with another number. As such, restructure your loops to count downwards where possible.
Num of instances: 2
Click to show findings
['54']
54: for (uint i = 0; i < blobHeader.quorumBlobParams.length; i++) { // <= FOUND
55:
56: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumNumbers[uint8(blobVerificationProof.quorumThresholdIndexes[i])]) == blobHeader.quorumBlobParams[i].quorumNumber,
57: "EigenDARollupUtils.verifyBlob: quorumNumber does not match"
58: );
59:
60:
61: require(blobHeader.quorumBlobParams[i].adversaryThresholdPercentage
62: < blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
63: "EigenDARollupUtils.verifyBlob: adversaryThresholdPercentage is not valid"
64: );
65:
66:
67: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumThresholdPercentages[uint8(blobVerificationProof.quorumThresholdIndexes[i])])
68: >= blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
69: "EigenDARollupUtils.verifyBlob: quorumThresholdPercentage is not met"
70: );
71:
72: }
['104']
104: for (uint i = 0; i < batchHeader.quorumThresholdPercentages.length; i++) { // <= FOUND
105:
106:
107: require(
108: quorumStakeTotals.signedStakeForQuorum[i] * THRESHOLD_DENOMINATOR >=
109: quorumStakeTotals.totalStakeForQuorum[i] * uint8(batchHeader.quorumThresholdPercentages[i]),
110: "EigenDAServiceManager.confirmBatch: signatories do not own at least threshold percentage of a quorum"
111: );
112: }
Using private visibility for constants and immutables in Solidity instead of public can save gas. This is because private elements are not included in the contract's ABI, reducing the deployment and interaction costs. To achieve better efficiency, it is recommended to use private visibility when external access is not needed.
Num of instances: 3
Click to show findings
['13']
13: uint256 public constant THRESHOLD_DENOMINATOR = 100; // <= FOUND
['17']
17: uint32 public constant STORE_DURATION_BLOCKS = 2 weeks / 12 seconds; // <= FOUND
['31']
31: uint32 public constant BLOCK_STALE_MEASURE = 150; // <= FOUND
In Solidity, marking functions as payable allows them to accept Ether. If a function is known to revert for regular users (non-admin or specific roles) but needs to be accessible to others, marking it as payable can be beneficial. This ensures that even if a regular user accidentally sends Ether to the function, the Ether won't be trapped, as the function reverts, returning the funds. This can save gas by avoiding unnecessary failure handling in the function itself. Resolution: Carefully assess the roles and access patterns, and mark functions that should revert for regular users as payable to handle accidental Ether transfers.
Num of instances: 1
Click to show findings
['126']
126: function setBatchConfirmer(address _batchConfirmer) external onlyOwner() {
127: _setBatchConfirmer(_batchConfirmer);
128: }
Function names in Solidity contracts can be optimized to save gas during both deployment and execution. Method IDs are the first four bytes of the keccak256 hash of the function signature, and having two leading zero bytes can save 128 gas each during deployment. Additionally, renaming functions to have lower method IDs can save 22 gas per call, per sorted position shifted. This optimization leverages the way EVM handles data storage, making the execution more efficient. While these savings might seem minor, they can add up in contracts with numerous calls, contributing to more economical and efficient code.
Num of instances: 2
Click to show findings
['24']
24: contract EigenDAServiceManager is EigenDAServiceManagerStorage, ServiceManagerBase, BLSSignatureChecker, Pausable // <= FOUND
['21']
21: contract MockRollup // <= FOUND
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: 2
Click to show findings
['54']
54: for (uint i = 0; i < blobHeader.quorumBlobParams.length; i++) {
55:
56: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumNumbers[uint8(blobVerificationProof.quorumThresholdIndexes[i])]) == blobHeader.quorumBlobParams[i].quorumNumber,
57: "EigenDARollupUtils.verifyBlob: quorumNumber does not match"
58: );
59:
60:
61: require(blobHeader.quorumBlobParams[i].adversaryThresholdPercentage
62: < blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
63: "EigenDARollupUtils.verifyBlob: adversaryThresholdPercentage is not valid"
64: );
65:
66:
67: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumThresholdPercentages[uint8(blobVerificationProof.quorumThresholdIndexes[i])])
68: >= blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
69: "EigenDARollupUtils.verifyBlob: quorumThresholdPercentage is not met"
70: );
71:
72: }
['104']
104: for (uint i = 0; i < batchHeader.quorumThresholdPercentages.length; i++) {
105:
106:
107: require(
108: quorumStakeTotals.signedStakeForQuorum[i] * THRESHOLD_DENOMINATOR >=
109: quorumStakeTotals.totalStakeForQuorum[i] * uint8(batchHeader.quorumThresholdPercentages[i]),
110: "EigenDAServiceManager.confirmBatch: signatories do not own at least threshold percentage of a quorum"
111: );
112: }
Using custom errors instead of 'require' statements in Solidity can lead to gas savings and improve developer experience. Custom errors provide explicit error messages, aiding in troubleshooting. Moreover, custom errors are cheaper as they don't require a string literal, thus saving gas. Hence, developers should replace 'require' statements with custom errors wherever possible for efficiency and readability.
Num of instances: 20
Click to show findings
['37']
37: require( // <= FOUND
38: EigenDAHasher.hashBatchMetadata(blobVerificationProof.batchMetadata)
39: == eigenDAServiceManager.batchIdToBatchMetadataHash(blobVerificationProof.batchId),
40: "EigenDARollupUtils.verifyBlob: batchMetadata does not match stored metadata"
41: ); // <= FOUND
['43']
43:
44: require( // <= FOUND
45: Merkle.verifyInclusionKeccak(
46: blobVerificationProof.inclusionProof,
47: blobVerificationProof.batchMetadata.batchHeader.blobHeadersRoot,
48: keccak256(abi.encodePacked(EigenDAHasher.hashBlobHeader(blobHeader))),
49: blobVerificationProof.blobIndex
50: ),
51: "EigenDARollupUtils.verifyBlob: inclusion proof is invalid"
52: ); // <= FOUND
['56']
56:
57: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumNumbers[uint8(blobVerificationProof.quorumThresholdIndexes[i])]) == blobHeader.quorumBlobParams[i].quorumNumber, // <= FOUND
58: "EigenDARollupUtils.verifyBlob: quorumNumber does not match"
59: ); // <= FOUND
['61']
61:
62:
63: require(blobHeader.quorumBlobParams[i].adversaryThresholdPercentage // <= FOUND
64: < blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
65: "EigenDARollupUtils.verifyBlob: adversaryThresholdPercentage is not valid"
66: ); // <= FOUND
['67']
67:
68:
69: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumThresholdPercentages[uint8(blobVerificationProof.quorumThresholdIndexes[i])]) // <= FOUND
70: >= blobHeader.quorumBlobParams[i].quorumThresholdPercentage,
71: "EigenDARollupUtils.verifyBlob: quorumThresholdPercentage is not met"
72: ); // <= FOUND
['32']
32: require(msg.sender == batchConfirmer, "onlyBatchConfirmer: not from batch confirmer"); // <= FOUND
['72']
72:
73: require(tx.origin == msg.sender, "EigenDAServiceManager.confirmBatch: header and nonsigner data must be in calldata"); // <= FOUND
['74']
74:
75: require( // <= FOUND
76: batchHeader.referenceBlockNumber <= block.number, "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is in future"
77: ); // <= FOUND
['78']
78:
79: require( // <= FOUND
80: (batchHeader.referenceBlockNumber + BLOCK_STALE_MEASURE) >= uint32(block.number),
81: "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is too far in past"
82: ); // <= FOUND
['84']
84:
85:
86: require( // <= FOUND
87: batchHeader.quorumNumbers.length == batchHeader.quorumThresholdPercentages.length,
88: "EigenDAServiceManager.confirmBatch: quorumNumbers and quorumThresholdPercentages must be of the same length"
89: ); // <= FOUND
['107']
107:
108:
109: require( // <= FOUND
110: quorumStakeTotals.signedStakeForQuorum[i] * THRESHOLD_DENOMINATOR >=
111: quorumStakeTotals.totalStakeForQuorum[i] * uint8(batchHeader.quorumThresholdPercentages[i]),
112: "EigenDAServiceManager.confirmBatch: signatories do not own at least threshold percentage of a quorum"
113: ); // <= FOUND
['44']
44: require(msg.value == stakeRequired, "MockRollup.registerValidator: Must send stake required to register"); // <= FOUND
['45']
45: require(!validators[msg.sender], "MockRollup.registerValidator: Validator already registered"); // <= FOUND
['46']
46: require(!blacklist[msg.sender], "MockRollup.registerValidator: Validator blacklisted"); // <= FOUND
['59']
59: require(validators[msg.sender], "MockRollup.postCommitment: Validator not registered"); // <= FOUND
['60']
60: require(commitments[block.timestamp].validator == address(0), "MockRollup.postCommitment: Commitment already posted"); // <= FOUND
['76']
76: require(commitment.validator != address(0), "MockRollup.challengeCommitment: Commitment not posted"); // <= FOUND
['79']
79:
80:
81: require(point < commitment.dataLength, "MockRollup.challengeCommitment: Point must be less than data length"); // <= FOUND
['82']
82:
83:
84: require(EigenDARollupUtils.openCommitment(point, illegalValue, tau, commitment.polynomialCommitment, proof), "MockRollup.challengeCommitment: Does not evaluate to illegal value"); // <= FOUND
['90']
90: require(success); // <= FOUND
When implementing certain functionalities in a smart contract, using libraries can sometimes incur additional costs compared to including the functionality directly within the contract. Libraries in Solidity are essentially contracts that get deployed separately and then linked to the main contract, so there's an overhead cost associated with calling them. If the functionality is relatively simple, this overhead can outweigh the benefits of code reuse and modularity provided by libraries. Thus, in such cases, implementing the functionality directly within the contract could be more gas-efficient. However, this approach should be balanced against considerations of code readability, maintainability, and the potential for code reuse.
Num of instances: 2
Click to show findings
['11']
11: library EigenDAHasher // <= FOUND
['14']
14: library EigenDARollupUtils // <= FOUND
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: 5
Click to show findings
['32']
32: require(msg.sender == batchConfirmer, "onlyBatchConfirmer: not from batch confirmer"); // <= FOUND
['72']
72:
73: require(tx.origin == msg.sender, "EigenDAServiceManager.confirmBatch: header and nonsigner data must be in calldata"); // <= FOUND
['45']
45: require(!validators[msg.sender], "MockRollup.registerValidator: Validator already registered"); // <= FOUND
['46']
46: require(!blacklist[msg.sender], "MockRollup.registerValidator: Validator blacklisted"); // <= FOUND
['59']
59: require(validators[msg.sender], "MockRollup.postCommitment: Validator not registered"); // <= FOUND
Optimizing the deployment size of a smart contract is vital to minimize gas costs, and one way to achieve this is by fine-tuning the IPFS hash appended by the Solidity compiler as metadata. This metadata, consisting of 53 bytes, increases the gas required for contract deployment by approximately 10,600 gas due to bytecode costs, and additionally, up to 848 gas due to calldata costs, depending on the proportion of zero and non-zero bytes. Utilize the --no-cbor-metadata compiler flag to prevent the compiler from appending metadata. However, this approach has a drawback as it can complicate the contract verification process on block explorers like Etherscan, potentially reducing transparency.
Num of instances: 2
Click to show findings
['24']
24: contract EigenDAServiceManager is EigenDAServiceManagerStorage, ServiceManagerBase, BLSSignatureChecker, Pausable // <= FOUND
['21']
21: contract MockRollup // <= FOUND
Public state variables in Solidity automatically generate getter functions, increasing contract size and potentially leading to higher deployment and interaction costs. To optimize gas usage and contract efficiency, minimize the use of public variables unless external access is necessary. Instead, use internal or private visibility combined with explicit getter functions when required. This practice not only reduces contract size but also provides better control over data access and manipulation, enhancing security and readability. Prioritize lean, efficient contracts to ensure cost-effectiveness and better performance on the blockchain.
Num of instances: 9
Click to show findings
['23']
23: IEigenDAServiceManager public eigenDAServiceManager; // <= FOUND
['24']
24: BN254.G1Point public tau; // <= FOUND
['25']
25: uint256 public illegalValue; // <= FOUND
['26']
26: uint256 public stakeRequired; // <= FOUND
['13']
13: uint256 public constant THRESHOLD_DENOMINATOR = 100; // <= FOUND
['17']
17: uint32 public constant STORE_DURATION_BLOCKS = 2 weeks / 12 seconds; // <= FOUND
['31']
31: uint32 public constant BLOCK_STALE_MEASURE = 150; // <= FOUND
['34']
34: uint32 public batchId; // <= FOUND
['40']
40: address public batchConfirmer; // <= FOUND
When emitting events in Solidity, using stack variables (local variables within a function) instead of state variables can lead to significant gas savings. Stack variables reside in memory only for the duration of the function execution and are less costly to access compared to state variables, which are stored on the blockchain. When an event is emitted, accessing these stack variables requires less gas than fetching data from state variables, which involves reading from the contract's storage - a more expensive operation. Thus, for efficiency, prefer using local variables within functions for event emission, especially in functions that are called frequently.
Num of instances: 1
Click to show findings
['134']
134: emit BatchConfirmerChanged(previousBatchConfirmer, batchConfirmer); // <= FOUND
Num of instances: 1
Click to show findings
['31']
31: modifier onlyBatchConfirmer() { // <= FOUND
32: require(msg.sender == batchConfirmer, "onlyBatchConfirmer: not from batch confirmer"); // <= FOUND
33: _;
34: }
Move the ++/-- action to the left of the variable
Num of instances: 2
Click to show findings
['56']
54:
55:
56: for (uint i = 0; i < blobHeader.quorumBlobParams.length; i++) { // <= FOUND
['106']
104:
105:
106: for (uint i = 0; i < batchHeader.quorumThresholdPercentages.length; i++) { // <= FOUND
Solidity 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: 1
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: 1
Click to show findings
['104']
104: for (uint i = 0; i < batchHeader.quorumThresholdPercentages.length; i++) { // <= FOUND
105:
106:
107: require(
108: quorumStakeTotals.signedStakeForQuorum[i] * THRESHOLD_DENOMINATOR >=
109: quorumStakeTotals.totalStakeForQuorum[i] * uint8(batchHeader.quorumThresholdPercentages[i]),
110: "EigenDAServiceManager.confirmBatch: signatories do not own at least threshold percentage of a quorum"
111: );
112: }
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: 2
Click to show findings
['54']
54: for (uint i = 0; i < blobHeader.quorumBlobParams.length; i++) // <= FOUND
['104']
104: for (uint i = 0; i < batchHeader.quorumThresholdPercentages.length; i++) // <= FOUND
If 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
['46']
46: function hashBatchMetadata( // <= FOUND
47: IEigenDAServiceManager.BatchMetadata memory batchMetadata
48: ) internal pure returns(bytes32)
['84']
84: function hashBlobHeader(IEigenDAServiceManager.BlobHeader memory blobHeader) internal pure returns(bytes32) // <= FOUND
['92']
92: function convertBatchHeaderToReducedBatchHeader(IEigenDAServiceManager.BatchHeader memory batchHeader) internal pure returns(IEigenDAServiceManager.ReducedBatchHeader memory) // <= FOUND
['103']
103: function hashBatchHeaderToReducedBatchHeader(IEigenDAServiceManager.BatchHeader memory batchHeader) internal pure returns(bytes32) // <= FOUND
['83']
83: function openCommitment( // <= FOUND
84: uint256 point,
85: uint256 evaluation,
86: BN254.G1Point memory tau,
87: BN254.G1Point memory commitment,
88: BN254.G2Point memory proof
89: ) internal view returns(bool)
Num of instances: 2
Click to show findings
['36']
36: constructor(
37: IAVSDirectory __avsDirectory,
38: IRegistryCoordinator __registryCoordinator,
39: IStakeRegistry __stakeRegistry
40: )
41: BLSSignatureChecker(__registryCoordinator)
42: ServiceManagerBase(__avsDirectory, __registryCoordinator, __stakeRegistry)
43: {
44: _disableInitializers();
45: }
['35']
35: constructor(IEigenDAServiceManager _eigenDAServiceManager, BN254.G1Point memory _tau, uint256 _illegalValue, uint256 _stakeRequired) {
36: eigenDAServiceManager = _eigenDAServiceManager;
37: tau = _tau;
38: illegalValue = _illegalValue;
39: stakeRequired = _stakeRequired;
40: }
Using Solidity's assembly scratch space for constructing calldata in external calls with one or two arguments can be a gas-efficient approach. This method leverages the designated memory area (the first 64 bytes of memory) for temporary data storage during assembly operations. By directly writing arguments into this scratch space, it eliminates the need for additional memory allocation typically required for calldata preparation. This technique can lead to notable gas savings, especially in high-frequency or gas-sensitive operations. However, it requires careful implementation to avoid data corruption and should be used with a thorough understanding of low-level EVM operations and memory handling. Proper testing and validation are crucial when employing such optimizations.
Num of instances: 15
Click to show findings
['37']
37: require( // <= FOUND
38: EigenDAHasher.hashBatchMetadata(blobVerificationProof.batchMetadata) // <= FOUND
39: == eigenDAServiceManager.batchIdToBatchMetadataHash(blobVerificationProof.batchId), // <= FOUND
40: "EigenDARollupUtils.verifyBlob: batchMetadata does not match stored metadata" // <= FOUND
41: );
['56']
56:
57: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumNumbers[uint8(blobVerificationProof.quorumThresholdIndexes[i])]) == blobHeader.quorumBlobParams[i].quorumNumber, // <= FOUND
58: "EigenDARollupUtils.verifyBlob: quorumNumber does not match" // <= FOUND
59: );
['67']
67:
68:
69: require(uint8(blobVerificationProof.batchMetadata.batchHeader.quorumThresholdPercentages[uint8(blobVerificationProof.quorumThresholdIndexes[i])]) // <= FOUND
70: >= blobHeader.quorumBlobParams[i].quorumThresholdPercentage, // <= FOUND
71: "EigenDARollupUtils.verifyBlob: quorumThresholdPercentage is not met" // <= FOUND
72: );
['78']
78:
79: require( // <= FOUND
80: (batchHeader.referenceBlockNumber + BLOCK_STALE_MEASURE) >= uint32(block.number), // <= FOUND
81: "EigenDAServiceManager.confirmBatch: specified referenceBlockNumber is too far in past" // <= FOUND
82: );
['107']
107:
108:
109: require( // <= FOUND
110: quorumStakeTotals.signedStakeForQuorum[i] * THRESHOLD_DENOMINATOR >= // <= FOUND
111: quorumStakeTotals.totalStakeForQuorum[i] * uint8(batchHeader.quorumThresholdPercentages[i]), // <= FOUND
112: "EigenDAServiceManager.confirmBatch: signatories do not own at least threshold percentage of a quorum" // <= FOUND
113: );
['46']
46:
52: function hashBatchMetadata( // <= FOUND
53: IEigenDAServiceManager.BatchMetadata memory batchMetadata // <= FOUND
54: ) internal pure returns(bytes32) { // <= FOUND
['60']
60:
65: function hashBatchHeaderMemory(IEigenDAServiceManager.BatchHeader memory batchHeader) internal pure returns(bytes32) { // <= FOUND
['68']
68:
73: function hashBatchHeader(IEigenDAServiceManager.BatchHeader calldata batchHeader) internal pure returns(bytes32) { // <= FOUND
['76']
76:
81: function hashReducedBatchHeader(IEigenDAServiceManager.ReducedBatchHeader memory reducedBatchHeader) internal pure returns(bytes32) { // <= FOUND
['84']
84:
89: function hashBlobHeader(IEigenDAServiceManager.BlobHeader memory blobHeader) internal pure returns(bytes32) { // <= FOUND
['92']
92:
97: function convertBatchHeaderToReducedBatchHeader(IEigenDAServiceManager.BatchHeader memory batchHeader) internal pure returns(IEigenDAServiceManager.ReducedBatchHeader memory) { // <= FOUND
['93']
93: return IEigenDAServiceManager.ReducedBatchHeader({ // <= FOUND
['103']
103:
108: function hashBatchHeaderToReducedBatchHeader(IEigenDAServiceManager.BatchHeader memory batchHeader) internal pure returns(bytes32) { // <= FOUND
['83']
83:
92: function openCommitment( // <= FOUND
93: uint256 point, // <= FOUND
94: uint256 evaluation, // <= FOUND
95: BN254.G1Point memory tau, // <= FOUND
96: BN254.G1Point memory commitment, // <= FOUND
97: BN254.G2Point memory proof // <= FOUND
98: ) internal view returns(bool) { // <= FOUND
['89']
89:
90:
91: (bool success, ) = msg.sender.call{value: 1 ether}(""); // <= FOUND
Utilizing Solidity's assembly scratch space to build calldata for emitting events with just one or two arguments can optimize gas usage. The scratch space, a designated area in the first 64 bytes of memory, is ideal for temporary storage during assembly-level operations. By directly writing the event arguments into this area, developers can bypass the standard memory allocation process required for event emission. This approach results in gas savings, particularly for contracts where events are frequently emitted. However, such low-level optimization requires a deep understanding of Ethereum Virtual Machine (EVM) mechanics and careful coding to prevent data mishandling. Rigorous testing is essential to ensure the integrity and correct functionality of the contract.
Num of instances: 2
Click to show findings
['119']
119:
120: emit BatchConfirmed(reducedBatchHeaderHash, batchIdMemory); // <= FOUND
['134']
134: emit BatchConfirmerChanged(previousBatchConfirmer, batchConfirmer); // <= FOUND
Using Solady's "FixedPointMathLib" for multiplication or division operations in Solidity can lead to significant gas savings. This library is designed to optimize fixed-point arithmetic operations, which are common in financial calculations involving tokens or currencies. By implementing more efficient algorithms and assembly optimizations, "FixedPointMathLib" minimizes the computational resources required for these operations. For contracts that frequently perform such calculations, integrating this library can reduce transaction costs, thereby enhancing overall performance and cost-effectiveness. However, developers must ensure compatibility with their existing codebase and thoroughly test for accuracy and expected behavior to avoid any unintended consequences.
Num of instances: 1
Click to show findings
['20']
17:
20: uint32 public constant STORE_DURATION_BLOCKS = 2 weeks / 12 seconds; // <= FOUND
Internal functions which are never used use unnecessary gas and should be safely removed.
Num of instances: 2
Click to show findings
['60']
60: function hashBatchHeaderMemory(IEigenDAServiceManager.BatchHeader memory batchHeader) internal pure returns(bytes32) // <= FOUND
['76']
76: function hashReducedBatchHeader(IEigenDAServiceManager.ReducedBatchHeader memory reducedBatchHeader) internal pure returns(bytes32) // <= FOUND
Using uint256 with values 1 and 0 instead of bool can save gas in Solidity contracts when changing state variables, as it avoids higher costs associated with switching from false to true. This technique reduces gas for "cold" storage writes to non-zero values. Implementing uint256 for binary states and employing getters to return boolean representations can optimize gas usage, particularly for contracts with frequent state toggling.
Num of instances: 2
Click to show findings
['31']
29:
31: mapping(address => bool) public validators; // <= FOUND
['32']
31:
32: mapping(address => bool) public blacklist; // <= FOUND
Enabling Intermediate Representation (IR)-based code generation in Solidity, via the --via-ir compiler flag or setting {"viaIR": true} in the configuration, activates an advanced optimization phase. This approach allows the compiler to perform more sophisticated optimizations across multiple functions, potentially leading to significant gas savings. IR-based compilation can optimize contract bytecode more efficiently, making smart contracts cheaper to deploy and execute by reducing the gas required for transactions
Num of instances: 2
Click to show findings
['24']
24: contract EigenDAServiceManager is EigenDAServiceManagerStorage, ServiceManagerBase, BLSSignatureChecker, Pausable { // <= FOUND
25: using EigenDAHasher for BatchHeader;
26: using EigenDAHasher for ReducedBatchHeader;
27:
28: uint8 internal constant PAUSED_CONFIRM_BATCH = 0;
29:
57: }
['21']
21: contract MockRollup { // <= FOUND
22:
23: IEigenDAServiceManager public eigenDAServiceManager;
24: BN254.G1Point public tau;
25: uint256 public illegalValue;
26: uint256 public stakeRequired;
27:
29: mapping(address => bool) public validators;
30:
31: mapping(address => bool) public blacklist;
32:
33: mapping(uint256 => Commitment) public commitments;
34:
55: }
Num of instances: 1
Click to show findings
['91']
89:
90:
91: (bool success, ) = msg.sender.call{value: 1 ether}(""); // <= FOUND
[Gas-42] Write direct outcome, instead of performing mathematical operations for constant state variables
In Solidity, it's highly efficient to directly assign constant values to state variables when these values are known at compile time and will not change. This practice avoids unnecessary computational operations and reduces gas costs for deploying and interacting with smart contracts.
Num of instances: 1
Click to show findings
['17']
17: uint32 public constant STORE_DURATION_BLOCKS = 2 weeks / 12 seconds; // <= FOUND