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

Liquidity Manager #66

Merged
merged 41 commits into from
Jan 8, 2025
Merged

Liquidity Manager #66

merged 41 commits into from
Jan 8, 2025

Conversation

carlosfebres
Copy link
Contributor

No description provided.

@carlosfebres carlosfebres self-assigned this Dec 6, 2024
@carlosfebres carlosfebres marked this pull request as ready for review December 8, 2024 00:06
@carlosfebres carlosfebres requested a review from StoyanD December 8, 2024 00:06
Copy link
Member

@StoyanD StoyanD left a comment

Choose a reason for hiding this comment

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

Reviewed. Left most as comments. Would like you to also structure the liquidity folder the way the rest of the project is, particularly the bullmq/queue/processor objects should be outside that dir. also you need to add tests for all your services and logic

package.json Outdated Show resolved Hide resolved
Comment on lines +70 to +73
case TokenState.SURPLUS:
return balance.current - balance.target
case TokenState.DEFICIT:
return balance.target - balance.current
Copy link
Member

Choose a reason for hiding this comment

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

would Math.abs() work better here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those values are BigInts and Math.abs doesn't support them

Copy link
Member

Choose a reason for hiding this comment

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

lol, didnt know that, can add a util like

export class Mathb {
  static abs(x: bigint): bigint {
    return x < 0 ? -x : x;
  }
}

src/liquidity-manager/utils/queue.ts Outdated Show resolved Hide resolved
src/balance/balance.service.ts Outdated Show resolved Hide resolved
import { TokenBalance, TokenConfig } from '@/balance/types'
import * as LiFi from '@lifi/sdk'

type TokenState = 'DEFICIT' | 'SURPLUS' | 'IN_RANGE'
Copy link
Member

Choose a reason for hiding this comment

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

u define this as an enum src/liquidity-manager/types/token-state.enum.ts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, enums couldn't be used from type definition files.

Copy link
Member

Choose a reason for hiding this comment

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

cant you define the type from the enum like so:

type TokenStateType = keyof typeof TokenState

that way you dont duplicate keys and risk forgetting to update both in the future

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think that'd be an issue, if an item in the enum doesn't match the TokenState type TypeScript will return an error.

Copy link
Member

Choose a reason for hiding this comment

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

yea but y have 2 places where u need to update?

src/liquidity-manager/utils/serialize.spec.ts Show resolved Hide resolved
StoyanD and others added 2 commits December 18, 2024 20:31
refactoring code and mongo record for new AUDIT protocol
@aromanoEco aromanoEco added the good first issue Good for newcomers label Jan 7, 2025
* move bigint conversion

* disabling storage prover from being accepted by solver
package.json Outdated Show resolved Hide resolved
* move bigint conversion

* disabling storage prover from being accepted by solver

* add sameChainFulfill check to validate intent
* move bigint conversion

* disabling storage prover from being accepted by solver

* updating routes-ts to ~0.2.10-beta update only on patch
* move bigint conversion

* moving watch services into own module. adding watch fulfill intent service

* creating watch fulfillment event

* refactoring watch create intent to use same abstract parent

* adding tests for watch fulfillment
adding inbox processor
adding intent utils method for update - > needs more work and tests

* adding default intervals, intentconfigs, and liquiditymanager to config defaults.ts
removing eventemitter from app modules
adding interval modeule and service
adding interval processor and queue
adding retry_intent to source intent queue

* adding skeleton of retry infeasable

* disabling storage prover from being accepted by solver

* updating routes-ts to ~0.2.10-beta update only on patch

* fixing merge

* adding retry infeasable intents tests
* adding balance api endpoint
adding cache manager to project

* adding cache configs to configs and defaults

* linter
@StoyanD StoyanD merged commit 907d236 into main Jan 8, 2025
3 checks passed
@StoyanD StoyanD deleted the carlos/liquidity-manager branch January 8, 2025 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants