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.

Sign up for GitHub

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

Jump to bottom

[WIP] Stableswap Internal Audit Tracking Issue #3328

Closed
19 tasks done
AlpinYukseloglu opened this issue Nov 10, 2022 · 1 comment
Closed
19 tasks done

[WIP] Stableswap Internal Audit Tracking Issue #3328

AlpinYukseloglu opened this issue Nov 10, 2022 · 1 comment
Labels
C:stableswap C:x/gamm Changes, features and bugs related to the gamm module.

Comments

@AlpinYukseloglu
Copy link
Contributor

AlpinYukseloglu commented Nov 10, 2022 •
edited
Loading

Full Report: https://hackmd.io/juU2yglrRTyYJKJF2Tt8Pw

This tracking issue covers the list of issues that came up during our end-to-end internal review of the stableswap package in preparation for v13.

Release blocking (safety issues):

No safety risk but either completeness issue or odd behavior:

Minor/code hygiene:

  • Exit single asset join early for zero input (currently runs over negative target for 1-token input)
  • Fixed by [x/gamm][stableswap]: Clean up rounding for single-asset joins #3355
  • Align single-asset join portion of joinPoolSharesInternal with all-asset portion (update error messages, variable names, and error return value)
  • Update error message for max stableswap numAssets (currently says 2 instead of 8)
  • Add comment to maxIterations that says to keep above 256 for it to converge
  • (Code hygiene) JoinPool error check
  • (Code hygiene) Move exit pool state update into updatePoolLiquidityForExit
  • New error message to be used in validateScalingFactors
  • Validate pool liquidity after reordering in scaledSortedPoolReserves
  • (Code hygiene) Use p.GetTotalPoolLiquidity() instead of raw p.PoolLiquidity in getLiquidityIndexMap
  • Update cfmmConstantMultiNoV comment to reflect that we no longer use v term
  • (Code hygiene) Switch uses of vSumSquares to wSumSquares in cfmmConstantMulti and cfmmConstantMultiNoV
  • Ensure calculated output leads to valid pool state in SwapOutAmtGivenIn
  • Locally defined pool update function in calcSingleAssetJoinShares uses tokenIn instead of its passed in newLiquidity parameter (which is remains unused)
  • Add "purpose" line under each function in report as docs
@AlpinYukseloglu AlpinYukseloglu added C:x/gamm Changes, features and bugs related to the gamm module. C:stableswap labels Nov 10, 2022
This was referenced Nov 14, 2022
Closed
Merged
@AlpinYukseloglu
Copy link
Contributor Author

All issues have been addressed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C:stableswap C:x/gamm Changes, features and bugs related to the gamm module.
Projects
Osmosis Chain Development
Archived in project
Milestone
No milestone
Development

No branches or pull requests
1 participant
@AlpinYukseloglu