Skip to content

Improve pix onramp payment handling#631

Merged
gianfra-t merged 8 commits into
stagingfrom
add-reference-label-check
May 14, 2025
Merged

Improve pix onramp payment handling#631
gianfra-t merged 8 commits into
stagingfrom
add-reference-label-check

Conversation

@gianfra-t
Copy link
Copy Markdown
Contributor

@gianfra-t gianfra-t commented May 9, 2025

Issue: #620.

Change description

  • Adds a reference label check to the pix payment. Reference is derived as the first 8 characters of the quote id used.
  • Force the ramp to a failed state after 30 minutes.
  • Teleport service now supports deleting an entry, which will be used when the ramp is declared a failed one. Additionally, when requesting, a teleport will not be added if existing already in the set.

Additional changes

  • Increase delay time to 30 seconds between teleport balance check and starting moonebamToPendulumXcm operation.

@netlify
Copy link
Copy Markdown

netlify Bot commented May 9, 2025

Deploy Preview for pendulum-pay ready!

Name Link
🔨 Latest commit 6c3e916
🔍 Latest deploy log https://app.netlify.com/sites/pendulum-pay/deploys/682226978f16140008c42343
😎 Deploy Preview https://deploy-preview-631--pendulum-pay.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Copy Markdown
Member

@ebma ebma 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 to me overall, cool that only the few changes are necessary to make this work. I suppose you still need to test whether it actually works @gianfra-t?

if (!(balanceCheckError instanceof Error)) throw balanceCheckError;
const { message } = balanceCheckError;

const isCheckTimeout = message.includes('did not meet the limit within the specified time');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This check seems quite brittle to me, any chance we can refactor to make it less reliant on the exact wording of the error message?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, we can start implementing some better practices and return custom errors probably, not only here but everywhere...

Comment thread api/src/api/services/phases/handlers/brla-teleport-handler.ts Outdated
@gianfra-t
Copy link
Copy Markdown
Contributor Author

@ebma yes only testing, and also removing the entry from the teleport service was missing. I decided to use the quote id instead of the address encoding to have more control over each specific ramp and the associated teleport operation.

For testing, well I think the best would be to merge and test on staging a true ramp, I don't think this can be mocked. We just need to be sure we are encoding correctly the reference on the br code.

@gianfra-t gianfra-t marked this pull request as ready for review May 12, 2025 15:10
Copy link
Copy Markdown
Member

@ebma ebma 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 to me overall ✅

Comment thread api/src/api/services/phases/handlers/brla-teleport-handler.ts Outdated
@gianfra-t gianfra-t requested a review from a team May 12, 2025 15:27
@gianfra-t gianfra-t merged commit 011e242 into staging May 14, 2025
5 checks passed
@gianfra-t gianfra-t deleted the add-reference-label-check branch May 14, 2025 10:41
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.

2 participants