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

Fix: bridgeXYZMutation to return null on error #3471

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jakubcolony
Copy link
Collaborator

Description

Cosmetic-looking change that caused the linked issue when Bridge API went down earlier today. The JSON response got stringified and a payment was attempted to { success: false }.

This PR changes the return value of the bridge mutation to null if error is thrown.

Testing

To test the fix, you will need to simulate an error in getUserLiquidationAddress handler, and make a payment that will trigger liquidation address swapping.

While you could set your native token address as USDC_LOCAL_ADDRESS in .env.local, go through KYC, add bank details and enable auto offramp, you can also create a file called diff with the contents below and apply the patch with git apply diff. It will make liquidation address swapping happen for any token and recipient, and throw an error inside the lambda handler.

diff --git a/amplify/backend/function/bridgeXYZMutation/src/handlers/getUserLiquidationAddress.js b/amplify/backend/function/bridgeXYZMutation/src/handlers/getUserLiquidationAddress.js
index af3204bf4..304e83abb 100644
--- a/amplify/backend/function/bridgeXYZMutation/src/handlers/getUserLiquidationAddress.js
+++ b/amplify/backend/function/bridgeXYZMutation/src/handlers/getUserLiquidationAddress.js
@@ -15,6 +15,8 @@ const getUserLiquidationAddressHandler = async (
 ) => {
   const userAddress = event.arguments.userAddress;
 
+  throw new Error('Demo error');
+
   try {
     const overrides = JSON.parse(temp_liquidationAddressOverrides);
     if (overrides[userAddress]) {
diff --git a/src/redux/sagas/utils/expenditures.ts b/src/redux/sagas/utils/expenditures.ts
index adcbcfe67..20c8d4625 100644
--- a/src/redux/sagas/utils/expenditures.ts
+++ b/src/redux/sagas/utils/expenditures.ts
@@ -236,7 +236,7 @@ export const adjustRecipientAddress = async (
 ) => {
   const USDCAddress = isDev ? DEV_USDC_ADDRESS : Tokens[network]?.USDC;
 
-  if (tokenAddress !== USDCAddress) {
+  if (false && tokenAddress !== USDCAddress) {
     return recipientAddress;
   }
 
@@ -251,7 +251,7 @@ export const adjustRecipientAddress = async (
   });
 
   const user = userData?.getUserByAddress?.items[0];
-  if (!user || !user.profile?.isAutoOfframpEnabled) {
+  if (!user || (false && !user.profile?.isAutoOfframpEnabled)) {
     return recipientAddress;
   }
 

Next, make a payment (simple or advanced) to leela. The payment should go through to her actual wallet address. On master, you would get the wrong address error as in the issue.

Resolves #3456

@jakubcolony jakubcolony self-assigned this Oct 24, 2024
@jakubcolony jakubcolony marked this pull request as ready for review October 24, 2024 19:11
@jakubcolony jakubcolony requested review from a team as code owners October 24, 2024 19:11
Copy link
Member

@rdig rdig left a comment

Choose a reason for hiding this comment

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

I've tested this with the patch and can confirm the expected error is being thrown

Screenshot from 2024-10-25 11-06-10
Screenshot from 2024-10-25 11-07-28

Also, this made me chuckle :)

and a payment was attempted to { success: false }.

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.

Advanced payment wrong address error and empty action
2 participants