diff --git a/contracts/SpokePool.sol b/contracts/SpokePool.sol index a9720518a..10b9d8ed3 100644 --- a/contracts/SpokePool.sol +++ b/contracts/SpokePool.sol @@ -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. */ @@ -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(); } @@ -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(); } @@ -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(); @@ -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. diff --git a/contracts/interfaces/V3SpokePoolInterface.sol b/contracts/interfaces/V3SpokePoolInterface.sol index 0be81c630..d004fdfcc 100644 --- a/contracts/interfaces/V3SpokePoolInterface.sol +++ b/contracts/interfaces/V3SpokePoolInterface.sol @@ -222,7 +222,6 @@ interface V3SpokePoolInterface { error DisabledRoute(); error InvalidQuoteTimestamp(); error InvalidFillDeadline(); - error InvalidExclusiveRelayer(); error InvalidExclusivityDeadline(); error MsgValueDoesNotMatchInputAmount(); error NotExclusiveRelayer(); diff --git a/test/evm/hardhat/SpokePool.Relay.ts b/test/evm/hardhat/SpokePool.Relay.ts index ca35500d9..88c0aeaf3 100644 --- a/test/evm/hardhat/SpokePool.Relay.ts +++ b/test/evm/hardhat/SpokePool.Relay.ts @@ -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, diff --git a/test/evm/hardhat/SpokePool.SlowRelay.ts b/test/evm/hardhat/SpokePool.SlowRelay.ts index a8da51d20..4a0ded716 100644 --- a/test/evm/hardhat/SpokePool.SlowRelay.ts +++ b/test/evm/hardhat/SpokePool.SlowRelay.ts @@ -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(