diff --git a/contracts/SpokePool.sol b/contracts/SpokePool.sol index c740e3258..37bb414de 100644 --- a/contracts/SpokePool.sol +++ b/contracts/SpokePool.sol @@ -343,87 +343,46 @@ abstract contract SpokePool is } /************************************** - * LEGACY DEPOSITOR FUNCTIONS * + * REMOVED DEPOSITOR FUNCTIONS * **************************************/ /** - * @dev DEPRECATION NOTICE: this function is deprecated and will be removed in the future. - * Please use deposit (under DEPOSITOR FUNCTIONS below) or depositV3 instead. - * @notice Called by user to bridge funds from origin to destination chain. Depositor will effectively lock - * tokens in this contract and receive a destination token on the destination chain. The origin => destination - * token mapping is stored on the L1 HubPool. - * @notice The caller must first approve this contract to spend amount of originToken. - * @notice The originToken => destinationChainId must be enabled. - * @notice This method is payable because the caller is able to deposit native token if the originToken is - * wrappedNativeToken and this function will handle wrapping the native token to wrappedNativeToken. - * @dev Produces a FundsDeposited event with an infinite expiry, meaning that this deposit can never expire. - * Moreover, the event's outputToken is set to 0x0 meaning that this deposit can always be slow filled. - * @param recipient Address to receive funds at on destination chain. - * @param originToken Token to lock into this contract to initiate deposit. - * @param amount Amount of tokens to deposit. Will be amount of tokens to receive less fees. - * @param destinationChainId Denotes network where user will receive funds from SpokePool by a relayer. - * @param relayerFeePct % of deposit amount taken out to incentivize a fast relayer. - * @param quoteTimestamp Timestamp used by relayers to compute this deposit's realizedLPFeePct which is paid - * to LP pool on HubPool. - * @param message Arbitrary data that can be used to pass additional information to the recipient along with the tokens. - * Note: this is intended to be used to pass along instructions for how a contract should use or allocate the tokens. + * @dev REMOVED: This function has been removed and is now disallowed to prevent selector reuse. + * @notice This function was removed from the protocol. Calling it will revert. + * Function selector is preserved to prevent accidental reuse in future versions. + * @notice This function shares the same selector as the original "deposit" function that was removed. + * The collision was intentionally created to allow reusing the "deposit" name for a different function signature. */ function depositDeprecated_5947912356( - address recipient, - address originToken, - uint256 amount, - uint256 destinationChainId, - int64 relayerFeePct, - uint32 quoteTimestamp, - bytes memory message, - uint256 // maxCount. Deprecated. - ) public payable nonReentrant unpausedDeposits { - _deposit( - msg.sender, - recipient, - originToken, - amount, - destinationChainId, - relayerFeePct, - quoteTimestamp, - message - ); + address, + address, + uint256, + uint256, + int64, + uint32, + bytes memory, + uint256 + ) public payable { + revert RemovedFunction(); } /** - * @dev DEPRECATION NOTICE: this function is deprecated and will be removed in the future. - * Please use the other deposit or depositV3 instead. - * @notice The only difference between depositFor and deposit is that the depositor address stored - * in the relay hash can be overridden by the caller. This means that the passed in depositor - * can speed up the deposit, which is useful if the deposit is taken from the end user to a middle layer - * contract, like an aggregator or the SpokePoolVerifier, before calling deposit on this contract. - * @notice The caller must first approve this contract to spend amount of originToken. - * @notice The originToken => destinationChainId must be enabled. - * @notice This method is payable because the caller is able to deposit native token if the originToken is - * wrappedNativeToken and this function will handle wrapping the native token to wrappedNativeToken. - * @param depositor Address who is credited for depositing funds on origin chain and can speed up the deposit. - * @param recipient Address to receive funds at on destination chain. - * @param originToken Token to lock into this contract to initiate deposit. - * @param amount Amount of tokens to deposit. Will be amount of tokens to receive less fees. - * @param destinationChainId Denotes network where user will receive funds from SpokePool by a relayer. - * @param relayerFeePct % of deposit amount taken out to incentivize a fast relayer. - * @param quoteTimestamp Timestamp used by relayers to compute this deposit's realizedLPFeePct which is paid - * to LP pool on HubPool. - * @param message Arbitrary data that can be used to pass additional information to the recipient along with the tokens. - * Note: this is intended to be used to pass along instructions for how a contract should use or allocate the tokens. + * @dev REMOVED: This function has been removed and is now disallowed to prevent selector reuse. + * @notice This function was removed from the protocol. Calling it will revert. + * Function selector is preserved to prevent accidental reuse in future versions. */ function depositFor( - address depositor, - address recipient, - address originToken, - uint256 amount, - uint256 destinationChainId, - int64 relayerFeePct, - uint32 quoteTimestamp, - bytes memory message, - uint256 // maxCount. Deprecated. - ) public payable nonReentrant unpausedDeposits { - _deposit(depositor, recipient, originToken, amount, destinationChainId, relayerFeePct, quoteTimestamp, message); + address, + address, + address, + uint256, + uint256, + int64, + uint32, + bytes memory, + uint256 + ) public payable { + revert RemovedFunction(); } /******************************************** @@ -1294,6 +1253,9 @@ abstract contract SpokePool is // Verify depositor is a valid EVM address. params.depositor.checkAddress(); + // Verify output token is not zero address. + if (params.outputToken == bytes32(0)) revert InvalidOutputToken(); + // Require that quoteTimestamp has a maximum age so that depositors pay an LP fee based on recent HubPool usage. // It is assumed that cross-chain timestamps are normally loosely in-sync, but clock drift can occur. If the // SpokePool time stalls or lags significantly, it is still possible to make deposits by setting quoteTimestamp @@ -1369,71 +1331,6 @@ abstract contract SpokePool is ); } - function _deposit( - address depositor, - address recipient, - address originToken, - uint256 amount, - uint256 destinationChainId, - int64 relayerFeePct, - uint32 quoteTimestamp, - bytes memory message - ) internal { - // We limit the relay fees to prevent the user spending all their funds on fees. - if (SignedMath.abs(relayerFeePct) >= 0.5e18) revert InvalidRelayerFeePct(); - if (amount > MAX_TRANSFER_SIZE) revert MaxTransferSizeExceeded(); - - // Require that quoteTimestamp has a maximum age so that depositors pay an LP fee based on recent HubPool usage. - // It is assumed that cross-chain timestamps are normally loosely in-sync, but clock drift can occur. If the - // SpokePool time stalls or lags significantly, it is still possible to make deposits by setting quoteTimestamp - // within the configured buffer. The owner should pause deposits if this is undesirable. This will underflow if - // quoteTimestamp is more than depositQuoteTimeBuffer; this is safe but will throw an unintuitive error. - - // slither-disable-next-line timestamp - if (getCurrentTime() - quoteTimestamp > depositQuoteTimeBuffer) revert InvalidQuoteTimestamp(); - - // Increment count of deposits so that deposit ID for this spoke pool is unique. - uint32 newDepositId = numberOfDeposits++; - - // If the address of the origin token is a wrappedNativeToken contract and there is a msg.value with the - // transaction then the user is sending ETH. In this case, the ETH should be deposited to wrappedNativeToken. - if (originToken == address(wrappedNativeToken) && msg.value > 0) { - if (msg.value != amount) revert MsgValueDoesNotMatchInputAmount(); - wrappedNativeToken.deposit{ value: msg.value }(); - // Else, it is a normal ERC20. In this case pull the token from the user's wallet as per normal. - // Note: this includes the case where the L2 user has WETH (already wrapped ETH) and wants to bridge them. - // In this case the msg.value will be set to 0, indicating a "normal" ERC20 bridging action. - } else { - IERC20Upgradeable(originToken).safeTransferFrom(msg.sender, address(this), amount); - } - - emit FundsDeposited( - originToken.toBytes32(), // inputToken - bytes32(0), // outputToken. Setting this to 0x0 means that the outputToken should be assumed to be the - // canonical token for the destination chain matching the inputToken. Therefore, this deposit - // can always be slow filled. - // - setting token to 0x0 will signal to off-chain validator that the "equivalent" - // token as the inputToken for the destination chain should be replaced here. - amount, // inputAmount - _computeAmountPostFees(amount, relayerFeePct), // outputAmount - // - output amount will be the deposit amount less relayerFeePct, which should now be set - // equal to realizedLpFeePct + gasFeePct + capitalCostFeePct where (gasFeePct + capitalCostFeePct) - // is equal to the old usage of `relayerFeePct`. - destinationChainId, - newDepositId, - quoteTimestamp, - INFINITE_FILL_DEADLINE, // fillDeadline. Default to infinite expiry because - // expired deposits refunds could be a breaking change for existing users of this function. - 0, // exclusivityDeadline. Setting this to 0 along with the exclusiveRelayer to 0x0 means that there - // is no exclusive deadline - depositor.toBytes32(), - recipient.toBytes32(), - bytes32(0), // exclusiveRelayer. Setting this to 0x0 will signal to off-chain validator that there - // is no exclusive relayer. - message - ); - } - function _distributeRelayerRefunds( uint256 _chainId, uint256 amountToReturn, diff --git a/contracts/interfaces/SpokePoolInterface.sol b/contracts/interfaces/SpokePoolInterface.sol index d8ce51cc0..2b4bf85ba 100644 --- a/contracts/interfaces/SpokePoolInterface.sol +++ b/contracts/interfaces/SpokePoolInterface.sol @@ -46,6 +46,16 @@ interface SpokePoolInterface { function emergencyDeleteRootBundle(uint256 rootBundleId) external; + // REMOVED FUNCTIONS: These functions have been removed and are now disallowed. + // Function selectors are preserved to prevent accidental reuse in future versions. + // All calls to these functions will revert. + + /** + * @dev REMOVED: This function has been removed and is now disallowed. + * @notice Calling this function will revert. Use deposit() or depositV3() instead. + * @notice This function shares the same selector as the original "deposit" function that was removed. + * The collision was intentionally created to allow reusing the "deposit" name for a different function signature. + */ function depositDeprecated_5947912356( address recipient, address originToken, @@ -57,6 +67,10 @@ interface SpokePoolInterface { uint256 maxCount ) external payable; + /** + * @dev REMOVED: This function has been removed and is now disallowed. + * @notice Calling this function will revert. Use deposit() or depositV3() instead. + */ function depositFor( address depositor, address recipient, @@ -79,8 +93,6 @@ interface SpokePoolInterface { error NotEOA(); error InvalidDepositorSignature(); - error InvalidRelayerFeePct(); - error MaxTransferSizeExceeded(); error InvalidCrossDomainAdmin(); error InvalidWithdrawalRecipient(); error DepositsArePaused(); diff --git a/contracts/interfaces/V3SpokePoolInterface.sol b/contracts/interfaces/V3SpokePoolInterface.sol index ea19237c3..a22fab443 100644 --- a/contracts/interfaces/V3SpokePoolInterface.sol +++ b/contracts/interfaces/V3SpokePoolInterface.sol @@ -327,6 +327,8 @@ interface V3SpokePoolInterface { error InvalidQuoteTimestamp(); error InvalidFillDeadline(); error InvalidExclusiveRelayer(); + error InvalidOutputToken(); + error RemovedFunction(); error MsgValueDoesNotMatchInputAmount(); error NotExclusiveRelayer(); error NoSlowFillsInExclusivityWindow(); diff --git a/test/evm/foundry/local/SpokePoolDeprecatedMethods.t.sol b/test/evm/foundry/local/SpokePoolDeprecatedMethods.t.sol deleted file mode 100644 index f47ed02b0..000000000 --- a/test/evm/foundry/local/SpokePoolDeprecatedMethods.t.sol +++ /dev/null @@ -1,141 +0,0 @@ -// SPDX-License-Identifier: BUSL-1.1 -pragma solidity ^0.8.0; - -import { Test } from "forge-std/Test.sol"; -import { MockSpokePool } from "../../../../contracts/test/MockSpokePool.sol"; -import { WETH9 } from "../../../../contracts/external/WETH9.sol"; -import { AddressToBytes32 } from "../../../../contracts/libraries/AddressConverters.sol"; -import { ERC1967Proxy } from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; - -// Deprecated interface used to show that we can still call deposit() on the spoke, which should route internally to the -// colliding function interface selector on depositDeprecated_5947912356 enabling legacy deposits to still work without -// breaking interface changes. -interface DeprecatedSpokePoolInterface { - function deposit( - address recipient, - address originToken, - uint256 amount, - uint256 destinationChainId, - int64 relayerFeePct, - uint32 quoteTimestamp, - bytes memory message, - uint256 - ) external payable; -} - -contract SpokePoolOverloadedDeprecatedMethodsTest is Test { - using AddressToBytes32 for address; - - MockSpokePool spokePool; - WETH9 mockWETH; - - address depositor; - address owner; - - uint256 destinationChainId = 10; - uint256 depositAmount = 0.5 * (10**18); - - function setUp() public { - mockWETH = new WETH9(); - - depositor = vm.addr(1); - owner = vm.addr(2); - - vm.startPrank(owner); - ERC1967Proxy proxy = new ERC1967Proxy( - address(new MockSpokePool(address(mockWETH))), - abi.encodeCall(MockSpokePool.initialize, (0, owner, address(420))) - ); - spokePool = MockSpokePool(payable(proxy)); - - vm.stopPrank(); - - deal(depositor, depositAmount * 2); - - vm.startPrank(depositor); - mockWETH.deposit{ value: depositAmount }(); - mockWETH.approve(address(spokePool), depositAmount); - vm.stopPrank(); - - // Assert that the spokePool balance is zero at the start - assertEq(mockWETH.balanceOf(address(spokePool)), 0, "SpokePool balance should be zero at setup"); - } - - function testDeprecatedDeposit() public { - // Here, we are calling the deprecated deposit method, as defined in the deprecated interface. This should, in - // theory, collide with the function selector depositDeprecated_5947912356, thereby calling the legacy deposit - // method on the spoke pool, while using the old old deposit function signature. - vm.startPrank(depositor); - - DeprecatedSpokePoolInterface(address(spokePool)).deposit( - depositor, // recipient - address(mockWETH), // originToken - depositAmount, // amount - destinationChainId, // destinationChainId - 0, // relayerFeePct - uint32(block.timestamp), // quoteTimestamp - bytes(""), // message - 0 // maxCount - ); - - assertEq(mockWETH.balanceOf(address(spokePool)), depositAmount, "SpokePool balance should increase"); - - // Test depositing native ETH directly - DeprecatedSpokePoolInterface(address(spokePool)).deposit{ value: depositAmount }( - depositor, // recipient - address(mockWETH), // originToken - still WETH address for native deposits - depositAmount, // amount - destinationChainId, // destinationChainId - 0, // relayerFeePct - uint32(block.timestamp), // quoteTimestamp - bytes(""), // message - 0 // maxCount - ); - vm.stopPrank(); - - assertEq(mockWETH.balanceOf(address(spokePool)), depositAmount * 2, "SpokePool balance should increase"); - } - - function testBytes32Deposit() public { - vm.prank(depositor); - - // Show the bytes32 variant of the new deposit method works. - spokePool.deposit( - address(depositor).toBytes32(), // depositor - address(depositor).toBytes32(), // recipient - address(mockWETH).toBytes32(), // inputToken - address(mockWETH).toBytes32(), // outputToken - depositAmount, // inputAmount - 0, // outputAmount - destinationChainId, // destinationChainId - bytes32(0), // exclusiveRelayer - uint32(block.timestamp), // quoteTimestamp - uint32(block.timestamp + 1 hours), // fillDeadline - 0, // exclusivityParameter - bytes("") // message - ); - - assertEq(mockWETH.balanceOf(address(spokePool)), depositAmount, "SpokePool balance should increase"); - } - - function testAddressDeposit() public { - // Show the address variant of the new deposit method works. - vm.prank(depositor); - spokePool.depositV3( - depositor, // depositor - depositor, // recipient - address(mockWETH), // inputToken - address(mockWETH), // outputToken - depositAmount, // inputAmount - 0, // outputAmount - destinationChainId, // destinationChainId - address(0), // exclusiveRelayer - uint32(block.timestamp), // quoteTimestamp - uint32(block.timestamp + 1 hours), // fillDeadline - 0, // exclusivityParameter - bytes("") // message - ); - - assertEq(mockWETH.balanceOf(address(spokePool)), depositAmount, "SpokePool balance should increase"); - } -} diff --git a/test/evm/hardhat/SpokePool.Deposit.ts b/test/evm/hardhat/SpokePool.Deposit.ts index db29ce863..e634ebee5 100644 --- a/test/evm/hardhat/SpokePool.Deposit.ts +++ b/test/evm/hardhat/SpokePool.Deposit.ts @@ -53,274 +53,6 @@ describe("SpokePool Depositor Logic", async function () { quoteTimestamp = (await spokePool.getCurrentTime()).toNumber(); }); - it("Depositing ERC20 tokens correctly pulls tokens and changes contract state", async function () { - const revertReason = "DepositsArePaused"; - - // Can't deposit when paused: - await spokePool.connect(depositor).pauseDeposits(true); - await expect( - spokePool.connect(depositor).depositDeprecated_5947912356( - ...getDepositParams({ - originToken: erc20.address, - amount: amountToDeposit, - destinationChainId, - relayerFeePct, - quoteTimestamp, - }) - ) - ).to.be.revertedWith(revertReason); - - await spokePool.connect(depositor).pauseDeposits(false); - - await expect( - spokePool.connect(depositor).depositDeprecated_5947912356( - ...getDepositParams({ - recipient: recipient.address, - originToken: erc20.address, - amount, - destinationChainId, - relayerFeePct, - quoteTimestamp, - }) - ) - ) - .to.emit(spokePool, "FundsDeposited") - .withArgs( - addressToBytes(erc20.address), - addressToBytes(ZERO_ADDRESS), - amountToDeposit, - amountReceived, - destinationChainId, - 0, - quoteTimestamp, - MAX_UINT32, - 0, - addressToBytes(depositor.address), - addressToBytes(recipient.address), - addressToBytes(ZERO_ADDRESS), - "0x" - ); - - // The collateral should have transferred from depositor to contract. - expect(await erc20.balanceOf(depositor.address)).to.equal(amountToSeedWallets.sub(amountToDeposit)); - expect(await erc20.balanceOf(spokePool.address)).to.equal(amountToDeposit); - - // Deposit nonce should increment. - expect(await spokePool.numberOfDeposits()).to.equal(1); - }); - - it("DepositFor overrrides the depositor", async function () { - const newDepositor = randomAddress(); - await expect( - spokePool.connect(depositor).depositFor( - newDepositor, - ...getDepositParams({ - recipient: recipient.address, - originToken: erc20.address, - amount, - destinationChainId, - relayerFeePct, - quoteTimestamp, - }) - ) - ) - .to.emit(spokePool, "FundsDeposited") - .withArgs( - addressToBytes(erc20.address), - addressToBytes(ZERO_ADDRESS), - amountToDeposit, - amountReceived, - destinationChainId, - 0, - quoteTimestamp, - BigNumber.from("0xFFFFFFFF"), - 0, - addressToBytes(newDepositor), // Depositor is overridden. - addressToBytes(recipient.address), - addressToBytes(ZERO_ADDRESS), - "0x" - ); - }); - it("Depositing ETH correctly wraps into WETH", async function () { - const revertReason = "MsgValueDoesNotMatchInputAmount"; - - // Fails if msg.value > 0 but doesn't match amount to deposit. - await expect( - spokePool.connect(depositor).depositDeprecated_5947912356( - ...getDepositParams({ - originToken: weth.address, - amount, - destinationChainId, - relayerFeePct, - quoteTimestamp, - }), - { value: 1 } - ) - ).to.be.revertedWith(revertReason); - - await expect(() => - spokePool.connect(depositor).depositDeprecated_5947912356( - ...getDepositParams({ - recipient: recipient.address, - originToken: weth.address, - amount, - destinationChainId, - relayerFeePct, - quoteTimestamp, - }), - { value: amountToDeposit } - ) - ).to.changeEtherBalances([depositor, weth], [amountToDeposit.mul(toBN("-1")), amountToDeposit]); // ETH should transfer from depositor to WETH contract. - - // WETH balance for user should be same as start, but WETH balancein pool should increase. - expect(await weth.balanceOf(depositor.address)).to.equal(amountToSeedWallets); - expect(await weth.balanceOf(spokePool.address)).to.equal(amountToDeposit); - }); - - it("Depositing ETH with msg.value = 0 pulls WETH from depositor", async function () { - await expect(() => - spokePool.connect(depositor).depositDeprecated_5947912356( - ...getDepositParams({ - originToken: weth.address, - amount, - destinationChainId, - relayerFeePct, - quoteTimestamp, - }), - { value: 0 } - ) - ).to.changeTokenBalances(weth, [depositor, spokePool], [amountToDeposit.mul(toBN("-1")), amountToDeposit]); - }); - - it("SpokePool is not approved to spend originToken", async function () { - const insufficientAllowance = "ERC20: insufficient allowance"; - - await erc20.connect(depositor).approve(spokePool.address, 0); - await expect( - spokePool.connect(depositor).depositDeprecated_5947912356( - ...getDepositParams({ - originToken: erc20.address, - amount, - destinationChainId, - relayerFeePct, - quoteTimestamp, - }) - ) - ).to.be.reverted; - - await erc20.connect(depositor).approve(spokePool.address, amountToDeposit); - await expect( - spokePool.connect(depositor).depositDeprecated_5947912356( - ...getDepositParams({ - originToken: erc20.address, - amount, - destinationChainId, - relayerFeePct, - quoteTimestamp, - }) - ) - ).to.emit(spokePool, "FundsDeposited"); - }); - - it("Relayer fee is invalid", async function () { - const revertReason = "InvalidRelayerFee"; - - await expect( - spokePool.connect(depositor).depositDeprecated_5947912356( - ...getDepositParams({ - originToken: erc20.address, - amount, - destinationChainId, - relayerFeePct: toWei("1"), // Fee > 50% - quoteTimestamp, - }) - ) - ).to.be.revertedWith(revertReason); - }); - - it("quoteTimestamp is out of range", async function () { - const revertReason = "InvalidQuoteTimestamp"; - const quoteTimeBuffer = await spokePool.depositQuoteTimeBuffer(); - - await expect( - spokePool.connect(depositor).depositDeprecated_5947912356( - ...getDepositParams({ - originToken: erc20.address, - amount, - destinationChainId, - relayerFeePct, - quoteTimestamp: quoteTimestamp + 1, - }) - ) - ).to.be.revertedWith("underflowed"); - - await expect( - spokePool.connect(depositor).depositDeprecated_5947912356( - ...getDepositParams({ - originToken: erc20.address, - amount, - destinationChainId, - relayerFeePct, - quoteTimestamp: quoteTimestamp - (quoteTimeBuffer + 1), - }) - ) - ).to.be.revertedWith(revertReason); - - // quoteTimestamp at the exact margins should succeed. - for (const offset of [0, quoteTimeBuffer]) { - await erc20.connect(depositor).approve(spokePool.address, amountToDeposit); - await expect( - spokePool.connect(depositor).depositDeprecated_5947912356( - ...getDepositParams({ - originToken: erc20.address, - amount, - destinationChainId, - relayerFeePct, - quoteTimestamp: quoteTimestamp - offset, - }) - ) - ).to.emit(spokePool, "FundsDeposited"); - } - }); - it("should call legacy deposit through overloaded interface", async function () { - // Define the deprecated interface - const DeprecatedSpokePoolInterface = new ethers.utils.Interface([ - "function deposit(address recipient, address originToken, uint256 amount, uint256 destinationChainId, int64 relayerFeePct, uint32 quoteTimestamp, bytes memory message, uint256 maxCount) external payable", - ]); - - // Create a new instance of the SpokePool with the deprecated interface - const deprecatedSpokePool = new ethers.Contract(spokePool.address, DeprecatedSpokePoolInterface, depositor); - - // Call the deprecated deposit method - await expect( - await deprecatedSpokePool.deposit( - depositor.address, // recipient - erc20.address, // originToken - amountToDeposit, // amount - destinationChainId, // destinationChainId - 0, // relayerFeePct - quoteTimestamp, // quoteTimestamp - "0x", // message - 0 // maxCount - ) - ).to.emit(spokePool, "FundsDeposited"); - - // Test depositing native ETH directly - await expect( - deprecatedSpokePool.deposit( - depositor.address, // recipient - weth.address, // originToken - still WETH address for native deposits - amountToDeposit, // amount - destinationChainId, // destinationChainId - 0, // relayerFeePct - quoteTimestamp, // quoteTimestamp - "0x", // message - 0, // maxCount - { value: amountToDeposit } // Send ETH - ) - ).to.emit(spokePool, "FundsDeposited"); - }); - describe("deposit V3", function () { let relayData: V3RelayData, depositArgs: any[]; function getDepositArgsFromRelayData( @@ -791,6 +523,16 @@ describe("SpokePool Depositor Logic", async function () { ]); await expect(spokePool.connect(depositor).callback(functionCalldata)).to.be.reverted; }); + it("output token cannot be zero address", async function () { + await expect( + spokePool.connect(depositor).deposit( + ...getDepositArgsFromRelayData({ + ...relayData, + outputToken: ZERO_ADDRESS, + }) + ) + ).to.be.revertedWith("InvalidOutputToken"); + }); it("unsafe deposit ID", async function () { // new deposit ID should be the uint256 equivalent of the keccak256 hash of packed {msg.sender, depositor, forcedDepositId}. const forcedDepositId = "99";