Add Cancel Eth Deposit on Bridge (#120)

Co-authored-by: Nikesh Nazareth <business@nikeshnazareth.com>
Co-authored-by: Gustavo Gonzalez <gustavo.gonzalez@openzeppelin.com>

Author: 3 people

offchain/sample_deposit_proof.rs

@@ -59,6 +59,7 @@ pub struct DepositSpecification {
     pub amount: U256,
     pub data: String,
     pub context: String,
+    pub canceler: Address,
 }
 
 fn deposit_specification() -> Vec<DepositSpecification> {
@@ -75,6 +76,8 @@ fn deposit_specification() -> Vec<DepositSpecification> {
         "5932a71200000000000000000000000000000000000000000000000000000000000004d2", // (valid) call to `someNonPayableFunction(1234)`
     ];
 
+    let zero_canceler = Address::ZERO;
+
     let mut specifications = vec![];
     for amount in amounts {
         for data in calldata.iter() {
@@ -83,6 +86,7 @@ fn deposit_specification() -> Vec<DepositSpecification> {
                 amount: U256::from(amount),
                 data: data.to_string(),
                 context: String::from(""),
+                canceler: zero_canceler,
             });
         }
     }
@@ -105,6 +109,7 @@ async fn main() -> Result<()> {
                 spec.recipient,
                 decode(spec.data.clone())?.into(),
                 decode(spec.context.clone())?.into(),
+                spec.canceler,
             )
             .value(spec.amount)
             .send()

src/protocol/ETHBridge.sol

@@ -5,14 +5,11 @@ import {IETHBridge} from "./IETHBridge.sol";
 import {ISignalService} from "./ISignalService.sol";
 import {ReentrancyGuardTransient} from "@openzeppelin/contracts/utils/ReentrancyGuardTransient.sol";
 
-/// @dev ETH bridging contract to send native ETH between L1 <-> L2 using storage proofs.
-/// @dev In contracts to the `SignalService`, this contract does not expect the bridge to be deployed on the same
+/// @dev In contrast to the `SignalService`, this contract does not expect the bridge to be deployed on the same
 /// address on both chains. This is because it is designed so that each rollup has its own independent bridge contract,
 /// and they may furthermore decide to deploy a new version of the bridge in the future.
-///
-/// IMPORTANT: No recovery mechanism is implemented in case an account creates a deposit that can't be claimed.
 contract ETHBridge is IETHBridge, ReentrancyGuardTransient {
-    mapping(bytes32 id => bool claimed) private _claimed;
+    mapping(bytes32 id => bool processed) private _processed;
 
     /// Incremental nonce to generate unique deposit IDs.
     uint256 private _globalDepositNonce;
@@ -39,8 +36,8 @@ contract ETHBridge is IETHBridge, ReentrancyGuardTransient {
     }
 
     /// @inheritdoc IETHBridge
-    function claimed(bytes32 id) public view returns (bool) {
-        return _claimed[id];
+    function processed(bytes32 id) public view returns (bool) {
+        return _processed[id];
     }
 
     /// @inheritdoc IETHBridge
@@ -49,8 +46,13 @@ contract ETHBridge is IETHBridge, ReentrancyGuardTransient {
     }
 
     /// @inheritdoc IETHBridge
-    function deposit(address to, bytes memory data, bytes memory context) public payable returns (bytes32 id) {
-        ETHDeposit memory ethDeposit = ETHDeposit(_globalDepositNonce, msg.sender, to, msg.value, data, context);
+    function deposit(address to, bytes memory data, bytes memory context, address canceler)
+        public
+        payable
+        returns (bytes32 id)
+    {
+        ETHDeposit memory ethDeposit =
+            ETHDeposit(_globalDepositNonce, msg.sender, to, msg.value, data, context, canceler);
         id = _generateId(ethDeposit);
         unchecked {
             ++_globalDepositNonce;
@@ -62,15 +64,36 @@ contract ETHBridge is IETHBridge, ReentrancyGuardTransient {
 
     /// @inheritdoc IETHBridge
     function claimDeposit(ETHDeposit memory ethDeposit, uint256 height, bytes memory proof) external nonReentrant {
-        bytes32 id = _generateId(ethDeposit);
-        require(!claimed(id), AlreadyClaimed());
+        bytes32 id = _claimDeposit(ethDeposit, ethDeposit.to, ethDeposit.data, height, proof);
+        emit DepositClaimed(id, ethDeposit);
+    }
 
-        signalService.verifySignal(height, trustedCommitmentPublisher, counterpart, id, proof);
+    /// @inheritdoc IETHBridge
+    function cancelDeposit(ETHDeposit memory ethDeposit, address claimee, uint256 height, bytes memory proof)
+        external
+        nonReentrant
+    {
+        require(msg.sender == ethDeposit.canceler, OnlyCanceler());
 
-        _claimed[id] = true;
-        _sendETH(ethDeposit.to, ethDeposit.amount, ethDeposit.data);
+        bytes32 id = _claimDeposit(ethDeposit, claimee, bytes(""), height, proof);
 
-        emit DepositClaimed(id, ethDeposit);
+        emit DepositCancelled(id, claimee);
+    }
+
+    function _claimDeposit(
+        ETHDeposit memory ethDeposit,
+        address to,
+        bytes memory data,
+        uint256 height,
+        bytes memory proof
+    ) internal returns (bytes32 id) {
+        id = _generateId(ethDeposit);
+        require(!processed(id), AlreadyClaimed());
+
+        signalService.verifySignal(height, trustedCommitmentPublisher, counterpart, id, proof);
+
+        _processed[id] = true;
+        _sendETH(to, ethDeposit.amount, data);
     }
 
     /// @dev Function to transfer ETH to the receiver but ignoring the returndata.

src/protocol/IETHBridge.sol

@@ -19,6 +19,9 @@ interface IETHBridge {
         bytes data;
         // Application-specific context data (e.g., for relayer selection, tips, etc.)
         bytes context;
+        // Address that is allowed to cancel the deposit on the destination chain (zero address means deposit is
+        // uncancellable)
+        address canceler;
     }
 
     /// @dev Emitted when a deposit is made.
@@ -31,15 +34,23 @@ interface IETHBridge {
     /// @param deposit The claimed ETH deposit
     event DepositClaimed(bytes32 indexed id, ETHDeposit deposit);
 
+    /// @dev Emitted when a deposit is cancelled.
+    /// @param id The deposit id
+    /// @param claimee The address that received the cancelled deposit
+    event DepositCancelled(bytes32 indexed id, address claimee);
+
     /// @dev Failed to call the receiver with value.
     error FailedClaim();
 
     /// @dev A deposit was already claimed.
     error AlreadyClaimed();
 
-    /// @dev Whether the deposit identified by `id` has been claimed.
+    /// @dev Only canceler can cancel a deposit.
+    error OnlyCanceler();
+
+    /// @dev Whether the deposit identified by `id` has been claimed or cancelled.
     /// @param id The deposit id
-    function claimed(bytes32 id) external view returns (bool);
+    function processed(bytes32 id) external view returns (bool);
 
     /// @dev ETH Deposit identifier.
     /// @param ethDeposit The ETH deposit struct
@@ -49,12 +60,25 @@ interface IETHBridge {
     /// @param to The receiver of the deposit
     /// @param data Any calldata to be sent to the receiver in case of a contract
     /// @param context Application-specific context data
-    function deposit(address to, bytes memory data, bytes memory context) external payable returns (bytes32 id);
+    /// @param canceler Address on the destination chain that is allowed to cancel the deposit (zero address means
+    /// deposit is uncancellable)
+    function deposit(address to, bytes memory data, bytes memory context, address canceler)
+        external
+        payable
+        returns (bytes32 id);
 
     /// @dev Claims an ETH deposit created by the sender (`from`) with `nonce`. The `value` ETH claimed  is
     /// sent to the receiver (`to`) after verifying a storage proof.
     /// @param ethDeposit The ETH deposit struct
     /// @param height The `height` of the checkpoint on the source chain (i.e. the block number or publicationId)
     /// @param proof Encoded proof of the storage slot where the deposit is stored
     function claimDeposit(ETHDeposit memory ethDeposit, uint256 height, bytes memory proof) external;
+
+    /// @dev Initiates a cancel on the deposit, must be called by the canceler on the destination chain.
+    /// @param ethDeposit The ETH deposit struct
+    /// @param claimee The address that will receive the cancelled deposit
+    /// @param height The `height` of the checkpoint on the source chain (i.e. the block number or publicationId)
+    /// @param proof Encoded proof of the storage slot where the deposit is stored
+    function cancelDeposit(ETHDeposit memory ethDeposit, address claimee, uint256 height, bytes memory proof)
+        external;
 }

test/ETHBridge/ClaimableScenarios.t.sol

@@ -17,9 +17,9 @@ abstract contract DepositIsClaimable is CrossChainDepositExists {
         bytes memory proof = abi.encode(sampleDepositProof.getDepositSignalProof(_depositIdx()));
         (, bytes32 id) = sampleDepositProof.getDepositInternals(_depositIdx());
 
-        assertFalse(bridge.claimed(id), "deposit already marked as claimed");
+        assertFalse(bridge.processed(id), "deposit already marked as claimed");
         bridge.claimDeposit(deposit, HEIGHT, proof);
-        assertTrue(bridge.claimed(id), "deposit not marked as claimed");
+        assertTrue(bridge.processed(id), "deposit not marked as claimed");
     }
 
     function test_claimDeposit_shouldEmitEvent() public {

test/ETHBridge/InitialState.t.sol

@@ -15,6 +15,9 @@ 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 trustedCommitmentPublisher = _randomAddress("trustedCommitmentPublisher");
 
     function setUp() public virtual {