Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 28 additions & 3 deletions offchain/sample_deposit_proof.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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();
Expand All @@ -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;
}
Expand All @@ -76,11 +78,13 @@ fn deposit_specification() -> Vec<DepositSpecification> {
"5932a71200000000000000000000000000000000000000000000000000000000000004d2", // (valid) call to `someNonPayableFunction(1234)`
];

// _randomAddress("canceler");
let canceler = address!("0xf9f5C5411F0bEf1880cE3B051BD14196479764D2");
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),
Expand All @@ -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;
}

Expand Down Expand Up @@ -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();
Expand Down
3 changes: 2 additions & 1 deletion src/protocol/ETHBridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ contract ETHBridge is IETHBridge, ReentrancyGuardTransient {
payable
returns (bytes32 id)
{
require(to != address(0), ZeroReceiver());
ETHDeposit memory ethDeposit =
ETHDeposit(_globalDepositNonce, msg.sender, to, msg.value, data, context, canceler);
id = _generateId(ethDeposit);
Expand Down Expand Up @@ -88,7 +89,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);

Expand Down
7 changes: 5 additions & 2 deletions src/protocol/IETHBridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,11 @@ 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();
Expand Down
88 changes: 88 additions & 0 deletions test/ETHBridge/CancelableScenarios.t.sol
Original file line number Diff line number Diff line change
@@ -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(_randomAddress("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);
}
}
26 changes: 26 additions & 0 deletions test/ETHBridge/ContractCallValidityScenarios.t.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// 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";
Expand Down Expand Up @@ -48,3 +49,28 @@ 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_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);
}
}
3 changes: 3 additions & 0 deletions test/ETHBridge/CrossChainDepositExists.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ abstract contract CrossChainDepositExists is UniversalTest {
// the sample deposit is to this address
address recipient = _randomAddress("recipient");

// the recipient that receives the cancelled deposit
address cancellationRecipient = _randomAddress("cancellationRecipient");

uint256 HEIGHT = 1;

function setUp() public virtual override {
Expand Down
55 changes: 54 additions & 1 deletion test/ETHBridge/EnumeratedScenarios.t.sol
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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)
Expand All @@ -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) {
Expand Down
6 changes: 4 additions & 2 deletions test/ETHBridge/InitialState.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 = _randomAddress("canceler");

address zeroCanceler = address(0);

address trustedCommitmentPublisher = _randomAddress("trustedCommitmentPublisher");

Expand Down
Loading