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

refactor(sdk): amm #217

Merged
merged 17 commits into from
Nov 7, 2022
Merged

refactor(sdk): amm #217

merged 17 commits into from
Nov 7, 2022

Conversation

lidatong
Copy link
Collaborator

@lidatong lidatong commented Oct 31, 2022

This PR aims to simplify the SDK's AMM API to be more self-documenting and obvious.

The previous API had some awkwardness around static methods / when state would get refreshed, etc. It required reading the pool from the chain just to initialize (vs. being able to just construct a pool with uninitialized state)

This refactor makes it so everything is a method and tries to separate querying metadata about the pool and the actual pool's state.

const pool = auxClient.pool({ coinTypeX: auxCoin, coinTypeY: btcCoin });

// alternatively...
// no such thing as package private constructors in typescript or would prefer that...
const pool = new Pool(auxClient, { coinTypeX: auxCoin, coinTypeY: btcCoin })

// reading the pool
const poolInfo = pool.info();  // memoizes in instance var
const poolState = pool.state();  // does not memoize

// everything is now a method, including creating the pool
if (pool.info() === undefined) {
    pool.create(...)
}

pool.addLiquidity(...)

// the idea is the pool type name, coin infos, etc. won't change
// whereas the reserves, events certainly will

@lidatong lidatong marked this pull request as draft October 31, 2022 02:37
@lidatong lidatong force-pushed the refactor/aux-client branch 2 times, most recently from d8616d8 to 76cff7d Compare November 4, 2022 01:08
Base automatically changed from refactor/aux-client to main November 4, 2022 01:09
@lidatong lidatong force-pushed the refactor/amm branch 2 times, most recently from a4c3b4f to 9ecb291 Compare November 6, 2022 04:14
@lidatong lidatong marked this pull request as ready for review November 6, 2022 04:14
@lidatong lidatong force-pushed the refactor/amm branch 3 times, most recently from ae6eb07 to a3fa721 Compare November 6, 2022 04:23
Copy link
Collaborator

@leina05 leina05 left a comment

Choose a reason for hiding this comment

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

Overall looks much cleaner, thanks! Will be great to eventually migrate other functionality to this structure too. Will approve once all tests are refactored.

Copy link
Collaborator

@leina05 leina05 left a comment

Choose a reason for hiding this comment

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

All tests passing, nice!

@lidatong lidatong merged commit 7266591 into main Nov 7, 2022
@lidatong lidatong deleted the refactor/amm branch November 7, 2022 21:59
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.

None yet

2 participants