Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions queue-manager/rango-preset/src/actions/scheduleNextStep.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ export function scheduleNextStep({
(step: PendingSwapStep) => step.status === 'failed'
);

console.log('what the heck is happening here');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Remove this debug log from the scheduler hot path.

Line 39 logs on every scheduleNextStep call, which can flood logs and add noise in production. The message is also non-actionable.

Proposed fix
-  console.log('what the heck is happening here');
📝 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
console.log('what the heck is happening here');
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@queue-manager/rango-preset/src/actions/scheduleNextStep.ts` at line 39,
Remove the noise-making debug log by deleting the console.log('what the heck is
happening here') call inside the scheduleNextStep function so the scheduler hot
path no longer emits non-actionable messages on every invocation; if you need
diagnostic info keep a guarded logger (e.g., process.env.DEBUG or a logger.debug
call) instead of an unconditional console.log.


Comment on lines +39 to +40
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Stray debug log 🐞 Bug ⚙ Maintainability

scheduleNextStep now prints a hardcoded debug message on every execution, creating noisy production
logs and potentially leaking internal behavior during swaps.
Agent Prompt
### Issue description
A stray `console.log` was committed and will run on every call to `scheduleNextStep`, generating noisy logs.

### Issue Context
This code runs in the queue-manager execution loop.

### Fix Focus Areas
- queue-manager/rango-preset/src/actions/scheduleNextStep.ts[39-40]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

if (!!currentStep && !isFailed) {
if (isTxAlreadyCreated(swap, currentStep)) {
if (currentStep.fromBlockchain === TransactionType.XRPL) {
Expand Down
1 change: 1 addition & 0 deletions wallets/core/src/namespaces/xrpl/mod.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export * as builders from './builders.js';
export type { XRPLActions } from './types.js';

export * as utils from './utils.js';
export { CAIP_XRPL_CHAIN_ID, CAIP_NAMESPACE } from './constants.js';
15 changes: 15 additions & 0 deletions wallets/core/src/namespaces/xrpl/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import type { CaipAccount } from '../common/mod.js';

import { AccountId } from 'caip';

import { CAIP_NAMESPACE, CAIP_XRPL_CHAIN_ID } from './constants.js';

export function formatAddressToCAIP(address: string): string {
return AccountId.format({
address,
chainId: {
namespace: CAIP_NAMESPACE,
reference: CAIP_XRPL_CHAIN_ID,
},
}) as CaipAccount;
}
3 changes: 2 additions & 1 deletion wallets/provider-all/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"@rango-dev/provider-enkrypt": "^0.58.1-next.2",
"@rango-dev/provider-exodus": "^0.59.1-next.2",
"@rango-dev/provider-keplr": "^0.58.1-next.2",
"@rango-dev/provider-gemwallet": "^0.1.0",
"@rango-dev/provider-leap-cosmos": "^0.58.1-next.2",
"@rango-dev/provider-ledger": "^0.29.1-next.2",
"@rango-dev/provider-math-wallet": "^0.59.1-next.2",
Expand Down Expand Up @@ -62,4 +63,4 @@
"publishConfig": {
"access": "public"
}
}
}
2 changes: 2 additions & 0 deletions wallets/provider-all/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { versions as cosmostation } from '@rango-dev/provider-cosmostation';
import * as defaultInjected from '@rango-dev/provider-default';
import { versions as enkrypt } from '@rango-dev/provider-enkrypt';
import { versions as exodus } from '@rango-dev/provider-exodus';
import { versions as gemwallet } from '@rango-dev/provider-gemwallet';
import { versions as keplr } from '@rango-dev/provider-keplr';
import { versions as leap } from '@rango-dev/provider-leap-cosmos';
import { versions as ledger } from '@rango-dev/provider-ledger';
Expand Down Expand Up @@ -130,5 +131,6 @@ export const allProviders = (
solflare,
slush,
unisat,
gemwallet,
];
};
32 changes: 32 additions & 0 deletions wallets/provider-gemwallet/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"name": "@rango-dev/provider-gemwallet",
"version": "0.1.0",
"license": "MIT",
"type": "module",
"source": "./src/mod.ts",
"main": "./dist/mod.js",
"exports": {
".": "./dist/mod.js"
},
"typings": "dist/mod.d.ts",
"files": [
"dist",
"src"
],
"scripts": {
"build": "node ../../scripts/build/command.mjs --path wallets/provider-gemwallet --inputs src/mod.ts",
"ts-check": "tsc --declaration --emitDeclarationOnly -p ./tsconfig.json",
"clean": "rimraf dist",
"format": "prettier --write '{.,src}/**/*.{ts,tsx}'",
"lint": "eslint \"**/*.{ts,tsx}\""
},
"dependencies": {
"@rango-dev/wallets-shared": "0.58.1-next.2",
"@gemwallet/api": "^3.8.0",
"xrpl": "^4.2.0",
"rango-types": "^0.1.95"
},
"publishConfig": {
"access": "public"
}
}
18 changes: 18 additions & 0 deletions wallets/provider-gemwallet/readme.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# GemWallet Provider
GemWallet integration for hub.
[Homepage](https://gemwallet.app/) | [Docs](https://gemwallet.app/docs/user-guide/introduction)

More about implementation status can be found [here](../readme.md).
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Use descriptive link text instead of “here”.

Line 5 uses non-descriptive link text, which hurts readability/accessibility in docs and rendered link lists.

Suggested doc tweak
-More about implementation status can be found [here](../readme.md).
+More about implementation status can be found in the [wallet providers implementation status](../readme.md).
📝 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
More about implementation status can be found [here](../readme.md).
More about implementation status can be found in the [wallet providers implementation status](../readme.md).
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)

[warning] 5-5: Link text should be descriptive

(MD059, descriptive-link-text)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@wallets/provider-gemwallet/readme.md` at line 5, Replace the non-descriptive
link text "here" with a descriptive phrase that conveys the target, e.g.,
"implementation status" or "the parent README", so the sentence reads something
like "More about implementation status can be found in the parent README" (or
"More about implementation status can be found in the implementation status
section of the parent README"); update the link target text in
wallets/provider-gemwallet/readme.md accordingly to improve accessibility and
clarity.


## Implementation notes/limitation

### Feature

#### ⚠️ Auto Connect

It doesn't have the feature to silently connect to wallet, it shows a popup and a loading for a few seconds.


---

More wallet information can be found in [readme.md](../readme.md).
40 changes: 40 additions & 0 deletions wallets/provider-gemwallet/src/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import type { ProviderMetadata } from '@rango-dev/wallets-core';
import type { BlockchainMeta } from 'rango-types';

import { xrplBlockchain } from 'rango-types';

import getSigners from './signer.js';

export const XRPL_PUBLIC_SERVER = 'wss://xrplcluster.com/';
export const WALLET_ID = 'gemwallet';

export const info: ProviderMetadata = {
name: 'GemWallet',
icon: 'https://raw.githubusercontent.com/rango-exchange/assets/main/wallets/gemwallet/icon.svg',
extensions: {
chrome:
'https://chromewebstore.google.com/detail/gemwallet/egebedonbdapoieedfcfkofloclfghab',
homepage: 'https://gemwallet.app/',
},
properties: [
{
name: 'namespaces',
value: {
selection: 'multiple',
data: [
{
label: 'XRPL',
value: 'XRPL',
id: 'XRPL',
getSupportedChains: (allBlockchains: BlockchainMeta[]) =>
xrplBlockchain(allBlockchains),
Comment on lines +29 to +30
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kody code-review Bug critical

Typos in variable names and file paths causing runtime errors. In constants.ts, allBlockactions is used instead of allBlockchains, and in mod.ts, singer.js is used instead of signer.js.

This issue appears in multiple locations:

  • wallets/provider-gemwallet/src/constants.ts: Lines 29-30
  • wallets/provider-gemwallet/src/namespaces/xrpl/mod.ts: Lines 2-2
    Please correct all typos in variable names and file paths to match their intended references.
            getSupportedChains: (allBlockchains: BlockchainMeta[]) =>
              xrplBlockchain(allBlockchains),
Prompt for LLM

File wallets/provider-gemwallet/src/constants.ts:

Line 29 to 30:

In the provided TypeScript code, there is a function `getSupportedChains` that takes a parameter named `allBlockchains`. However, inside the function's body, it calls another function `xrplBlockchain` with an argument `allBlockactions`, which is a typo. This will lead to a `ReferenceError` at runtime. Please correct the typo in the argument passed to `xrplBlockchain` to match the parameter name `allBlockchains`.

Suggested Code:

            getSupportedChains: (allBlockchains: BlockchainMeta[]) =>
              xrplBlockchain(allBlockchains),

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

},
],
},
},
{
name: 'signers',
value: { getSigners: async () => getSigners() },
},
],
};
8 changes: 8 additions & 0 deletions wallets/provider-gemwallet/src/mod.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { defineVersions } from '@rango-dev/wallets-core/utils';

import { buildProvider } from './provider.js';

const versions = () =>
defineVersions().version('1.0.0', buildProvider()).build();

export { versions };
88 changes: 88 additions & 0 deletions wallets/provider-gemwallet/src/namespaces/xrpl/helpers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import type {
Memo,
SendPaymentRequest,
SetTrustlineRequest,
} from '@gemwallet/api';
import type {
XrplPaymentTransactionData,
XrplTransactionDataIssuedCurrencyAmount,
XrplTransactionDataMPTAmount,
XrplTrustSetTransactionData,
} from 'rango-types/mainApi';

function isIssuedCurrencyAmount(
amount: XrplPaymentTransactionData['Amount']
): amount is XrplTransactionDataIssuedCurrencyAmount {
return (
typeof amount === 'object' &&
// @ts-expect-error it never throw an runtime error, since we are checking it should be an object first
typeof amount.currency === 'string' &&
// @ts-expect-error it never throw an runtime error, since we are checking it should be an object first
typeof amount.issuer === 'string' &&
typeof amount.value === 'string'
Comment on lines +16 to +22
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Make the XRPL amount guards null-safe.

typeof null === 'object', so both guards can still throw on malformed input before the caller reaches its fallback error path. These should return false, not crash.

🛠️ Suggested change
 function isIssuedCurrencyAmount(
   amount: XrplPaymentTransactionData['Amount']
 ): amount is XrplTransactionDataIssuedCurrencyAmount {
-  return (
-    typeof amount === 'object' &&
-    // `@ts-expect-error` it never throw an runtime error, since we are checking it should be an object first
-    typeof amount.currency === 'string' &&
-    // `@ts-expect-error` it never throw an runtime error, since we are checking it should be an object first
-    typeof amount.issuer === 'string' &&
-    typeof amount.value === 'string'
-  );
+  return !!amount &&
+    typeof amount === 'object' &&
+    'currency' in amount &&
+    typeof amount.currency === 'string' &&
+    'issuer' in amount &&
+    typeof amount.issuer === 'string' &&
+    'value' in amount &&
+    typeof amount.value === 'string';
 }
 
 function isMPTokenAmount(
   amount: XrplPaymentTransactionData['Amount']
 ): amount is XrplTransactionDataMPTAmount {
-  return (
-    typeof amount === 'object' &&
-    // `@ts-expect-error` it never throw an runtime error, since we are checking it should be an object first
-    typeof amount.mpt_issuance_id === 'string' &&
-    typeof amount.value === 'string'
-  );
+  return !!amount &&
+    typeof amount === 'object' &&
+    'mpt_issuance_id' in amount &&
+    typeof amount.mpt_issuance_id === 'string' &&
+    'value' in amount &&
+    typeof amount.value === 'string';
 }

Also applies to: 30-33

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@wallets/provider-gemwallet/src/namespaces/xrpl/helpers.ts` around lines 16 -
22, The guard that validates an XRPL Amount incorrectly treats null as an object
and can throw; update the predicates that check the shape of `amount` (the
boolean expression around typeof amount === 'object' and subsequent checks for
amount.currency, amount.issuer, amount.value) to first ensure amount is not null
(e.g., amount !== null && typeof amount === 'object') before accessing
properties, and apply the same null-safe change to the other guard used at lines
30-33 so both return false for null or malformed inputs instead of throwing.

);
}

function isMPTokenAmount(
amount: XrplPaymentTransactionData['Amount']
): amount is XrplTransactionDataMPTAmount {
return (
typeof amount === 'object' &&
// @ts-expect-error it never throw an runtime error, since we are checking it should be an object first
typeof amount.mpt_issuance_id === 'string' &&
typeof amount.value === 'string'
);
}

function fromPaymentTransactionMemoToGemWalletMemo(
memos: XrplPaymentTransactionData['Memos']
): Memo[] {
if (!memos) {
return [];
}

return memos.map((memo) => {
return {
memo: {
memoType: memo.Memo.MemoType,
memoData: memo.Memo.MemoData,
memoFormat: memo.Memo.MemoFormat,
},
};
});
}

export function fromTrustSetTransactionDataToGemWalletRequest(
data: XrplTrustSetTransactionData
): SetTrustlineRequest {
return {
limitAmount: data.LimitAmount,
memos: fromPaymentTransactionMemoToGemWalletMemo(data.Memos),
flags: data.Flags,
};
}

export function fromPaymentTransactionDataToGemWalletRequest(
data: XrplPaymentTransactionData
): SendPaymentRequest {
let amount: SendPaymentRequest['amount'];
if (isMPTokenAmount(data.Amount)) {
throw new Error("Current implemented signer doesn't have support for MPT");
} else if (isIssuedCurrencyAmount(data.Amount)) {
amount = data.Amount;
} else if (typeof data.Amount === 'string') {
amount = data.Amount;
} else {
throw new Error(
"There is an unexpected type for Amount. current signer doesn't have support for that."
);
}

return {
amount,
destination: data.Destination,
destinationTag: data.DestinationTag,
memos: fromPaymentTransactionMemoToGemWalletMemo(data.Memos),
flags: data.Flags,
};
}
35 changes: 35 additions & 0 deletions wallets/provider-gemwallet/src/namespaces/xrpl/hooks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import type { XRPLActions } from '@rango-dev/wallets-core/namespaces/xrpl';

import { on } from '@gemwallet/api';
import { ChangeAccountSubscriberBuilder } from '@rango-dev/wallets-core/namespaces/common';
import { utils } from '@rango-dev/wallets-core/namespaces/xrpl';

type WalletChangedEventPayload = {
wallet: {
publicAddress: string;
};
};

export function changeAccountSubscriberBuilder() {
// `true` instead of ProviderAPI is just a workaround. we don't need to have instance here.
return new ChangeAccountSubscriberBuilder<
WalletChangedEventPayload,
true,
XRPLActions
>()
.getInstance(() => true)
.format(async (_, payload) => [
utils.formatAddressToCAIP(payload.wallet.publicAddress),
])
.addEventListener((_, callback) => {
on('walletChanged', callback);
})
.removeEventListener((_instance, _callback) => {
/*
* TODO: gem wallet doesn't have support for unsubscribing.
* Making a variable and keep the callback refrence then here make it `undefined` is a quick fix
* but it makes new bugs, where if two subscribers added at once, we will loose the track of the first one and it will be staled.
*/
})
Comment on lines +27 to +33
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kody code-review Bug high

Resource leaks due to improper cleanup. In hooks.ts, removeEventListener is a no-op, and in namespace.ts, the XRPL client is not disconnected on error.

This issue appears in multiple locations:

  • wallets/provider-gemwallet/src/namespaces/xrpl/hooks.ts: Lines 27-33
  • wallets/provider-gemwallet/src/namespaces/xrpl/namespace.ts: Lines 55-68
    Please ensure all resources are properly cleaned up, including event listeners and network connections, especially in error cases.
.removeEventListener((_instance, _callback) => {
      // TODO: gem wallet doesn't have support for unsubscribing.
      // This feature will cause memory leaks until the underlying API supports listener removal.
      console.warn(
        'GemWallet provider does not support unsubscribing from "walletChanged" events. This will cause a memory leak.'
      );
    })
Prompt for LLM

File wallets/provider-gemwallet/src/namespaces/xrpl/hooks.ts:

Line 27 to 33:

The provided code implements an event subscriber for GemWallet. The `removeEventListener` function is currently a no-op, as indicated by the TODO comment, because the underlying GemWallet API allegedly does not support unsubscribing from events. This creates a memory leak, as any subscribed listeners will never be cleaned up. This is especially problematic in single-page applications where components subscribe and unsubscribe during their lifecycle. Suggest a way to handle this. Should it throw an error, log a warning, or is there a pattern to manage listeners internally even if the source doesn't support removal?

Suggested Code:

.removeEventListener((_instance, _callback) => {
      // TODO: gem wallet doesn't have support for unsubscribing.
      // This feature will cause memory leaks until the underlying API supports listener removal.
      console.warn(
        'GemWallet provider does not support unsubscribing from "walletChanged" events. This will cause a memory leak.'
      );
    })

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

Comment on lines +24 to +33
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Does @gemwallet/apiversion 3.8.0 provide an official way to unsubscribe or remove listeners registered viaon('walletChanged', callback)?

💡 Result:

No, @gemwallet/api version 3.8.0 does not provide an official way to unsubscribe or remove listeners registered via on('walletChanged', callback). Official documentation across multiple versions (including 3.6.0, 3.7.0, and earlier) shows examples of using on to register listeners for events like walletChanged, but no mention of an off, removeListener, or unsubscribe method. The npm package files list various helpers and methods but no explicit event management beyond registration. Source code is in the GitHub repo (GemWallet/gemwallet-extension), but no snippets indicate unsubscription support.

Citations:


Remove no-op removeEventListener and implement local callback registry to prevent subscriber accumulation.

@gemwallet/api does not support unsubscribing from listeners, so the current no-op removeEventListener at line 27 can accumulate callbacks across repeated subscribe/unsubscribe cycles, causing stale and duplicate updates.

Implement a local Set-based fan-out registry to manage subscribers independently of the provider's single registration:

Suggested implementation
+const walletChangedSubscribers = new Set<(payload: WalletChangedEventPayload) => void>();
+let bridgeRegistered = false;
+
+const walletChangedBridge = (payload: WalletChangedEventPayload) => {
+  for (const cb of walletChangedSubscribers) cb(payload);
+};
+
 export function changeAccountSubscriberBuilder() {
   return new ChangeAccountSubscriberBuilder<
     WalletChangedEventPayload,
     true,
     XRPLActions
   >()
     .getInstance(() => true)
     .format(async (_, payload) => [
       utils.formatAddressToCAIP(payload.wallet.publicAddress),
     ])
     .addEventListener((_, callback) => {
-      on('walletChanged', callback);
+      if (!bridgeRegistered) {
+        on('walletChanged', walletChangedBridge);
+        bridgeRegistered = true;
+      }
+      walletChangedSubscribers.add(callback);
     })
-    .removeEventListener((_instance, _callback) => {
-      /*
-       * TODO: gem wallet doesn't have support for unsubscribing.
-       * Making a variable and keep the callback refrence then here make it `undefined` is a quick fix
-       * but it makes new bugs, where if two subscribers added at once, we will loose the track of the first one and it will be staled.
-       */
+    .removeEventListener((_instance, callback) => {
+      walletChangedSubscribers.delete(callback);
     })
     .build();
 }
📝 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
.addEventListener((_, callback) => {
on('walletChanged', callback);
})
.removeEventListener((_instance, _callback) => {
/*
* TODO: gem wallet doesn't have support for unsubscribing.
* Making a variable and keep the callback refrence then here make it `undefined` is a quick fix
* but it makes new bugs, where if two subscribers added at once, we will loose the track of the first one and it will be staled.
*/
})
const walletChangedSubscribers = new Set<(payload: WalletChangedEventPayload) => void>();
let bridgeRegistered = false;
const walletChangedBridge = (payload: WalletChangedEventPayload) => {
for (const cb of walletChangedSubscribers) cb(payload);
};
export function changeAccountSubscriberBuilder() {
return new ChangeAccountSubscriberBuilder<
WalletChangedEventPayload,
true,
XRPLActions
>()
.getInstance(() => true)
.format(async (_, payload) => [
utils.formatAddressToCAIP(payload.wallet.publicAddress),
])
.addEventListener((_, callback) => {
if (!bridgeRegistered) {
on('walletChanged', walletChangedBridge);
bridgeRegistered = true;
}
walletChangedSubscribers.add(callback);
})
.removeEventListener((_instance, callback) => {
walletChangedSubscribers.delete(callback);
})
.build();
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@wallets/provider-gemwallet/src/namespaces/xrpl/hooks.ts` around lines 24 -
33, Remove the no-op removeEventListener and add a local Set-based registry to
fan out provider events: create a Set called subscribers (or similar) in the
same module, change addEventListener to add the callback to subscribers and
ensure a single provider listener is registered that calls each callback in
subscribers (use on('walletChanged', providerCallback) once); implement
removeEventListener to delete the callback from subscribers and, if subscribers
becomes empty, stop forwarding (you can keep the provider listener registered if
gemwallet lacks off/unsubscribe, but the providerCallback should no longer call
any callbacks when the set is empty). Reference addEventListener,
removeEventListener, on('walletChanged', callback) and the
providerCallback/subscribers Set when making the change.

Comment on lines +24 to +33
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

3. Account listener not removable 🐞 Bug ☼ Reliability

GemWallet's account-change subscriber registers a walletChanged listener but provides a no-op
removeEventListener, so cleanup cannot detach callbacks and repeated subscriptions can accumulate
and duplicate state updates.
Agent Prompt
### Issue description
The GemWallet account-change subscription cannot be cleaned up: `addEventListener` registers a listener, but `removeEventListener` is a no-op. Core cleanup calls `removeEventListener`, so callbacks can leak and duplicate.

### Issue Context
GemWallet API may not support true unsubscribe, but we still need to prevent duplicated callbacks and stale state updates.

### Fix Focus Areas
- wallets/provider-gemwallet/src/namespaces/xrpl/hooks.ts[13-34]
- wallets/core/src/namespaces/common/hooks/changeAccountSubscriber.ts[210-227]

### Implementation direction
- Prefer returning an unsubscribe function from `.addEventListener(...)` if GemWallet `on(...)` can provide one.
- If GemWallet cannot unsubscribe, implement a single underlying `walletChanged` listener and multiplex to a `Set` of subscribers; `removeEventListener` should delete from the set so cleanup becomes effective (even if the underlying listener remains).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

.build();
}
2 changes: 2 additions & 0 deletions wallets/provider-gemwallet/src/namespaces/xrpl/mod.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export { namespace } from './namespace.js';
export { Signer } from './singer.js';
75 changes: 75 additions & 0 deletions wallets/provider-gemwallet/src/namespaces/xrpl/namespace.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import type { XRPLActions } from '@rango-dev/wallets-core/namespaces/xrpl';

import { getAddress } from '@gemwallet/api';
import { ActionBuilder, NamespaceBuilder } from '@rango-dev/wallets-core';
import { builders, utils } from '@rango-dev/wallets-core/namespaces/xrpl';
import { Client } from 'xrpl';

import { WALLET_ID, XRPL_PUBLIC_SERVER } from '../../constants.js';
import { checkInstallationOnLoad } from '../../utils.js';

import { changeAccountSubscriberBuilder } from './hooks.js';

const [changeAccountSubscriber, changeAccountCleanup] =
changeAccountSubscriberBuilder();

const connect = builders
.connect()
.action(async function () {
const response = await getAddress();
if (!response.result?.address) {
throw new Error(`Couldn't access to your wallet address.`);
}

return [utils.formatAddressToCAIP(response.result.address)];
})
.before(changeAccountSubscriber)
.or(changeAccountCleanup)
.build();

const canEagerConnect = new ActionBuilder<XRPLActions, 'canEagerConnect'>(
'canEagerConnect'
)
.action(async () => {
const isInstalled = await checkInstallationOnLoad();
if (!isInstalled) {
throw new Error(
'Trying to eagerly connect to your EVM wallet, but seems its instance is not available.'
);
Comment on lines +36 to +38
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix wallet type in error message.

Line 37 says “EVM wallet” in an XRPL namespace path; this is misleading for users and debugging.

Suggested text fix
-        'Trying to eagerly connect to your EVM wallet, but seems its instance is not available.'
+        'Trying to eagerly connect to your XRPL wallet, but its instance is not available.'
📝 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
throw new Error(
'Trying to eagerly connect to your EVM wallet, but seems its instance is not available.'
);
throw new Error(
'Trying to eagerly connect to your XRPL wallet, but its instance is not available.'
);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@wallets/provider-gemwallet/src/namespaces/xrpl/namespace.ts` around lines 36
- 38, The error message in the XRPL namespace uses the wrong wallet type; update
the thrown Error in namespace.ts (the throw new Error(...) inside the XRPL
namespace connect/init path) to refer to "XRPL wallet" (or a neutral phrase like
"wallet" if preferred) instead of "EVM wallet" so the message correctly reflects
the XRPL context; keep the rest of the message intact and ensure any similar
messages in the same file also use "XRPL" where appropriate.

}

try {
const response = await getAddress();
const address = response.result?.address;

return !!address;
} catch {
return false;
}
})
.build();

const accountLines = new ActionBuilder<XRPLActions, 'accountLines'>(
'accountLines'
)
.action(async (_, account, options) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kody code-review Kody Rules high

The accountLines action uses implicit any types for its parameters, violating type safety rules.

This issue appears in multiple locations:

  • wallets/provider-gemwallet/src/namespaces/xrpl/namespace.ts: Lines 55-68
  • wallets/provider-gemwallet/src/namespaces/xrpl/helpers.ts: Lines 29-34
  • wallets/provider-gemwallet/src/namespaces/xrpl/helpers.ts: Lines 16-23
  • wallets/provider-gemwallet/src/utils.ts: Lines 4-16
    Please fix this Kody Rule violation in all listed locations.
Suggested change
.action(async (_, account, options) => {
.action(async (_: unknown, account: string, options?: { peer?: string }) => {
Warning

This is an experimental feature that generates committable changes. Review the diff before applying. Results may be incorrect.

Prompt for LLM

File wallets/provider-gemwallet/src/namespaces/xrpl/namespace.ts:

Line 55 to 68:

My project enforces a strict rule against using the `any` type to maintain type safety, especially in critical areas like wallet interactions. The following TypeScript code defines an `accountLines` action where the function parameters `_`, `account`, and `options` are implicitly typed as `any`. Refactor this function to include explicit types. From the code's context, the first parameter `_` is unused, `account` is a string representing an address, and `options` is an optional object with a potential `peer` property.

Suggested Code:

  .action(async (_: unknown, account: string, options?: { peer?: string }) => {
    const client = new Client(XRPL_PUBLIC_SERVER);
    await client.connect();

    const response = await client.request({
      command: 'account_lines',
      ledger_index: 'current',
      account: account,
      peer: options?.peer,
    });

    await client.disconnect();
    return response.result.lines;
  })

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

const client = new Client(XRPL_PUBLIC_SERVER);
await client.connect();

const response = await client.request({
command: 'account_lines',
ledger_index: 'current',
account: account,
peer: options?.peer,
});

await client.disconnect();
return response.result.lines;
Comment on lines +55 to +67
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect other XRPL namespace implementations for client lifecycle pattern consistency.
rg -n -C3 'new Client\(|client\.connect\(|client\.disconnect\(' --type=ts

Repository: RyukTheCoder/rango-client

Length of output: 3090


🏁 Script executed:

# Check the full namespace file to see all action handlers and client usage patterns
cat -n wallets/provider-gemwallet/src/namespaces/xrpl/namespace.ts

Repository: RyukTheCoder/rango-client

Length of output: 2687


🏁 Script executed:

# Search for all client lifecycle patterns in the XRPL namespace directory
rg -n 'new Client|\.connect\(|\.disconnect\(' wallets/provider-gemwallet/src/namespaces/xrpl/ --type=ts -C2

Repository: RyukTheCoder/rango-client

Length of output: 1440


Wrap client request in try-finally to ensure disconnect on errors.

If client.request() at line 59 throws, the disconnect() call is skipped, leaving the connection open.

Proposed fix
 .action(async (_, account, options) => {
   const client = new Client(XRPL_PUBLIC_SERVER);
-  await client.connect();
-
-  const response = await client.request({
-    command: 'account_lines',
-    ledger_index: 'current',
-    account: account,
-    peer: options?.peer,
-  });
-
-  await client.disconnect();
-  return response.result.lines;
+  await client.connect();
+  try {
+    const response = await client.request({
+      command: 'account_lines',
+      ledger_index: 'current',
+      account: account,
+      peer: options?.peer,
+    });
+    return response.result.lines;
+  } finally {
+    await client.disconnect();
+  }
 })
📝 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
.action(async (_, account, options) => {
const client = new Client(XRPL_PUBLIC_SERVER);
await client.connect();
const response = await client.request({
command: 'account_lines',
ledger_index: 'current',
account: account,
peer: options?.peer,
});
await client.disconnect();
return response.result.lines;
.action(async (_, account, options) => {
const client = new Client(XRPL_PUBLIC_SERVER);
await client.connect();
try {
const response = await client.request({
command: 'account_lines',
ledger_index: 'current',
account: account,
peer: options?.peer,
});
return response.result.lines;
} finally {
await client.disconnect();
}
})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@wallets/provider-gemwallet/src/namespaces/xrpl/namespace.ts` around lines 55
- 67, The async action handler that creates a Client (new
Client(XRPL_PUBLIC_SERVER)) must ensure client.disconnect() runs even if
client.request(...) throws; wrap the connect/request sequence in a try { await
client.connect(); const response = await client.request(...); return
response.result.lines; } finally { await client.disconnect(); } so disconnect is
always called (use the existing Client, XRPL_PUBLIC_SERVER, .action async
handler and client.connect/request/disconnect symbols).

Comment on lines +55 to +67
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

4. Xrpl client leak on error 🐞 Bug ☼ Reliability

accountLines connects an xrpl Client and disconnects only on the success path; if connect() or
request() throws, the websocket may remain open and leak resources.
Agent Prompt
### Issue description
`accountLines` does not guarantee `client.disconnect()` will run when `connect()` or `request()` throws.

### Issue Context
This can leak websocket connections and degrade long-running sessions.

### Fix Focus Areas
- wallets/provider-gemwallet/src/namespaces/xrpl/namespace.ts[55-67]

### Implementation direction
Wrap connect/request in `try { ... } finally { await client.disconnect().catch(() => {}) }` (and consider guarding `disconnect` if `connect` fails).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

})
.build();

export const namespace = new NamespaceBuilder<XRPLActions>('XRPL', WALLET_ID)
.action(connect)
.action(canEagerConnect)
.action(accountLines)
.build();
Loading
Loading