diff --git a/config/common/smart-contract-errors.toml b/config/common/smart-contract-errors.toml index 04bd92f2a..36015605a 100644 --- a/config/common/smart-contract-errors.toml +++ b/config/common/smart-contract-errors.toml @@ -42,6 +42,7 @@ "3b174434" = "MessageHashesListLengthHigherThanOneHundred" "ca389c44" = "InvalidProofOrProofVerificationRanOutOfGas" # L2 Message Service +"6446cc9c" = "MessageHashesListLengthIsZero" "d39e75f9" = "L1MessageNumberSynchronizationWrong" "7557a60a" = "L1RollingHashSynchronizationWrong" "36a4bb94" = "FinalRollingHashIsZero" diff --git a/contracts/contracts/LineaRollup.sol b/contracts/contracts/LineaRollup.sol index 24faeab8d..410fef2d1 100644 --- a/contracts/contracts/LineaRollup.sol +++ b/contracts/contracts/LineaRollup.sol @@ -172,6 +172,10 @@ contract LineaRollup is revert BlobSubmissionDataIsMissing(); } + if (blobhash(blobSubmissionLength) != EMPTY_HASH) { + revert BlobSubmissionDataEmpty(blobSubmissionLength); + } + bytes32 currentDataEvaluationPoint; bytes32 currentDataHash; uint256 lastFinalizedBlockNumber = currentL2BlockNumber; diff --git a/contracts/contracts/interfaces/l1/ILineaRollup.sol b/contracts/contracts/interfaces/l1/ILineaRollup.sol index a18efb84c..230d02c48 100644 --- a/contracts/contracts/interfaces/l1/ILineaRollup.sol +++ b/contracts/contracts/interfaces/l1/ILineaRollup.sol @@ -194,10 +194,15 @@ interface ILineaRollup { error EmptyBlobDataAtIndex(uint256 index); /** - * @dev Thrown when the data for multiple blobs' submission has length zero. + * @dev Thrown when the data for multiple blobs submission has length zero. */ error BlobSubmissionDataIsMissing(); + /** + * @dev Thrown when a blob has been submitted but there is no data for it. + */ + error BlobSubmissionDataEmpty(uint256 emptyBlobIndex); + /** * @dev Thrown when the starting block in the data item is out of sequence with the last block number. */ diff --git a/contracts/contracts/interfaces/l2/IL2MessageManager.sol b/contracts/contracts/interfaces/l2/IL2MessageManager.sol index ccb711ab4..616dc8ef2 100644 --- a/contracts/contracts/interfaces/l2/IL2MessageManager.sol +++ b/contracts/contracts/interfaces/l2/IL2MessageManager.sol @@ -16,6 +16,11 @@ interface IL2MessageManager { */ event RollingHashUpdated(uint256 indexed messageNumber, bytes32 indexed rollingHash); + /** + * @dev Reverts when the message hashes array length is zero. + */ + error MessageHashesListLengthIsZero(); + /** * @dev Reverts when message number synchronization is mismatched. */ diff --git a/contracts/contracts/messageService/MessageServiceBase.sol b/contracts/contracts/messageService/MessageServiceBase.sol index fb3d1bab0..b40346107 100644 --- a/contracts/contracts/messageService/MessageServiceBase.sol +++ b/contracts/contracts/messageService/MessageServiceBase.sol @@ -18,6 +18,13 @@ abstract contract MessageServiceBase is Initializable, IGenericErrors { /// @dev Keep 10 free storage slots for future implementation updates to avoid storage collision. uint256[10] private __base_gap; + /** + * @dev Event emitted when the remote sender is set. + * @param remoteSender The address of the new remote sender. + * @param setter The address of the account that set the remote sender. + */ + event RemoteSenderSet(address indexed remoteSender, address indexed setter); + /** * @dev Thrown when the caller address is not the message service address */ @@ -71,6 +78,7 @@ abstract contract MessageServiceBase is Initializable, IGenericErrors { /** * @notice Sets the remote sender + * @dev This function sets the remote sender address and emits the RemoteSenderSet event. * @param _remoteSender The authorized remote sender address, cannot be empty. */ function _setRemoteSender(address _remoteSender) internal { @@ -79,5 +87,6 @@ abstract contract MessageServiceBase is Initializable, IGenericErrors { } remoteSender = _remoteSender; + emit RemoteSenderSet(_remoteSender, msg.sender); } } diff --git a/contracts/contracts/messageService/l2/L2MessageManager.sol b/contracts/contracts/messageService/l2/L2MessageManager.sol index f268f627d..48fa16b2d 100644 --- a/contracts/contracts/messageService/l2/L2MessageManager.sol +++ b/contracts/contracts/messageService/l2/L2MessageManager.sol @@ -40,6 +40,10 @@ abstract contract L2MessageManager is AccessControlUpgradeable, IL2MessageManage ) external whenTypeNotPaused(GENERAL_PAUSE_TYPE) onlyRole(L1_L2_MESSAGE_SETTER_ROLE) { uint256 messageHashesLength = _messageHashes.length; + if (messageHashesLength == 0) { + revert MessageHashesListLengthIsZero(); + } + if (messageHashesLength > 100) { revert MessageHashesListLengthHigherThanOneHundred(messageHashesLength); } diff --git a/contracts/contracts/messageService/l2/v1/L2MessageServiceV1.sol b/contracts/contracts/messageService/l2/v1/L2MessageServiceV1.sol index 7479cac21..f31df6434 100644 --- a/contracts/contracts/messageService/l2/v1/L2MessageServiceV1.sol +++ b/contracts/contracts/messageService/l2/v1/L2MessageServiceV1.sol @@ -152,7 +152,7 @@ abstract contract L2MessageServiceV1 is uint256 previousMinimumFee = minimumFeeInWei; minimumFeeInWei = _feeInWei; - emit MinimumFeeChanged(previousMinimumFee, minimumFeeInWei, msg.sender); + emit MinimumFeeChanged(previousMinimumFee, _feeInWei, msg.sender); } /** diff --git a/contracts/contracts/messageService/lib/RateLimiter.sol b/contracts/contracts/messageService/lib/RateLimiter.sol index fd2ec83ae..538b23370 100644 --- a/contracts/contracts/messageService/lib/RateLimiter.sol +++ b/contracts/contracts/messageService/lib/RateLimiter.sol @@ -60,20 +60,17 @@ contract RateLimiter is Initializable, IRateLimiter, AccessControlUpgradeable { */ function _addUsedAmount(uint256 _usedAmount) internal { if (_usedAmount != 0) { - uint256 currentPeriodAmountTemp; - if (currentPeriodEnd < block.timestamp) { currentPeriodEnd = block.timestamp + periodInSeconds; - currentPeriodAmountTemp = _usedAmount; } else { - currentPeriodAmountTemp = currentPeriodAmountInWei + _usedAmount; + _usedAmount += currentPeriodAmountInWei; } - if (currentPeriodAmountTemp > limitInWei) { + if (_usedAmount > limitInWei) { revert RateLimitExceeded(); } - currentPeriodAmountInWei = currentPeriodAmountTemp; + currentPeriodAmountInWei = _usedAmount; } } diff --git a/contracts/contracts/messageService/lib/SparseMerkleTreeVerifier.sol b/contracts/contracts/messageService/lib/SparseMerkleTreeVerifier.sol index e8bc42b9b..b67cf4824 100644 --- a/contracts/contracts/messageService/lib/SparseMerkleTreeVerifier.sol +++ b/contracts/contracts/messageService/lib/SparseMerkleTreeVerifier.sol @@ -1,12 +1,16 @@ // SPDX-License-Identifier: AGPL-3.0 pragma solidity 0.8.26; +import { Utils } from "../../lib/Utils.sol"; + /** * @title Library to verify sparse merkle proofs and to get the leaf hash value * @author ConsenSys Software Inc. * @custom:security-contact security-report@linea.build */ library SparseMerkleTreeVerifier { + using Utils for *; + /** * @dev Custom error for when the leaf index is out of bounds. */ @@ -35,24 +39,11 @@ library SparseMerkleTreeVerifier { for (uint256 height; height < _proof.length; ++height) { if (((_leafIndex >> height) & 1) == 1) { - node = _efficientKeccak(_proof[height], node); + node = Utils._efficientKeccak(_proof[height], node); } else { - node = _efficientKeccak(node, _proof[height]); + node = Utils._efficientKeccak(node, _proof[height]); } } return node == _root; } - - /** - * @notice Performs a gas optimized keccak hash - * @param _left Left value. - * @param _right Right value. - */ - function _efficientKeccak(bytes32 _left, bytes32 _right) internal pure returns (bytes32 value) { - assembly { - mstore(0x00, _left) - mstore(0x20, _right) - value := keccak256(0x00, 0x40) - } - } } diff --git a/contracts/contracts/test-contracts/TestMessageServiceBase.sol b/contracts/contracts/test-contracts/TestMessageServiceBase.sol index 7dc94c8d1..63b548b9f 100644 --- a/contracts/contracts/test-contracts/TestMessageServiceBase.sol +++ b/contracts/contracts/test-contracts/TestMessageServiceBase.sol @@ -17,4 +17,8 @@ contract TestMessageServiceBase is MessageServiceBase { __MessageServiceBase_init(_messageService); _setRemoteSender(_remoteSender); } + + function testSetRemoteSender(address _remoteSender) external { + _setRemoteSender(_remoteSender); + } } diff --git a/contracts/contracts/test-contracts/TestSparseMerkleTreeVerifier.sol b/contracts/contracts/test-contracts/TestSparseMerkleTreeVerifier.sol index c46754701..22ab121aa 100644 --- a/contracts/contracts/test-contracts/TestSparseMerkleTreeVerifier.sol +++ b/contracts/contracts/test-contracts/TestSparseMerkleTreeVerifier.sol @@ -2,9 +2,11 @@ pragma solidity 0.8.26; import { SparseMerkleTreeVerifier } from "../messageService/lib/SparseMerkleTreeVerifier.sol"; +import { Utils } from "../lib/Utils.sol"; contract TestSparseMerkleTreeVerifier { using SparseMerkleTreeVerifier for *; + using Utils for *; function verifyMerkleProof( bytes32 _leafHash, @@ -16,7 +18,7 @@ contract TestSparseMerkleTreeVerifier { } function efficientKeccak(bytes32 _left, bytes32 _right) external pure returns (bytes32 value) { - return SparseMerkleTreeVerifier._efficientKeccak(_left, _right); + return Utils._efficientKeccak(_left, _right); } function getLeafHash( diff --git a/contracts/test/L2MessageManager.ts b/contracts/test/L2MessageManager.ts index 709352c92..066183c61 100644 --- a/contracts/test/L2MessageManager.ts +++ b/contracts/test/L2MessageManager.ts @@ -77,6 +77,16 @@ describe("L2MessageManager", () => { ); }); + it("Should revert if message hashes array length is zero", async () => { + const messageHashes: [] = []; + + const anchorCall = l2MessageManager + .connect(l1l2MessageSetter) + .anchorL1L2MessageHashes(messageHashes, 1, 100, HASH_ZERO); + + await expectRevertWithCustomError(l2MessageManager, anchorCall, "MessageHashesListLengthIsZero"); + }); + it("Should update rolling hash and messages emitting events", async () => { const messageHashes = generateNKeccak256Hashes("message", 100); const expectedRollingHash = calculateRollingHashFromCollection(ethers.ZeroHash, messageHashes.slice(0, 100)); diff --git a/contracts/test/LineaRollup.ts b/contracts/test/LineaRollup.ts index 86ff5b10e..e3bfde0c6 100644 --- a/contracts/test/LineaRollup.ts +++ b/contracts/test/LineaRollup.ts @@ -1106,6 +1106,50 @@ describe("Linea Rollup contract", () => { ); }); + it("Should revert if there is less data than blobs", async () => { + const operatorHDSigner = getWalletForIndex(2); + const lineaRollupAddress = await lineaRollup.getAddress(); + + const { + blobDataSubmission: blobSubmission, + compressedBlobs: compressedBlobs, + parentShnarf: parentShnarf, + finalShnarf: finalShnarf, + } = generateBlobDataSubmission(0, 2, true); + + const encodedCall = lineaRollup.interface.encodeFunctionData("submitBlobs", [ + [blobSubmission[0]], + parentShnarf, + finalShnarf, + ]); + + const { maxFeePerGas, maxPriorityFeePerGas } = await ethers.provider.getFeeData(); + const nonce = await operatorHDSigner.getNonce(); + + const transaction = Transaction.from({ + data: encodedCall, + maxPriorityFeePerGas: maxPriorityFeePerGas!, + maxFeePerGas: maxFeePerGas!, + to: lineaRollupAddress, + chainId: (await ethers.provider.getNetwork()).chainId, + type: 3, + nonce: nonce, + value: 0, + gasLimit: 5_000_000, + kzg, + maxFeePerBlobGas: 1n, + blobs: compressedBlobs, + }); + + const signedTx = await operatorHDSigner.signTransaction(transaction); + await expectRevertWithCustomError( + lineaRollup, + ethers.provider.broadcastTransaction(signedTx), + "BlobSubmissionDataEmpty", + [1], + ); + }); + it("Should fail to finalize with not enough gas for the rollup (pre-verifier)", async () => { // Submit 2 blobs await sendBlobTransaction(0, 2); diff --git a/contracts/test/MessageServiceBase.ts b/contracts/test/MessageServiceBase.ts index b256a17e0..42f0824f3 100644 --- a/contracts/test/MessageServiceBase.ts +++ b/contracts/test/MessageServiceBase.ts @@ -13,7 +13,7 @@ import { unpauseTypeRoles, } from "./utils/constants"; import { deployUpgradableFromFactory } from "./utils/deployment"; -import { expectRevertWithCustomError, expectRevertWithReason } from "./utils/helpers"; +import { expectEvent, expectRevertWithCustomError, expectRevertWithReason } from "./utils/helpers"; describe("MessageServiceBase", () => { let messageServiceBase: TestMessageServiceBase; @@ -85,6 +85,18 @@ describe("MessageServiceBase", () => { }); }); + describe("RemoteSenderSet event", () => { + it("Should emit RemoteSenderSet event when testSetRemoteSender is called", async () => { + const newRemoteSender = ethers.Wallet.createRandom().address; + await expectEvent( + messageServiceBase, + messageServiceBase.testSetRemoteSender(newRemoteSender), + "RemoteSenderSet", + [newRemoteSender, admin.address], + ); + }); + }); + describe("onlyMessagingService() modifier", () => { it("Should revert if msg.sender is not the message service address", async () => { await expectRevertWithCustomError( diff --git a/coordinator/app/src/test/kotlin/net/consensys/zkevm/coordinator/app/config/CoordinatorConfigTest.kt b/coordinator/app/src/test/kotlin/net/consensys/zkevm/coordinator/app/config/CoordinatorConfigTest.kt index da155ad33..1635f669b 100644 --- a/coordinator/app/src/test/kotlin/net/consensys/zkevm/coordinator/app/config/CoordinatorConfigTest.kt +++ b/coordinator/app/src/test/kotlin/net/consensys/zkevm/coordinator/app/config/CoordinatorConfigTest.kt @@ -180,6 +180,7 @@ class CoordinatorConfigTest { "3b174434" to "MessageHashesListLengthHigherThanOneHundred", "ca389c44" to "InvalidProofOrProofVerificationRanOutOfGas", // L2 Message Service + "6446cc9c" to "MessageHashesListLengthIsZero", "d39e75f9" to "L1MessageNumberSynchronizationWrong", "7557a60a" to "L1RollingHashSynchronizationWrong", "36a4bb94" to "FinalRollingHashIsZero"