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

Improve TDex flow (discovery & tdex registry) #11

Merged
merged 6 commits into from
Dec 8, 2021

Conversation

louisinger
Copy link
Contributor

@louisinger louisinger commented Dec 6, 2021

This PR improves the tdex flow in the app:

  • fetch providers (and markets) from registry (use a new Svelte store: tdexstore.ts).
  • handle all assets, fetch Infos from explorer (new svelte store: coinstore.ts).
  • use Discoverer to select best orders from providers (Trade.svelte).
  • use the marina UTXO events to get and delete utxos instead of marina.getCoins call.

it reuses the functions of tdex-app (see utils/tdex.ts file)

Note that this PR does not add support for testnet or regtest (does it make sense on dex.vulpem.com?). And do not add any new "visual elements" like the number of providers/markets available etc... (may be useful ??).

it closes #4

@tiero @bordalix please review

@tiero
Copy link
Member

tiero commented Dec 6, 2021

Let's have @bordalix to review as well :)

@tiero tiero requested a review from bordalix December 6, 2021 17:02
@louisinger louisinger requested a review from bordalix December 7, 2021 09:09
Copy link
Contributor

@bordalix bordalix left a comment

Choose a reason for hiding this comment

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

ACK.

My only issue is not showing the user the true reason for the error.

Copy link
Contributor

@bordalix bordalix left a comment

Choose a reason for hiding this comment

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

Yes, I also prefer to have all the constants on the same file.

@louisinger louisinger requested a review from tiero December 8, 2021 10:34
@tiero tiero merged commit 46ca6fe into vulpemventures:master Dec 8, 2021
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.

Use Discoverer class instead of hardcoding two providers
3 participants