proposal: ensure that EVM errors are always consistant on underflows (#720)

Author: chrismaree

contracts/SpokePool.sol

@@ -520,12 +520,11 @@ abstract contract SpokePool is
         // 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();
+        if (quoteTimestamp > currentTime || 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.

test/evm/hardhat/SpokePool.Deposit.ts

@@ -431,6 +431,12 @@ describe("SpokePool Depositor Logic", async function () {
           ...getDepositArgsFromRelayData(relayData, destinationChainId, currentTime.sub(quoteTimeBuffer).sub(1))
         )
       ).to.be.revertedWith("InvalidQuoteTimestamp");
+      await expect(
+        spokePool.connect(depositor)[depositV3Bytes](
+          // quoteTimestamp in the future should also revert with InvalidQuoteTimestamp
+          ...getDepositArgsFromRelayData(relayData, destinationChainId, currentTime.add(500))
+        )
+      ).to.be.revertedWith("InvalidQuoteTimestamp");
       await expect(
         spokePool.connect(depositor)[depositV3Bytes](
           // quoteTimestamp right at the buffer is OK