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

Rework Fixed Send Fee Calculation #3055

Open
BlairCurrey opened this issue Oct 31, 2024 · 0 comments
Open

Rework Fixed Send Fee Calculation #3055

BlairCurrey opened this issue Oct 31, 2024 · 0 comments
Labels
pkg: backend Changes in the backend package.

Comments

@BlairCurrey
Copy link
Contributor

Change the fixed send fee calculation to happen before creating the quote (paymentMethodHandlerService.getQuote) and base on the debit amount (instead of receive amount, as it is currently). This will result in a behavior change (ie different receiveAmount) and the quote/service.test.ts tests should be updated. Not entirely dependent, but probably best to address after this PR to add local payments is merged. This adds a debitAmountMinusFees which is determined during the amount calculations - ensure this field is still populated correctly.

Context

Currently with fixed send quotes, we get the quote then patch the debit/receive amount based on the fees. For fixed send, we apply the percentage and fixed fee to the receiveAmount (after it's been converted by the exchange rate, if any). Then it gets converted by the exchange rate. Given the percentage amount is derived from the already converted receiveAmount, applying the exchange rate again messes up the percentage based fee.

Worked example

debitAmountValue basisPointFee exchangeRate
200 200 (2%) 0.5

The current calculation is:

Note: initial receiveAmount comes from getQuote

$receiveAmount = 200 \times 0.5$
$receiveAmount = receiveAmount - ((receiveAmount \times 0.02) \times 0.5) = 99$

The new calculation should be:

$200 - (200 \times 0.02) = 196$
pass into getQuote where 0.5 exchange rate is applied
$receiveAmount =</code>98<code> $

More details on required changes

Basically, we could simply change this:

https://github.com/interledger/rafiki/blob/main/packages/backend/src/open_payments/quote/service.ts#L232

To use debitAmount like this: const fees = quote.fee?.calculate(quote.debitAmount.value) ?? BigInt(0)

At time of writing this causes the following tests to fail:

This would give us the correct calculation. However, this change is insufficient by itself - we need to move this ahead of creating the quote. We don't need the returned quote to make these calculations so doing it upfront should make the logic a bit clearer.

@BlairCurrey BlairCurrey added the discussions: ideas Convert to an idea discussion label Oct 31, 2024
@github-project-automation github-project-automation bot moved this to Backlog in Rafiki Oct 31, 2024
@BlairCurrey BlairCurrey added pkg: backend Changes in the backend package. and removed discussions: ideas Convert to an idea discussion labels Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: backend Changes in the backend package.
Projects
Status: Backlog
Development

No branches or pull requests

1 participant