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

feat: larger gamma #12

Merged
merged 135 commits into from
Jun 4, 2024
Merged

feat: larger gamma #12

merged 135 commits into from
Jun 4, 2024

Conversation

michwill
Copy link
Contributor

@michwill michwill commented Mar 26, 2024

Features implemented

  • Restore max gamma to MAX_GAMMA_SMALL 0.02 (as before) and even allow to go as far as MAX_GAMMA 0.199 with some more restrictive checks on convergence.
  • newton_D convergence limits are now symmetric.
  • Removal of xcp_oracle due to low usage and gas consumption.
  • More precise ramping condition (before there was some delay before you could start a new ramp).

Testing

  • Stateful testing was previously inefficient and broken, has been rebuilt from the ground up:
    • strategies.py now contains a collection of SearchStrategy that can be used for stateful testing (and fuzzing) instead of fixtures, to unlock all the hypothesis features (like statistics and shrinking) that were previously unavailable with injected pytest fixtures.
    • every single aspect of a pool is fuzzed, from the parameters to the fee receiver, from the decimals of the tokens to the moving average time window.
  • constants have been centralized in a single python file that is now used by all tests. Previously each file had its own constants reducing readbility.
  • bumped boa version
  • ci now has a separate job for stateful testing to avoid hitting the 6 hours limit.
  • moved fuzz_multicoin_curve (now simulator.py) to utils since it's not a proper test, but rather a simulation helper to find bounds. Lots of dead code has been removed.
  • more unit tests for ramping edge cases have been added (increase coverage).
  • minor changes in test (removed unused parts, better documentation).

In `ImbaalncedLiquidiytStateful` we start claiming admin fees, which can sometimes break the virtual price invariants.
After a long fight to get some reasonable tests to pass on this one, it feels almost out of scope to check so many conditions inside stateful testing. The pool can fail depending on too many parameters which make the test very tedious to write. Might come back to this in a later time but will leave a simpler check for now.
Before cases where one of the amounts was 0 would not run
restored imbalanced liquidity based on a percentage like it used to before, with a stricter limit on the imbalance, and created one more rule for fully imbalanced deposits.
assumption should be checked only if amount is non-zero
Testing managed to get an overflow with a number that was very close to one.
Added a list of presets that will be expanded in the future. This helps keep testing sane since drawing random parameters for the pool finds edge cases that are not relevant to production pools.
We only use decimals that we have actually seen in production
Refactored the pool strategy so that we can build a strategy that samples params from presets on top of it.
Goal is to remove as many assertions as possible as they hurt testing.
One coin deposits are already handled by `add_liquidity_imbalanced`.
Prices now don't go too low with the decimals because testing for shitcoins is not the goal here.
Copy link
Collaborator

@bout3fiddy bout3fiddy left a comment

Choose a reason for hiding this comment

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

LGTM. Very good work with reworking the tests and indeed not going to arbitrary A, gamma way definitely was the better approach.

@bout3fiddy bout3fiddy merged commit 3b0226a into main Jun 4, 2024
5 checks passed
@bout3fiddy bout3fiddy deleted the larger-gamma branch June 4, 2024 10:19
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.

4 participants