Feat/rf 2840 migrating tonconnect to hub#4
Open
RyukTheCoder wants to merge 7 commits intonextfrom
Open
Conversation
# Conflicts: # wallets/core/package.json # wallets/core/src/hub/provider/types.ts # wallets/react/src/hub/helpers.ts # wallets/readme.md # Conflicts: # wallets/provider-all/src/index.ts # wallets/readme.md
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
📝 WalkthroughWalkthroughAdding comprehensive TON blockchain support to the wallet management system. A new TON namespace module is created in wallets/core with types, utilities, and builders. The Hub is extended to accept and pass per-provider configuration arguments. The TonConnect provider is refactored from a monolithic structure into a modular architecture using versioned exports. React integration is updated to configure wallet-specific options. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
![kody-ai[bot]](https://avatars.githubusercontent.com/in/413034?s=60&v=4){kind=small}
Comment on lines
+24
to
+25
| We set **'installed'** to `true` on initialization even if we can't initialize the TonConnect instance. | ||
| Instead, we throw an error when the user tries to connect and we haven't initialized the TonConnect instance. |
There was a problem hiding this comment.
Logic error: The 'installed' property is hardcoded to true regardless of whether the TonConnect instance is successfully initialized. This violates the provider's availability invariant, causing the UI to display the wallet as available even when it is in an unusable state (e.g., due to missing or invalid configuration). Consequently, users will encounter unexpected errors when attempting to connect to a wallet that incorrectly claimed to be installed.
We set **'installed'** to `true` only after the TonConnect instance is successfully initialized with the provided configuration.
If initialization fails or configuration is missing, **'installed'** remains `false` to prevent the UI from offering an unusable connection option.Prompt for LLM
File wallets/provider-tonconnect/readme.md:
Line 24 to 25:
Identify and suggest improvements for the logic error in the TonConnect initialization process.
Suggested Code:
We set **'installed'** to `true` only after the TonConnect instance is successfully initialized with the provided configuration.
If initialization fails or configuration is missing, **'installed'** remains `false` to prevent the UI from offering an unusable connection option.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (5)
wallets/core/src/namespaces/ton/types.ts (1)
14-15: PreferunknownoveranyforProviderAPI.
Record<string, unknown>keeps flexibility while avoiding unsafe propagation ofany.♻️ Proposed refactor
-// eslint-disable-next-line `@typescript-eslint/no-explicit-any` -export type ProviderAPI = Record<string, any>; +export type ProviderAPI = Record<string, unknown>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wallets/core/src/namespaces/ton/types.ts` around lines 14 - 15, Replace the unsafe any-based type alias ProviderAPI with a safer unknown-based type: change export type ProviderAPI = Record<string, any> to export type ProviderAPI = Record<string, unknown>, and update any downstream places that rely on implicit any (e.g., functions or methods that index into ProviderAPI or pass it to other APIs) to perform proper type guards or casts before accessing properties or assigning to narrower types; ensure symbols referencing ProviderAPI (the ProviderAPI type itself, and any functions accepting or returning ProviderAPI) compile after the change by adding explicit casts or runtime checks where necessary.wallets/core/src/namespaces/ton/utils.ts (1)
7-17: Remove redundant.toString()call.Since
accountis already typed asstring(fromaccounts: string[]), the.toString()call on line 11 is unnecessary.♻️ Suggested simplification
export function formatAccountsToCAIP(accounts: string[]) { return accounts.map( (account) => AccountId.format({ - address: account.toString(), + address: account, chainId: { namespace: CAIP_NAMESPACE, reference: CAIP_TON_CHAIN_ID, }, }) as CaipAccount ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wallets/core/src/namespaces/ton/utils.ts` around lines 7 - 17, The function formatAccountsToCAIP unnecessarily calls .toString() on each account string; remove the redundant .toString() and pass account directly into AccountId.format in formatAccountsToCAIP so it uses the string value (replace account.toString() with account) while keeping the existing AccountId.format payload (namespace/ reference) intact.wallets/react/src/legacy/types.ts (1)
83-90: Index signature vs mapped type syntax.If
WalletTypeis a string literal union (e.g.,'metamask' | 'tonconnect' | ...), the current[key: WalletType]syntax creates an index signature that accepts anyWalletTypeas a key but doesn't enforce that only valid wallet types can be used. Consider using a mapped type withPartialfor stricter typing:♻️ Suggested type refinement
walletOptions?: { - [key: WalletType]: { + [key in WalletType]?: { provider?: unknown; namespaces?: { [namespaceId: string]: unknown; }; - }; + }; };Or alternatively:
walletOptions?: Partial<Record<WalletType, { provider?: unknown; namespaces?: Record<string, unknown>; }>>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wallets/react/src/legacy/types.ts` around lines 83 - 90, The walletOptions type uses an index signature "[key: WalletType]" which doesn't enforce that only the union members of WalletType are allowed as keys; change it to a mapped/utility type so keys are limited to WalletType (e.g., use Partial<Record<WalletType, { provider?: unknown; namespaces?: Record<string, unknown> }>> or an equivalent mapped type like { [K in WalletType]?: { provider?: unknown; namespaces?: Record<string, unknown> } }). Update the namespaces inner type from the ad-hoc index signature to Record<string, unknown> for clarity.wallets/provider-tonconnect/src/provider.ts (1)
19-21: Don’t silently swallow initialization failures.The empty catch makes TonConnect init failures invisible in logs and harder to diagnose in production.
🔧 Proposed improvement
- } catch (_) { - /* empty */ + } catch (error) { + console.warn('[ton] initialization failed.', error); } finally {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wallets/provider-tonconnect/src/provider.ts` around lines 19 - 21, The catch block in provider.ts that swallows errors during TonConnect initialization (the catch (_) { /* empty */ } around the TonConnect init) hides failures; change it to capture the error (e) and surface it by logging the error with context (e.g., "Failed to initialize TonConnect" plus error.stack/message) using the existing logger or console.error, and either rethrow the error or set a visible initialization-failed state/flag so callers can handle it instead of silently continuing.wallets/provider-tonconnect/src/actions/ton.ts (1)
46-52: NarrowgetInstancetype to eliminate unsafe cast.Line 50 casts
getInstance()toTonConnectUIdespite the parameter type being() => TonProviderAPI. SinceTonConnect.getInstance()already returnsTonConnectUIand the sole call site passestonConnect.getInstance, narrowing the parameter type is safe and aligns with the pattern used in theconnect()function in the same file.Proposed refactor
export function canEagerConnect( - getInstance: () => TonProviderAPI + getInstance: () => TonConnectUI ): FunctionWithContext<TonActions['canEagerConnect'], Context> { return async () => { - const tonConnectUI = getInstance() as TonConnectUI; + const tonConnectUI = getInstance(); const connectionRestored = await tonConnectUI.connectionRestored; return connectionRestored; }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wallets/provider-tonconnect/src/actions/ton.ts` around lines 46 - 52, The parameter getInstance for canEagerConnect is typed as () => TonProviderAPI but the implementation casts its result to TonConnectUI; change the parameter type to () => TonConnectUI (matching the actual caller tonConnect.getInstance and the pattern used in connect()) and remove the unsafe cast inside canEagerConnect so you call getInstance() directly as a TonConnectUI to access connectionRestored.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@wallets/core/src/hub/hub.ts`:
- Around line 77-79: The per-namespace init arguments passed from hub (where
namespaceMethod is invoked with
args?.[provider.id]?.namespaces?.[namespace.namespaceId]) are dropped because
Namespace.init is defined without parameters; update the Namespace API to accept
and forward those args: change the signature of init in
wallets/core/src/hub/namespaces/namespace.ts (and its implementations) to accept
an optional parameter (e.g., init(initArgs?: unknown)) and ensure each concrete
namespace implementation consumes or forwards initArgs appropriately so the
hub-provided per-namespace args are honored.
- Around line 77-79: The namespace methods (e.g., Namespace.init(),
Namespace.state() and other instance methods) are being invoked detached via
namespaceMethod(...) so their `this` is lost and private fields/methods like
`this.#initiated`, `this.#store` and `this.#storeId()` fail; fix by invoking the
method with the namespace instance as its context (call the function with
.call(namespace, ...) or otherwise bind namespaceMethod to the namespace) when
calling namespaceMethod in hub.ts so the method runs with the correct `this`.
In `@wallets/provider-tonconnect/readme.md`:
- Around line 5-10: Change the vague link text "More about implementation
status" to a descriptive label such as "Implementation status in the project
README" (so screen readers know destination) and update the link target as in
the diff; also fix the heading-level jump by using a consistent heading level
(e.g., change "#### ⚠️ Initialization" to "### ⚠️ Initialization" or adjust
surrounding headings to avoid skipping from H2 to H4), and ensure the config
keys mentioned remain code-formatted exactly as
configs.walletOptions[WalletTypes.TON_CONNECT] (alias
configs.walletOptions.tonconnect) for clarity and accessibility.
In `@wallets/provider-tonconnect/src/hooks/ton.ts`:
- Around line 17-36: The handler (context, err) => { ... } currently checks
instance() before rethrowing an upstream error, which can mask the original
error; modify the logic so that if err is truthy and instanceof Error the
function rethrows err immediately before calling instance(), so the original
error is preserved; keep the subsequent logic that creates eventCallback, calls
instance(), assigns unsubscribe = tonInstance.onStatusChange(eventCallback), and
only then handles missing instance errors.
In `@wallets/provider-tonconnect/src/tonConnect.ts`:
- Around line 16-19: The initialization guard in initialize() is inverted: after
assigning this.#env the code uses "if (this.#env) throw new Error('Environments
are not set')", which always throws; change the condition to check for missing
env (e.g., if (!this.#env) throw ...) so initialize() only throws when the
environment value is falsy; update the guard around this.#env in the
initialize() method of the TonConnect-related class accordingly.
In `@wallets/provider-tonconnect/src/types.ts`:
- Line 1: The import of LegacyNetworks must be a runtime import (not a type-only
import) because the enum LegacyNetworks is used as a value in a computed
property key (LegacyNetworks.TON); change the line that currently uses "import
type { LegacyNetworks }" to a normal import so LegacyNetworks is emitted at
runtime and can be accessed by the computed key in this file (types.ts),
ensuring any usage like LegacyNetworks.TON works as intended.
In `@wallets/react/src/hub/useHubAdapter.ts`:
- Line 89: The init call getHub().init(params.configs?.walletOptions) can use a
stale first-render walletOptions; update the logic so init uses the latest
walletOptions snapshot (not just the initial params) — e.g., store walletOptions
in the shared dataRef used by the readystatechange handler and ensure dataRef is
updated whenever params.configs?.walletOptions changes so the delayed
readystatechange/init uses the current config; specifically, update the place
that sets dataRef (and any related useEffect or subscription) to include
walletOptions and change the readystatechange/init code to read walletOptions
from dataRef instead of the original params variable.
In `@widget/embedded/src/containers/Wallets/Wallets.tsx`:
- Around line 108-112: walletOptions currently always includes a
WalletTypes.TON_CONNECT entry with provider.manifestUrl set from
config.tonConnect?.manifestUrl which can be undefined; change the construction
of walletOptions so the TON_CONNECT entry is only added when
config.tonConnect?.manifestUrl is truthy (e.g., compute manifestUrl =
config.tonConnect?.manifestUrl and conditionally spread or assign the
[WalletTypes.TON_CONNECT] object only if manifestUrl exists) to avoid emitting
an invalid manifestUrl into provider options.
---
Nitpick comments:
In `@wallets/core/src/namespaces/ton/types.ts`:
- Around line 14-15: Replace the unsafe any-based type alias ProviderAPI with a
safer unknown-based type: change export type ProviderAPI = Record<string, any>
to export type ProviderAPI = Record<string, unknown>, and update any downstream
places that rely on implicit any (e.g., functions or methods that index into
ProviderAPI or pass it to other APIs) to perform proper type guards or casts
before accessing properties or assigning to narrower types; ensure symbols
referencing ProviderAPI (the ProviderAPI type itself, and any functions
accepting or returning ProviderAPI) compile after the change by adding explicit
casts or runtime checks where necessary.
In `@wallets/core/src/namespaces/ton/utils.ts`:
- Around line 7-17: The function formatAccountsToCAIP unnecessarily calls
.toString() on each account string; remove the redundant .toString() and pass
account directly into AccountId.format in formatAccountsToCAIP so it uses the
string value (replace account.toString() with account) while keeping the
existing AccountId.format payload (namespace/ reference) intact.
In `@wallets/provider-tonconnect/src/actions/ton.ts`:
- Around line 46-52: The parameter getInstance for canEagerConnect is typed as
() => TonProviderAPI but the implementation casts its result to TonConnectUI;
change the parameter type to () => TonConnectUI (matching the actual caller
tonConnect.getInstance and the pattern used in connect()) and remove the unsafe
cast inside canEagerConnect so you call getInstance() directly as a TonConnectUI
to access connectionRestored.
In `@wallets/provider-tonconnect/src/provider.ts`:
- Around line 19-21: The catch block in provider.ts that swallows errors during
TonConnect initialization (the catch (_) { /* empty */ } around the TonConnect
init) hides failures; change it to capture the error (e) and surface it by
logging the error with context (e.g., "Failed to initialize TonConnect" plus
error.stack/message) using the existing logger or console.error, and either
rethrow the error or set a visible initialization-failed state/flag so callers
can handle it instead of silently continuing.
In `@wallets/react/src/legacy/types.ts`:
- Around line 83-90: The walletOptions type uses an index signature "[key:
WalletType]" which doesn't enforce that only the union members of WalletType are
allowed as keys; change it to a mapped/utility type so keys are limited to
WalletType (e.g., use Partial<Record<WalletType, { provider?: unknown;
namespaces?: Record<string, unknown> }>> or an equivalent mapped type like { [K
in WalletType]?: { provider?: unknown; namespaces?: Record<string, unknown> }
}). Update the namespaces inner type from the ad-hoc index signature to
Record<string, unknown> for clarity.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 83e50fb1-f760-4849-8f2b-e483fbb7d0b2
📒 Files selected for processing (31)
wallets/core/namespaces/ton/package.jsonwallets/core/package.jsonwallets/core/src/hub/hub.tswallets/core/src/hub/provider/provider.tswallets/core/src/hub/provider/types.tswallets/core/src/namespaces/ton/builders.tswallets/core/src/namespaces/ton/constants.tswallets/core/src/namespaces/ton/mod.tswallets/core/src/namespaces/ton/types.tswallets/core/src/namespaces/ton/utils.tswallets/provider-all/src/index.tswallets/provider-tonconnect/package.jsonwallets/provider-tonconnect/readme.mdwallets/provider-tonconnect/src/actions/ton.tswallets/provider-tonconnect/src/constants.tswallets/provider-tonconnect/src/helpers.tswallets/provider-tonconnect/src/hooks/ton.tswallets/provider-tonconnect/src/index.tswallets/provider-tonconnect/src/mod.tswallets/provider-tonconnect/src/namespaces/ton.tswallets/provider-tonconnect/src/provider.tswallets/provider-tonconnect/src/signer.tswallets/provider-tonconnect/src/signers/ton.tswallets/provider-tonconnect/src/tonConnect.tswallets/provider-tonconnect/src/types.tswallets/provider-tonconnect/src/utils.tswallets/react/src/hub/helpers.tswallets/react/src/hub/useHubAdapter.tswallets/react/src/legacy/types.tswallets/readme.mdwidget/embedded/src/containers/Wallets/Wallets.tsx
💤 Files with no reviewable changes (2)
- wallets/provider-tonconnect/src/index.ts
- wallets/provider-tonconnect/src/helpers.ts
Comment on lines
+77
to
+79
| const result = namespaceMethod( | ||
| args?.[provider.id]?.namespaces?.[namespace.namespaceId] | ||
| ); |
There was a problem hiding this comment.
Namespace init args are forwarded but currently not consumed.
runAll('init', args) passes per-namespace args, but wallets/core/src/hub/namespaces/namespace.ts defines init() without parameters, so namespace init args are effectively dropped.
🔧 Suggested follow-up change (outside this file)
- public init(): void {
+ public init(args?: unknown): void {
if (this.#initiated) {
return;
}
const definedInitByUser = this.#actions.get('init');
if (definedInitByUser) {
- definedInitByUser(this.#context());
+ definedInitByUser(this.#context(), args);
}
this.#initiated = true;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wallets/core/src/hub/hub.ts` around lines 77 - 79, The per-namespace init
arguments passed from hub (where namespaceMethod is invoked with
args?.[provider.id]?.namespaces?.[namespace.namespaceId]) are dropped because
Namespace.init is defined without parameters; update the Namespace API to accept
and forward those args: change the signature of init in
wallets/core/src/hub/namespaces/namespace.ts (and its implementations) to accept
an optional parameter (e.g., init(initArgs?: unknown)) and ensure each concrete
namespace implementation consumes or forwards initArgs appropriately so the
hub-provided per-namespace args are honored.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify detached namespace method invocation and `this` usage in namespace methods.
rg -n -C3 'const namespaceMethod = namespace\[action\]|namespaceMethod\(' wallets/core/src/hub/hub.ts
rg -n -C4 'public (init|state)\(' wallets/core/src/hub/namespaces/namespace.ts
rg -n -C2 'this\.\#' wallets/core/src/hub/namespaces/namespace.tsRepository: RyukTheCoder/rango-client
Length of output: 7057
Bind namespace methods to preserve this context.
Namespace methods are invoked detached from their instance. The init(), state(), and all other methods in the Namespace class access private fields (e.g., this.#initiated, this.#store) or call private methods (e.g., this.#storeId(), this.#context()). Without proper binding, this will be undefined, causing runtime errors. Invoke with .call(namespace, ...).
🔧 Proposed fix
- const result = namespaceMethod(
- args?.[provider.id]?.namespaces?.[namespace.namespaceId]
- );
+ const result = namespaceMethod.call(
+ namespace,
+ args?.[provider.id]?.namespaces?.[namespace.namespaceId]
+ );📝 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 result = namespaceMethod( | |
| args?.[provider.id]?.namespaces?.[namespace.namespaceId] | |
| ); | |
| const result = namespaceMethod.call( | |
| namespace, | |
| args?.[provider.id]?.namespaces?.[namespace.namespaceId] | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wallets/core/src/hub/hub.ts` around lines 77 - 79, The namespace methods
(e.g., Namespace.init(), Namespace.state() and other instance methods) are being
invoked detached via namespaceMethod(...) so their `this` is lost and private
fields/methods like `this.#initiated`, `this.#store` and `this.#storeId()` fail;
fix by invoking the method with the namespace instance as its context (call the
function with .call(namespace, ...) or otherwise bind namespaceMethod to the
namespace) when calling namespaceMethod in hub.ts so the method runs with the
correct `this`.
Comment on lines
+5
to
+10
| More about implementation status can be found [here](../readme.md). | ||
|
|
||
| ## Implementation notes/limitations | ||
|
|
||
| #### ⚠️ Initialization | ||
| You should provide TonConnect configs in configs.walletOptions[WalletTypes.TON_CONNECT] (which equals configs.walletOptions.tonconnect) |
There was a problem hiding this comment.
Fix markdown accessibility/readability lint issues.
Use descriptive link text (Line 5) and avoid heading-level jumps (Line 9).
🔧 Proposed doc fix
-More about implementation status can be found [here](../readme.md).
+More about implementation status can be found in the [wallet providers README](../readme.md).
-#### ⚠️ Initialization
+### ⚠️ Initialization📝 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). | |
| ## Implementation notes/limitations | |
| #### ⚠️ Initialization | |
| You should provide TonConnect configs in configs.walletOptions[WalletTypes.TON_CONNECT] (which equals configs.walletOptions.tonconnect) | |
| More about implementation status can be found in the [wallet providers README](../readme.md). | |
| ## Implementation notes/limitations | |
| ### ⚠️ Initialization | |
| You should provide TonConnect configs in configs.walletOptions[WalletTypes.TON_CONNECT] (which equals configs.walletOptions.tonconnect) |
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 5-5: Link text should be descriptive
(MD059, descriptive-link-text)
[warning] 9-9: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wallets/provider-tonconnect/readme.md` around lines 5 - 10, Change the vague
link text "More about implementation status" to a descriptive label such as
"Implementation status in the project README" (so screen readers know
destination) and update the link target as in the diff; also fix the
heading-level jump by using a consistent heading level (e.g., change "#### ⚠️
Initialization" to "### ⚠️ Initialization" or adjust surrounding headings to
avoid skipping from H2 to H4), and ensure the config keys mentioned remain
code-formatted exactly as configs.walletOptions[WalletTypes.TON_CONNECT] (alias
configs.walletOptions.tonconnect) for clarity and accessibility.
Comment on lines
+17
to
+36
| (context, err) => { | ||
| const tonInstance = instance(); | ||
|
|
||
| if (!tonInstance) { | ||
| throw new Error( | ||
| 'Trying to subscribe to your Ton wallet, but seems its instance is not available.' | ||
| ); | ||
| } | ||
|
|
||
| eventCallback = (event) => { | ||
| if (!event.payload) { | ||
| context.action('disconnect'); | ||
| } | ||
| }; | ||
|
|
||
| unsubscribe = tonInstance.onStatusChange(eventCallback); | ||
|
|
||
| if (err instanceof Error) { | ||
| throw err; | ||
| } |
There was a problem hiding this comment.
Preserve upstream errors before trying to subscribe.
When err exists, this block should rethrow immediately. Current ordering can replace the original failure with a secondary “instance not available” error.
🔧 Proposed fix
(context, err) => {
+ if (err instanceof Error) {
+ throw err;
+ }
+
const tonInstance = instance();
if (!tonInstance) {
throw new Error(
'Trying to subscribe to your Ton wallet, but seems its instance is not available.'
);
}
@@
- if (err instanceof Error) {
- throw err;
- }
},📝 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
| (context, err) => { | |
| const tonInstance = instance(); | |
| if (!tonInstance) { | |
| throw new Error( | |
| 'Trying to subscribe to your Ton wallet, but seems its instance is not available.' | |
| ); | |
| } | |
| eventCallback = (event) => { | |
| if (!event.payload) { | |
| context.action('disconnect'); | |
| } | |
| }; | |
| unsubscribe = tonInstance.onStatusChange(eventCallback); | |
| if (err instanceof Error) { | |
| throw err; | |
| } | |
| (context, err) => { | |
| if (err instanceof Error) { | |
| throw err; | |
| } | |
| const tonInstance = instance(); | |
| if (!tonInstance) { | |
| throw new Error( | |
| 'Trying to subscribe to your Ton wallet, but seems its instance is not available.' | |
| ); | |
| } | |
| eventCallback = (event) => { | |
| if (!event.payload) { | |
| context.action('disconnect'); | |
| } | |
| }; | |
| unsubscribe = tonInstance.onStatusChange(eventCallback); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wallets/provider-tonconnect/src/hooks/ton.ts` around lines 17 - 36, The
handler (context, err) => { ... } currently checks instance() before rethrowing
an upstream error, which can mask the original error; modify the logic so that
if err is truthy and instanceof Error the function rethrows err immediately
before calling instance(), so the original error is preserved; keep the
subsequent logic that creates eventCallback, calls instance(), assigns
unsubscribe = tonInstance.onStatusChange(eventCallback), and only then handles
missing instance errors.
Comment on lines
+16
to
+19
| this.#env = env; | ||
| if (this.#env) { | ||
| throw new Error('Environments are not set'); | ||
| } |
There was a problem hiding this comment.
Initialization always fails due to inverted guard logic.
After Line 16 assigns this.#env, Line 17 always evaluates true and throws, so initialize() can never succeed.
Proposed fix
async initialize(env?: Environments) {
if (!env) {
throw new Error('Environments are not set');
}
- this.#env = env;
- if (this.#env) {
- throw new Error('Environments are not set');
+ if (this.#env) {
+ throw new Error('TonConnect is already initialized');
}
+ this.#env = env;
this.#tonModule = await dynamicImportWithRefinedError(
async () => await import('@tonconnect/ui')
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wallets/provider-tonconnect/src/tonConnect.ts` around lines 16 - 19, The
initialization guard in initialize() is inverted: after assigning this.#env the
code uses "if (this.#env) throw new Error('Environments are not set')", which
always throws; change the condition to check for missing env (e.g., if
(!this.#env) throw ...) so initialize() only throws when the environment value
is falsy; update the guard around this.#env in the initialize() method of the
TonConnect-related class accordingly.
| export interface Environments extends Record<string, string | undefined> { | ||
| manifestUrl: string; | ||
| } | ||
| import type { LegacyNetworks } from '@rango-dev/wallets-core/legacy'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the file and check its content
find . -name "types.ts" -path "*/provider-tonconnect/src/*" -type fRepository: RyukTheCoder/rango-client
Length of output: 111
🏁 Script executed:
# Read the first 20 lines of the types.ts file to see the import and usage
head -20 wallets/provider-tonconnect/src/types.ts | cat -nRepository: RyukTheCoder/rango-client
Length of output: 562
🏁 Script executed:
# Search for LegacyNetworks usage in the file
rg -n "LegacyNetworks" wallets/provider-tonconnect/src/types.ts -A 2 -B 1Repository: RyukTheCoder/rango-client
Length of output: 254
🏁 Script executed:
# Check the LegacyNetworks declaration in the core package
find . -path "*wallets-core*" -name "*.ts" -o -name "*.tsx" | head -20Repository: RyukTheCoder/rango-client
Length of output: 937
🏁 Script executed:
# Search for LegacyNetworks definition
rg -n "export.*LegacyNetworks" --type ts --type tsxRepository: RyukTheCoder/rango-client
Length of output: 96
🏁 Script executed:
# Search for LegacyNetworks export definition
rg -n "export.*LegacyNetworks" -t tsRepository: RyukTheCoder/rango-client
Length of output: 183
🏁 Script executed:
# Try searching in the wallets-core package
find . -path "*wallets-core*" -name "*.ts" | head -30Repository: RyukTheCoder/rango-client
Length of output: 51
🏁 Script executed:
# Look for LegacyNetworks in legacy file
fd -e ts -e js | xargs rg -l "LegacyNetworks" | grep legacyRepository: RyukTheCoder/rango-client
Length of output: 279
🏁 Script executed:
# Search for the export directly
rg "LegacyNetworks\s*=" -t tsRepository: RyukTheCoder/rango-client
Length of output: 51
🏁 Script executed:
# Check the Networks definition in the types file
cat -n wallets/core/src/legacy/types.ts | head -50Repository: RyukTheCoder/rango-client
Length of output: 1556
🏁 Script executed:
# Search for Networks enum or const in the legacy types
rg -n "export.*Networks" wallets/core/src/legacy/types.ts -A 5Repository: RyukTheCoder/rango-client
Length of output: 375
🏁 Script executed:
# Check if TON exists in the Networks enum
rg -n "TON\s*=" wallets/core/src/legacy/types.tsRepository: RyukTheCoder/rango-client
Length of output: 86
🏁 Script executed:
# Get the full Networks enum to see all members
sed -n '5,100p' wallets/core/src/legacy/types.ts | cat -nRepository: RyukTheCoder/rango-client
Length of output: 2694
LegacyNetworks should not be imported with import type here.
Line 1 uses a type-only import, but line 10 uses LegacyNetworks.TON as a computed property key. LegacyNetworks is an enum, which requires value-space access at runtime. Type-only imports are erased and cannot be accessed in computed keys.
🐛 Proposed fix
-import type { LegacyNetworks } from '@rango-dev/wallets-core/legacy';
+import { LegacyNetworks } from '@rango-dev/wallets-core/legacy';Also applies to: 10-10
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wallets/provider-tonconnect/src/types.ts` at line 1, The import of
LegacyNetworks must be a runtime import (not a type-only import) because the
enum LegacyNetworks is used as a value in a computed property key
(LegacyNetworks.TON); change the line that currently uses "import type {
LegacyNetworks }" to a normal import so LegacyNetworks is emitted at runtime and
can be accessed by the computed key in this file (types.ts), ensuring any usage
like LegacyNetworks.TON works as intended.
| useEffect(() => { | ||
| const runOnInit = () => { | ||
| getHub().init(); | ||
| getHub().init(params.configs?.walletOptions); |
There was a problem hiding this comment.
Hub init is using potentially stale wallet options.
Because this effect runs once, Line 89 reads the first-render params.configs?.walletOptions. If wallet configs are populated/updated after mount, init may run without TON options.
💡 Suggested direction
- getHub().init(params.configs?.walletOptions);
+ getHub().init(dataRef.current.walletOptions);And include walletOptions in dataRef updates so delayed readystatechange init uses the latest config snapshot.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wallets/react/src/hub/useHubAdapter.ts` at line 89, The init call
getHub().init(params.configs?.walletOptions) can use a stale first-render
walletOptions; update the logic so init uses the latest walletOptions snapshot
(not just the initial params) — e.g., store walletOptions in the shared dataRef
used by the readystatechange handler and ensure dataRef is updated whenever
params.configs?.walletOptions changes so the delayed readystatechange/init uses
the current config; specifically, update the place that sets dataRef (and any
related useEffect or subscription) to include walletOptions and change the
readystatechange/init code to read walletOptions from dataRef instead of the
original params variable.
Comment on lines
+108
to
+112
| walletOptions: { | ||
| [WalletTypes.TON_CONNECT]: { | ||
| provider: { manifestUrl: config.tonConnect?.manifestUrl }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Guard TON walletOptions to avoid emitting an invalid manifestUrl.
Line 110 can pass undefined into provider options. Build this entry conditionally so TON config is only sent when manifestUrl exists.
🐛 Proposed fix
- walletOptions: {
- [WalletTypes.TON_CONNECT]: {
- provider: { manifestUrl: config.tonConnect?.manifestUrl },
- },
- },
+ walletOptions: config.tonConnect?.manifestUrl
+ ? {
+ [WalletTypes.TON_CONNECT]: {
+ provider: { manifestUrl: config.tonConnect.manifestUrl },
+ },
+ }
+ : undefined,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@widget/embedded/src/containers/Wallets/Wallets.tsx` around lines 108 - 112,
walletOptions currently always includes a WalletTypes.TON_CONNECT entry with
provider.manifestUrl set from config.tonConnect?.manifestUrl which can be
undefined; change the construction of walletOptions so the TON_CONNECT entry is
only added when config.tonConnect?.manifestUrl is truthy (e.g., compute
manifestUrl = config.tonConnect?.manifestUrl and conditionally spread or assign
the [WalletTypes.TON_CONNECT] object only if manifestUrl exists) to avoid
emitting an invalid manifestUrl into provider options.
Review Summary by QodoMigrate TonConnect to hub architecture with TON namespace support
WalkthroughsDescription• Migrated TonConnect wallet to hub architecture with new namespace support • Added TON blockchain namespace with builders, utils, and types • Implemented Hub initialization with provider-specific configuration arguments • Refactored TonConnect provider to use new hub provider pattern • Updated documentation with TON wallet support status and implementation notes Diagramflowchart LR
A["Hub Init System"] -->|"accepts RunAllArgs"| B["Provider Init"]
B -->|"passes config"| C["TonConnect Provider"]
C -->|"initializes"| D["TonConnect Instance"]
E["TON Namespace"] -->|"connect/disconnect"| D
E -->|"uses builders & utils"| F["CAIP Account Formatting"]
File Changes1. wallets/core/src/hub/hub.ts
|
Code Review by Qodo
1. TonConnect init always fails
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+12
to
+26
| async initialize(env?: Environments) { | ||
| if (!env) { | ||
| throw new Error('Environments are not set'); | ||
| } | ||
| this.#env = env; | ||
| if (this.#env) { | ||
| throw new Error('Environments are not set'); | ||
| } | ||
|
|
||
| this.#tonModule = await dynamicImportWithRefinedError( | ||
| async () => await import('@tonconnect/ui') | ||
| ); | ||
| const { TonConnectUI } = this.#tonModule; | ||
| this.#tonConnectInstance = new TonConnectUI(this.#env); | ||
| } |
There was a problem hiding this comment.
1. Tonconnect init always fails 🐞 Bug ≡ Correctness
TonConnect.initialize() assigns this.#env = env and then immediately throws when this.#env is truthy, so the TonConnect module/instance never gets created. As a result, provider init silently fails and later getInstance() calls will throw, breaking connect/canEagerConnect.
Agent Prompt
### Issue description
`TonConnect.initialize()` always throws after assigning `this.#env`, preventing TonConnect UI module import and instance creation.
### Issue Context
This makes the TonConnect provider appear installed (provider init sets `installed=true` in `finally`) but unusable because `getInstance()` later throws.
### Fix Focus Areas
- wallets/provider-tonconnect/src/tonConnect.ts[12-26]
- wallets/provider-tonconnect/src/provider.ts[14-30]
### Suggested fix
- Remove the second guard entirely, or change it to the intended condition (e.g. `if (!this.#env) throw ...`).
- Optionally add a clearer error for missing/invalid `manifestUrl` if that’s required by `TonConnectUI`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Added Ton builders, utils and types to the wallets core.
Migrated TonConnect to use the new hub APIs for ton namespaces and updated documentation:
How did you test this change?
TonConnect was tested end-to-end within the Hub environment:
Checklist:
Summary by CodeRabbit
Release Notes
New Features
Documentation