diff --git a/contracts/SpokePool.sol b/contracts/SpokePool.sol index b588b67e..a6e77d07 100644 --- a/contracts/SpokePool.sol +++ b/contracts/SpokePool.sol @@ -143,6 +143,10 @@ 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 * ****************************************/ @@ -490,9 +494,8 @@ 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 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. + * 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. * @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 @@ -531,9 +534,17 @@ 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 relayer deadline, - * which is the deadline for the exclusiveRelayer to fill the deposit. After this destination chain timestamp, - * anyone can fill the deposit. + * @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 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. */ @@ -548,7 +559,7 @@ abstract contract SpokePool is address exclusiveRelayer, uint32 quoteTimestamp, uint32 fillDeadline, - uint32 exclusivityPeriod, + uint32 exclusivityParameter, bytes calldata message ) public payable override nonReentrant unpausedDeposits { _depositV3( @@ -566,7 +577,7 @@ abstract contract SpokePool is numberOfDeposits++, quoteTimestamp, fillDeadline, - uint32(getCurrentTime()) + exclusivityPeriod, + exclusivityParameter, message ); } @@ -600,7 +611,7 @@ abstract contract SpokePool is * @param exclusiveRelayer See identically named parameter in depositV3() comments. * @param quoteTimestamp See identically named parameter in depositV3() comments. * @param fillDeadline See identically named parameter in depositV3() comments. - * @param exclusivityPeriod See identically named parameter in depositV3() comments. + * @param exclusivityParameter See identically named parameter in depositV3() comments. * @param message See identically named parameter in depositV3() comments. */ function unsafeDepositV3( @@ -615,7 +626,7 @@ abstract contract SpokePool is uint256 depositNonce, uint32 quoteTimestamp, uint32 fillDeadline, - uint32 exclusivityPeriod, + uint32 exclusivityParameter, bytes calldata message ) public payable nonReentrant unpausedDeposits { // @dev Create the uint256 deposit ID by concatenating the msg.sender and depositor address with the inputted @@ -636,7 +647,7 @@ abstract contract SpokePool is depositId, quoteTimestamp, fillDeadline, - uint32(getCurrentTime()) + exclusivityPeriod, + exclusivityParameter, message ); } @@ -857,7 +868,9 @@ 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. + * 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. * - 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 @@ -872,7 +885,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.exclusiveRelayer, relayData.exclusivityDeadline, uint32(getCurrentTime())) && + _fillIsExclusive(relayData.exclusivityDeadline, uint32(getCurrentTime())) && relayData.exclusiveRelayer != msg.sender ) { revert NotExclusiveRelayer(); @@ -918,7 +931,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.exclusiveRelayer, relayData.exclusivityDeadline, uint32(getCurrentTime())) && + _fillIsExclusive(relayData.exclusivityDeadline, uint32(getCurrentTime())) && relayData.exclusiveRelayer != msg.sender ) { revert NotExclusiveRelayer(); @@ -967,7 +980,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.exclusiveRelayer, relayData.exclusivityDeadline, currentTime)) { + if (_fillIsExclusive(relayData.exclusivityDeadline, currentTime)) { revert NoSlowFillsInExclusivityWindow(); } if (relayData.fillDeadline < currentTime) revert ExpiredFillDeadline(); @@ -1165,7 +1178,7 @@ abstract contract SpokePool is uint256 depositId, uint32 quoteTimestamp, uint32 fillDeadline, - uint32 exclusivityDeadline, + 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 @@ -1192,6 +1205,32 @@ abstract contract SpokePool is // 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; + if (exclusivityParameter == 0) { + exclusivityDeadline = 0; + } else { + if (exclusivityParameter <= MAX_EXCLUSIVITY_PERIOD_SECONDS) { + exclusivityDeadline = uint32(currentTime) + exclusivityParameter; + } else { + exclusivityDeadline = exclusivityParameter; + } + + // 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. @@ -1535,13 +1574,9 @@ 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); + // Determine whether the exclusivityDeadline implies active exclusivity. + function _fillIsExclusive(uint32 exclusivityDeadline, uint32 currentTime) internal pure returns (bool) { + return exclusivityDeadline >= currentTime; } // Implementing contract needs to override this to ensure that only the appropriate cross chain admin can execute diff --git a/contracts/interfaces/V3SpokePoolInterface.sol b/contracts/interfaces/V3SpokePoolInterface.sol index 603838d2..326ee43a 100644 --- a/contracts/interfaces/V3SpokePoolInterface.sol +++ b/contracts/interfaces/V3SpokePoolInterface.sol @@ -222,7 +222,7 @@ interface V3SpokePoolInterface { error DisabledRoute(); error InvalidQuoteTimestamp(); error InvalidFillDeadline(); - error InvalidExclusivityDeadline(); + error InvalidExclusiveRelayer(); error MsgValueDoesNotMatchInputAmount(); error NotExclusiveRelayer(); error NoSlowFillsInExclusivityWindow(); diff --git a/test/evm/hardhat/SpokePool.Deposit.ts b/test/evm/hardhat/SpokePool.Deposit.ts index fd6b7a4e..5be855d1 100644 --- a/test/evm/hardhat/SpokePool.Deposit.ts +++ b/test/evm/hardhat/SpokePool.Deposit.ts @@ -25,6 +25,8 @@ import { amountReceived, MAX_UINT32, originChainId, + MAX_EXCLUSIVITY_OFFSET_SECONDS, + zeroAddress, } from "./constants"; const { AddressZero: ZERO_ADDRESS } = ethers.constants; @@ -457,6 +459,170 @@ describe("SpokePool Depositor Logic", async function () { ) ).to.not.be.reverted; }); + it("invalid exclusivity params", async function () { + const currentTime = await spokePool.getCurrentTime(); + + // If exclusive deadline is not zero, then exclusive relayer must be set. + await expect( + spokePool.connect(depositor).depositV3( + ...getDepositArgsFromRelayData({ + ...relayData, + exclusiveRelayer: zeroAddress, + exclusivityDeadline: 1, + }) + ) + ).to.be.revertedWith("InvalidExclusiveRelayer"); + await expect( + spokePool.connect(depositor).depositV3( + ...getDepositArgsFromRelayData({ + ...relayData, + exclusiveRelayer: zeroAddress, + exclusivityDeadline: MAX_EXCLUSIVITY_OFFSET_SECONDS, + }) + ) + ).to.be.revertedWith("InvalidExclusiveRelayer"); + await expect( + spokePool.connect(depositor).depositV3( + ...getDepositArgsFromRelayData({ + ...relayData, + exclusiveRelayer: zeroAddress, + exclusivityDeadline: MAX_EXCLUSIVITY_OFFSET_SECONDS + 1, + }) + ) + ).to.be.revertedWith("InvalidExclusiveRelayer"); + await expect( + spokePool.connect(depositor).depositV3( + ...getDepositArgsFromRelayData({ + ...relayData, + exclusiveRelayer: zeroAddress, + exclusivityDeadline: currentTime.sub(1), + }) + ) + ).to.be.revertedWith("InvalidExclusiveRelayer"); + await expect( + spokePool.connect(depositor).depositV3( + ...getDepositArgsFromRelayData({ + ...relayData, + exclusiveRelayer: zeroAddress, + exclusivityDeadline: currentTime.add(1), + }) + ) + ).to.be.revertedWith("InvalidExclusiveRelayer"); + await expect( + spokePool.connect(depositor).depositV3( + ...getDepositArgsFromRelayData({ + ...relayData, + exclusiveRelayer: zeroAddress, + exclusivityDeadline: 0, + }) + ) + ).to.not.be.reverted; + }); + it("exclusivity param is used as an offset", async function () { + const currentTime = (await spokePool.getCurrentTime()).toNumber(); + const fillDeadlineOffset = 1000; + const exclusivityDeadlineOffset = MAX_EXCLUSIVITY_OFFSET_SECONDS; + await expect( + spokePool.connect(depositor).depositV3( + ...getDepositArgsFromRelayData( + { + ...relayData, + exclusiveRelayer: depositor.address, + exclusivityDeadline: exclusivityDeadlineOffset, + }, + undefined, + currentTime + ) + ) + ) + .to.emit(spokePool, "V3FundsDeposited") + .withArgs( + relayData.inputToken, + relayData.outputToken, + relayData.inputAmount, + relayData.outputAmount, + destinationChainId, + // deposit ID is 0 for first deposit + 0, + currentTime, // quoteTimestamp should be current time + currentTime + fillDeadlineOffset, // fillDeadline should be current time + offset + currentTime + exclusivityDeadlineOffset, // exclusivityDeadline should be current time + offset + relayData.depositor, + relayData.recipient, + depositor.address, + relayData.message + ); + }); + it("exclusivity param is used as a timestamp", async function () { + const currentTime = (await spokePool.getCurrentTime()).toNumber(); + const fillDeadlineOffset = 1000; + const exclusivityDeadlineTimestamp = MAX_EXCLUSIVITY_OFFSET_SECONDS + 1; + await expect( + spokePool.connect(depositor).depositV3( + ...getDepositArgsFromRelayData( + { + ...relayData, + exclusiveRelayer: depositor.address, + exclusivityDeadline: exclusivityDeadlineTimestamp, + }, + undefined, + currentTime + ) + ) + ) + .to.emit(spokePool, "V3FundsDeposited") + .withArgs( + relayData.inputToken, + relayData.outputToken, + relayData.inputAmount, + relayData.outputAmount, + destinationChainId, + // deposit ID is 0 for first deposit + 0, + currentTime, // quoteTimestamp should be current time + currentTime + fillDeadlineOffset, // fillDeadline should be current time + offset + exclusivityDeadlineTimestamp, // exclusivityDeadline should be passed in time + relayData.depositor, + relayData.recipient, + depositor.address, + relayData.message + ); + }); + it("exclusivity param is set to 0", async function () { + const currentTime = (await spokePool.getCurrentTime()).toNumber(); + const fillDeadlineOffset = 1000; + const zeroExclusivity = 0; + await expect( + spokePool.connect(depositor).depositV3( + ...getDepositArgsFromRelayData( + { + ...relayData, + exclusiveRelayer: depositor.address, + exclusivityDeadline: zeroExclusivity, + }, + undefined, + currentTime + ) + ) + ) + .to.emit(spokePool, "V3FundsDeposited") + .withArgs( + relayData.inputToken, + relayData.outputToken, + relayData.inputAmount, + relayData.outputAmount, + destinationChainId, + // deposit ID is 0 for first deposit + 0, + currentTime, // quoteTimestamp should be current time + currentTime + fillDeadlineOffset, // fillDeadline should be current time + offset + 0, // Exclusivity deadline should always be 0 + relayData.depositor, + relayData.recipient, + depositor.address, + relayData.message + ); + }); it("if input token is WETH and msg.value > 0, msg.value must match inputAmount", async function () { await expect( spokePool @@ -527,7 +693,7 @@ describe("SpokePool Depositor Logic", async function () { 0, currentTime, // quoteTimestamp should be current time currentTime + fillDeadlineOffset, // fillDeadline should be current time + offset - currentTime, + 0, relayData.depositor, relayData.recipient, relayData.exclusiveRelayer, @@ -535,7 +701,6 @@ describe("SpokePool Depositor Logic", async function () { ); }); it("emits V3FundsDeposited event with correct deposit ID", async function () { - const currentTime = (await spokePool.getCurrentTime()).toNumber(); await expect(spokePool.connect(depositor).depositV3(...depositArgs)) .to.emit(spokePool, "V3FundsDeposited") .withArgs( @@ -548,7 +713,7 @@ describe("SpokePool Depositor Logic", async function () { 0, quoteTimestamp, relayData.fillDeadline, - currentTime, + 0, relayData.depositor, relayData.recipient, relayData.exclusiveRelayer, @@ -560,7 +725,6 @@ describe("SpokePool Depositor Logic", async function () { expect(await spokePool.numberOfDeposits()).to.equal(1); }); it("tokens are always pulled from caller, even if different from specified depositor", async function () { - const currentTime = (await spokePool.getCurrentTime()).toNumber(); const balanceBefore = await erc20.balanceOf(depositor.address); const newDepositor = randomAddress(); await expect( @@ -578,7 +742,7 @@ describe("SpokePool Depositor Logic", async function () { 0, quoteTimestamp, relayData.fillDeadline, - currentTime, + 0, // New depositor newDepositor, relayData.recipient, @@ -598,7 +762,6 @@ describe("SpokePool Depositor Logic", async function () { ); }); it("unsafe deposit ID", async function () { - const currentTime = (await spokePool.getCurrentTime()).toNumber(); // new deposit ID should be the uint256 equivalent of the keccak256 hash of packed {msg.sender, depositor, forcedDepositId}. const forcedDepositId = "99"; const expectedDepositId = BigNumber.from( @@ -629,7 +792,7 @@ describe("SpokePool Depositor Logic", async function () { expectedDepositId, quoteTimestamp, relayData.fillDeadline, - currentTime, + 0, recipient.address, relayData.recipient, relayData.exclusiveRelayer, diff --git a/test/evm/hardhat/SpokePool.Relay.ts b/test/evm/hardhat/SpokePool.Relay.ts index 88c0aeaf..88ca7de9 100644 --- a/test/evm/hardhat/SpokePool.Relay.ts +++ b/test/evm/hardhat/SpokePool.Relay.ts @@ -349,26 +349,6 @@ describe("SpokePool Relayer Logic", async function () { ) ).to.not.be.reverted; }); - it("if no exclusive relayer is set, exclusivity deadline can be in future", async function () { - const _relayData = { - ...relayData, - // Overwrite exclusivity deadline - exclusivityDeadline: relayData.fillDeadline, - }; - - // Can send it after exclusivity deadline - await expect(spokePool.connect(relayer).fillV3Relay(_relayData, consts.repaymentChainId)).to.not.be.reverted; - }); - it("can have empty exclusive relayer before exclusivity deadline", async function () { - const _relayData = { - ...relayData, - // Overwrite exclusivity deadline - exclusivityDeadline: relayData.fillDeadline, - }; - - // Can send it before exclusivity deadline if exclusive relayer is empty - await expect(spokePool.connect(relayer).fillV3Relay(_relayData, consts.repaymentChainId)).to.not.be.reverted; - }); it("calls _fillRelayV3 with expected params", async function () { await expect(spokePool.connect(relayer).fillV3Relay(relayData, consts.repaymentChainId)) .to.emit(spokePool, "FilledV3Relay") @@ -420,7 +400,7 @@ describe("SpokePool Relayer Logic", async function () { spokePool.connect(relayer).fillV3RelayWithUpdatedDeposit( { ...relayData, - exclusiveRelayer: consts.zeroAddress, + exclusivityDeadline: 0, }, consts.repaymentChainId, updatedOutputAmount, diff --git a/test/evm/hardhat/SpokePool.SlowRelay.ts b/test/evm/hardhat/SpokePool.SlowRelay.ts index 4a0ded71..c72dcad0 100644 --- a/test/evm/hardhat/SpokePool.SlowRelay.ts +++ b/test/evm/hardhat/SpokePool.SlowRelay.ts @@ -54,9 +54,10 @@ describe("SpokePool Slow Relay Logic", async function () { 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"); + await expect(spokePool.connect(relayer).requestV3SlowFill({ ...relayData, exclusivityDeadline: 0 })).to.emit( + spokePool, + "RequestedV3SlowFill" + ); }); it("during exclusivity deadline", async function () { await spokePool.setCurrentTime(relayData.exclusivityDeadline); diff --git a/test/evm/hardhat/constants.ts b/test/evm/hardhat/constants.ts index cb4bf6de..14c2f607 100644 --- a/test/evm/hardhat/constants.ts +++ b/test/evm/hardhat/constants.ts @@ -100,6 +100,8 @@ export const l1TokenTransferThreshold = toWei(100); export const MAX_UINT32 = BigNumber.from("0xFFFFFFFF"); +export const MAX_EXCLUSIVITY_OFFSET_SECONDS = 24 * 60 * 60 * 365; // 1 year + // DAI's Rate model. export const sampleRateModel = { UBar: toWei(0.8).toString(),