Cancel eth tests

gas-reports/minimal_inbox_publish.json

@@ -1,2 +1,2 @@
-{ "num_publications": 20, "average_gas_used_publish": 44563 }
-
+{num_publications:20,
+average_gas_used_publish: 44563}

offchain/sample_deposit_proof.rs

@@ -7,7 +7,7 @@ mod utils;
 use utils::{deploy_eth_bridge, deploy_signal_service, get_proofs, get_provider, SignalProof};
 
 use alloy::hex::decode;
-use alloy::primitives::{Address, Bytes, FixedBytes, U256};
+use alloy::primitives::{address, Address, Bytes, FixedBytes, U256};
 use std::fs;
 
 fn expand_vector(vec: Vec<Bytes>, name: &str) -> String {
@@ -28,6 +28,7 @@ fn create_deposit_call(
     amount: U256,
     data: &str,
     context: &str,
+    canceler: &str,
     id: &FixedBytes<32>,
 ) -> String {
     let mut result = String::new();
@@ -50,6 +51,7 @@ fn create_deposit_call(
     result += format!("\t\tdeposit.amount = {};\n", amount).as_str();
     result += format!("\t\tdeposit.data = bytes(hex\"{}\");\n", data).as_str();
     result += format!("\t\tdeposit.context = bytes(hex\"{}\");\n", context).as_str();
+    result += format!("\t\tdeposit.canceler = address({});\n", canceler).as_str();
     result += format!("\t\t_createDeposit(\n\t\t\taccountProof,\n\t\t\tstorageProof,\n\t\t\tdeposit,\n\t\t\tbytes32({}),\n\t\t\tbytes32({})\n\t\t);\n", proof.slot, id).as_str();
     return result;
 }
@@ -76,11 +78,13 @@ fn deposit_specification() -> Vec<DepositSpecification> {
         "5932a71200000000000000000000000000000000000000000000000000000000000004d2", // (valid) call to `someNonPayableFunction(1234)`
     ];
 
+    // makeAddr("canceler");
+    let canceler = address!("0x738b9be4596e37015bA15F17116c9B2eE971c238");
     let zero_canceler = Address::ZERO;
 
     let mut specifications = vec![];
-    for amount in amounts {
-        for data in calldata.iter() {
+    for &amount in &amounts {
+        for &data in &calldata {
             specifications.push(DepositSpecification {
                 recipient: recipient.parse().unwrap(),
                 amount: U256::from(amount),
@@ -90,6 +94,26 @@ fn deposit_specification() -> Vec<DepositSpecification> {
             });
         }
     }
+
+    // Special canceler cases
+
+    // Case 1: Empty calldata with non-zero canceler
+    specifications.push(DepositSpecification {
+        recipient: recipient.parse().unwrap(),
+        amount: U256::from(amounts[1]),
+        data: calldata[0].to_string(),
+        context: String::from(""),
+        canceler,
+    });
+
+    // Case 2: Valid calldata with non-zero canceler
+    specifications.push(DepositSpecification {
+        recipient: recipient.parse().unwrap(),
+        amount: U256::from(amounts[1]),
+        data: calldata[1].to_string(),
+        context: String::from(""),
+        canceler,
+    });
     return specifications;
 }
 
@@ -145,6 +169,7 @@ async fn main() -> Result<()> {
             d.amount,
             d.data.as_str(),
             d.context.as_str(),
+            &d.canceler.to_string(),
             id,
         )
         .as_str();

src/protocol/ETHBridge.sol

@@ -94,7 +94,7 @@ contract ETHBridge is IETHBridge, ReentrancyGuardTransient {
         bytes memory proof
     ) internal returns (bytes32 id) {
         id = _generateId(ethDeposit);
-        require(!processed(id), AlreadyClaimed());
+        require(!processed(id), AlreadyProcessed());
 
         signalService.verifySignal(height, trustedCommitmentPublisher, counterpart, id, proof);
 

src/protocol/IETHBridge.sol

@@ -52,15 +52,15 @@ interface IETHBridge {
     /// @dev Failed to call the receiver with value.
     error FailedClaim();
 
-    /// @dev A deposit was already claimed.
-    error AlreadyClaimed();
+    /// @dev To address is set to zero
+    error ZeroReceiver();
+
+    /// @dev A deposit was already claimed or cancelled
+    error AlreadyProcessed();
 
     /// @dev Only canceler can cancel a deposit.
     error OnlyCanceler();
 
-    /// @dev Zero receiver
-    error ZeroReceiver();
-
     /// @dev Whether the deposit identified by `id` has been claimed or cancelled.
     /// @param id The deposit id
     function processed(bytes32 id) external view returns (bool);

test/ETHBridge/CancelDepositTest.t.sol

@@ -165,7 +165,7 @@ contract CancelDepositTest is InitialState {
 
         // Try to cancel again - should revert
         vm.prank(canceler);
-        vm.expectRevert(IETHBridge.AlreadyClaimed.selector);
+        vm.expectRevert(IETHBridge.AlreadyProcessed.selector);
         bridge.cancelDeposit(ethDeposit, charlie, "cancel_data", HEIGHT, "proof");
     }
 

test/ETHBridge/CancelableScenarios.t.sol

@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.28;
+
+import {CrossChainDepositExists} from "./CrossChainDepositExists.t.sol";
+import {IETHBridge} from "src/protocol/IETHBridge.sol";
+
+/// This contract describes behaviours that should be valid when the deposit is cancelable.
+abstract contract DepositIsCancelable is CrossChainDepositExists {
+    function test_cancelDeposit_shouldSucceed() public {
+        IETHBridge.ETHDeposit memory deposit = sampleDepositProof.getEthDeposit(_depositIdx());
+        bytes memory proof = abi.encode(sampleDepositProof.getDepositSignalProof(_depositIdx()));
+        vm.prank(cancelerAddress);
+        bridge.cancelDeposit(deposit, cancellationRecipient, "", HEIGHT, proof);
+    }
+
+    function test_cancelDeposit_shouldSetClaimedFlag() public {
+        IETHBridge.ETHDeposit memory deposit = sampleDepositProof.getEthDeposit(_depositIdx());
+        bytes memory proof = abi.encode(sampleDepositProof.getDepositSignalProof(_depositIdx()));
+        (, bytes32 id) = sampleDepositProof.getDepositInternals(_depositIdx());
+
+        assertFalse(bridge.processed(id), "deposit already marked as claimed");
+
+        vm.prank(cancelerAddress);
+        bridge.cancelDeposit(deposit, cancellationRecipient, "", HEIGHT, proof);
+        assertTrue(bridge.processed(id), "deposit not marked as claimed");
+    }
+
+    function test_cancelDeposit_shouldEmitEvent() public {
+        IETHBridge.ETHDeposit memory deposit = sampleDepositProof.getEthDeposit(_depositIdx());
+        bytes memory proof = abi.encode(sampleDepositProof.getDepositSignalProof(_depositIdx()));
+        (, bytes32 id) = sampleDepositProof.getDepositInternals(_depositIdx());
+
+        vm.expectEmit();
+        emit IETHBridge.DepositCancelled(id, cancellationRecipient, "");
+
+        vm.prank(cancelerAddress);
+        bridge.cancelDeposit(deposit, cancellationRecipient, "", HEIGHT, proof);
+    }
+
+    function test_cancelDeposit_shouldSendETH() public {
+        IETHBridge.ETHDeposit memory deposit = sampleDepositProof.getEthDeposit(_depositIdx());
+        bytes memory proof = abi.encode(sampleDepositProof.getDepositSignalProof(_depositIdx()));
+
+        uint256 initialCancellationRecipientBalance = cancellationRecipient.balance;
+        uint256 initialRecipientBalance = recipient.balance;
+        uint256 initialBridgeBalance = address(bridge).balance;
+
+        vm.prank(cancelerAddress);
+        bridge.cancelDeposit(deposit, cancellationRecipient, "", HEIGHT, proof);
+        assertEq(recipient.balance, initialRecipientBalance, "recipient balance mismatch");
+        assertEq(
+            cancellationRecipient.balance,
+            initialCancellationRecipientBalance + deposit.amount,
+            "cancel recipient balance mismatch"
+        );
+        assertEq(address(bridge).balance, initialBridgeBalance - deposit.amount, "bridge balance mismatch");
+    }
+
+    function test_claimDeposit_shouldRevertWhen_DepositIsCancelled() public {
+        IETHBridge.ETHDeposit memory deposit = sampleDepositProof.getEthDeposit(_depositIdx());
+        bytes memory proof = abi.encode(sampleDepositProof.getDepositSignalProof(_depositIdx()));
+
+        vm.prank(cancelerAddress);
+        bridge.cancelDeposit(deposit, cancellationRecipient, "", HEIGHT, proof);
+        vm.expectRevert(IETHBridge.AlreadyProcessed.selector);
+        bridge.claimDeposit(deposit, HEIGHT, proof);
+    }
+
+    function test_cancelDeposit_shouldRevertWhen_CancellerIsNotCaller() public {
+        IETHBridge.ETHDeposit memory deposit = sampleDepositProof.getEthDeposit(_depositIdx());
+        bytes memory proof = abi.encode(sampleDepositProof.getDepositSignalProof(_depositIdx()));
+
+        vm.expectRevert(IETHBridge.OnlyCanceler.selector);
+        vm.prank(makeAddr("notCanceller"));
+        bridge.cancelDeposit(deposit, cancellationRecipient, "", HEIGHT, proof);
+    }
+}
+
+abstract contract DepositIsNotCancelable is CrossChainDepositExists {
+    function test_cancelDeposit_shouldRevertWhen_NoCancelerIsSet() public {
+        IETHBridge.ETHDeposit memory deposit = sampleDepositProof.getEthDeposit(_depositIdx());
+        bytes memory proof = abi.encode(sampleDepositProof.getDepositSignalProof(_depositIdx()));
+
+        vm.expectRevert(IETHBridge.OnlyCanceler.selector);
+        vm.prank(cancelerAddress);
+        bridge.cancelDeposit(deposit, cancellationRecipient, "", HEIGHT, proof);
+    }
+}

test/ETHBridge/ContractCallValidityScenarios.t.sol

@@ -1,10 +1,11 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.28;
 
+import {DepositIsCancelable} from "./CancelableScenarios.t.sol";
 import {BridgeSufficientlyCapitalized} from "./CapitalizationScenarios.t.sol";
 import {DepositIsClaimable, DepositIsNotClaimable} from "./ClaimableScenarios.t.sol";
 import {CrossChainDepositExists} from "./CrossChainDepositExists.t.sol";
-import {RecipientIsAContract} from "./RecipientScenarios.t.sol";
+import {REQUIRED_INPUT, RecipientIsAContract, TransferRecipient} from "./RecipientScenarios.t.sol";
 import {TransferRecipient} from "./RecipientScenarios.t.sol";
 import {IETHBridge} from "src/protocol/IETHBridge.sol";
 
@@ -48,3 +49,39 @@ abstract contract DepositIsInvalidContractCall is
         super.setUp();
     }
 }
+
+// This contract describes the secnario where a cancelable deposit is made but the canceler
+// specifies a contract as the recipient.
+abstract contract CancelableDepositIsValidContractCall is
+    RecipientIsAContract,
+    BridgeSufficientlyCapitalized,
+    DepositIsCancelable
+{
+    function setUp()
+        public
+        virtual
+        override(CrossChainDepositExists, RecipientIsAContract, BridgeSufficientlyCapitalized)
+    {
+        super.setUp();
+    }
+
+    function test_cancelDeposit_canInvokeCancelerContract() public {
+        IETHBridge.ETHDeposit memory deposit = sampleDepositProof.getEthDeposit(_depositIdx());
+        bytes memory proof = abi.encode(sampleDepositProof.getDepositSignalProof(_depositIdx()));
+
+        bytes memory cancelCalldata = abi.encodeWithSignature("somePayableFunction(uint256)", (REQUIRED_INPUT));
+        vm.prank(cancelerAddress);
+        vm.expectEmit();
+        emit TransferRecipient.FunctionCalled();
+        bridge.cancelDeposit(deposit, recipient, cancelCalldata, HEIGHT, proof);
+    }
+
+    function test_cancelDeposit_shouldNotInvokeRecipientContract() public {
+        IETHBridge.ETHDeposit memory deposit = sampleDepositProof.getEthDeposit(_depositIdx());
+        bytes memory proof = abi.encode(sampleDepositProof.getDepositSignalProof(_depositIdx()));
+
+        vm.prank(cancelerAddress);
+        vm.expectRevert(IETHBridge.FailedClaim.selector);
+        bridge.cancelDeposit(deposit, recipient, "", HEIGHT, proof);
+    }
+}

test/ETHBridge/CrossChainDepositExists.t.sol

@@ -15,6 +15,9 @@ abstract contract CrossChainDepositExists is UniversalTest {
     // the sample deposit is to this address
     address recipient = makeAddr("recipient");
 
+    // the recipient that receives the cancelled deposit
+    address cancellationRecipient = makeAddr("cancellationRecipient");
+
     uint256 HEIGHT = 1;
 
     function setUp() public virtual override {

test/ETHBridge/EnumeratedScenarios.t.sol

@@ -1,16 +1,23 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.28;
 
+import {DepositIsCancelable, DepositIsNotCancelable} from "./CancelableScenarios.t.sol";
 import {BridgeHasNoEther, BridgeSufficientlyCapitalized} from "./CapitalizationScenarios.t.sol";
 import {DepositIsClaimable, DepositIsNotClaimable} from "./ClaimableScenarios.t.sol";
-import {DepositIsInvalidContractCall, DepositIsValidContractCall} from "./ContractCallValidityScenarios.t.sol";
+import {
+    CancelableDepositIsValidContractCall,
+    DepositIsInvalidContractCall,
+    DepositIsValidContractCall
+} from "./ContractCallValidityScenarios.t.sol";
 import {CrossChainDepositExists} from "./CrossChainDepositExists.t.sol";
 import {RecipientIsAContract, RecipientIsAnEOA} from "./RecipientScenarios.t.sol";
 import {
     NonzeroETH_InvalidCallToPayableFn,
     NonzeroETH_NoCalldata,
+    NonzeroETH_NoCalldata_IsCancellable,
     NonzeroETH_ValidCallToNonpayableFn,
     NonzeroETH_ValidCallToPayableFn,
+    NonzeroETH_ValidCallToPayableFn_IsCancelable,
     ZeroETH_InvalidCallToPayableFn,
     ZeroETH_NoCalldata,
     ZeroETH_ValidCallToNonpayableFn,
@@ -49,6 +56,42 @@ contract SimpleDepositToEOA_BridgeUndercollateralized is
     DepositIsNotClaimable
 {}
 
+// A cancelable deposit is made to an EOA with no calldata
+contract CancelDeposit_WithNoCalldata_ToEOACancelRecipient is
+    NonzeroETH_NoCalldata_IsCancellable,
+    RecipientIsAnEOA,
+    BridgeSufficientlyCapitalized,
+    DepositIsCancelable
+{
+    function setUp() public override(CrossChainDepositExists, BridgeSufficientlyCapitalized) {
+        super.setUp();
+    }
+}
+
+// A cancelable deposit is made to an contract with calldata, cancel recipient is an EOA
+contract CancelDeposit_WithCalldata_ToEOACancelRecipient is
+    NonzeroETH_ValidCallToPayableFn_IsCancelable,
+    RecipientIsAnEOA,
+    BridgeSufficientlyCapitalized,
+    DepositIsCancelable
+{
+    function setUp() public override(CrossChainDepositExists, BridgeSufficientlyCapitalized) {
+        super.setUp();
+    }
+}
+
+// Same transfer as above, but no canceler is set. Cancellation attempts should fail.
+contract SimpleDepositToEOA_NoCancelerIsSet is
+    NonzeroETH_NoCalldata,
+    RecipientIsAnEOA,
+    BridgeSufficientlyCapitalized,
+    DepositIsNotCancelable
+{
+    function setUp() public override(CrossChainDepositExists, BridgeSufficientlyCapitalized) {
+        super.setUp();
+    }
+}
+
 // The bridge does a direct call (without the standard function invocation syntax). This means calldata passed to an EOA
 // is ignored
 // (and the call still succeeds)
@@ -70,6 +113,16 @@ contract DepositToPayableFunction is NonzeroETH_ValidCallToPayableFn, DepositIsV
     }
 }
 
+// Should not be able to claim a deposit if the recipient of a cancelable deposit is a contract
+contract CancelDeposit_WithCalldata_ToContractCancelRecipient is
+    NonzeroETH_ValidCallToPayableFn_IsCancelable,
+    CancelableDepositIsValidContractCall
+{
+    function setUp() public override(CrossChainDepositExists, CancelableDepositIsValidContractCall) {
+        super.setUp();
+    }
+}
+
 // We should not be able to send ETH to a nonpayable function on a contract.
 contract DepositToNonPayableFunction is NonzeroETH_ValidCallToNonpayableFn, DepositIsInvalidContractCall {
     function setUp() public override(CrossChainDepositExists, DepositIsInvalidContractCall) {

test/ETHBridge/InitialState.t.sol

@@ -15,8 +15,10 @@ abstract contract InitialState is Test {
     // zero address means any relayer is allowed
     bytes anyRelayer = new bytes(0);
 
-    // zero address means deposit is uncancellable
-    address nonCancellableAddress = address(0);
+    // address that can cancel deposits (if specified in the ETHDeposit)
+    address cancelerAddress = makeAddr("canceler");
+
+    address zeroCanceler = address(0);
 
     address trustedCommitmentPublisher = makeAddr("trustedCommitmentPublisher");
 

test/ETHBridge/RecipientScenarios.t.sol

@@ -19,11 +19,11 @@ abstract contract RecipientIsAnEOA is CrossChainDepositExists {
 // no need to do anything because the recipient defaults to no code
 }
 
-contract TransferRecipient {
-    // Magic values to make the behaviour identifiable
-    uint256 public constant REQUIRED_INPUT = 1234;
-    uint256 public constant RETURN_VALUE = 5678;
+// Magic values to make the behaviour identifiable
+uint256 constant REQUIRED_INPUT = 1234;
+uint256 constant RETURN_VALUE = 5678;
 
+contract TransferRecipient {
     event FunctionCalled();
 
     // valid calldata: 0x9b28f6fb00000000000000000000000000000000000000000000000000000000000004d2