Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
108 changes: 87 additions & 21 deletions contracts/SpokePool.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure you decrement the __gap array size at the bottom of the file to keep storage aligned.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just decrement by 1 for a 2d mapping? I'm not totally sure how this gap variable works.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spoke implementations for each chain inherit this SpokePool, which is also upgradable, so we need to ensure the storage layout on derived contracts stays the same


/**************************************************************
* CONSTANT/IMMUTABLE VARIABLES *
**************************************************************/
Expand Down Expand Up @@ -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 *
Expand All @@ -170,6 +172,7 @@ abstract contract SpokePool is
uint32 indexed leafId,
bytes32 l2TokenAddress,
bytes32[] refundAddresses,
bool deferredRefunds,
address caller
);
event TokensBridged(
Expand Down Expand Up @@ -585,7 +588,7 @@ abstract contract SpokePool is
if (currentTime - quoteTimestamp > depositQuoteTimeBuffer) revert InvalidQuoteTimestamp();

// fillDeadline is relative to the destination chain.
// Dont 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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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().
Expand Down Expand Up @@ -1254,7 +1257,7 @@ abstract contract SpokePool is

_setClaimedLeaf(rootBundleId, relayerRefundLeaf.leafId);

_distributeRelayerRefunds(
bool deferredRefunds = _distributeRelayerRefunds(
relayerRefundLeaf.chainId,
relayerRefundLeaf.amountToReturn,
relayerRefundLeaf.refundAmounts,
Expand All @@ -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 *
**************************************/
Expand All @@ -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 *
**************************************/
Expand Down Expand Up @@ -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());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to check here that amountToReturn <= erc20.balanceOf(address(this)) because this case would be handled by the revert?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup.


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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, this function is exactly what we need to work with old tokens, like USDT.

bool success;
uint256 returnSize;
uint256 returnValue;
bytes memory data = abi.encodeCall(IERC20Upgradeable.transfer, (to, amount));
assembly {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be annotated as memory-safe, like in OZ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the solidity version we are using does not support this. it gave me an error when trying to do that.

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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
9 changes: 9 additions & 0 deletions contracts/interfaces/V3SpokePoolInterface.sol
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,13 @@ interface V3SpokePoolInterface {
bytes message
);

event ClaimedRelayerRefund(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ordering is different than in SVM. This also has the caller added. Do you think we later update SVM to match this EVM interface?

address indexed l2TokenAddress,
address indexed caller,
address indexed refundAddress,
uint256 amount
);

/**************************************
* FUNCTIONS *
**************************************/
Expand Down Expand Up @@ -242,4 +249,6 @@ interface V3SpokePoolInterface {
error InvalidPayoutAdjustmentPct();
error WrongERC7683OrderId();
error LowLevelCallFailed(bytes data);
error InsufficientSpokePoolBalanceToExecuteLeaf();
error NoRelayerRefundToClaim();
}
27 changes: 27 additions & 0 deletions contracts/test/ExpandedERC20WithBlacklist.sol
Original file line number Diff line number Diff line change
@@ -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);
}
}
8 changes: 7 additions & 1 deletion storage-layouts/Arbitrum_SpokePool.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
8 changes: 7 additions & 1 deletion storage-layouts/Base_SpokePool.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
8 changes: 7 additions & 1 deletion storage-layouts/Blast_SpokePool.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
8 changes: 7 additions & 1 deletion storage-layouts/Ethereum_SpokePool.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
8 changes: 7 additions & 1 deletion storage-layouts/Linea_SpokePool.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
8 changes: 7 additions & 1 deletion storage-layouts/Mode_SpokePool.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
8 changes: 7 additions & 1 deletion storage-layouts/Optimism_SpokePool.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
8 changes: 7 additions & 1 deletion storage-layouts/Polygon_SpokePool.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading
Loading