Skip to content

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

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Nov 22, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3e34e74
WIP
chrismaree Nov 4, 2024
268af70
Merge branch 'master' into chrismaree/standardize-errors
chrismaree Nov 4, 2024
dd88eac
WIP
chrismaree Nov 4, 2024
a3df3c1
WIP
chrismaree Nov 4, 2024
f3f0c76
WIP
chrismaree Nov 4, 2024
162ce53
WIP
chrismaree Nov 4, 2024
c4cef8c
Merge branch 'svm-dev' into chrismaree/standardize-errors
chrismaree Nov 19, 2024
bb87650
Merge branch 'svm-dev' into chrismaree/standardize-errors
chrismaree Nov 21, 2024
e891bd7
WIP
chrismaree Nov 21, 2024
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
5 changes: 2 additions & 3 deletions contracts/SpokePool.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Copy link
Contributor

@Reinis-FRP Reinis-FRP Nov 4, 2024

Choose a reason for hiding this comment

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

need to update comment above as there should be no overflow anymore

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.
Expand Down
6 changes: 6 additions & 0 deletions test/evm/hardhat/SpokePool.Deposit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading