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

chore(backend): estimate ILP quote receive amount, and fail early #2783

Merged
merged 4 commits into from
Jun 26, 2024

Conversation

mkurapov
Copy link
Contributor

@mkurapov mkurapov commented Jun 25, 2024

Changes proposed in this pull request

Error during quoting if the ILP quote will not fulfil at least a single unit of the receiving asset. See #2770 for detailed explanation.

Context

Fixes #2770

Checklist

  • Related issues linked using fixes #number
  • Tests added/updated
  • Documentation added
  • Make sure that all checks pass
  • Bruno collection updated

@github-actions github-actions bot added type: tests Testing related pkg: backend Changes in the backend package. type: source Changes business logic labels Jun 25, 2024
Copy link

netlify bot commented Jun 25, 2024

Deploy Preview for brilliant-pasca-3e80ec canceled.

Name Link
🔨 Latest commit 83493b1
🔍 Latest deploy log https://app.netlify.com/sites/brilliant-pasca-3e80ec/deploys/667bfbfe6ad4e70009213ae2

@mkurapov mkurapov force-pushed the 2770/mk/minimum-delivery-amount branch from 20dc3c7 to 4ce6353 Compare June 25, 2024 12:45
@mkurapov mkurapov changed the title 2770/mk/minimum delivery amount chore(backend): estimate quote receive amount, and fail early Jun 25, 2024
@mkurapov mkurapov changed the title chore(backend): estimate quote receive amount, and fail early chore(backend): estimate ILP quote receive amount, and fail early Jun 25, 2024
Comment on lines +167 to +172
if (
err instanceof PaymentMethodHandlerError &&
err.code === PaymentMethodHandlerErrorCode.QuoteNonPositiveReceiveAmount
) {
return QuoteError.NonPositiveReceiveAmount
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Handle gracefully, avoid returning 500 for an unhandled PaymentMethodHandlerError.
This is also prepping for having better error codes in the RS in general.

@mkurapov mkurapov marked this pull request as ready for review June 25, 2024 12:56
sabineschaller
sabineschaller previously approved these changes Jun 25, 2024
Copy link
Member

@sabineschaller sabineschaller left a comment

Choose a reason for hiding this comment

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

Looks good, just one curiosity question

receiver: receiver.incomingPayment!.id,
method: 'ilp',
debitAmount: {
value: 2n,
Copy link
Member

Choose a reason for hiding this comment

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

Why does that end up being non-positive? Because of slippage?

Copy link
Contributor Author

@mkurapov mkurapov Jun 25, 2024

Choose a reason for hiding this comment

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

We mock the call to paymentMethodHandlerService.getQuote so this value doesn't matter too much.
Main test is here

@mkurapov mkurapov force-pushed the 2770/mk/minimum-delivery-amount branch from 1897492 to 83493b1 Compare June 26, 2024 11:31
@mkurapov mkurapov merged commit 48a286b into main Jun 26, 2024
42 checks passed
@mkurapov mkurapov deleted the 2770/mk/minimum-delivery-amount branch June 26, 2024 12:25
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. type: source Changes business logic type: tests Testing related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix minimum delivery amounts
3 participants