Dp502/throw stellar account non activated error#375
Dp502/throw stellar account non activated error#375depressedPlumber502 wants to merge 9 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR introduces validation for non-activated Stellar accounts in the Hot Bridge withdrawal flow. When a destination account isn't activated, the SDK now throws Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🦋 Changeset detectedLatest commit: 654e366 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/intents-sdk/src/bridges/hot-bridge/hot-bridge.ts`:
- Around line 279-305: The current logic calls
this.hotSdk.stellar.isTrustlineExists with token="native", which is wrong for
XLM: change the flow in the Stellar validation block so native XLM skips
trustline validation entirely — detect isXlm first; if isXlm, call an
account-existence check (e.g. this.hotSdk.stellar.isAccountExists or
fetchAccount equivalent) and on NotFoundError throw
StellarAccountNotActivatedError(args.destinationAddress, args.assetId,
assetInfo.blockchain); otherwise (non-native) call
this.hotSdk.stellar.isTrustlineExists(args.destinationAddress, token) and if it
returns false throw TrustlineNotFoundError(args.destinationAddress,
args.assetId, assetInfo.blockchain, token); preserve rethrow of unexpected
errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1dfdd291-0897-4765-8671-7fcd6ef6f23f
📒 Files selected for processing (2)
packages/intents-sdk/index.tspackages/intents-sdk/src/bridges/hot-bridge/hot-bridge.ts
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/intents-sdk/src/bridges/hot-bridge/hot-bridge.ts (1)
279-292:⚠️ Potential issue | 🟠 MajorHandle native XLM separately from trustline boolean checks.
For native XLM, trustlines are not a meaningful requirement. If
isTrustlineExists(address, "native")returnsfalsefor an activated account, this branch throwsTrustlineNotFoundErrorincorrectly. KeepNotFoundError -> StellarAccountNotActivatedError, but avoidhasTrustline === falsehandling for native assets.Suggested fix
if (assetInfo.blockchain === Chains.Stellar) { try { - const token = "native" in assetInfo ? "native" : assetInfo.address; - const hasTrustline = await this.hotSdk.stellar.isTrustlineExists( - args.destinationAddress, - token, - ); - - if (!hasTrustline) { - throw new TrustlineNotFoundError( - args.destinationAddress, - args.assetId, - assetInfo.blockchain, - token, - ); - } + if ("native" in assetInfo) { + // Native XLM: only verify account activation via SDK error path. + await this.hotSdk.stellar.isTrustlineExists( + args.destinationAddress, + "native", + ); + } else { + const token = assetInfo.address; + const hasTrustline = await this.hotSdk.stellar.isTrustlineExists( + args.destinationAddress, + token, + ); + if (!hasTrustline) { + throw new TrustlineNotFoundError( + args.destinationAddress, + args.assetId, + assetInfo.blockchain, + token, + ); + } + } } catch (error) { if (error instanceof NotFoundError) { throw new StellarAccountNotActivatedError(On Stellar, are trustlines required for the native XLM asset, or only for issued assets?#!/bin/bash # Verify whether native-XLM behavior is explicitly tested in this repo. rg -nP --type=ts -C3 'StellarAccountNotActivatedError|TrustlineNotFoundError|isTrustlineExists\(|native' \ packages/intents-sdk/src/sdk.test.ts \ packages/intents-sdk/src/bridges/hot-bridge/hot-bridge.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/intents-sdk/src/bridges/hot-bridge/hot-bridge.ts` around lines 279 - 292, The current check throws TrustlineNotFoundError when isTrustlineExists returns false even for native XLM; update the logic in the block using token (computed from "native" in assetInfo) and this.hotSdk.stellar.isTrustlineExists so that if token === "native" you skip the trustline boolean branch (do not throw TrustlineNotFoundError) while still mapping not-found/account errors to StellarAccountNotActivatedError where appropriate; modify the code around isTrustlineExists, TrustlineNotFoundError, and StellarAccountNotActivatedError to treat native assets as exempt from trustline validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/intents-sdk/src/bridges/hot-bridge/hot-bridge.ts`:
- Around line 279-292: The current check throws TrustlineNotFoundError when
isTrustlineExists returns false even for native XLM; update the logic in the
block using token (computed from "native" in assetInfo) and
this.hotSdk.stellar.isTrustlineExists so that if token === "native" you skip the
trustline boolean branch (do not throw TrustlineNotFoundError) while still
mapping not-found/account errors to StellarAccountNotActivatedError where
appropriate; modify the code around isTrustlineExists, TrustlineNotFoundError,
and StellarAccountNotActivatedError to treat native assets as exempt from
trustline validation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1a1d9f98-31d7-4284-b2f1-47d76ee6c7ba
📒 Files selected for processing (2)
packages/intents-sdk/src/bridges/hot-bridge/hot-bridge.tspackages/intents-sdk/src/sdk.test.ts
✅ Files skipped from review due to trivial changes (1)
- packages/intents-sdk/src/sdk.test.ts
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/intents-sdk/src/bridges/hot-bridge/hot-bridge.ts (1)
279-293:⚠️ Potential issue | 🟠 MajorSkip trustline validation for native XLM.
Line 280 still routes native Stellar assets through
isTrustlineExists(..., "native"). Trustlines only exist for issued assets, so this can still reject an activated XLM destination withTrustlineNotFoundErrorbefore the new activation error path helps. Split this branch so native XLM only checks account existence, and keepisTrustlineExistsfor issued assets.On Stellar, do trustlines apply to the native XLM asset, or only to issued assets?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/intents-sdk/src/bridges/hot-bridge/hot-bridge.ts` around lines 279 - 293, The current branch treats native Stellar as token "native" and calls this.hotSdk.stellar.isTrustlineExists, which incorrectly rejects native XLM; update the try block so that when assetInfo has "native" you call the Stellar account-existence check for args.destinationAddress (use the SDK/account helper method your project exposes, e.g., isAccountExists) instead of isTrustlineExists, and only call this.hotSdk.stellar.isTrustlineExists for issued assets; keep throwing TrustlineNotFoundError for missing issued-asset trustlines and use the appropriate activation/error path when the native account does not exist.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/intents-sdk/src/bridges/hot-bridge/hot-bridge.ts`:
- Around line 279-293: The current branch treats native Stellar as token
"native" and calls this.hotSdk.stellar.isTrustlineExists, which incorrectly
rejects native XLM; update the try block so that when assetInfo has "native" you
call the Stellar account-existence check for args.destinationAddress (use the
SDK/account helper method your project exposes, e.g., isAccountExists) instead
of isTrustlineExists, and only call this.hotSdk.stellar.isTrustlineExists for
issued assets; keep throwing TrustlineNotFoundError for missing issued-asset
trustlines and use the appropriate activation/error path when the native account
does not exist.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d135dd8a-bd24-424e-a49e-1286bbc9f15a
📒 Files selected for processing (4)
packages/intents-sdk/index.tspackages/intents-sdk/src/bridges/hot-bridge/error.tspackages/intents-sdk/src/bridges/hot-bridge/hot-bridge.tspackages/intents-sdk/src/sdk.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/intents-sdk/index.ts
- packages/intents-sdk/src/sdk.test.ts
1 participant
No description provided.