update test

Author: LeoPatOZ

src/protocol/ETHBridge.sol

@@ -6,7 +6,7 @@ import {IETHBridge} from "./IETHBridge.sol";
 /// @dev Abstract ETH bridging contract to send native ETH between L1 <-> L2 using storage proofs.
 ///
 /// IMPORTANT: No recovery mechanism is implemented in case an account creates a deposit that can't be claimed.
-abstract contract ETHBridge is IETHBridge {
+contract ETHBridge is IETHBridge {
     mapping(bytes32 id => bool claimed) private _claimed;
 
     /// Incremental nonce to generate unique deposit IDs.
@@ -36,10 +36,12 @@ abstract contract ETHBridge is IETHBridge {
     }
 
     /// @inheritdoc IETHBridge
-    function claimDeposit(ETHDeposit memory deposit, uint256 height, bytes memory proof)
+    function claimDeposit(ETHDeposit memory ethDeposit, uint256 height, bytes memory proof)
         external
-        virtual
-        returns (bytes32 id);
+        returns (bytes32 id)
+    {
+        id = _generateId(ethDeposit);
+    }
 
     /// @dev Processes deposit claim by id.
     /// @param id Identifier of the deposit

src/protocol/ICheckpointTracker.sol

@@ -10,7 +10,7 @@ interface ICheckpointTracker {
     /// @notice Emitted when the proven checkpoint is updated
     /// @param publicationId the publication ID of the latest proven checkpoint
     /// @param commitment the commitment of the latest proven checkpoint
-    event CheckpointUpdated(uint256 publicationId, bytes32 commitment);
+    event CheckpointUpdated(uint256 indexed publicationId, bytes32 commitment);
 
     /// @dev If genesis is not 0 it should be the latest publication
     /// @param latestPublicationId the current latest publication ID

src/protocol/SignalService.sol

@@ -44,27 +44,6 @@ contract SignalService is ISignalService, CommitmentStore {
         emit SignalVerified(sender, value);
     }
 
-    /// @dev Overrides ETHBridge.depositETH to add signaling functionality.
-    function deposit(address to, bytes memory data) public payable override returns (bytes32 id) {
-        id = super.deposit(to, data);
-        id.signal(msg.sender, ETH_BRIDGE_NAMESPACE);
-    }
-
-    // CHECK: Should this function be non-reentrant?
-    /// @inheritdoc ETHBridge
-    /// @dev Overrides ETHBridge.claimDeposit to add signal verification logic.
-    function claimDeposit(ETHDeposit memory ethDeposit, uint256 height, bytes memory proof)
-        external
-        override
-        returns (bytes32 id)
-    {
-        id = _generateId(ethDeposit);
-
-        _verifySignal(height, commitmentPublisher, ethDeposit.from, id, ETH_BRIDGE_NAMESPACE, proof);
-
-        super._processClaimDepositWithId(id, ethDeposit);
-    }
-
     function _verifySignal(
         uint256 height,
         address commitmentPublisher,

test/CheckpointTracker.t.sol

@@ -64,7 +64,7 @@ contract CheckpointTrackerTest is Test {
             ICheckpointTracker.Checkpoint({publicationId: 0, commitment: genesisCommitment});
 
         vm.expectEmit();
-        emit ICheckpointTracker.CheckpointUpdated(genesisCheckpoint);
+        emit ICheckpointTracker.CheckpointUpdated(genesisCheckpoint.publicationId, genesisCheckpoint.commitment);
         new CheckpointTracker(
             genesisCommitment, address(feed), address(verifier), proverManager, address(signalService)
         );
@@ -78,7 +78,7 @@ contract CheckpointTrackerTest is Test {
         uint256 numRelevantPublications = 2;
 
         vm.expectEmit();
-        emit ICheckpointTracker.CheckpointUpdated(end);
+        emit ICheckpointTracker.CheckpointUpdated(end.publicationId, end.commitment);
         tracker.proveTransition(start, end, numRelevantPublications, proof);
 
         ICheckpointTracker.Checkpoint memory provenCheckpoint = tracker.getProvenCheckpoint();

test/ProverManager.t.sol

@@ -43,11 +43,9 @@ contract ProverManagerTest is Test {
     uint256 constant INITIAL_PERIOD = 1;
 
     function setUp() public {
-        signalService = new SignalService(rollupOperator);
         checkpointTracker = new MockCheckpointTracker(address(signalService));
         publicationFeed = new PublicationFeed();
         vm.prank(rollupOperator);
-        signalService.setAuthorizedCommitter(address(checkpointTracker));
 
         // Fund the initial prover so the constructor can receive the required livenessBond.
         vm.deal(initialProver, 10 ether);

test/signal/ETHBridge.t.sol

@@ -19,18 +19,26 @@ contract BridgeETHState is BaseState {
     uint256 public depositAmount = 4 ether;
     ETHBridge.ETHDeposit public depositOne;
 
+    ETHBridge public L1ethBridge;
+    ETHBridge public L2ethBridge;
+
     // this is a valid deposit ID but is sent via a signal not a deposit
     // hence "invalid" and should not be claimable
     bytes32 invalidDepositId = 0xbf8ce3088406c4ddbc32e32404ca006c3ef57f07d5139479f16c9124d6490f2e;
 
     function setUp() public virtual override {
         super.setUp();
+
+        vm.selectFork(L2Fork);
+        L2ethBridge = new ETHBridge();
+
         vm.selectFork(L1Fork);
+        L1ethBridge = new ETHBridge();
 
         vm.prank(defaultSender);
         bytes memory emptyData = "";
         vm.recordLogs();
-        depositIdOne = L1signalService.deposit{value: depositAmount}(defaultSender, emptyData);
+        depositIdOne = L1ethBridge.deposit{value: depositAmount}(defaultSender, emptyData);
 
         Vm.Log[] memory entries = vm.getRecordedLogs();
 
@@ -47,7 +55,7 @@ contract ETHBridgeTest is BridgeETHState {
         assertEq(defaultSender.balance, senderBalanceL1 - depositAmount);
 
         vm.selectFork(L2Fork);
-        assertEq(L2signalService.claimed(depositIdOne), false);
+        assertEq(L2ethBridge.claimed(depositIdOne), false);
         assertEq(defaultSender.balance, senderBalanceL2);
     }
 }
@@ -99,9 +107,9 @@ contract ClaimDepositTest is CommitmentStoredState {
         ISignalService.SignalProof memory signalProof = ISignalService.SignalProof(accountProof, storageProof);
         bytes memory encodedProof = abi.encode(signalProof);
 
-        L2signalService.claimDeposit(depositOne, commitmentHeight, encodedProof);
+        L2ethBridge.claimDeposit(depositOne, commitmentHeight, encodedProof);
 
-        assertEq(address(L2signalService).balance, ETHBridgeInitBalance - depositAmount);
+        assertEq(address(L2ethBridge).balance, ETHBridgeInitBalance - depositAmount);
         assertEq(defaultSender.balance, senderBalanceL2 + depositAmount);
     }
 
@@ -129,9 +137,9 @@ contract ClaimDepositTest is CommitmentStoredState {
 
         vm.selectFork(L2Fork);
         // to be extra sure its not a problem with the proof
-        L2signalService.verifySignal(commitmentHeight, defaultSender, invalidDepositId, encodedProof);
+        L2signalService.verifySignal(commitmentHeight, address(anchor), defaultSender, invalidDepositId, encodedProof);
         // I believe this error means that the proof is not valid for this deposit id
         vm.expectRevert("MerkleTrie: invalid large internal hash");
-        L2signalService.claimDeposit(invalidDeposit, commitmentHeight, encodedProof);
+        L2ethBridge.claimDeposit(invalidDeposit, commitmentHeight, encodedProof);
     }
 }

test/signal/SignalService.t.sol

@@ -48,7 +48,6 @@ contract BaseState is Test {
         vm.deal(defaultSender, senderBalanceL1);
 
         vm.prank(defaultSender);
-        L1signalService = new SignalService(rollupOperator);
         vm.deal(address(L1signalService), ETHBridgeInitBalance);
 
         checkpointTracker = new MockCheckpointTracker(address(L1signalService));
@@ -60,13 +59,11 @@ contract BaseState is Test {
         vm.deal(defaultSender, senderBalanceL2);
 
         vm.prank(defaultSender);
-        L2signalService = new SignalService(rollupOperator);
         vm.deal(address(L2signalService), ETHBridgeInitBalance);
 
         anchor = new MockAnchor(address(L2signalService));
 
         vm.prank(rollupOperator);
-        L2signalService.setAuthorizedCommitter(address(anchor));
 
         // Labels for debugging
         vm.label(address(L1signalService), "L1signalService");
@@ -132,7 +129,7 @@ contract SendL1SignalTest is SendL1SignalState {
         uint256 height = 1;
         anchor.anchor(height, stateRoot);
 
-        L2signalService.verifySignal(height, defaultSender, signal, encodedProof);
+        L2signalService.verifySignal(height, address(anchor), defaultSender, signal, encodedProof);
     }
 
     function test_verifyL1Signal_UsingStorageProof() public {
@@ -145,6 +142,6 @@ contract SendL1SignalTest is SendL1SignalState {
         uint256 height = 1;
         anchor.anchor(height, storageRoot);
 
-        L2signalService.verifySignal(height, defaultSender, signal, encodedProof);
+        L2signalService.verifySignal(height, address(anchor), defaultSender, signal, encodedProof);
     }
 }