improve(SpokePool): Prevent future deposit quoteTimestamps

Future quoteTimestamps create unusual challenges for the bots that
process Deposit events. Fundamentally, it's impossible to compute the
correct realizedLpFeePct for a deposit with a future quoteTimestamp, so
all implementations must correctly park these early deposits and pull
them out at the right time. Given that we are now seeing diversity in
relayer implementations, it seems improbable that all will implement
this correctly, and the downside is that they will lose funds for a
slight mistake here.

From the depositor side, for the most part it is hard to justify
deliberately setting a

The scenarios where this might harm users is:
 - A chain lags slightly behind the current time, meaning that deposits
   will fail due to very small time deltas.
 - A deposit from a multisig normally wants the widest possible time
   range to collect the necessary signatures, and is technically losing
   half of that window.

For the former, this could be managed by the API, which supplies the
suggested quoteTimestamp to the depositor. In cases where the chain has
stalled, the API could simply throw a descriptive error.

For the latter, this could be mitigated by doubling the current deposit
window.

contracts/SpokePool.sol

@@ -439,17 +439,16 @@ abstract contract SpokePool is
         require(amount <= MAX_TRANSFER_SIZE, "Amount too large");
         require(depositCounter[originToken] <= maxCount, "Above max count");
 
-        // This function assumes that L2 timing cannot be compared accurately and consistently to L1 timing. Therefore,
-        // block.timestamp is different from the L1 EVM's. Therefore, the quoteTimestamp must be within a configurable
-        // buffer of this contract's block time to allow for this variance.
-        // Note also that quoteTimestamp cannot be less than the buffer otherwise the following arithmetic can result
-        // in underflow. This isn't a problem as the deposit will revert, but the error might be unexpected for clients.
+        // 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 if this is undesirable. This will underflow if
+        // quoteTimestamp is less than depositQuoteTimeBuffer; this is safe but will throw an unintuitive error.
 
-        //slither-disable-next-line timestamp
+        // slither-disable-next-line timestamp
         require(
-            getCurrentTime() >= quoteTimestamp - depositQuoteTimeBuffer &&
-                getCurrentTime() <= quoteTimestamp + depositQuoteTimeBuffer,
-            "invalid quote time"
+            getCurrentTime() >= quoteTimestamp && getCurrentTime() <= quoteTimestamp + depositQuoteTimeBuffer,
+            "invalid quoteTimestamp"
         );
 
         // Increment count of deposits so that deposit ID for this spoke pool is unique.

test/SpokePool.Deposit.ts

@@ -288,7 +288,7 @@ describe("SpokePool Depositor Logic", async function () {
   });
 
   it("quoteTimestamp is out of range", async function () {
-    const revertReason = "invalid quote time";
+    const revertReason = "invalid quoteTimestamp";
     const quoteTimeBuffer = await spokePool.depositQuoteTimeBuffer();
 
     await expect(
@@ -301,7 +301,7 @@ describe("SpokePool Depositor Logic", async function () {
             amountToDeposit,
             destinationChainId,
             depositRelayerFeePct,
-            toBN(currentSpokePoolTime).add(quoteTimeBuffer + 1),
+            toBN(currentSpokePoolTime).add(1),
             maxUint256
           )
         )
@@ -323,22 +323,25 @@ describe("SpokePool Depositor Logic", async function () {
         )
     ).to.be.revertedWith(revertReason);
 
-    // quoteTimestamp at max age.
-    await expect(
-      spokePool
-        .connect(depositor)
-        .deposit(
-          ...getDepositParams(
-            recipient.address,
-            erc20.address,
-            amountToDeposit,
-            destinationChainId,
-            depositRelayerFeePct,
-            currentSpokePoolTime.sub(quoteTimeBuffer),
-            maxUint256
+    // quoteTimestamp at the exact margins should succeed.
+    for (const offset of [0, quoteTimeBuffer]) {
+      await erc20.connect(depositor).approve(spokePool.address, amountToDeposit);
+      await expect(
+        spokePool
+          .connect(depositor)
+          .deposit(
+            ...getDepositParams(
+              recipient.address,
+              erc20.address,
+              amountToDeposit,
+              destinationChainId,
+              depositRelayerFeePct,
+              currentSpokePoolTime.sub(offset),
+              maxUint256
+            )
           )
-        )
-    ).to.emit(spokePool, "FundsDeposited");
+      ).to.emit(spokePool, "FundsDeposited");
+    }
   });
 
   it("maxCount is too low", async function () {