-
Notifications
You must be signed in to change notification settings - Fork 19
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
feat: implement router for LBP #565
Conversation
… for limits in the trade_execution
… distributed assets
) | ||
.map_err(|_| ExecutorError::Error(Error::<T>::Overflow.into()))?; | ||
|
||
Ok(calculated_in) //TODO: Double check with someone if this is correct |
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.
I am not sure about this. Although it seems it works with this according to the integration tests, I expected to remove the fee from this amount, like in calculate_sell
line 46, as the pool is responsible for sending the fee to the fee-collector
What do you guys think?
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.
checks out, looks the same as lbp impl, pls remove TODO
Crate versions that have been updated:
Runtime version has been increased. |
…e it work with tarpaulin code coverage tool
Codecov ReportBase: 79.24% // Head: 78.75% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #565 +/- ##
==========================================
- Coverage 79.24% 78.75% -0.49%
==========================================
Files 21 22 +1
Lines 2038 2104 +66
==========================================
+ Hits 1615 1657 +42
- Misses 423 447 +24
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
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.
i would prefer instead of rewriting existing tests to include LBP write new ones specifically for it
Description
Added implementation and integration tests for router in the context of LBP
Related Issue
Motivation and Context
How Has This Been Tested?
Checklist: