Skip to content

Revert "improve(SpokePool): _depositV3 interprets exclusivityParameter as an offset, or a timestamp (#670)" #881

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 3, 2025
Merged

Revert "improve(SpokePool): _depositV3 interprets exclusivityParameter as an offset, or a timestamp (#670)" #881

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
187 changes: 62 additions & 125 deletions contracts/SpokePool.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,6 @@ abstract contract SpokePool is
// 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.
uint32 public constant INFINITE_FILL_DEADLINE = type(uint32).max;

// One year in seconds. If `exclusivityParameter` is set to a value less than this, then the emitted
// exclusivityDeadline in a deposit event will be set to the current time plus this value.
uint32 public constant MAX_EXCLUSIVITY_PERIOD_SECONDS = 31_536_000;
/****************************************
* EVENTS *
****************************************/
Expand Down Expand Up @@ -433,8 +429,9 @@ abstract contract SpokePool is
/**
* @notice Previously, this function allowed the caller to specify the exclusivityDeadline, otherwise known as the
* as exact timestamp on the destination chain before which only the exclusiveRelayer could fill the deposit. Now,
* the caller is expected to pass in a number that will be interpreted either as an offset or a fixed
* timestamp depending on its value.
* the caller is expected to pass in an exclusivityPeriod which is the number of seconds to be added to the
* block.timestamp to produce the exclusivityDeadline. This allows the caller to ignore any latency associated
* with this transaction being mined and propagating this transaction to the miner.
* @notice Request to bridge input token cross chain to a destination chain and receive a specified amount
* of output tokens. The fee paid to relayers and the system should be captured in the spread between output
* amount and input amount when adjusted to be denominated in the input token. A relayer on the destination
Expand Down Expand Up @@ -473,17 +470,9 @@ 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 exclusivityParameter This value is used to set the exclusivity deadline timestamp in the emitted deposit
* event. Before this destinationchain timestamp, only the exclusiveRelayer (if set to a non-zero address),
* can fill this deposit. There are three ways to use this parameter:
* 1. NO EXCLUSIVITY: If this value is set to 0, then a timestamp of 0 will be emitted,
* meaning that there is no exclusivity period.
* 2. OFFSET: If this value is less than MAX_EXCLUSIVITY_PERIOD_SECONDS, then add this value to
* the block.timestamp to derive the exclusive relayer deadline. Note that using the parameter in this way
* will expose the filler of the deposit to the risk that the block.timestamp of this event gets changed
* due to a chain-reorg, which would also change the exclusivity timestamp.
* 3. TIMESTAMP: Otherwise, set this value as the exclusivity deadline timestamp.
* which is the deadline for the exclusiveRelayer to fill the deposit.
* @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.
* If the message is not empty, the recipient contract must implement handleV3AcrossMessage() or the fill will revert.
*/
Expand All @@ -498,23 +487,62 @@ abstract contract SpokePool is
address exclusiveRelayer,
uint32 quoteTimestamp,
uint32 fillDeadline,
uint32 exclusivityParameter,
uint32 exclusivityPeriod,
bytes calldata message
) public payable override nonReentrant unpausedDeposits {
_depositV3(
depositor,
recipient,
// Check that deposit route is enabled for the input token. There are no checks required for the output token
// which is pulled from the relayer at fill time and passed through this contract atomically to the recipient.
if (!enabledDepositRoutes[inputToken][destinationChainId]) revert DisabledRoute();

// 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/fills 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
uint256 currentTime = getCurrentTime();
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.
// 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
// unless they are significantly out of sync or the depositor is setting very short fill deadlines. This latter
// situation won't be a problem for honest users.
if (fillDeadline < currentTime || fillDeadline > currentTime + fillDeadlineBuffer) revert InvalidFillDeadline();

// 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 the native token. In this case, the native token should be
// wrapped.
if (inputToken == address(wrappedNativeToken) && msg.value > 0) {
if (msg.value != inputAmount) revert MsgValueDoesNotMatchInputAmount();
wrappedNativeToken.deposit{ value: msg.value }();
// Else, it is a normal ERC20. In this case pull the token from the caller as per normal.
// Note: this includes the case where the L2 caller 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 {
// msg.value should be 0 if input token isn't the wrapped native token.
if (msg.value != 0) revert MsgValueDoesNotMatchInputAmount();
IERC20Upgradeable(inputToken).safeTransferFrom(msg.sender, address(this), inputAmount);
}

emit V3FundsDeposited(
inputToken,
outputToken,
inputAmount,
outputAmount,
destinationChainId,
exclusiveRelayer,
// Increment count of deposits so that deposit ID for this spoke pool is unique.
numberOfDeposits++,
quoteTimestamp,
fillDeadline,
exclusivityParameter,
uint32(currentTime) + exclusivityPeriod,
depositor,
recipient,
exclusiveRelayer,
message
);
}
Expand Down Expand Up @@ -735,9 +763,7 @@ abstract contract SpokePool is
* - fillDeadline The deadline for the caller to fill the deposit. After this timestamp,
* the fill will revert on the destination chain.
* - exclusivityDeadline: The deadline for the exclusive relayer to fill the deposit. After this
* timestamp, anyone can fill this deposit. Note that if this value was set in depositV3 by adding an offset
* to the deposit's block.timestamp, there is re-org risk for the caller of this method because the event's
* block.timestamp can change. Read the comments in `depositV3` about the `exclusivityParameter` for more details.
* timestamp, anyone can fill this deposit.
* - message The message to send to the recipient if the recipient is a contract that implements a
* handleV3AcrossMessage() public function
* @param repaymentChainId Chain of SpokePool where relayer wants to be refunded after the challenge window has
Expand All @@ -752,7 +778,7 @@ abstract contract SpokePool is
// Exclusivity deadline is inclusive and is the latest timestamp that the exclusive relayer has sole right
// to fill the relay.
if (
_fillIsExclusive(relayData.exclusivityDeadline, uint32(getCurrentTime())) &&
_fillIsExclusive(relayData.exclusiveRelayer, relayData.exclusivityDeadline, uint32(getCurrentTime())) &&
relayData.exclusiveRelayer != msg.sender
) {
revert NotExclusiveRelayer();
Expand Down Expand Up @@ -798,7 +824,7 @@ abstract contract SpokePool is
// Exclusivity deadline is inclusive and is the latest timestamp that the exclusive relayer has sole right
// to fill the relay.
if (
_fillIsExclusive(relayData.exclusivityDeadline, uint32(getCurrentTime())) &&
_fillIsExclusive(relayData.exclusiveRelayer, relayData.exclusivityDeadline, uint32(getCurrentTime())) &&
relayData.exclusiveRelayer != msg.sender
) {
revert NotExclusiveRelayer();
Expand Down Expand Up @@ -847,7 +873,7 @@ abstract contract SpokePool is
// fast fill within this deadline. Moreover, the depositor should expect to get *fast* filled within
// this deadline, not slow filled. As a simplifying assumption, we will not allow slow fills to be requested
// during this exclusivity period.
if (_fillIsExclusive(relayData.exclusivityDeadline, currentTime)) {
if (_fillIsExclusive(relayData.exclusiveRelayer, relayData.exclusivityDeadline, currentTime)) {
revert NoSlowFillsInExclusivityWindow();
}
if (relayData.fillDeadline < currentTime) revert ExpiredFillDeadline();
Expand Down Expand Up @@ -1022,99 +1048,6 @@ abstract contract SpokePool is
* INTERNAL FUNCTIONS *
**************************************/

function _depositV3(
address depositor,
address recipient,
address inputToken,
address outputToken,
uint256 inputAmount,
uint256 outputAmount,
uint256 destinationChainId,
address exclusiveRelayer,
uint32 depositId,
uint32 quoteTimestamp,
uint32 fillDeadline,
uint32 exclusivityParameter,
bytes calldata message
) internal {
// Check that deposit route is enabled for the input token. There are no checks required for the output token
// which is pulled from the relayer at fill time and passed through this contract atomically to the recipient.
if (!enabledDepositRoutes[inputToken][destinationChainId]) revert DisabledRoute();

// 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/fills 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
uint256 currentTime = getCurrentTime();
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.
// 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
// unless they are significantly out of sync or the depositor is setting very short fill deadlines. This latter
// situation won't be a problem for honest users.
if (fillDeadline < currentTime || fillDeadline > currentTime + fillDeadlineBuffer) revert InvalidFillDeadline();

// There are three cases for setting the exclusivity deadline using the exclusivity parameter:
// 1. If this parameter is 0, then there is no exclusivity period and emit 0 for the deadline. This
// means that fillers of this deposit do not have to worry about the block.timestamp of this event changing
// due to re-orgs when filling this deposit.
// 2. If the exclusivity parameter is less than or equal to MAX_EXCLUSIVITY_PERIOD_SECONDS, then the exclusivity
// deadline is set to the block.timestamp of this event plus the exclusivity parameter. This means that the
// filler of this deposit assumes re-org risk when filling this deposit because the block.timestamp of this
// event affects the exclusivity deadline.
// 3. Otherwise, interpret this parameter as a timestamp and emit it as the exclusivity deadline. This means
// that the filler of this deposit will not assume re-org risk related to the block.timestamp of this
// event changing.
uint32 exclusivityDeadline = exclusivityParameter;
if (exclusivityDeadline > 0) {
if (exclusivityDeadline <= MAX_EXCLUSIVITY_PERIOD_SECONDS) {
exclusivityDeadline += uint32(currentTime);
}

// As a safety measure, prevent caller from inadvertently locking funds during exclusivity period
// by forcing them to specify an exclusive relayer.
if (exclusiveRelayer == address(0)) revert InvalidExclusiveRelayer();
}

// 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 the native token. In this case, the native token should be
// wrapped.
if (inputToken == address(wrappedNativeToken) && msg.value > 0) {
if (msg.value != inputAmount) revert MsgValueDoesNotMatchInputAmount();
wrappedNativeToken.deposit{ value: msg.value }();
// Else, it is a normal ERC20. In this case pull the token from the caller as per normal.
// Note: this includes the case where the L2 caller 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 {
// msg.value should be 0 if input token isn't the wrapped native token.
if (msg.value != 0) revert MsgValueDoesNotMatchInputAmount();
IERC20Upgradeable(inputToken).safeTransferFrom(msg.sender, address(this), inputAmount);
}

emit V3FundsDeposited(
inputToken,
outputToken,
inputAmount,
outputAmount,
destinationChainId,
depositId,
quoteTimestamp,
fillDeadline,
exclusivityDeadline,
depositor,
recipient,
exclusiveRelayer,
message
);
}

function _deposit(
address depositor,
address recipient,
Expand Down Expand Up @@ -1426,9 +1359,13 @@ abstract contract SpokePool is
}
}

// Determine whether the exclusivityDeadline implies active exclusivity.
function _fillIsExclusive(uint32 exclusivityDeadline, uint32 currentTime) internal pure returns (bool) {
return exclusivityDeadline >= currentTime;
// Determine whether the combination of exlcusiveRelayer and exclusivityDeadline implies active exclusivity.
function _fillIsExclusive(
address exclusiveRelayer,
uint32 exclusivityDeadline,
uint32 currentTime
) internal pure returns (bool) {
return exclusivityDeadline >= currentTime && exclusiveRelayer != address(0);
}

// Implementing contract needs to override this to ensure that only the appropriate cross chain admin can execute
Expand Down
2 changes: 1 addition & 1 deletion contracts/interfaces/V3SpokePoolInterface.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ interface V3SpokePoolInterface {
error DisabledRoute();
error InvalidQuoteTimestamp();
error InvalidFillDeadline();
error InvalidExclusiveRelayer();
error InvalidExclusivityDeadline();
error MsgValueDoesNotMatchInputAmount();
error NotExclusiveRelayer();
error NoSlowFillsInExclusivityWindow();
Expand Down
Loading
Loading