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

improve(SpokePool): Apply rounding consistently to partial and full fills #268

Merged
merged 1 commit into from
Apr 11, 2023

Conversation

nicholaspai
Copy link
Member

@nicholaspai nicholaspai commented Mar 30, 2023

Rounding errors are possible when computing fillAmountPreFees and we want to enforce that the error is considered in amountToSend both to partial and full fills. Currently amountToSend is only reset to its postFee amount for situations where maxTokensToSend > amountRemainingInRelay which is often the case for slow fills. We should apply the error consistently regardless of the type of fill.

…ills

Currently the `amountToSend` could have rounding errors based on the division in `_computeAmountPreFees` and `_computeAmountPostFees`. The rounding errors are applied differently for partial fills and full fills, the latter of which only applies the preFees computation, while the former applies both preFees and postFees.

This PR enforces consistently both the preFees and postFees computations to `amountToSend`
@nicholaspai nicholaspai marked this pull request as ready for review March 30, 2023 15:29
@nicholaspai nicholaspai requested a review from mrice32 March 30, 2023 15:44
Copy link
Contributor

@mrice32 mrice32 left a comment

Choose a reason for hiding this comment

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

LGTM!

@nicholaspai nicholaspai merged commit cb137f6 into master Apr 11, 2023
@nicholaspai nicholaspai deleted the rounding branch April 11, 2023 14:24
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