-
Notifications
You must be signed in to change notification settings - Fork 429
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
Add Int3faceBridgeProvider #4045
base: stage
Are you sure you want to change the base?
Add Int3faceBridgeProvider #4045
Conversation
@artakmik-nitka is attempting to deploy a commit to the OsmoLabs Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis pull request integrates the Changes
Sequence Diagram(s)sequenceDiagram
participant TP as Int3faceTransferStatusProvider
participant API as getTransferStatus
participant Delegate as statusReceiverDelegate
TP->>API: Call getTransferStatus(sendTxHash, env)
API-->>TP: Return transfer status
TP->>TP: Evaluate status (pending, failed, finalized)
TP->>Delegate: Call receiveConclusiveStatus(status)
Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
packages/web/server/api/routers/local-bridge-transfer.tsOops! Something went wrong! :( ESLint: 8.50.0 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/packages/web/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. packages/web/components/bridge/amount-screen.tsxOops! Something went wrong! :( ESLint: 8.50.0 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/packages/web/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. packages/web/utils/bridge.tsOops! Something went wrong! :( ESLint: 8.50.0 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/packages/web/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team.
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (5)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (8)
packages/bridge/src/int3face/transfer-status.ts (2)
28-56
: Consider adding a timeout or maximum attempts for indefinite polling.
Unbounded polling can potentially lead to an endless loop if the transaction is never updated, negatively impacting resources. A maximum retry count or a mechanism to stop polling after a certain condition might provide a better user experience.
63-63
: Use optional chaining for cleaner null checks.
Substituteif (txStatus?.id)
in place of the current null check to streamline the condition.- if (txStatus && txStatus.id) { + if (txStatus?.id) {🧰 Tools
🪛 Biome (1.9.4)
[error] 63-63: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/bridge/src/int3face/index.ts (2)
126-137
: Provide enhanced error handling for IBC queries.
IfgetTxMessages
orestimateTransferTime
throw an error, the exception will bubble up without clear context. Consider capturing these errors to display more user-friendly information or attempt a fallback logic.
196-198
: Use a more descriptive error message.
Throwing a simple"Int3face transactions are currently not supported."
message may confuse end-users. Consider clarifying the reason or providing further guidance on future support.packages/utils/src/dogecoin.ts (1)
6-9
: Handle unrecognized environments or extend for new networks.
While this function covers "mainnet" and "testnet," consider throwing an error or providing a fallback if another value is passed forenv
. This helps ensure consistent behavior if new environments are introduced.packages/bridge/src/int3face/queries.ts (2)
2-7
: Add JSDoc documentation for the enum.Consider adding JSDoc to document the purpose and usage of each status.
+/** Represents the possible states of an Int3face bridge transfer. */ export enum Int3faceTransferStatus { + /** Initial state when the status is unknown or not yet determined. */ TRANSACTION_STATUS_UNSPECIFIED = 'TRANSACTION_STATUS_UNSPECIFIED', + /** Transaction has been initiated but not yet completed. */ TRANSACTION_STATUS_PENDING = 'TRANSACTION_STATUS_PENDING', + /** Transaction has failed to complete. */ TRANSACTION_STATUS_FAILED = 'TRANSACTION_STATUS_FAILED', + /** Transaction has been successfully completed. */ TRANSACTION_STATUS_FINALIZED = 'TRANSACTION_STATUS_FINALIZED', }
9-25
: Add JSDoc documentation for the interface.Consider adding JSDoc to document the purpose and fields of the response interface.
+/** Response structure for Int3face bridge transfer status queries. */ interface TransferStatusResponse { + /** Unique identifier for the transfer. */ "id": string, + /** External system's transaction identifier. */ "externalId": string, // Add similar JSDoc for other fields... }packages/web/stores/root.ts (1)
3-3
: Consider consistent ordering of transfer status providers.The Int3faceTransferStatusProvider integration follows the pattern of other providers. However, consider maintaining alphabetical order in both imports and provider instantiation for consistency.
import { AxelarTransferStatusProvider } from "@osmosis-labs/bridge/build/axelar/transfer-status"; import { IbcTransferStatusProvider } from "@osmosis-labs/bridge/build/ibc/transfer-status"; -import { Int3faceTransferStatusProvider } from "@osmosis-labs/bridge/build/int3face/transfer-status"; import { NomicTransferStatusProvider } from "@osmosis-labs/bridge/build/nomic/transfer-status"; import { SkipTransferStatusProvider } from "@osmosis-labs/bridge/build/skip/transfer-status"; import { SquidTransferStatusProvider } from "@osmosis-labs/bridge/build/squid/transfer-status"; +import { Int3faceTransferStatusProvider } from "@osmosis-labs/bridge/build/int3face/transfer-status"; // ... and in the transferStatusProviders array: new IbcTransferStatusProvider(ChainList, AssetLists), + new Int3faceTransferStatusProvider( + ChainList, + IS_TESTNET ? "testnet" : "mainnet" + ), new NomicTransferStatusProvider( ChainList, IS_TESTNET ? "testnet" : "mainnet" ), - new Int3faceTransferStatusProvider( - ChainList, - IS_TESTNET ? "testnet" : "mainnet" - ),Also applies to: 265-268
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/bridge/src/bridge-providers.ts
(3 hunks)packages/bridge/src/int3face/index.ts
(1 hunks)packages/bridge/src/int3face/queries.ts
(1 hunks)packages/bridge/src/int3face/transfer-status.ts
(1 hunks)packages/bridge/src/int3face/utils.ts
(1 hunks)packages/bridge/src/interface.ts
(2 hunks)packages/utils/src/dogecoin.ts
(1 hunks)packages/utils/src/index.ts
(1 hunks)packages/web/stores/root.ts
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/bridge/src/int3face/utils.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/bridge/src/int3face/transfer-status.ts
[error] 63-63: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (6)
packages/bridge/src/int3face/transfer-status.ts (1)
90-91
: Handle absent or undefined explorers array.
The code only checks forchain.explorers.length === 0
. Ifexplorers
is missing or undefined, this will throw an error. Confirm that all chain definitions guarantee anexplorers
array, or add a fallback check.packages/bridge/src/int3face/index.ts (1)
154-159
: Confirm zero bridging fee assumption.
Currently, thetransferFee
is set to "0". Verify whether no bridging fee truly applies for this provider or if you need to incorporate any relevant network fees.packages/utils/src/index.ts (1)
12-12
: LGTM!The new export follows the established pattern and is correctly placed in alphabetical order.
packages/bridge/src/bridge-providers.ts (1)
3-3
: LGTM!The Int3faceBridgeProvider integration follows the established pattern of other bridge providers in the system.
Also applies to: 27-27, 48-48
packages/bridge/src/interface.ts (2)
194-205
: LGTM! Well-structured Dogecoin chain schema.The schema follows the established pattern and includes appropriate documentation.
214-214
: LGTM! Correctly integrated into bridgeChainSchema.The Dogecoin schema is properly added to the discriminated union.
export async function getTransferStatus( | ||
sendTxHash: string, | ||
env: "testnet" | "mainnet", | ||
): Promise<TransferStatusResponse | null> { | ||
try { | ||
// Todo: check origin | ||
const origin = env === 'testnet' ? 'https://api.testnet.int3face.zone/int3face/bridge' : 'https://api.mainnet.int3face.zone/int3face/bridge' | ||
|
||
// Todo: update url when BE is ready | ||
const url = new URL("/be-in-progress", origin); | ||
url.searchParams.set("external_id", sendTxHash); | ||
|
||
// return apiClient<TransferStatusResponse>(url.toString()); | ||
|
||
return null | ||
} catch { | ||
console.error("Failed to fetch transfer status for tx hash: ", sendTxHash); | ||
return null; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Complete the implementation and address TODOs.
The function has several issues that need attention:
- TODOs need to be addressed regarding origin check and URL update
- The API call is commented out
- The function currently returns null by default
Consider implementing proper error handling with specific error types instead of catching all errors silently.
Let's verify the API endpoint:
❌ Verification inconclusive
🌐 Web query:
What is the current API endpoint for Int3face bridge transfer status?
💡 Result:
Based on the provided search results, there is no explicit mention of an "Int3face bridge transfer status" API endpoint. However, several key points about API endpoints can be inferred from related systems:
- NetApp ONTAP S3 APIs (from Result 10) show a pattern for transfer-related endpoints:
/api/protocols/s3/services/{svm.uuid}/users
This demonstrates how storage systems often structure endpoints with UUID identifiers for specific services.
- Confluent Metadata API (Result 9) uses authentication patterns like:
GET /security/1.0/authenticate
POST /security/1.0/impersonate
Though unrelated to transfers, these examples show common security patterns in API design.
- Intuiface API Explorer (Result 2) enables creation of custom interface assets for REST APIs but doesn't expose specific transfer status endpoints.
For "bridge transfer status" functionality in systems like Int3face:
- Check vendor documentation for endpoints matching patterns like
/api/transfers/{id}/status
or/bridge/transfers
- Look for monitoring endpoints similar to Automation Anywhere's execution status APIs (Result 7)
- Consider pagination parameters if dealing with multiple transfers (as shown in Result 2's multi-page result handling)
Without explicit documentation about "Int3face bridge" in the search results, we recommend consulting official platform documentation or contacting vendor support for endpoint specifics[2][5][10].
Citations:
- 1: https://docs.aws.amazon.com/eventbridge/latest/userguide/eb-api-destinations.html
- 2: https://support.intuiface.com/hc/en-us/articles/360007431091-API-Explorer-Create-a-REST-based-Web-Service-Interface-Asset
- 3: https://docs.automationanywhere.com/bundle/enterprise-v2019/page/enterprise-cloud/topics/control-room/control-room-api/cloud-control-room-apis.html
- 4: https://docs.confluent.io/cloud/current/kafka-rest/kafka-rest-cc.html
- 5: https://support.intuiface.com/hc/en-us/articles/360007296832--Using-the-Intuiface-Share-and-Deploy-API-Sample
- 6: https://docs.confluent.io/platform/current/ksqldb/developer-guide/ksqldb-rest-api/rest-api-reference.html
- 7: https://docs.automationanywhere.com/bundle/enterprise-v2019/page/api-task-real-time-endpoint.html
- 8: https://community.bridgeapp.com/hc/en-us/articles/360054637574-How-to-use-the-Bridge-API
- 9: https://docs.confluent.io/platform/current/security/authorization/rbac/mds-api.html
- 10: https://docs.netapp.com/us-en/ontap-restapi/ontap/security_roles_endpoint_overview.html
Action Required: Complete the API integration and improve error handling
The current implementation in packages/bridge/src/int3face/queries.ts
remains a stub. The API call is commented out and the function returns null, while the TODO comments indicate that the origin needs further validation and the backend endpoint updated. The web query did not return a definitive API endpoint for Int3face bridge transfer status, so this part is still pending vendor or documentation confirmation. Until those details are provided, the implementation issues noted in the review remain valid and require further work.
- Remove or update the TODO comments once the correct endpoint is confirmed.
- Reinstate the API call (uncomment
apiClient<TransferStatusResponse>(url.toString())
) and ensure it uses the updated URL. - Enhance error handling to use specific error types rather than a blanket catch.
packages/bridge/src/interface.ts
Outdated
export type BridgeTransactionRequest = | ||
| EvmBridgeTransactionRequest | ||
| CosmosBridgeTransactionRequest; | ||
| CosmosBridgeTransactionRequest | ||
// | UTXOBridgeTransactionRequest; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify UTXO support requirements for Dogecoin.
The commented-out UTXOBridgeTransactionRequest
suggests incomplete UTXO support. Since Dogecoin is UTXO-based, we should verify if this functionality is required for the Int3faceBridgeProvider
implementation.
Let's check the codebase for any UTXO-related implementations or TODOs:
🏁 Script executed:
#!/bin/bash
# Search for UTXO-related code and comments
echo "Searching for UTXO-related implementations..."
rg -i "utxo" -A 5
echo "Searching for TODO comments related to transactions..."
rg -i "todo.*transaction" -A 5
Length of output: 65718
Action Required: Address Incomplete UTXO Support for Dogecoin
The searches for UTXO-related code (and associated TODO comments) only revealed the commented-out UTXOBridgeTransactionRequest
in the packages/bridge/src/interface.ts
file with no accompanying implementation elsewhere. There are also no Dogecoin-specific references suggesting that UTXO support is handled in any other part of the codebase.
- The commented union type in the interface indicates that UTXO transactions were considered but not enabled.
- No additional UTXO logic or TODO comments related to transaction handling were found.
If Dogecoin (a UTXO-based coin) is expected to be supported by the Int3faceBridgeProvider
, then further implementation is required to handle UTXO transactions. Please verify the intended requirements for Dogecoin support and add the necessary UTXO handling functionality if it is indeed needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/utils/src/dogecoin.ts (1)
1-9
: Add error handling for invalid environment values.The function should handle cases where
env
might be undefined or have an invalid value.Apply this diff to add error handling:
export const getInt3DOGEMinimalDenom = ({ env, }: { env: "mainnet" | "testnet"; }) => { + if (!env) { + throw new Error("Environment is required for getInt3DOGEMinimalDenom"); + } return env === "mainnet" ? "ibc/B3DFDC2958A2BE482532DA3B6B5729B469BE7475598F7487D98B1B3E085245DE" // DOGE.int3 : "ibc/FCB9537564D517E821D0438AB6CA3BBE03B9E2B2C661B89311181329DFD331C4"; // Testnet DOGE.int3 };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/web/public/networks/dogecoin.svg
is excluded by!**/*.svg
,!**/*.svg
📒 Files selected for processing (5)
packages/utils/src/dogecoin.ts
(1 hunks)packages/web/components/bridge/amount-screen.tsx
(2 hunks)packages/web/components/bridge/use-bridges-supported-assets.ts
(1 hunks)packages/web/server/api/routers/bridge-transfer.ts
(3 hunks)packages/web/server/api/routers/local-bridge-transfer.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (6)
packages/utils/src/dogecoin.ts (1)
11-17
: LGTM!The
DogecoinChainInfo
constant follows the same structure as other chain info objects and provides all necessary metadata.packages/web/server/api/routers/local-bridge-transfer.ts (1)
67-67
: LGTM!The addition of "dogecoin" as a supported asset type follows the same pattern as other asset types and is correctly integrated into the existing structure.
packages/web/components/bridge/use-bridges-supported-assets.ts (1)
17-17
: LGTM!The addition of "Int3face" to the supported bridges list follows the same pattern as other bridges and is correctly integrated.
packages/web/server/api/routers/bridge-transfer.ts (2)
50-50
: LGTM!The addition of Dogecoin to the
BridgeChainWithDisplayInfo
type union follows the same pattern as other chain types.
434-440
: LGTM!The Dogecoin chain info handling follows the same pattern as other chain types and correctly uses the
DogecoinChainInfo
constant.packages/web/components/bridge/amount-screen.tsx (1)
437-444
: LGTM: Dogecoin chain type implementation.The implementation follows the established pattern for chain type handling and maintains type safety through proper TypeScript assertions.
console.log({ | ||
isLoading, | ||
hasSupportedChains, | ||
areAssetTransfersDisabled, | ||
fromChain, | ||
fromAsset, | ||
toChain, | ||
toAsset, | ||
hasBalanceError, | ||
quoteEnabled: quote.enabled, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove debug console log statement.
Debug console logs should not be committed to production code as they:
- Could expose sensitive state information
- Add unnecessary noise to the browser console
- May impact performance in production
Apply this diff to remove the debug log:
- console.log({
- isLoading,
- hasSupportedChains,
- areAssetTransfersDisabled,
- fromChain,
- fromAsset,
- toChain,
- toAsset,
- hasBalanceError,
- quoteEnabled: quote.enabled,
- });
📝 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.
console.log({ | |
isLoading, | |
hasSupportedChains, | |
areAssetTransfersDisabled, | |
fromChain, | |
fromAsset, | |
toChain, | |
toAsset, | |
hasBalanceError, | |
quoteEnabled: quote.enabled, | |
}); |
What is the purpose of the change:
Linear Task
Brief Changelog
Testing and Verifying