Skip to content

Add can-transfer check #4078

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: stage
Choose a base branch
from
Open

Add can-transfer check #4078

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion packages/bridge/src/int3face/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
GetBridgeSupportedAssetsParams,
} from "../interface";
import { getGasAsset } from "../utils/gas";
import { checkCanTransfer } from "./queries";
import { Int3faceProviderId } from "./utils";

export class Int3faceBridgeProvider implements BridgeProvider {
Expand Down Expand Up @@ -50,7 +51,7 @@ export class Int3faceBridgeProvider implements BridgeProvider {
}

async getQuote(params: GetBridgeQuoteParams): Promise<BridgeQuote> {
const { fromAddress, toChain, toAddress, fromAsset, fromAmount } = params;
const { fromAddress, fromChain, toChain, toAddress, fromAsset, fromAmount } = params;

if (toChain.chainId !== "dogecoin") {
throw new BridgeQuoteError({
Expand All @@ -60,6 +61,22 @@ export class Int3faceBridgeProvider implements BridgeProvider {
});
}

// Check if transfer is available on Int3face side
const canTransfer = await checkCanTransfer(
fromChain.chainId,
toChain.chainId,
fromAsset.denom,
this.ctx.env
);

if (!canTransfer?.can_transfer) {
throw new BridgeQuoteError({
bridgeId: Int3faceProviderId,
errorType: "UnsupportedQuoteError",
message: canTransfer?.reason || "Transfer is not available at this time",
});
}
Comment on lines +72 to +78
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve null handling in the can-transfer check

The current check may not handle all edge cases properly. It's better to be more explicit about the conditions that should trigger the error.

-    if (!canTransfer?.can_transfer) {
+    if (!canTransfer || canTransfer.can_transfer === false) {
       throw new BridgeQuoteError({
         bridgeId: Int3faceProviderId,
         errorType: "UnsupportedQuoteError",
-        message: canTransfer?.reason || "Transfer is not available at this time",
+        message: canTransfer?.reason || "Transfer verification failed or is not available at this time",
       });
     }

This change makes the check more explicit by verifying both that the response exists and that the can_transfer property is specifically false. It also provides a more descriptive message when the reason is empty or when canTransfer is null.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!canTransfer?.can_transfer) {
throw new BridgeQuoteError({
bridgeId: Int3faceProviderId,
errorType: "UnsupportedQuoteError",
message: canTransfer?.reason || "Transfer is not available at this time",
});
}
if (!canTransfer || canTransfer.can_transfer === false) {
throw new BridgeQuoteError({
bridgeId: Int3faceProviderId,
errorType: "UnsupportedQuoteError",
message: canTransfer?.reason || "Transfer verification failed or is not available at this time",
});
}


const destMemo = `{"dest-address": "${toAddress}", "dest-chain-id": "dogecoin"}`;

const int3Doge = this.ctx.assetLists
Expand Down
32 changes: 32 additions & 0 deletions packages/bridge/src/int3face/queries.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,35 @@ export async function getTransferStatus(
return null;
});
}

interface CanTransferResponse {
can_transfer: boolean;
reason: string;
}

const denomOfInt3face: Record<string, string> = {
DOGE: 'dogecoin-doge'
}
Comment on lines +50 to +52
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling for unsupported assets

The mapping only contains one entry for DOGE. When this mapping is used in checkCanTransfer, there's no validation to ensure the assetId exists in the mapping, which could lead to undefined values in the API URL.

 const denomOfInt3face: Record<string, string> = {
   DOGE: 'dogecoin-doge'
 }
+
+export function getInt3faceDenom(assetId: string): string {
+  const denom = denomOfInt3face[assetId];
+  if (!denom) {
+    throw new Error(`Unsupported asset ID: ${assetId}`);
+  }
+  return denom;
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const denomOfInt3face: Record<string, string> = {
DOGE: 'dogecoin-doge'
}
const denomOfInt3face: Record<string, string> = {
DOGE: 'dogecoin-doge'
}
export function getInt3faceDenom(assetId: string): string {
const denom = denomOfInt3face[assetId];
if (!denom) {
throw new Error(`Unsupported asset ID: ${assetId}`);
}
return denom;
}


export async function checkCanTransfer(
srcChainId: string | number,
destChainId: string,
assetId: string,
env: "testnet" | "mainnet"
): Promise<CanTransferResponse> {
let srcChainIdConverted;

if (typeof srcChainId === "string" && srcChainId.startsWith("osmosis")) {
srcChainIdConverted = "osmosis"
} else {
srcChainIdConverted = srcChainId.toString();
}

const origin = env === "mainnet"
? "https://api.mainnet.int3face.zone/int3face"
: "https://api.testnet.int3face.zone/int3face";

const url = new URL(`/bridge/v1beta1/can-transfer/${srcChainIdConverted}/${destChainId}/${denomOfInt3face[assetId]}`, origin);

return apiClient<CanTransferResponse>(url.toString());
}
Comment on lines +54 to +75
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling and use the safer denom lookup method

The function needs to handle possible API errors and provide better context for debugging. Also, it should use a safer method to get the denom to avoid potential undefined values in the URL.

 export async function checkCanTransfer(
     srcChainId: string | number,
     destChainId: string,
     assetId: string,
     env: "testnet" | "mainnet"
 ): Promise<CanTransferResponse> {
   let srcChainIdConverted;

   if (typeof srcChainId === "string" && srcChainId.startsWith("osmosis")) {
     srcChainIdConverted = "osmosis"
   } else {
     srcChainIdConverted = srcChainId.toString();
   }

   const origin = env === "mainnet"
       ? "https://api.mainnet.int3face.zone/int3face"
       : "https://api.testnet.int3face.zone/int3face";

-  const url = new URL(`/bridge/v1beta1/can-transfer/${srcChainIdConverted}/${destChainId}/${denomOfInt3face[assetId]}`, origin);
+  const denom = getInt3faceDenom(assetId);
+  const url = new URL(`/bridge/v1beta1/can-transfer/${srcChainIdConverted}/${destChainId}/${denom}`, origin);

-  return apiClient<CanTransferResponse>(url.toString());
+  try {
+    return await apiClient<CanTransferResponse>(url.toString());
+  } catch (error) {
+    console.error(`Failed to check transfer availability: ${error}`);
+    // Rethrow the error to be handled by the caller
+    throw error;
+  }
 }

Committable suggestion skipped: line range outside the PR's diff.