Skip to content
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.

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

fix(SpokePool): Enforce exclusivity consistently #649

Merged
merged 8 commits into from
Oct 30, 2024
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
28 changes: 21 additions & 7 deletions contracts/SpokePool.sol
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ abstract contract SpokePool is
/**
* @notice Pauses deposit-related functions. This is intended to be used if this contract is deprecated or when
* something goes awry.
* @dev Affects `deposit()` but not `speedUpDeposit()`, so that existing deposits can be sped up and still
* @dev Affects `deposit()` but not `speedUpV3Deposit()`, so that existing deposits can be sped up and still
* relayed.
* @param pause true if the call is meant to pause the system, false if the call is meant to unpause it.
*/
Expand Down Expand Up @@ -839,9 +839,8 @@ 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 (
relayData.exclusiveRelayer != msg.sender &&
relayData.exclusivityDeadline >= getCurrentTime() &&
relayData.exclusiveRelayer != address(0)
_fillIsExclusive(relayData.exclusiveRelayer, relayData.exclusivityDeadline, uint32(getCurrentTime())) &&
relayData.exclusiveRelayer != msg.sender
) {
revert NotExclusiveRelayer();
}
Expand Down Expand Up @@ -885,7 +884,10 @@ abstract contract SpokePool is
) public override nonReentrant unpausedFills {
// Exclusivity deadline is inclusive and is the latest timestamp that the exclusive relayer has sole right
// to fill the relay.
if (relayData.exclusivityDeadline >= getCurrentTime() && relayData.exclusiveRelayer != msg.sender) {
if (
_fillIsExclusive(relayData.exclusiveRelayer, relayData.exclusivityDeadline, uint32(getCurrentTime())) &&
relayData.exclusiveRelayer != msg.sender
) {
revert NotExclusiveRelayer();
}

Expand Down Expand Up @@ -927,12 +929,15 @@ abstract contract SpokePool is
* then Across will not include a slow fill for the intended deposit.
*/
function requestV3SlowFill(V3RelayData calldata relayData) public override nonReentrant unpausedFills {
uint32 currentTime = uint32(getCurrentTime());
// If a depositor has set an exclusivity deadline, then only the exclusive relayer should be able to
// 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 (relayData.exclusivityDeadline >= getCurrentTime()) revert NoSlowFillsInExclusivityWindow();
if (relayData.fillDeadline < getCurrentTime()) revert ExpiredFillDeadline();
if (_fillIsExclusive(relayData.exclusiveRelayer, relayData.exclusivityDeadline, currentTime)) {
revert NoSlowFillsInExclusivityWindow();
}
if (relayData.fillDeadline < currentTime) revert ExpiredFillDeadline();

bytes32 relayHash = _getV3RelayHash(relayData);
if (fillStatuses[relayHash] != uint256(FillStatus.Unfilled)) revert InvalidSlowFillRequest();
Expand Down Expand Up @@ -1408,6 +1413,15 @@ abstract contract SpokePool is
}
}

// 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
// certain admin functions. For L2 contracts, the cross chain admin refers to some L1 address or contract, and for
// L1, this would just be the same admin of the HubPool.
Expand Down
1 change: 0 additions & 1 deletion contracts/interfaces/V3SpokePoolInterface.sol
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,6 @@ interface V3SpokePoolInterface {
error DisabledRoute();
error InvalidQuoteTimestamp();
error InvalidFillDeadline();
error InvalidExclusiveRelayer();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Driveby change - this error is unused. It was dropped with the recent changes to permit relative exclusivity.

error InvalidExclusivityDeadline();
error MsgValueDoesNotMatchInputAmount();
error NotExclusiveRelayer();
Expand Down
17 changes: 17 additions & 0 deletions test/evm/hardhat/SpokePool.Relay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,23 @@ describe("SpokePool Relayer Logic", async function () {
updatedMessage
);
});
it("in absence of exclusivity", async function () {
// Clock drift between spokes can mean exclusivityDeadline is in future even when no exclusivity was applied.
await spokePool.setCurrentTime(relayData.exclusivityDeadline - 1);
await expect(
spokePool.connect(relayer).fillV3RelayWithUpdatedDeposit(
{
...relayData,
exclusiveRelayer: consts.zeroAddress,
},
consts.repaymentChainId,
updatedOutputAmount,
updatedRecipient,
updatedMessage,
signature
)
).to.emit(spokePool, "FilledV3Relay");
});
it("must be exclusive relayer before exclusivity deadline", async function () {
const _relayData = {
...relayData,
Expand Down
7 changes: 7 additions & 0 deletions test/evm/hardhat/SpokePool.SlowRelay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ describe("SpokePool Slow Relay Logic", async function () {
relayData.fillDeadline = (await spokePool.getCurrentTime()).sub(1);
await expect(spokePool.connect(relayer).requestV3SlowFill(relayData)).to.be.revertedWith("ExpiredFillDeadline");
});
it("in absence of exclusivity", async function () {
// Clock drift between spokes can mean exclusivityDeadline is in future even when no exclusivity was applied.
await spokePool.setCurrentTime(relayData.exclusivityDeadline - 1);
await expect(
spokePool.connect(relayer).requestV3SlowFill({ ...relayData, exclusiveRelayer: consts.zeroAddress })
).to.emit(spokePool, "RequestedV3SlowFill");
});
it("during exclusivity deadline", async function () {
await spokePool.setCurrentTime(relayData.exclusivityDeadline);
await expect(spokePool.connect(relayer).requestV3SlowFill(relayData)).to.be.revertedWith(
Expand Down
Loading