diff --git a/contracts/SpokePool.sol b/contracts/SpokePool.sol index d02370a80..fa9835d0d 100644 --- a/contracts/SpokePool.sol +++ b/contracts/SpokePool.sol @@ -102,6 +102,8 @@ abstract contract SpokePool is // to eliminate any chance of collision between pre and post V3 relay hashes. mapping(bytes32 => uint256) public fillStatuses; + mapping(address => mapping(address => uint256)) public relayerRefund; + /************************************************************** * CONSTANT/IMMUTABLE VARIABLES * **************************************************************/ @@ -149,7 +151,7 @@ abstract contract SpokePool is // used as a fillDeadline in deposit(), a soon to be deprecated function that also hardcodes outputToken to // the zero address, which forces the off-chain validator to replace the output token with the equivalent // token for the input token. By using this magic value, off-chain validators do not have to keep - // this event in their lookback window when querying for expired deposts. + // this event in their lookback window when querying for expired deposits. uint32 public constant INFINITE_FILL_DEADLINE = type(uint32).max; /**************************************** * EVENTS * @@ -170,6 +172,7 @@ abstract contract SpokePool is uint32 indexed leafId, bytes32 l2TokenAddress, bytes32[] refundAddresses, + bool deferredRefunds, address caller ); event TokensBridged( @@ -585,7 +588,7 @@ abstract contract SpokePool is if (currentTime - quoteTimestamp > depositQuoteTimeBuffer) revert InvalidQuoteTimestamp(); // fillDeadline is relative to the destination chain. - // Don’t allow fillDeadline to be more than several bundles into the future. + // Don't allow fillDeadline to be more than several bundles into the future. // This limits the maximum required lookback for dataworker and relayer instances. // Also, don't allow fillDeadline to be in the past. This poses a potential UX issue if the destination // chain time keeping and this chain's time keeping are out of sync but is not really a practical hurdle @@ -846,7 +849,7 @@ abstract contract SpokePool is * @param fillDeadline The deadline for the relayer to fill the deposit. After this destination chain timestamp, * the fill will revert on the destination chain. Must be set between [currentTime, currentTime + fillDeadlineBuffer] * where currentTime is block.timestamp on this chain or this transaction will revert. - * @param exclusivityPeriod Added to the current time to set the exclusive reayer deadline, + * @param exclusivityPeriod Added to the current time to set the exclusive relayer deadline, * which is the deadline for the exclusiveRelayer to fill the deposit. After this destination chain timestamp, * anyone can fill the deposit. * @param message The message to send to the recipient on the destination chain if the recipient is a contract. @@ -1062,7 +1065,7 @@ abstract contract SpokePool is * recipient, and/or message. The relayer should only use this function if they can supply a message signed * by the depositor that contains the fill's matching deposit ID along with updated relay parameters. * If the signature can be verified, then this function will emit a FilledV3Event that will be used by - * the system for refund verification purposes. In otherwords, this function is an alternative way to fill a + * the system for refund verification purposes. In other words, this function is an alternative way to fill a * a deposit than fillV3Relay. * @dev Subject to same exclusivity deadline rules as fillV3Relay(). * @param relayData struct containing all the data needed to identify the deposit to be filled. See fillV3Relay(). @@ -1254,7 +1257,7 @@ abstract contract SpokePool is _setClaimedLeaf(rootBundleId, relayerRefundLeaf.leafId); - _distributeRelayerRefunds( + bool deferredRefunds = _distributeRelayerRefunds( relayerRefundLeaf.chainId, relayerRefundLeaf.amountToReturn, relayerRefundLeaf.refundAmounts, @@ -1271,10 +1274,27 @@ abstract contract SpokePool is relayerRefundLeaf.leafId, relayerRefundLeaf.l2TokenAddress, relayerRefundLeaf.refundAddresses, + deferredRefunds, msg.sender ); } + /** + * @notice Enables a relayer to claim outstanding repayments. Should virtually never be used, unless for some reason + * relayer repayment transfer fails for reasons such as token transfer reverts due to blacklisting. In this case, + * the relayer can still call this method and claim the tokens to a new address. + * @param l2TokenAddress Address of the L2 token to claim refunds for. + * @param refundAddress Address to send the refund to. + */ + function claimRelayerRefund(address l2TokenAddress, address refundAddress) public { + uint256 refund = relayerRefund[l2TokenAddress][msg.sender]; + if (refund == 0) revert NoRelayerRefundToClaim(); + relayerRefund[l2TokenAddress][msg.sender] = 0; + IERC20Upgradeable(l2TokenAddress).safeTransfer(refundAddress, refund); + + emit ClaimedRelayerRefund(l2TokenAddress, msg.sender, refundAddress, refund); + } + /************************************** * VIEW FUNCTIONS * **************************************/ @@ -1295,6 +1315,10 @@ abstract contract SpokePool is return block.timestamp; // solhint-disable-line not-rely-on-time } + function getRelayerRefund(address l2TokenAddress, address refundAddress) public view returns (uint256) { + return relayerRefund[l2TokenAddress][refundAddress]; + } + /************************************** * INTERNAL FUNCTIONS * **************************************/ @@ -1373,29 +1397,71 @@ abstract contract SpokePool is uint32 leafId, bytes32 l2TokenAddress, bytes32[] memory refundAddresses - ) internal { - if (refundAddresses.length != refundAmounts.length) revert InvalidMerkleLeaf(); - - address l2TokenAddressParsed = l2TokenAddress.toAddress(); - - // Send each relayer refund address the associated refundAmount for the L2 token address. - // Note: Even if the L2 token is not enabled on this spoke pool, we should still refund relayers. - uint256 length = refundAmounts.length; - for (uint256 i = 0; i < length; ++i) { - uint256 amount = refundAmounts[i]; - if (amount > 0) - IERC20Upgradeable(l2TokenAddressParsed).safeTransfer(refundAddresses[i].toAddress(), amount); + ) internal returns (bool deferredRefunds) { + uint256 numRefunds = refundAmounts.length; + if (refundAddresses.length != numRefunds) revert InvalidMerkleLeaf(); + + if (numRefunds > 0) { + uint256 spokeStartBalance = IERC20Upgradeable(l2TokenAddress.toAddress()).balanceOf(address(this)); + uint256 totalRefundedAmount = 0; // Track the total amount refunded. + + // Send each relayer refund address the associated refundAmount for the L2 token address. + // Note: Even if the L2 token is not enabled on this spoke pool, we should still refund relayers. + for (uint256 i = 0; i < numRefunds; ++i) { + if (refundAmounts[i] > 0) { + totalRefundedAmount += refundAmounts[i]; + + // Only if the total refunded amount exceeds the spoke starting balance, should we revert. This + // ensures that bundles are atomic, if we have sufficient balance to refund all relayers and + // prevents can only re-pay some of the relayers. + if (totalRefundedAmount > spokeStartBalance) revert InsufficientSpokePoolBalanceToExecuteLeaf(); + + bool success = _noRevertTransfer( + l2TokenAddress.toAddress(), + refundAddresses[i].toAddress(), + refundAmounts[i] + ); + + // If the transfer failed then track a deferred transfer for the relayer. Given this function would + // have revered if there was insufficient balance, this will only happen if the transfer call + // reverts. This will only occur if the underlying transfer method on the l2Token reverts due to + // recipient blacklisting or other related modifications to the l2Token.transfer method. + if (!success) { + relayerRefund[l2TokenAddress.toAddress()][refundAddresses[i].toAddress()] += refundAmounts[i]; + deferredRefunds = true; + } + } + } } - // If leaf's amountToReturn is positive, then send L2 --> L1 message to bridge tokens back via // chain-specific bridging method. if (amountToReturn > 0) { - _bridgeTokensToHubPool(amountToReturn, l2TokenAddressParsed); + _bridgeTokensToHubPool(amountToReturn, l2TokenAddress.toAddress()); emit TokensBridged(amountToReturn, _chainId, leafId, l2TokenAddress, msg.sender); } } + // Re-implementation of OZ _callOptionalReturnBool to use private logic. Function executes a transfer and returns a + // bool indicating if the external call was successful, rather than reverting. Original method: + // https://github.com/OpenZeppelin/openzeppelin-contracts/blob/28aed34dc5e025e61ea0390c18cac875bfde1a78/contracts/token/ERC20/utils/SafeERC20.sol#L188 + function _noRevertTransfer( + address token, + address to, + uint256 amount + ) internal returns (bool) { + bool success; + uint256 returnSize; + uint256 returnValue; + bytes memory data = abi.encodeCall(IERC20Upgradeable.transfer, (to, amount)); + assembly { + success := call(gas(), token, 0, add(data, 0x20), mload(data), 0, 0x20) + returnSize := returndatasize() + returnValue := mload(0) + } + return success && (returnSize == 0 ? address(token).code.length > 0 : returnValue == 1); + } + function _setCrossDomainAdmin(address newCrossDomainAdmin) internal { if (newCrossDomainAdmin == address(0)) revert InvalidCrossDomainAdmin(); crossDomainAdmin = newCrossDomainAdmin; @@ -1466,7 +1532,7 @@ abstract contract SpokePool is bytes memory depositorSignature ) internal view virtual { // Note: - // - We don't need to worry about reentrancy from a contract deployed at the depositor address since the method + // - We don't need to worry about re-entrancy from a contract deployed at the depositor address since the method // `SignatureChecker.isValidSignatureNow` is a view method. Re-entrancy can happen, but it cannot affect state. // - EIP-1271 signatures are supported. This means that a signature valid now, may not be valid later and vice-versa. // - For an EIP-1271 signature to work, the depositor contract address must map to a deployed contract on the destination @@ -1621,5 +1687,5 @@ abstract contract SpokePool is // Reserve storage slots for future versions of this base contract to add state variables without // affecting the storage layout of child contracts. Decrement the size of __gap whenever state variables // are added. This is at bottom of contract to make sure it's always at the end of storage. - uint256[999] private __gap; + uint256[998] private __gap; } diff --git a/contracts/interfaces/V3SpokePoolInterface.sol b/contracts/interfaces/V3SpokePoolInterface.sol index 2b01f5322..49e16f876 100644 --- a/contracts/interfaces/V3SpokePoolInterface.sol +++ b/contracts/interfaces/V3SpokePoolInterface.sol @@ -154,6 +154,13 @@ interface V3SpokePoolInterface { bytes message ); + event ClaimedRelayerRefund( + address indexed l2TokenAddress, + address indexed caller, + address indexed refundAddress, + uint256 amount + ); + /************************************** * FUNCTIONS * **************************************/ @@ -242,4 +249,6 @@ interface V3SpokePoolInterface { error InvalidPayoutAdjustmentPct(); error WrongERC7683OrderId(); error LowLevelCallFailed(bytes data); + error InsufficientSpokePoolBalanceToExecuteLeaf(); + error NoRelayerRefundToClaim(); } diff --git a/contracts/test/ExpandedERC20WithBlacklist.sol b/contracts/test/ExpandedERC20WithBlacklist.sol new file mode 100644 index 000000000..607609eb7 --- /dev/null +++ b/contracts/test/ExpandedERC20WithBlacklist.sol @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import "@uma/core/contracts/common/implementation/ExpandedERC20.sol"; + +contract ExpandedERC20WithBlacklist is ExpandedERC20 { + mapping(address => bool) public isBlackListed; + + constructor( + string memory name, + string memory symbol, + uint8 decimals + ) ExpandedERC20(name, symbol, decimals) {} + + function setBlacklistStatus(address account, bool status) external { + isBlackListed[account] = status; + } + + function _beforeTokenTransfer( + address from, + address to, + uint256 amount + ) internal override { + require(!isBlackListed[to], "Recipient is blacklisted"); + super._beforeTokenTransfer(from, to, amount); + } +} diff --git a/storage-layouts/Arbitrum_SpokePool.json b/storage-layouts/Arbitrum_SpokePool.json index 132a22efe..14bd92871 100644 --- a/storage-layouts/Arbitrum_SpokePool.json +++ b/storage-layouts/Arbitrum_SpokePool.json @@ -146,10 +146,16 @@ }, { "contract": "contracts/Arbitrum_SpokePool.sol:Arbitrum_SpokePool", - "label": "__gap", + "label": "relayerRefund", "offset": 0, "slot": "2163" }, + { + "contract": "contracts/Arbitrum_SpokePool.sol:Arbitrum_SpokePool", + "label": "__gap", + "offset": 0, + "slot": "2164" + }, { "contract": "contracts/Arbitrum_SpokePool.sol:Arbitrum_SpokePool", "label": "l2GatewayRouter", diff --git a/storage-layouts/Base_SpokePool.json b/storage-layouts/Base_SpokePool.json index 925a6ab42..80934a3f8 100644 --- a/storage-layouts/Base_SpokePool.json +++ b/storage-layouts/Base_SpokePool.json @@ -146,10 +146,16 @@ }, { "contract": "contracts/Base_SpokePool.sol:Base_SpokePool", - "label": "__gap", + "label": "relayerRefund", "offset": 0, "slot": "2163" }, + { + "contract": "contracts/Base_SpokePool.sol:Base_SpokePool", + "label": "__gap", + "offset": 0, + "slot": "2164" + }, { "contract": "contracts/Base_SpokePool.sol:Base_SpokePool", "label": "l1Gas", diff --git a/storage-layouts/Blast_SpokePool.json b/storage-layouts/Blast_SpokePool.json index e287f83a7..2be5208f9 100644 --- a/storage-layouts/Blast_SpokePool.json +++ b/storage-layouts/Blast_SpokePool.json @@ -146,10 +146,16 @@ }, { "contract": "contracts/Blast_SpokePool.sol:Blast_SpokePool", - "label": "__gap", + "label": "relayerRefund", "offset": 0, "slot": "2163" }, + { + "contract": "contracts/Blast_SpokePool.sol:Blast_SpokePool", + "label": "__gap", + "offset": 0, + "slot": "2164" + }, { "contract": "contracts/Blast_SpokePool.sol:Blast_SpokePool", "label": "l1Gas", diff --git a/storage-layouts/Ethereum_SpokePool.json b/storage-layouts/Ethereum_SpokePool.json index 73fc3f2e2..416c2be2c 100644 --- a/storage-layouts/Ethereum_SpokePool.json +++ b/storage-layouts/Ethereum_SpokePool.json @@ -146,10 +146,16 @@ }, { "contract": "contracts/Ethereum_SpokePool.sol:Ethereum_SpokePool", - "label": "__gap", + "label": "relayerRefund", "offset": 0, "slot": "2163" }, + { + "contract": "contracts/Ethereum_SpokePool.sol:Ethereum_SpokePool", + "label": "__gap", + "offset": 0, + "slot": "2164" + }, { "contract": "contracts/Ethereum_SpokePool.sol:Ethereum_SpokePool", "label": "__gap", diff --git a/storage-layouts/Linea_SpokePool.json b/storage-layouts/Linea_SpokePool.json index 4a14b02c3..9dae90739 100644 --- a/storage-layouts/Linea_SpokePool.json +++ b/storage-layouts/Linea_SpokePool.json @@ -146,10 +146,16 @@ }, { "contract": "contracts/Linea_SpokePool.sol:Linea_SpokePool", - "label": "__gap", + "label": "relayerRefund", "offset": 0, "slot": "2163" }, + { + "contract": "contracts/Linea_SpokePool.sol:Linea_SpokePool", + "label": "__gap", + "offset": 0, + "slot": "2164" + }, { "contract": "contracts/Linea_SpokePool.sol:Linea_SpokePool", "label": "l2MessageService", diff --git a/storage-layouts/Mode_SpokePool.json b/storage-layouts/Mode_SpokePool.json index 55de1b86b..b0daad98f 100644 --- a/storage-layouts/Mode_SpokePool.json +++ b/storage-layouts/Mode_SpokePool.json @@ -146,10 +146,16 @@ }, { "contract": "contracts/Mode_SpokePool.sol:Mode_SpokePool", - "label": "__gap", + "label": "relayerRefund", "offset": 0, "slot": "2163" }, + { + "contract": "contracts/Mode_SpokePool.sol:Mode_SpokePool", + "label": "__gap", + "offset": 0, + "slot": "2164" + }, { "contract": "contracts/Mode_SpokePool.sol:Mode_SpokePool", "label": "l1Gas", diff --git a/storage-layouts/Optimism_SpokePool.json b/storage-layouts/Optimism_SpokePool.json index 8732f142e..ded4492f7 100644 --- a/storage-layouts/Optimism_SpokePool.json +++ b/storage-layouts/Optimism_SpokePool.json @@ -146,10 +146,16 @@ }, { "contract": "contracts/Optimism_SpokePool.sol:Optimism_SpokePool", - "label": "__gap", + "label": "relayerRefund", "offset": 0, "slot": "2163" }, + { + "contract": "contracts/Optimism_SpokePool.sol:Optimism_SpokePool", + "label": "__gap", + "offset": 0, + "slot": "2164" + }, { "contract": "contracts/Optimism_SpokePool.sol:Optimism_SpokePool", "label": "l1Gas", diff --git a/storage-layouts/Polygon_SpokePool.json b/storage-layouts/Polygon_SpokePool.json index 50c271f81..924750f6e 100644 --- a/storage-layouts/Polygon_SpokePool.json +++ b/storage-layouts/Polygon_SpokePool.json @@ -146,10 +146,16 @@ }, { "contract": "contracts/Polygon_SpokePool.sol:Polygon_SpokePool", - "label": "__gap", + "label": "relayerRefund", "offset": 0, "slot": "2163" }, + { + "contract": "contracts/Polygon_SpokePool.sol:Polygon_SpokePool", + "label": "__gap", + "offset": 0, + "slot": "2164" + }, { "contract": "contracts/Polygon_SpokePool.sol:Polygon_SpokePool", "label": "fxChild", diff --git a/storage-layouts/ZkSync_SpokePool.json b/storage-layouts/ZkSync_SpokePool.json index 7d1a0c5ce..d76f701b5 100644 --- a/storage-layouts/ZkSync_SpokePool.json +++ b/storage-layouts/ZkSync_SpokePool.json @@ -146,10 +146,16 @@ }, { "contract": "contracts/ZkSync_SpokePool.sol:ZkSync_SpokePool", - "label": "__gap", + "label": "relayerRefund", "offset": 0, "slot": "2163" }, + { + "contract": "contracts/ZkSync_SpokePool.sol:ZkSync_SpokePool", + "label": "__gap", + "offset": 0, + "slot": "2164" + }, { "contract": "contracts/ZkSync_SpokePool.sol:ZkSync_SpokePool", "label": "l2Eth", diff --git a/test/evm/foundry/fork/BlacklistedRelayerRecipient.t.sol b/test/evm/foundry/fork/BlacklistedRelayerRecipient.t.sol new file mode 100644 index 000000000..a2be448a8 --- /dev/null +++ b/test/evm/foundry/fork/BlacklistedRelayerRecipient.t.sol @@ -0,0 +1,164 @@ +// SPDX-License-Identifier: Unlicense +pragma solidity ^0.8.0; + +import { Test } from "forge-std/Test.sol"; +import { MockSpokePool } from "../../../../contracts/test/MockSpokePool.sol"; + +import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol"; + +// Define a minimal interface for USDT. Note USDT does NOT return anything after a transfer. +interface IUSDT { + function balanceOf(address account) external view returns (uint256); + + function transfer(address recipient, uint256 amount) external; + + function transferFrom( + address from, + address to, + uint256 value + ) external; + + function addBlackList(address _evilUser) external; + + function getBlackListStatus(address _evilUser) external view returns (bool); +} + +// Define a minimal interface for USDC. Note USDC returns a boolean after a transfer. +interface IUSDC { + function balanceOf(address account) external view returns (uint256); + + function transfer(address recipient, uint256 amount) external returns (bool); + + function transferFrom( + address from, + address to, + uint256 value + ) external returns (bool); + + function blacklist(address _account) external; + + function isBlacklisted(address _account) external view returns (bool); +} + +contract MockSpokePoolTest is Test { + MockSpokePool spokePool; + IUSDT usdt; + IUSDC usdc; + + address largeUSDTAccount = 0x99C9fc46f92E8a1c0deC1b1747d010903E884bE1; + address largeUSDCAccount = 0x37305B1cD40574E4C5Ce33f8e8306Be057fD7341; + uint256 seedAmount = 10_000 * 10**6; + + address recipient1 = address(0x6969691111111420); + address recipient2 = address(0x6969692222222420); + + function setUp() public { + spokePool = new MockSpokePool(address(0x123)); + // Create an instance of USDT & USDCusing its mainnet address + usdt = IUSDT(address(0xdAC17F958D2ee523a2206206994597C13D831ec7)); + usdc = IUSDC(address(0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48)); + + // Impersonate a large USDT & USDC holders and send tokens to spokePool contract. + assertTrue(usdt.balanceOf(largeUSDTAccount) > seedAmount, "Large USDT holder has less USDT than expected"); + assertTrue(usdc.balanceOf(largeUSDCAccount) > seedAmount, "Large USDC holder has less USDC than expected"); + + vm.prank(largeUSDTAccount); + usdt.transfer(address(spokePool), seedAmount); + assertEq(usdt.balanceOf(address(spokePool)), seedAmount, "Seed transfer failed"); + + vm.prank(largeUSDCAccount); + usdc.transfer(address(spokePool), seedAmount); + assertEq(usdc.balanceOf(address(spokePool)), seedAmount, "USDC seed transfer failed"); + } + + function testStandardRefundsWorks() public { + // Test USDT + assertEq(usdt.balanceOf(recipient1), 0, "Recipient should start with 0 USDT balance"); + assertEq(usdt.balanceOf(address(spokePool)), seedAmount, "SpokePool should have seed USDT balance"); + + uint256[] memory refundAmounts = new uint256[](1); + refundAmounts[0] = 420 * 10**6; + + bytes32[] memory refundAddresses = new bytes32[](1); + refundAddresses[0] = toBytes32(recipient1); + spokePool.distributeRelayerRefunds(1, 0, refundAmounts, 0, toBytes32(address(usdt)), refundAddresses); + + assertEq(usdt.balanceOf(recipient1), refundAmounts[0], "Recipient should have received refund"); + assertEq(usdt.balanceOf(address(spokePool)), seedAmount - refundAmounts[0], "SpokePool bal should drop"); + + // Test USDC + assertEq(usdc.balanceOf(recipient1), 0, "Recipient should start with 0 USDC balance"); + assertEq(usdc.balanceOf(address(spokePool)), seedAmount, "SpokePool should have seed USDC balance"); + + spokePool.distributeRelayerRefunds(1, 0, refundAmounts, 0, toBytes32(address(usdc)), refundAddresses); + + assertEq(usdc.balanceOf(recipient1), refundAmounts[0], "Recipient should have received refund"); + assertEq(usdc.balanceOf(address(spokePool)), seedAmount - refundAmounts[0], "SpokePool bal should drop"); + } + + function testSomeRecipientsBlacklistedDoesNotBlockTheWholeRefundUsdt() public { + // Note that USDT does NOT block blacklisted recipients, only blacklisted senders. This means that even + // if a recipient is blacklisted the bundle payment should still work to them, even though they then cant + // send the tokens after the fact. + assertEq(usdt.getBlackListStatus(recipient1), false, "Recipient1 should not be blacklisted"); + vm.prank(0xC6CDE7C39eB2f0F0095F41570af89eFC2C1Ea828); // USDT owner. + usdt.addBlackList(recipient1); + assertEq(usdt.getBlackListStatus(recipient1), true, "Recipient1 should be blacklisted"); + + assertEq(usdt.balanceOf(recipient1), 0, "Recipient1 should start with 0 USDT balance"); + assertEq(usdt.balanceOf(recipient2), 0, "Recipient2 should start with 0 USDT balance"); + + uint256[] memory refundAmounts = new uint256[](2); + refundAmounts[0] = 420 * 10**6; + refundAmounts[1] = 69 * 10**6; + + bytes32[] memory refundAddresses = new bytes32[](2); + refundAddresses[0] = toBytes32(recipient1); + refundAddresses[1] = toBytes32(recipient2); + spokePool.distributeRelayerRefunds(1, 0, refundAmounts, 0, toBytes32(address(usdt)), refundAddresses); + + assertEq(usdt.balanceOf(recipient1), refundAmounts[0], "Recipient1 should have received their refund"); + assertEq(usdt.balanceOf(recipient2), refundAmounts[1], "Recipient2 should have received their refund"); + + assertEq(spokePool.getRelayerRefund(address(usdt), recipient1), 0); + assertEq(spokePool.getRelayerRefund(address(usdt), recipient2), 0); + } + + function testSomeRecipientsBlacklistedDoesNotBlockTheWholeRefundUsdc() public { + // USDC blacklist blocks both the sender and recipient. Therefore if we a recipient within a bundle is + // blacklisted, they should be credited for the refund amount that can be claimed later to a new address. + assertEq(usdc.isBlacklisted(recipient1), false, "Recipient1 should not be blacklisted"); + vm.prank(0x10DF6B6fe66dd319B1f82BaB2d054cbb61cdAD2e); // USDC blacklister + usdc.blacklist(recipient1); + assertEq(usdc.isBlacklisted(recipient1), true, "Recipient1 should be blacklisted"); + + assertEq(usdc.balanceOf(recipient1), 0, "Recipient1 should start with 0 USDc balance"); + assertEq(usdc.balanceOf(recipient2), 0, "Recipient2 should start with 0 USDc balance"); + + uint256[] memory refundAmounts = new uint256[](2); + refundAmounts[0] = 420 * 10**6; + refundAmounts[1] = 69 * 10**6; + + bytes32[] memory refundAddresses = new bytes32[](2); + refundAddresses[0] = toBytes32(recipient1); + refundAddresses[1] = toBytes32(recipient2); + spokePool.distributeRelayerRefunds(1, 0, refundAmounts, 0, toBytes32(address(usdc)), refundAddresses); + + assertEq(usdc.balanceOf(recipient1), 0, "Recipient1 should have 0 refund as blacklisted"); + assertEq(usdc.balanceOf(recipient2), refundAmounts[1], "Recipient2 should have received their refund"); + + assertEq(spokePool.getRelayerRefund(address(usdc), recipient1), refundAmounts[0]); + assertEq(spokePool.getRelayerRefund(address(usdc), recipient2), 0); + + // Now, blacklisted recipient should be able to claim refund to a new address. + address newRecipient = address(0x6969693333333420); + vm.prank(recipient1); + spokePool.claimRelayerRefund(address(usdc), newRecipient); + assertEq(usdc.balanceOf(newRecipient), refundAmounts[0], "New recipient should have received relayer2 refund"); + assertEq(spokePool.getRelayerRefund(address(usdt), recipient1), 0); + } + + function toBytes32(address _address) internal pure returns (bytes32) { + return bytes32(uint256(uint160(_address))); + } +} diff --git a/test/evm/hardhat/SpokePool.ClaimRelayerRefund.ts b/test/evm/hardhat/SpokePool.ClaimRelayerRefund.ts new file mode 100644 index 000000000..84826c640 --- /dev/null +++ b/test/evm/hardhat/SpokePool.ClaimRelayerRefund.ts @@ -0,0 +1,90 @@ +import { + SignerWithAddress, + seedContract, + seedWallet, + expect, + Contract, + ethers, + toBN, + addressToBytes, +} from "../../../utils/utils"; +import * as consts from "./constants"; +import { spokePoolFixture } from "./fixtures/SpokePool.Fixture"; + +let spokePool: Contract, destErc20: Contract, weth: Contract; +let deployerWallet: SignerWithAddress, relayer: SignerWithAddress, rando: SignerWithAddress; + +let destinationChainId: number; + +describe("SpokePool with Blacklisted destErc20", function () { + beforeEach(async function () { + [deployerWallet, relayer, rando] = await ethers.getSigners(); + ({ spokePool, destErc20, weth } = await spokePoolFixture()); + + destinationChainId = Number(await spokePool.chainId()); + await seedContract(spokePool, deployerWallet, [destErc20], weth, consts.amountHeldByPool); + }); + + it("Blacklist destErc20 operates as expected", async function () { + // Transfer tokens to relayer before blacklisting works as expected. + await seedWallet(deployerWallet, [destErc20], weth, consts.amountToRelay); + await destErc20.connect(deployerWallet).transfer(relayer.address, consts.amountToRelay); + expect(await destErc20.balanceOf(relayer.address)).to.equal(consts.amountToRelay); + + await destErc20.setBlacklistStatus(relayer.address, true); // Blacklist the relayer + + // Attempt to transfer tokens to the blacklisted relayer + await expect(destErc20.connect(deployerWallet).transfer(relayer.address, consts.amountToRelay)).to.be.revertedWith( + "Recipient is blacklisted" + ); + }); + + it("Executes repayments and handles blacklisted addresses", async function () { + // No starting relayer liability. + expect(await spokePool.getRelayerRefund(destErc20.address, relayer.address)).to.equal(toBN(0)); + expect(await destErc20.balanceOf(rando.address)).to.equal(toBN(0)); + expect(await destErc20.balanceOf(relayer.address)).to.equal(toBN(0)); + // Blacklist the relayer + await destErc20.setBlacklistStatus(relayer.address, true); + + // Distribute relayer refunds. some refunds go to blacklisted address and some go to non-blacklisted address. + + await spokePool + .connect(deployerWallet) + .distributeRelayerRefunds( + destinationChainId, + consts.amountToReturn, + [consts.amountToRelay, consts.amountToRelay], + 0, + addressToBytes(destErc20.address), + [addressToBytes(relayer.address), addressToBytes(rando.address)] + ); + + // Ensure relayerRepaymentLiability is incremented + expect(await spokePool.getRelayerRefund(destErc20.address, relayer.address)).to.equal(consts.amountToRelay); + expect(await destErc20.balanceOf(rando.address)).to.equal(consts.amountToRelay); + expect(await destErc20.balanceOf(relayer.address)).to.equal(toBN(0)); + }); + it("Relayer with failed repayment can claim their refund", async function () { + await destErc20.setBlacklistStatus(relayer.address, true); + + await spokePool + .connect(deployerWallet) + .distributeRelayerRefunds( + destinationChainId, + consts.amountToReturn, + [consts.amountToRelay], + 0, + addressToBytes(destErc20.address), + [addressToBytes(relayer.address)] + ); + + await expect(spokePool.connect(relayer).claimRelayerRefund(destErc20.address, relayer.address)).to.be.revertedWith( + "Recipient is blacklisted" + ); + + expect(await destErc20.balanceOf(rando.address)).to.equal(toBN(0)); + await spokePool.connect(relayer).claimRelayerRefund(destErc20.address, rando.address); + expect(await destErc20.balanceOf(rando.address)).to.equal(consts.amountToRelay); + }); +}); diff --git a/test/evm/hardhat/SpokePool.Deposit.ts b/test/evm/hardhat/SpokePool.Deposit.ts index 135744d01..82d758eec 100644 --- a/test/evm/hardhat/SpokePool.Deposit.ts +++ b/test/evm/hardhat/SpokePool.Deposit.ts @@ -27,7 +27,6 @@ import { amountReceived, MAX_UINT32, originChainId, - zeroAddress, } from "./constants"; const { AddressZero: ZERO_ADDRESS } = ethers.constants; diff --git a/test/evm/hardhat/SpokePool.ExecuteRootBundle.ts b/test/evm/hardhat/SpokePool.ExecuteRootBundle.ts index c0326d1ca..76d85bca4 100644 --- a/test/evm/hardhat/SpokePool.ExecuteRootBundle.ts +++ b/test/evm/hardhat/SpokePool.ExecuteRootBundle.ts @@ -73,6 +73,7 @@ describe("SpokePool Root Bundle Execution", function () { addressToBytes(relayer.address), addressToBytes(rando.address), ]); + expect(relayTokensEvents[0].args?.deferredRefunds).to.equal(false); // Should emit TokensBridged event if amountToReturn is positive. let tokensBridgedEvents = await spokePool.queryFilter(spokePool.filters.TokensBridged()); @@ -129,6 +130,28 @@ describe("SpokePool Root Bundle Execution", function () { await expect(spokePool.connect(dataWorker).executeRelayerRefundLeaf(0, leaves[0], tree.getHexProof(leaves[0]))).to .be.reverted; }); + it("Execution correctly logs deferred refunds", async function () { + const { leaves, tree } = await constructSimpleTree(destErc20, destinationChainId); + + // Store new tree. + await spokePool.connect(dataWorker).relayRootBundle( + tree.getHexRoot(), // relayer refund root. Generated from the merkle tree constructed before. + consts.mockSlowRelayRoot + ); + + // Blacklist the relayer EOA to prevent it from receiving refunds. The execution should still succeed, though. + await destErc20.setBlacklistStatus(relayer.address, true); + await spokePool.connect(dataWorker).executeRelayerRefundLeaf(0, leaves[0], tree.getHexProof(leaves[0])); + + // Only the non-blacklisted recipient should receive their refund. + expect(await destErc20.balanceOf(spokePool.address)).to.equal(consts.amountHeldByPool.sub(consts.amountToRelay)); + expect(await destErc20.balanceOf(relayer.address)).to.equal(toBN(0)); + expect(await destErc20.balanceOf(rando.address)).to.equal(consts.amountToRelay); + + // Check event that tracks deferred refunds. + let relayTokensEvents = await spokePool.queryFilter(spokePool.filters.ExecutedRelayerRefundRoot()); + expect(relayTokensEvents[0].args?.deferredRefunds).to.equal(true); + }); describe("_distributeRelayerRefunds", function () { it("refund address length mismatch", async function () { @@ -219,5 +242,21 @@ describe("SpokePool Root Bundle Execution", function () { .withArgs(toBN(1), destErc20.address); }); }); + describe("Total refundAmounts > spokePool's balance", function () { + it("Reverts and emits log", async function () { + await expect( + spokePool.connect(dataWorker).distributeRelayerRefunds( + destinationChainId, + toBN(1), + [consts.amountHeldByPool, consts.amountToRelay], // spoke has only amountHeldByPool. + 0, + addressToBytes(destErc20.address), + [addressToBytes(relayer.address), addressToBytes(rando.address)] + ) + ).to.be.revertedWith("InsufficientSpokePoolBalanceToExecuteLeaf"); + + expect((await destErc20.queryFilter(destErc20.filters.Transfer(spokePool.address))).length).to.equal(0); + }); + }); }); }); diff --git a/test/evm/hardhat/fixtures/SpokePool.Fixture.ts b/test/evm/hardhat/fixtures/SpokePool.Fixture.ts index b4943e218..a87f6dd66 100644 --- a/test/evm/hardhat/fixtures/SpokePool.Fixture.ts +++ b/test/evm/hardhat/fixtures/SpokePool.Fixture.ts @@ -7,7 +7,6 @@ import { ethers, BigNumber, defaultAbiCoder, - addressToBytes, } from "../../../../utils/utils"; import * as consts from "../constants"; @@ -41,7 +40,7 @@ export async function deploySpokePool( ).deploy("Unwhitelisted", "UNWHITELISTED", 18); await unwhitelistedErc20.addMember(consts.TokenRolesEnum.MINTER, deployerWallet.address); const destErc20 = await ( - await getContractFactory("ExpandedERC20", deployerWallet) + await getContractFactory("ExpandedERC20WithBlacklist", deployerWallet) ).deploy("L2 USD Coin", "L2 USDC", 18); await destErc20.addMember(consts.TokenRolesEnum.MINTER, deployerWallet.address);