-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Unit tests for AMM offer overflow fix #4986
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to add expected offer balances? This way we can see the amounts going via AMM/LOB.
@gregtatcam I agree, please view it here: Bronek#1 Qualitatively, the LOB offers are being used when one side of the AMM is being overwhelmed by However, I'm not familiar enough with the synthetic offer generation process to validate the actual numbers myself. Let me know if I can improve the validation further. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test looks good to me as it is. If you want to enhance it, I'm good with that too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #4986 +/- ##
===========================================
+ Coverage 76.97% 76.98% +0.01%
===========================================
Files 1129 1129
Lines 131958 132026 +68
Branches 39576 39604 +28
===========================================
+ Hits 101570 101639 +69
- Misses 24423 24447 +24
+ Partials 5965 5940 -25 ☔ View full report in Codecov by Sentry. |
High Level Overview of Change
Unit tests for #4968
Context of Change
Type of Change
.gitignore
, formatting, dropping support for older tooling)