feat: add base parameter to /rates API #1527
Conversation
✅ Deploy Preview for brilliant-pasca-3e80ec canceled.
|
| (opts.sourceAmount * BigInt(scaledExchangeRate * shiftUp)) / BigInt(shiftUp) | ||
| ) | ||
|
|
||
| return BigInt(Math.round(Number(opts.sourceAmount) * scaledExchangeRate)) |
There was a problem hiding this comment.
BigInt divided by BigInt ends up truncating and not rounding the result. I think for currency, rounding up or down to the nearest unit makes more sense (and is standard). We can't really get around using number, since we are dealing with decimals for the exchange rate
There was a problem hiding this comment.
Do we need to be concerned about exceeding the MAX_SAFE_INTEGER (in the Number(opts.sourceAmount) case)?
There was a problem hiding this comment.
I'm quite certain we will never hit payment amounts over 9007199254740991 in any realistic scenario, but I can add a log just to be extra safe
There was a problem hiding this comment.
We don't have any tooling right now for proper alerting/error management, and I feel like a log would just get swallowed. Going to merge this in for now.
| expect( | ||
| convert({ | ||
| exchangeRate: 0.5, | ||
| sourceAmount: 101n, | ||
| sourceAsset: asset(9), | ||
| destinationAsset: asset(9) | ||
| }) | ||
| ).toBe(50n) |
There was a problem hiding this comment.
| (opts.sourceAmount * BigInt(scaledExchangeRate * shiftUp)) / BigInt(shiftUp) | ||
| ) | ||
|
|
||
| return BigInt(Math.round(Number(opts.sourceAmount) * scaledExchangeRate)) |
There was a problem hiding this comment.
Do we need to be concerned about exceeding the MAX_SAFE_INTEGER (in the Number(opts.sourceAmount) case)?
* feat: support exchangeRates with multiple decimal points * feat: add base parameter to rates service * test: update tests with new /rates api * feat: add base parameter to openapi spec
Changes proposed in this pull request
baseparameter to/rates, updating MASE to properly return with thebaseparamContext
/ratesAPI to take in abaseparameter #1524Checklist
fixes #number