feat(smart-contracts): add stableswap support for provide liquidity #351

nseguias · 2024-05-06T14:19:15Z

Description and Motivation

This PR adds support for StableSwap pools when providing liquidity through the Pool Manager.

Related Issues

#311

Checklist:

I have read Migaloo's contribution guidelines.
My pull request has a sound title and description (not something vague like Update index.md)
All existing and new tests are passing.
I updated/added relevant documentation.
The code is formatted properly cargo fmt --all --.
Clippy doesn't report any issues cargo clippy -- -D warnings.
I have regenerated the schemas if needed cargo schema.

…so we can provide liquidity with 2, 3, or N assets

codecov · 2024-05-07T13:07:28Z

Codecov Report

Attention: Patch coverage is 83.91304% with 111 lines in your changes missing coverage. Please review.

Project coverage is 90.90%. Comparing base (1d6ffbc) to head (2f40957).
Report is 36 commits behind head on release/v2_contracts.

Files	Patch %	Lines
...ontracts/liquidity_hub/pool-manager/sim/src/lib.rs	25.45%	82 Missing ⚠️
...ontracts/liquidity_hub/pool-manager/src/helpers.rs	92.75%	20 Missing ⚠️
...quidity_hub/pool-manager/src/liquidity/commands.rs	64.70%	6 Missing ⚠️
...work/stableswap_3pool/src/stableswap_math/curve.rs	60.00%	2 Missing ⚠️
...ty_hub/pool-manager/src/tests/integration_tests.rs	99.64%	1 Missing ⚠️

Additional details and impacted files

@@                   Coverage Diff                    @@
##           release/v2_contracts     #351      +/-   ##
========================================================
- Coverage                 91.15%   90.90%   -0.26%     
========================================================
  Files                       279      280       +1     
  Lines                     31119    31830     +711     
========================================================
+ Hits                      28368    28936     +568     
- Misses                     2751     2894     +143

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…len()

0xFable

LGTM great work you did it exactly as I was thinking it needed to be done.
I think now with mimimal changes we can bump N_coins to offer more pools down the line.

0xFable · 2024-05-09T07:11:32Z

contracts/liquidity_hub/pool-manager/src/helpers.rs

@@ -18,15 +18,15 @@ const NEWTON_ITERATIONS: u64 = 32;

 // todo isn't this for the 3pool? shouldn't it be 3
 // the number of assets in the pool
-const N_COINS: Uint256 = Uint256::from_u128(2);
+pub const N_COINS: u8 = 3;


Nice, was hoping other than some calculation changes it would work like this.

using Uint256::from(pool_info.assets.len() as u128) now

0xFable · 2024-05-09T07:12:28Z

contracts/liquidity_hub/pool-manager/src/helpers.rs

+                if d > d_prev {
+                    if d.checked_sub(d_prev).unwrap() <= Uint256::one() {
+                        break;
+                    }


Recommend adding some unit tests here. Logic looks right but its complicated

All 3pool tests && proptests have been refactored and integrated into the pool-manager

0xFable · 2024-05-09T07:13:00Z

contracts/liquidity_hub/pool-manager/src/liquidity/commands.rs

+                if total_share == Uint128::zero() {
+                    // Make sure at least MINIMUM_LIQUIDITY_AMOUNT is deposited to mitigate the risk of the first
+                    // depositor preventing small liquidity providers from joining the pool
+                    let min_lp_token_amount = MINIMUM_LIQUIDITY_AMOUNT * Uint128::from(3u8);


I actually change it to work with N amount of assets let min_lp_token_amount = MINIMUM_LIQUIDITY_AMOUNT * Uint128::from(pool_assets.len() as u128)

0xFable · 2024-05-09T07:14:23Z

contracts/liquidity_hub/pool-manager/src/tests/integration_tests.rs

+            vec![
+                Coin {
+                    denom: "uwhale".to_string(),
+                    amount: Uint128::from(MINIMUM_LIQUIDITY_AMOUNT),


can you add a test or enhance this test to do a swap against the 3 pool

we're doing a swap a lil bit further down.

kerber0x

It looks good overall, however i think it won't work for a stableswap with 2 assets as there's N_COINS and other hardcoded value equal to 3. Also, the unit tests for computing the d and other values for the stableswap were not transferred from the stableswap code.

I'd suggest all these compute_d and related methods to be moved to a file called math.

contracts/liquidity_hub/pool-manager/src/helpers.rs

kerber0x · 2024-05-20T10:21:59Z

contracts/liquidity_hub/pool-manager/src/helpers.rs

+                if d > d_prev {
+                    if d.checked_sub(d_prev).unwrap() <= Uint256::one() {
+                        break;
+                    }


contracts/liquidity_hub/pool-manager/src/helpers.rs

contracts/liquidity_hub/pool-manager/src/liquidity/commands.rs

contracts/liquidity_hub/pool-manager/src/tests/integration_tests.rs

0xFable

Only re-reviewed changes since my last review.
Looks really good! I like how you were able to preserve what looked like deccent test coverage from the stableswap contract.

Some nice descriptive comments in here explaining the hard math too I love to see it
LGTM and great work 🚀

0xFable · 2024-05-29T07:47:15Z

contracts/liquidity_hub/pool-manager/src/helpers.rs

    offer_pool: Decimal256,
    ask_pool: Decimal256,
    amp: &u64,
    precision: u8,
 ) -> Result<Decimal256, ContractError> {
-    let n_coins = Decimal256::from_ratio(Uint256::from(N_COINS), Uint256::from_u128(1));
+    let n_coins_decimal = Decimal256::from_ratio(n_coins, Uint256::one());


Good change, N_COINS was only ever meant to be a placeholder CONSTANT, better to have it as an input like u did here and now its generalized to N

0xFable · 2024-05-29T07:48:07Z

contracts/liquidity_hub/pool-manager/src/helpers.rs

@@ -180,6 +192,7 @@ pub fn compute_swap(
            let offer_amount = Decimal256::decimal_with_precision(offer_amount, offer_precision)?;

            let new_pool = calculate_stableswap_y(
+                n_coins,


Good stuff, I am sure ill see it lower in the rewview but I think the N value here becomes very important for all calculcations now

0xFable · 2024-05-29T07:49:10Z

contracts/liquidity_hub/pool-manager/src/helpers.rs

+                    .fold(Uint256::zero(), |acc, x| acc.checked_add(x).unwrap());
+                let deposits_total: Uint256 = deposits
+                    .into_iter()
+                    .fold(Uint256::zero(), |acc, x| acc.checked_add(x).unwrap());


Im fine with you keeping the unwraps, I learned over my journey unwraps are actually sound enough if you are checking all the usual stuff, overflow errors will be fine to depend on as long as you have added them to the errors file.

0xFable · 2024-05-29T07:50:06Z

contracts/liquidity_hub/pool-manager/src/helpers.rs

@@ -688,26 +714,27 @@ pub fn compute_d(amp_factor: &u64, deposits: &Vec<Coin>) -> Option<Uint256> {
        // Newton's method to approximate D
        let mut d_prev: Uint256;
        let mut d: Uint256 = sum_x.into();
-        for amount in amount_times_coins.into_iter() {
-            for _ in 0..256 {


is this just like removing the outer loop? Cool if so

0xFable · 2024-05-29T07:51:20Z

contracts/liquidity_hub/pool-manager/src/helpers.rs

+        .checked_add(no_swap.into())
+        .unwrap();
+
+    // Solve for y by approximating: y**2 + b*y = c


good comments, we could maybe have even more of them but im happy with these.
This is super complex stuff and the code is in no way 'self-documenting'.. hate that term lol these comments are huge to help understand what all these purple functions are doing

contracts/liquidity_hub/pool-manager/src/liquidity/commands.rs

0xFable · 2024-05-30T15:51:48Z

Test with diff amp factors on our integration test PR branch

github-actions · 2024-05-31T11:20:38Z

Schema generation had missing jobs:

Please run just schemas locally and upload the generated schemas.

…er with N coins

…-> WIP

…/bonding-manager

Co-authored-by: 0xFable <[email protected]>

… defitions (#362)

) * fix: Rework all v2 contracts to use bonding-manager not whale-lair + Removes all references to whale-lair for V2 contracts + Updates all schemas and most unit tests + Start of integration test rework for incentive manager The tests rework will take a bigger effort/another pass * fix: Update reference to whale_lair

…for-lp

nseguias added 2 commits May 6, 2024 16:08

feat(smart-contracts): add stableswap support for provide liquidity

84ae06b

chore: please the lord clippy

4cb71f5

nseguias requested review from kerber0x, 0xFable, kaimen-sano and Sen-Com as code owners May 6, 2024 14:19

nseguias added 2 commits May 7, 2024 14:56

feat(smart-contracts): make provide liquidity for stableswap generic …

c14fb19

…so we can provide liquidity with 2, 3, or N assets

chore(smart-contracts): please the lord clippy

0176c1a

nseguias added 2 commits May 7, 2024 16:40

fix(smart-contracts): refactor to use generic n_coins using deposits.…

9b00cc2

…len()

chore(smart-contracts): remvoe unnecessary comments

9fe2631

0xFable approved these changes May 9, 2024

View reviewed changes

kerber0x requested changes May 20, 2024

View reviewed changes

0xFable approved these changes May 29, 2024

View reviewed changes

nseguias and others added 14 commits June 4, 2024 11:28

test(smart-contracts): add tests (including proptests) for pool manag…

7f3d4e5

…er with N coins

feat(smart-contracts): test math for pool manager

249d0bc

feat(smart-contracts): address PR comments

106de06

feat(smart-contracts): address PR comments

93bfa2d

feat(smart-contracts): address PR comments

ef3f1ca

fix(smart-contracts): update slippage tolerance to work with N coins …

45c7cf6

…-> WIP

fix(smart-contracts): update slippage tolerance to work with N coins

0ea5afd

chore: bump cosmwasm-* versions

045a8f1

refactor: remove the temporary FillRewardsCoin msg from the whalelair…

da16403

…/bonding-manager

refactor: abstract bank burn msg

a4bbed0

refactor: make epoch id u64 on epoch manager

6a5522e

test: use the bonding-manager in pool-manager tests

6c8ea5b

chore: remove factory flags from token_factory package

db23e50

refactor: bonding-manager

26472df

kerber0x and others added 24 commits June 4, 2024 11:35

refactor: move functions to proper folders

1f5d722

refactor: using upcoming_reward_bucket when filling rewards

f1b18fb

chore: add last_claimed_epoch when bonding

5de11ef

chore: add rewards query

76aa0f7

test: enable rewards test

ff9c23f

test: add more test coverage, to helpers and bonding

bcb7676

test: add unbond and withdraw tests

0e8c4e5

chore: fix broken tests

4b8f840

refactor: move a few functiosn to helpers.rs

9b124e3

refactor: cleanup state

55990d1

chore: final refactor

01cf973

test: use test_case for extract_pool_identifier unit test

64176dd

docs: add readme to bonding-manager

35aaa12

chore: fix PR comments

e26f79d

docs: tweak v2 architecture diagram

39b3bc7

docs(epoch-manager): add readme to epoch manager

ae0591c

docs(epoch-manager): add epoch details and hook mechanismm diagram

52bed91

docs(incentive-manager): add readme to incentive manager

15896a2

Co-authored-by: 0xFable <[email protected]>

docs(vault-manager): add readme for vault manager

daed937

Co-authored-by: 0xFable <[email protected]>

docs(bonding-manager): add diagram of reward distribution (#359)

b7ea839

docs(pool-manager): add readme to pool manager (#356)

2fda9be

docs(epopoch-manager): add missing docummentation to structs and enum…

78ca31f

… defitions (#362)

chore(repo): resolving dependency & package conflicts

8302636

kerber0x force-pushed the feat/stableswap-case-for-lp branch from 37aef80 to 8302636 Compare June 4, 2024 10:39

kerber0x added 2 commits June 4, 2024 11:54

test: fix min liquidity amount check for stableswap

750b02a

chore: merge branch 'release/v2_contracts' into feat/stableswap-case-…

2f40957

…for-lp

kerber0x approved these changes Jun 4, 2024

View reviewed changes

nseguias merged commit e6e6d2a into release/v2_contracts Jun 4, 2024
6 of 7 checks passed

nseguias deleted the feat/stableswap-case-for-lp branch June 4, 2024 16:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(smart-contracts): add stableswap support for provide liquidity #351

feat(smart-contracts): add stableswap support for provide liquidity #351

nseguias commented May 6, 2024

codecov bot commented May 7, 2024 •

edited

Loading

0xFable left a comment

0xFable May 9, 2024

nseguias May 23, 2024

0xFable May 9, 2024

kerber0x May 20, 2024

nseguias May 23, 2024

0xFable May 9, 2024

nseguias May 23, 2024

0xFable May 9, 2024

nseguias May 27, 2024

kerber0x left a comment

kerber0x May 20, 2024

0xFable left a comment

0xFable May 29, 2024

0xFable May 29, 2024

0xFable May 29, 2024

0xFable May 29, 2024

0xFable May 29, 2024

0xFable commented May 30, 2024

github-actions bot commented May 31, 2024

feat(smart-contracts): add stableswap support for provide liquidity #351

feat(smart-contracts): add stableswap support for provide liquidity #351

Conversation

nseguias commented May 6, 2024

Description and Motivation

Related Issues

Checklist:

codecov bot commented May 7, 2024 • edited Loading

Codecov Report

0xFable left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kerber0x left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xFable left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xFable commented May 30, 2024

github-actions bot commented May 31, 2024

codecov bot commented May 7, 2024 •

edited

Loading