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

Use IBEX server for real time price #1

Open
Nodirbek75 opened this issue Sep 11, 2024 · 11 comments
Open

Use IBEX server for real time price #1

Nodirbek75 opened this issue Sep 11, 2024 · 11 comments
Assignees

Comments

@Nodirbek75
Copy link

Converted amount showing different in send-bitcoin-details-screen and receive screen
363949903-757f1077-5281-4bdc-ba76-c45be2b06939

@brh28
Copy link

brh28 commented Sep 12, 2024

Doing some preliminary research and I see two high-level approaches:

Option 1: Update Price Server to call Ibex.

What's required?

  • Add Ibex as an exchange to the price codebase. See CurrencyBeacon for a sample.
    For this approach, we should also consider whether we want to make our changes to the lnflash fork or the downstream GaloyMoney repo. Using lnflash means making our changes to the current codebase and building the new image ourselves. Using GaloyMoney would be mean rebasing and opening a PR into Galoy's codebase and then leveraging the Galoy CI to build the new image.
  • Update the Price config to enable Ibex and disable other exchanges.

It's also worth noting that the Ibex rates API requires authorization, similar to what we're doing in the graphql-api server. The code we write in the price codebase could either:

  • Re-implement authorization, potentially by extracting our Ibex code into it's own library
  • Fetch the existing accessToken from Redis
    The former requires more work but adds reusability. The latter is a quicker solution applied to our use case, but would also likely have occasional failures when the accessToken is expired, unless we add authorization handling.

Option 2: Call Ibex directly

Remove Price Server entirely and update all references to call Ibex directly. The advantage of this approach is it's less code and infrastructure to maintain on our part. The downside is there are potentially many references that need to be updated to our codebase and could introduce bugs if not tested thoroughly.

My thoughts

I lean towards option 1, as it isolates the changes and therefore less likely to effect the calling code (e.g api server, price-history cronjob, etc). Extracting the Ibex code to it's own library and then adding it to the downstream GaloyMoney/price codebase is likely the cleanest and most "out-in-the-open" approach (Ibex benefits having a client library, Galoy benefits having a Ibex as a potential exchange provider). And by using Galoy's codebase, we don't have to worry about CI, just updating our deployment configs with the new image & config.

@islandbitcoin
Copy link

Let's go with Option #1, making changes to the lnflash fork, build the new image ourselves, and storing the image on our own hosted registry (GCR or DigitalOcean?).

My preference is to give our ibex authorization code its own library and make it reusable/scalable.

@brh28
Copy link

brh28 commented Sep 16, 2024

Decouple tokens from graphql.

Use Ibex client library

@brh28
Copy link

brh28 commented Sep 20, 2024

@islandbitcoin Many of the currencies that we're fetching prices for are not available in the supported list of Ibex currencies.

  - { code: "XCD", symbol: "$", name: "East Caribbean Dollar", flag: "🇦🇬" }
  - { code: "ANG", symbol: "ƒ", name: "Netherlands Antillean Guilder", flag: "🇨🇼" }
  - { code: "BSD", symbol: "$", name: "Bahamian Dollar", flag: "🇧🇸" }
  - { code: "BBD", symbol: "$", name: "Barbadian Dollar", flag: "🇧🇧" }
  - { code: "BZD", symbol: "BZ$", name: "Belize Dollar", flag: "🇧🇿" }
  - { code: "KYD", symbol: "$", name: "Cayman Islands Dollar", flag: "🇰🇾" }
  - { code: "CUP", symbol: "$", name: "Cuban Peso", flag: "🇨🇺" }
  - { code: "DOP", symbol: "RD$", name: "Dominican Peso", flag: "🇩🇴" }
  - { code: "TTD", symbol: "TT$", name: "Trinidad and Tobago Dollar", flag: "🇹🇹" }

How do you want to proceed? Considering we aren't focusing on these customer bases to start, it may be easiest just to remove them.

@islandbitcoin
Copy link

islandbitcoin commented Sep 23, 2024

I think the more long term solution would be to add IBEX as one of two feeds, and if IBEX doesn't have it, we pull from the other feed if IBEX doesnt have it.

The best solution is to ask IBEX to add currencies as we go. They added JMD for me in a few days.

@brh28
Copy link

brh28 commented Sep 23, 2024

Price server does not currently support a fallback as you're describing, so we'd need to make those changes, and the codebase is a bit convoluted imo, so this may be a bit more involved than it would ideally be.

If Ibex is accommodating, that's certainly the easiest on our part.

@brh28
Copy link

brh28 commented Sep 26, 2024

Looking into our options after our discussion this morning. To my understanding, there two types of currencies we're interested in:

  1. Currencies that we actually hold & exchange
  2. Currencies that we want to display to users but do not hold/exchange

For type 1, we want to be synchronized as closely as possible with our exchange (e.g Ibex), so that invoices match expected amounts.

For type 2, we don't have a single source-of-truth, so we can fetch from one or more other price feeds.

Looking back at the price codebase, I actually think we can configure the price server to fetch the USD<->BTC rate using Ibex, but fetch a different set of rates using using a different set of exchanges by using the excludedQuotes field. I haven't fully verified the results, but the logs at least seem to indicate there aren't errors and the expected apis are being called.

Also worth noting is fetching rates from Ibex does not guarantee parity between users. User A might generate an invoice at t0, and user B might scan the invoice a minute later (t1), so there still might be a small difference in price similar to the screenshot shown above. Looking at the ibex docs, this may be a fundamental Ibex limitation:

In some rare cases you will not be credited by the exact fiat amount inputted.
The fiat amount credited to your account will be the market value of the sats, calculated at the settlement time.

@islandbitcoin
Copy link

Thanks for the insight. I like the idea of using IBEX for USD->BTC and using our own average for all other rates.

I think it is acceptable for users to see a small delta when paying an invoice if volatility is high. This is prob the reason why Ibex limits the invoice to 5 minutes.

We still need to talk about the possibility to add contingency to the rate spread for Flash.
This is to account for the local bank rate that we need to stay ahead of in order to be a viable business model.

@islandbitcoin
Copy link

@aliantjamaica FYI

@brh28
Copy link

brh28 commented Sep 27, 2024

@islandbitcoin Unless you have any objections, I'll proceed with fetching USD<->BTC from Ibex, while leaving the other currencies to their existing sources

@islandbitcoin
Copy link

no objections.

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

No branches or pull requests

3 participants