fix: remove A precision and clean up borked logic #1188
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Non convergence issue
The non convergence issue is annoying and sometimes extremly minor. We've seen cases where either D or stableswap calculation keep oscillating between n and n+2 for example.
This changes, which replicates the logic from the live 3pool from curve ensure that we always compute a value for both by taking the last value of the convergence.
Precision issue
After investigation, #1152 comes from using integer division in the calculation of D. This was replicated in the native python 3 model which mean that it' not coming from an issue in the implementation of the division of uint256.
This is replicating perfectly the behavior of the 3pool. It is possible though to add more precision in the calculation of D by multiplying A by 100 and dividing by this precision in a couple of places in the code.
Even setting this precision to 1 million is not enough to remove this off by one rounding issue. I've left the implementation in branch mat/stableswap_precision_issue
Based on the fact that increasing A precision does not seems to be doing anything for this particular issue, and that 3pool is already working fine with this implementation, we can just accept the current implementation.
This is also not really exploitable because of swap fees.
Purpose
Ensure we have convergence for pools that are ok.