Skip to content
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.

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

Account for subtraction underflow in order creation #14

Merged
merged 2 commits into from
Feb 13, 2024

Conversation

fedgiac
Copy link
Collaborator

@fedgiac fedgiac commented Feb 12, 2024

This PR adds an explicit underflow check for the subtractions in the getTradeableOrder function.

This is done because a revert due to underflow during a watchtower call to getTradeableOrder would cause the watchtower to stop indexing AMM order, stopping the automated order creation on the orderbook until the order is deleted and then recreated.

Note that we're mainly concerned about subtraction overflows. We're leaving all multiplication and addition overflow there because they are only likely to occur when the amounts are so large that they are close to the amount the CoW AMM isn't able to handle by design. In the case of subtraction, an underflow could happen when the AMM reserves are extremely close to the reference price. This may occur by chance for pairs with tokens that have low number of decimals, like USDC.

How to test

New unit test. Note that I changed setUpDefaultData to getDefaultData when we set the reserves by hand because otherwise there'd be two mocks set for the same oracle/pair.

@fedgiac fedgiac force-pushed the avoid-reverting-for-subtraction-overflow branch from 060ec90 to f2190d4 Compare February 12, 2024 11:39
@fedgiac fedgiac requested a review from a team February 12, 2024 11:45
Copy link
Contributor

@mfw78 mfw78 left a comment

Choose a reason for hiding this comment

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

Minor nit, not affecting overall logic

src/ConstantProduct.sol Show resolved Hide resolved
@fedgiac fedgiac merged commit 1c78b0d into main Feb 13, 2024
3 checks passed
@fedgiac fedgiac deleted the avoid-reverting-for-subtraction-overflow branch February 13, 2024 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants