P-992 Add Solana Wallet Adapter integration#157
Conversation
This commit adds support for tracking Solana wallet events in the Formo Analytics SDK. Features: - SolanaWalletAdapterHandler for tracking Solana wallet events - Support for connect/disconnect, signature, and transaction tracking - Solana address validation utilities (Base58 format) - Cluster-to-chainId mapping (mainnet-beta: 900001, etc.) - Transaction confirmation polling - Works alongside EVM tracking (not mutually exclusive) - Dynamic wallet/connection updates for React apps Changes: - Add src/solana/ module with handler, types, and address utilities - Extend Options interface with SolanaOptions - Add setSolanaWallet, setSolanaConnection, setSolanaCluster methods - Add optional peer dependencies for @solana/wallet-adapter-* - Add comprehensive tests and documentation https://claude.ai/code/session_011PKjHn7x4Pmqwm7tojPz4E
yosriady
changed the title
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| public cleanup(): void { | ||
| logger.info("SolanaWalletAdapterHandler: Cleaning up"); | ||
|
|
||
| this.cleanupWalletListeners(); | ||
| this.processedSignatures.clear(); | ||
| this.pendingTransactions.clear(); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- Remove unused imports (DEFAULT_SOLANA_CHAIN_ID, getValidSolanaAddress, SolanaCluster, etc.) - Add validateMultiChainAddress() to support both EVM and Solana addresses in connect() - Fix orphaned setTimeout in pollTransactionConfirmation - now tracks timeout IDs and cancels them on cleanup - Add isCleanedUp flag to prevent polls from running after cleanup https://claude.ai/code/session_011PKjHn7x4Pmqwm7tojPz4E
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| public setWallet( | ||
| wallet: SolanaWalletAdapter | SolanaWalletContext | null | ||
| ): void { | ||
| // Clean up previous wallet listeners | ||
| this.cleanupWalletListeners(); | ||
|
|
There was a problem hiding this comment.
Nonexistent cleanup call
setWallet() calls this.cleanupWalletListeners(), but the only cleanup helper defined is cleanupWalletListeners() (without the extra this. prefix in the name). As written, setWallet() will throw TypeError: this.cleanupWalletListeners is not a function the first time it’s called, breaking runtime wallet updates.
| public setWallet( | |
| wallet: SolanaWalletAdapter | SolanaWalletContext | null | |
| ): void { | |
| // Clean up previous wallet listeners | |
| this.cleanupWalletListeners(); | |
| this.cleanupWalletListeners(); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/solana/SolanaWalletAdapterHandler.ts
Line: 125:130
Comment:
**Nonexistent cleanup call**
`setWallet()` calls `this.cleanupWalletListeners()`, but the only cleanup helper defined is `cleanupWalletListeners()` (without the extra `this.` prefix in the name). As written, `setWallet()` will throw `TypeError: this.cleanupWalletListeners is not a function` the first time it’s called, breaking runtime wallet updates.
```suggestion
this.cleanupWalletListeners();
```
How can I resolve this? If you propose a fix, please make it concise.| public setSolanaWallet(wallet: Parameters<SolanaWalletAdapterHandler["setWallet"]>[0]): void { | ||
| if (this.solanaHandler) { | ||
| this.solanaHandler.setWallet(wallet); | ||
| } else if (wallet) { |
There was a problem hiding this comment.
Runtime crash on setSolanaWallet
FormoAnalytics.setSolanaWallet() delegates to solanaHandler.setWallet(), which currently throws due to calling a nonexistent cleanupWalletListeners() method in SolanaWalletAdapterHandler (see comment there). This makes the new public API method unusable in the common “wallet context updates” scenario.
Also appears in: src/solana/SolanaWalletAdapterHandler.ts:125.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/FormoAnalytics.ts
Line: 1977:1980
Comment:
**Runtime crash on setSolanaWallet**
`FormoAnalytics.setSolanaWallet()` delegates to `solanaHandler.setWallet()`, which currently throws due to calling a nonexistent `cleanupWalletListeners()` method in `SolanaWalletAdapterHandler` (see comment there). This makes the new public API method unusable in the common “wallet context updates” scenario.
Also appears in: `src/solana/SolanaWalletAdapterHandler.ts:125`.
How can I resolve this? If you propose a fix, please make it concise.
| wallet: SolanaWalletAdapter | SolanaWalletContext | null | ||
| ): void { | ||
| // Clean up previous wallet listeners | ||
| this.cleanupWalletListeners(); | ||
|
|
||
| this.wallet = wallet; | ||
|
|
||
| if (this.wallet) { | ||
| this.setupWalletListeners(); | ||
| } | ||
| } |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| address: string, | ||
| chainId?: number | ||
| ): Address | undefined { | ||
| // If chain ID is in Solana range, validate as Solana address | ||
| const solanaChainIds = Object.values(SOLANA_CHAIN_IDS); | ||
| if (chainId && solanaChainIds.includes(chainId)) { | ||
| const validSolanaAddress = getValidSolanaAddress(address); | ||
| return validSolanaAddress || undefined; | ||
| } | ||
|
|
||
| // Check if it looks like a Solana address (Base58, no 0x prefix) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- EventFactory: Add validateMultiChainAddress() to support both EVM and Solana addresses - SolanaWalletAdapterHandler: Store context/adapter references for cleanup - SolanaWalletAdapterHandler: Add restoreOriginalMethods() helper called in setWallet() and cleanup() - SolanaWalletAdapterHandler: Fix double-wrapping by adding setupAdapterEventListenersOnly() for contexts - SolanaWalletAdapterHandler: Rename original method properties to originalAdapter*/originalContext* https://claude.ai/code/session_011PKjHn7x4Pmqwm7tojPz4E
|
|
||
| if (address && this.formo.isAutocaptureEnabled("signature")) { | ||
| // Track signature confirmed | ||
| const signatureHex = Buffer.from(signature).toString("hex"); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- Replace Buffer.from().toString('hex') with uint8ArrayToHex() helper
function for browser compatibility (Buffer is a Node.js API)
- Fix polling timeout ID memory leak by tracking currentTimeoutId and
removing it from the Set when the poll callback fires
https://claude.ai/code/session_011PKjHn7x4Pmqwm7tojPz4E
| /** | ||
| * Set up listeners for a wallet context (useWallet) | ||
| */ | ||
| private setupContextListeners(context: SolanaWalletContext): void { | ||
| // For context-based wallets, we set up event listeners on the inner adapter | ||
| // but only wrap the context methods (not adapter methods) to avoid double tracking. | ||
| // The context methods are the user-facing API that we want to track. | ||
|
|
||
| if (context.wallet) { | ||
| // Only add event listeners (connect/disconnect) on the inner adapter | ||
| // Do NOT wrap adapter methods - we'll wrap context methods instead | ||
| this.setupAdapterEventListenersOnly(context.wallet); | ||
| } | ||
|
|
||
| // Wrap context methods for transaction/signature tracking | ||
| this.wrapContextMethods(context); | ||
| } | ||
|
|
There was a problem hiding this comment.
Context wallet adapter swap
setupContextListeners() only attaches connect/disconnect listeners to context.wallet once at initialization. In @solana/wallet-adapter-react, context.wallet can change when the user selects a different wallet; after that happens, events from the new adapter won’t be tracked, and the old adapter will keep listeners attached (leak + stale events). This needs a mechanism to rebind listeners when context.wallet changes (e.g., require callers to call setSolanaWallet on wallet changes, or add a small polling/observer strategy to detect adapter swaps and re-run setupAdapterEventListenersOnly with cleanup).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/solana/SolanaWalletAdapterHandler.ts
Line: 268:285
Comment:
**Context wallet adapter swap**
`setupContextListeners()` only attaches connect/disconnect listeners to `context.wallet` once at initialization. In `@solana/wallet-adapter-react`, `context.wallet` can change when the user selects a different wallet; after that happens, events from the new adapter won’t be tracked, and the old adapter will keep listeners attached (leak + stale events). This needs a mechanism to rebind listeners when `context.wallet` changes (e.g., require callers to call `setSolanaWallet` on wallet changes, or add a small polling/observer strategy to detect adapter swaps and re-run `setupAdapterEventListenersOnly` with cleanup).
How can I resolve this? If you propose a fix, please make it concise.| private validateMultiChainAddress(address: string | null | undefined): Address | null { | ||
| if (!address) { | ||
| return null; | ||
| } | ||
|
|
||
| // Check if it's a valid Solana address first (Base58, no 0x prefix) | ||
| if (isSolanaAddress(address)) { | ||
| return getValidSolanaAddress(address); | ||
| } | ||
|
|
||
| // Try EVM address validation | ||
| const validEvmAddress = getValidAddress(address); | ||
| if (validEvmAddress) { | ||
| return toChecksumAddress(validEvmAddress); | ||
| } | ||
|
|
||
| return null; |
There was a problem hiding this comment.
EVM address misclassified
validateMultiChainAddress() checks isSolanaAddress() first, so any 32–44 char string comprised solely of Base58 chars will be treated as Solana and returned as-is. This includes some valid EVM addresses without a 0x prefix (40 hex chars are a subset of Base58), which will now bypass EVM validation/checksumming and change the emitted address value. Consider preferring EVM validation first (via getValidAddress) and only falling back to Solana if EVM fails, or require a Solana chainId/context signal before accepting Base58.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/event/EventFactory.ts
Line: 59:75
Comment:
**EVM address misclassified**
`validateMultiChainAddress()` checks `isSolanaAddress()` first, so any 32–44 char string comprised solely of Base58 chars will be treated as Solana and returned as-is. This includes some valid EVM addresses *without* a `0x` prefix (40 hex chars are a subset of Base58), which will now bypass EVM validation/checksumming and change the emitted `address` value. Consider preferring EVM validation first (via `getValidAddress`) and only falling back to Solana if EVM fails, or require a Solana chainId/context signal before accepting Base58.
How can I resolve this? If you propose a fix, please make it concise.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
- Add trackConnectEventOnly/trackDisconnectEventOnly internal methods to FormoAnalytics for emitting events without mutating shared state - Update Solana handler to use internal methods, preventing EVM state corruption in multi-chain setups - Fix transaction polling stopping prematurely on "processed" status by only deduplicating terminal states (confirmed/finalized/err) - Fix checkInitialConnection to emit connect event for auto-connected wallets that were already connected at initialization - Remove redundant SYSTEM_PROGRAM check in isBlockedSolanaAddress https://claude.ai/code/session_011PKjHn7x4Pmqwm7tojPz4E
This comment has been minimized.
This comment has been minimized.
| // Check if it's a valid Solana address first (Base58, no 0x prefix) | ||
| if (isSolanaAddress(address)) { | ||
| return getValidSolanaAddress(address); |
There was a problem hiding this comment.
Solana checked before EVM may misclassify EVM addresses without 0x prefix as Solana (40 hex chars are valid Base58 subset). Prefer EVM validation first or require chainId/context:
| // Check if it's a valid Solana address first (Base58, no 0x prefix) | |
| if (isSolanaAddress(address)) { | |
| return getValidSolanaAddress(address); | |
| // Try EVM address validation first | |
| const validEvmAddress = getValidAddress(address); | |
| if (validEvmAddress) { | |
| return toChecksumAddress(validEvmAddress); | |
| } | |
| // Check if it's a valid Solana address (Base58, no 0x prefix) | |
| if (isSolanaAddress(address)) { | |
| return getValidSolanaAddress(address); | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/event/EventFactory.ts
Line: 64:66
Comment:
Solana checked before EVM may misclassify EVM addresses without `0x` prefix as Solana (40 hex chars are valid Base58 subset). Prefer EVM validation first or require chainId/context:
```suggestion
// Try EVM address validation first
const validEvmAddress = getValidAddress(address);
if (validEvmAddress) {
return toChecksumAddress(validEvmAddress);
}
// Check if it's a valid Solana address (Base58, no 0x prefix)
if (isSolanaAddress(address)) {
return getValidSolanaAddress(address);
}
```
How can I resolve this? If you propose a fix, please make it concise.| private setupContextListeners(context: SolanaWalletContext): void { | ||
| // For context-based wallets, we set up event listeners on the inner adapter | ||
| // but only wrap the context methods (not adapter methods) to avoid double tracking. | ||
| // The context methods are the user-facing API that we want to track. | ||
|
|
||
| if (context.wallet) { | ||
| // Only add event listeners (connect/disconnect) on the inner adapter | ||
| // Do NOT wrap adapter methods - we'll wrap context methods instead | ||
| this.setupAdapterEventListenersOnly(context.wallet); | ||
| } | ||
|
|
||
| // Wrap context methods for transaction/signature tracking | ||
| this.wrapContextMethods(context); | ||
| } |
There was a problem hiding this comment.
setupContextListeners only attaches listeners to context.wallet once. In @solana/wallet-adapter-react, context.wallet changes when user switches wallets. After a wallet switch, new adapter events won't be tracked and old adapter keeps listeners (memory leak + stale events). Either document that callers must call setSolanaWallet on wallet changes, or add a mechanism to detect and rebind listeners when context.wallet changes.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/solana/SolanaWalletAdapterHandler.ts
Line: 271:284
Comment:
`setupContextListeners` only attaches listeners to `context.wallet` once. In `@solana/wallet-adapter-react`, `context.wallet` changes when user switches wallets. After a wallet switch, new adapter events won't be tracked and old adapter keeps listeners (memory leak + stale events). Either document that callers must call `setSolanaWallet` on wallet changes, or add a mechanism to detect and rebind listeners when `context.wallet` changes.
How can I resolve this? If you propose a fix, please make it concise.| } | ||
|
|
||
| // Stop polling if cleaned up | ||
| if (this.isCleanedUp) { | ||
| this.pendingTransactions.delete(signature); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| const result = await conn.getSignatureStatus!(signature); | ||
| const status = result.value; | ||
|
|
||
| if (status) { | ||
| const signatureKey = `${signature}:${status.confirmationStatus}`; | ||
|
|
||
| // Only deduplicate terminal states (confirmed/finalized/err) to prevent | ||
| // duplicate event emissions. Do NOT deduplicate intermediate states like | ||
| // "processed" since we need to keep polling until a terminal state. | ||
| const isTerminalState = | ||
| !!status.err || | ||
| status.confirmationStatus === "confirmed" || | ||
| status.confirmationStatus === "finalized"; | ||
|
|
||
| // Check for duplicate processing of terminal states only | ||
| if (isTerminalState && this.processedSignatures.has(signatureKey)) { | ||
| return; | ||
| } | ||
| if (isTerminalState) { | ||
| this.processedSignatures.add(signatureKey); | ||
| } | ||
|
|
||
| if (status.err) { | ||
| // Transaction failed | ||
| logger.info( | ||
| "SolanaWalletAdapterHandler: Transaction reverted", | ||
| { | ||
| signature, | ||
| error: status.err, | ||
| } | ||
| ); | ||
|
|
||
| this.formo.transaction({ | ||
| status: TransactionStatus.REVERTED, | ||
| chainId: this.chainId, | ||
| address, | ||
| transactionHash: signature, | ||
| }); | ||
|
|
||
| this.pendingTransactions.delete(signature); | ||
| return; | ||
| } | ||
|
|
||
| if ( | ||
| status.confirmationStatus === "confirmed" || | ||
| status.confirmationStatus === "finalized" | ||
| ) { | ||
| // Transaction confirmed | ||
| logger.info( | ||
| "SolanaWalletAdapterHandler: Transaction confirmed", | ||
| { | ||
| signature, | ||
| confirmationStatus: status.confirmationStatus, | ||
| } | ||
| ); | ||
|
|
||
| this.formo.transaction({ | ||
| status: TransactionStatus.CONFIRMED, | ||
| chainId: this.chainId, | ||
| address, | ||
| transactionHash: signature, | ||
| }); | ||
|
|
||
| this.pendingTransactions.delete(signature); | ||
| return; | ||
| } | ||
| } | ||
| } catch (error) { | ||
| logger.error( | ||
| "SolanaWalletAdapterHandler: Error polling transaction status", | ||
| error | ||
| ); | ||
| } | ||
|
|
||
| attempts++; | ||
| if (attempts < maxAttempts && !this.isCleanedUp) { | ||
| currentTimeoutId = setTimeout(poll, intervalMs); | ||
| this.pollingTimeouts.add(currentTimeoutId); | ||
| } else { | ||
| // Cleanup after max attempts | ||
| this.pendingTransactions.delete(signature); | ||
| } | ||
| }; | ||
|
|
||
| // Start polling | ||
| currentTimeoutId = setTimeout(poll, intervalMs); | ||
| this.pollingTimeouts.add(currentTimeoutId); | ||
|
|
||
| // Clean up old processed signatures | ||
| cleanupOldEntries(this.processedSignatures); | ||
| } |
There was a problem hiding this comment.
Polling continues until confirmation/error or 30 attempts (60s). Solana block time is ~400ms, so 2s intervals may be longer than needed. Consider reducing interval for faster confirmation detection. Note: pendingTransactions map cleans up on terminal states but not on max attempts timeout (minor memory accumulation for stuck transactions).
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/solana/SolanaWalletAdapterHandler.ts
Line: 844:971
Comment:
Polling continues until confirmation/error or 30 attempts (60s). Solana block time is ~400ms, so 2s intervals may be longer than needed. Consider reducing interval for faster confirmation detection. Note: `pendingTransactions` map cleans up on terminal states but not on max attempts timeout (minor memory accumulation for stuck transactions).
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| // Only deduplicate terminal states (confirmed/finalized/err) to prevent | ||
| // duplicate event emissions. Do NOT deduplicate intermediate states like | ||
| // "processed" since we need to keep polling until a terminal state. | ||
| const isTerminalState = | ||
| !!status.err || | ||
| status.confirmationStatus === "confirmed" || | ||
| status.confirmationStatus === "finalized"; | ||
|
|
||
| // Check for duplicate processing of terminal states only | ||
| if (isTerminalState && this.processedSignatures.has(signatureKey)) { | ||
| return; | ||
| } | ||
| if (isTerminalState) { | ||
| this.processedSignatures.add(signatureKey); | ||
| } |
There was a problem hiding this comment.
Deduplicates only terminal states (confirmed/finalized/err) to prevent duplicate events, allowing intermediate "processed" states to continue polling until terminal state reached. This design correctly handles Solana's multi-stage confirmation model.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/solana/SolanaWalletAdapterHandler.ts
Line: 886:901
Comment:
Deduplicates only terminal states (confirmed/finalized/err) to prevent duplicate events, allowing intermediate "processed" states to continue polling until terminal state reached. This design correctly handles Solana's multi-stage confirmation model.
How can I resolve this? If you propose a fix, please make it concise.| private validateMultiChainAddress( | ||
| address: string, | ||
| chainId?: number | ||
| ): Address | undefined { | ||
| // If chain ID is in Solana range, validate as Solana address | ||
| const solanaChainIds = Object.values(SOLANA_CHAIN_IDS); | ||
| if (chainId && solanaChainIds.includes(chainId)) { | ||
| const validSolanaAddress = getValidSolanaAddress(address); | ||
| return validSolanaAddress || undefined; | ||
| } | ||
|
|
||
| // Check if it looks like a Solana address (Base58, no 0x prefix) | ||
| if (isSolanaAddress(address)) { | ||
| return getValidSolanaAddress(address) || undefined; | ||
| } | ||
|
|
||
| // Default to EVM address validation | ||
| return this.validateAndChecksumAddress(address); | ||
| } |
There was a problem hiding this comment.
Same address validation order issue as EventFactory.ts:64 - checking isSolanaAddress first may misclassify EVM addresses without 0x as Solana since 40 hex chars are a Base58 subset. Should prefer EVM validation first (via validateAndChecksumAddress), then fall back to Solana only if EVM validation fails.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/FormoAnalytics.ts
Line: 2401:2419
Comment:
Same address validation order issue as `EventFactory.ts:64` - checking `isSolanaAddress` first may misclassify EVM addresses without `0x` as Solana since 40 hex chars are a Base58 subset. Should prefer EVM validation first (via `validateAndChecksumAddress`), then fall back to Solana only if EVM validation fails.
How can I resolve this? If you propose a fix, please make it concise.
| } | ||
|
|
||
| return null; | ||
| } |
There was a problem hiding this comment.
Address validation prioritizes Solana over EVM without chain context
Medium Severity
EventFactory.validateMultiChainAddress checks isSolanaAddress before EVM validation without any chainId context. Unlike the FormoAnalytics version which checks chainId first, this method guesses purely from format. isSolanaAddress matches any 32–44 character string containing only Base58 characters, which can overlap with non-0x-prefixed EVM addresses (about 7.6% of random hex addresses contain no 0 digit). While normal SDK-generated EVM addresses always carry the 0x prefix (making this safe in practice), the FormoAnalytics.validateMultiChainAddress has the same fallback path when chainId is absent or non-Solana — an address matching Base58 will skip EVM checksumming entirely.
Additional Locations (1)
| transactions: SolanaTransaction[] | ||
| ): Promise<SolanaTransaction[]>; | ||
| signMessage?(message: Uint8Array): Promise<Uint8Array>; | ||
| } |
There was a problem hiding this comment.
SolanaWalletContext.wallet type mismodels the real wallet-adapter-react interface
High Severity
SolanaWalletContext.wallet is typed as SolanaWalletAdapter | null, but in the real @solana/wallet-adapter-react, useWallet().wallet returns { adapter: Adapter, readyState: WalletReadyState } | null — the adapter with .on()/.off() lives at .adapter, not directly on the wallet object. setupContextListeners passes context.wallet to setupAdapterEventListenersOnly, which calls .on("connect", ...) on it. At runtime with a real wallet-adapter-react context (e.g., via type coercion), this would throw because the Wallet wrapper object lacks on/off methods. Similarly, getWalletName accesses this.wallet.wallet?.name which would be undefined on a real Wallet object (the name is at .adapter.name).
Additional Locations (1)
This comment has been minimized.
This comment has been minimized.
Applied via @cursor push command
| // Wrap sendTransaction | ||
| context.sendTransaction = async ( | ||
| transaction: SolanaTransaction, | ||
| connection: SolanaConnection, | ||
| options?: SendTransactionOptions |
There was a problem hiding this comment.
Cleanup breaks future polling
cleanup() sets isCleanedUp = true, and pollTransactionConfirmation() bails out early when that flag is set. If the handler is later reused via setWallet() (or lazy init in FormoAnalytics.setSolanaWallet()), isCleanedUp is never reset, so confirmation polling will never run for any subsequent transactions/signatures.
This triggers when consumers call FormoAnalytics.cleanup() (or solanaHandler.cleanup()) and later re-initialize/reuse the same FormoAnalytics instance with Solana enabled.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/solana/SolanaWalletAdapterHandler.ts
Line: 398:402
Comment:
**Cleanup breaks future polling**
`cleanup()` sets `isCleanedUp = true`, and `pollTransactionConfirmation()` bails out early when that flag is set. If the handler is later reused via `setWallet()` (or lazy init in `FormoAnalytics.setSolanaWallet()`), `isCleanedUp` is never reset, so confirmation polling will never run for any subsequent transactions/signatures.
This triggers when consumers call `FormoAnalytics.cleanup()` (or `solanaHandler.cleanup()`) and later re-initialize/reuse the same `FormoAnalytics` instance with Solana enabled.
How can I resolve this? If you propose a fix, please make it concise.| ): Promise<Uint8Array> => { | ||
| const address = this.getCurrentAddress(); | ||
| const messageString = new TextDecoder().decode(message); | ||
|
|
There was a problem hiding this comment.
Unconditional TextDecoder usage
new TextDecoder() will throw in environments without the Web TextEncoder/TextDecoder globals (e.g., some Node/test setups). Since the SDK supports non-browser contexts, signMessage autocapture can crash when enabled.
Also occurs in wrappedSignMessage at src/solana/SolanaWalletAdapterHandler.ts:617.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/solana/SolanaWalletAdapterHandler.ts
Line: 459:462
Comment:
**Unconditional `TextDecoder` usage**
`new TextDecoder()` will throw in environments without the Web TextEncoder/TextDecoder globals (e.g., some Node/test setups). Since the SDK supports non-browser contexts, `signMessage` autocapture can crash when enabled.
Also occurs in `wrappedSignMessage` at src/solana/SolanaWalletAdapterHandler.ts:617.
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| /** | ||
| * Poll for transaction confirmation | ||
| */ | ||
| private async pollTransactionConfirmation( | ||
| signature: string, |
There was a problem hiding this comment.
Polling uses nonstandard API
The code calls conn.getSignatureStatus(signature) but the common @solana/web3.js Connection API is getSignatureStatuses([signature]) (plural) rather than getSignatureStatus. With a real web3.js Connection, getSignatureStatus will be undefined, so confirmation polling will never emit CONFIRMED/REVERTED and pending transactions are only cleaned up on max-attempts.
If this is intentional (supporting a custom connection wrapper), it likely needs to also support getSignatureStatuses to work with the standard adapter stack.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/solana/SolanaWalletAdapterHandler.ts
Line: 841:846
Comment:
**Polling uses nonstandard API**
The code calls `conn.getSignatureStatus(signature)` but the common `@solana/web3.js` Connection API is `getSignatureStatuses([signature])` (plural) rather than `getSignatureStatus`. With a real web3.js `Connection`, `getSignatureStatus` will be `undefined`, so confirmation polling will never emit CONFIRMED/REVERTED and pending transactions are only cleaned up on max-attempts.
If this is intentional (supporting a custom connection wrapper), it likely needs to also support `getSignatureStatuses` to work with the standard adapter stack.
How can I resolve this? If you propose a fix, please make it concise.
| chainId: this.options.solana?.chainId, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| /** |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- Reset isCleanedUp flag in setWallet() to enable polling after reuse
- Add trackChainEventOnly() internal method for Solana chain events
- Update setCluster to use trackChainEventOnly to avoid EVM state corruption
- Add safeDecodeMessage() helper for Node.js environment compatibility
- Support both getSignatureStatuses (standard) and getSignatureStatus APIs
- Pass connection from initial options in setSolanaWallet lazy-init
- Add duplicate check in checkInitialConnection to prevent repeat events
- Fix SolanaWalletContext.wallet type to match real wallet-adapter-react
(wallet is { adapter, readyState }, not the adapter directly)
- Update setupContextListeners to use context.wallet.adapter
- Update getWalletName to use wallet.adapter.name
https://claude.ai/code/session_011PKjHn7x4Pmqwm7tojPz4E
1. Fix setSolanaConnection/setSolanaCluster silent drop:
- Add pendingSolanaConnection and pendingSolanaCluster properties
- Store values when called before handler is initialized
- Apply pending values when handler is lazily created via setSolanaWallet()
2. Add syncSolanaWalletState() method:
- Public method for consumers to sync wallet state
- Checks if adapter changed and rebinds if needed
- Useful in React effects when wallet.wallet changes but context
object reference stays the same
- Ensures connect/disconnect events are tracked without waiting
for next transaction call
https://claude.ai/code/session_011PKjHn7x4Pmqwm7tojPz4E
1. Add tests for wallet swap detection:
- syncWalletState() with adapter change detection
- Adapter becoming null in context
- No-op for non-context wallets
2. Add tests for disconnect guards:
- No disconnect event when no prior connection exists
- Disconnect only emits with prior connection
3. Add tests for double-wrapping prevention:
- Same adapter not double-wrapped on setWallet
- Different adapter properly re-wrapped
4. Add tests for chainId consistency:
- Captured chainId used for all events
- ChainId preserved even if setCluster called mid-transaction
5. Add tests for all system address blocking:
- System Program, Token Program, Token 2022, Associated Token,
Rent Sysvar, Clock Sysvar
6. Simplify getPublicKey() method:
- Both wallet types have same publicKey property
- Reduced from 10 lines to 1 line
Test count: 515 -> 530 (+15 new tests)
https://claude.ai/code/session_011PKjHn7x4Pmqwm7tojPz4E
Update test to verify that setWallet re-wraps cleanly without causing duplicate event emissions, rather than checking method reference equality. https://claude.ai/code/session_011PKjHn7x4Pmqwm7tojPz4E
1. Add emitTransactionEvent() helper: - Consolidates repeated pattern of checking address and autocapture - Used in wrappedSendTransaction for STARTED/BROADCASTED/REJECTED 2. Add emitSignatureEvent() helper: - Same consolidation for signature events - Used in wrappedSignMessage and wrappedSignTransaction 3. Add registerAdapterListener() helper: - Simplifies listener registration with automatic unsubscriber tracking - Reduces boilerplate in setupAdapterEventListenersOnly 4. Simplify wrapped methods: - wrappedSendTransaction: 64 -> 40 lines - wrappedSignMessage: 48 -> 24 lines - wrappedSignTransaction: 47 -> 25 lines Net reduction: 34 lines of code (-108 +74) https://claude.ai/code/session_011PKjHn7x4Pmqwm7tojPz4E
| handler: (...args: unknown[]) => void | ||
| ): void { | ||
| adapter.on(event as "connect", handler as () => void); | ||
| this.unsubscribers.push(() => adapter.off(event as "connect", handler as () => void)); | ||
| } |
There was a problem hiding this comment.
Incorrect event listener types
registerAdapterListener() always casts the event to "connect" for both on and off, which means disconnect/error listeners are registered/removed under the wrong event name. This will prevent disconnect/error tracking and leak listeners on cleanup.
| handler: (...args: unknown[]) => void | |
| ): void { | |
| adapter.on(event as "connect", handler as () => void); | |
| this.unsubscribers.push(() => adapter.off(event as "connect", handler as () => void)); | |
| } | |
| adapter.on(event as any, handler as any); | |
| this.unsubscribers.push(() => adapter.off(event, handler as any)); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/solana/SolanaWalletAdapterHandler.ts
Line: 383:387
Comment:
**Incorrect event listener types**
`registerAdapterListener()` always casts the event to `"connect"` for both `on` and `off`, which means `disconnect`/`error` listeners are registered/removed under the wrong event name. This will prevent disconnect/error tracking and leak listeners on cleanup.
```suggestion
adapter.on(event as any, handler as any);
this.unsubscribers.push(() => adapter.off(event, handler as any));
```
How can I resolve this? If you propose a fix, please make it concise.
| this.checkAndRebindContextAdapter(); | ||
|
|
||
| if (!this.originalAdapterSendTransaction) { | ||
| throw new Error("sendTransaction not available"); | ||
| } |
There was a problem hiding this comment.
🔴 checkAndRebindContextAdapter inside wrapped methods replaces originalAdapter* references mid-flight, routing calls to the wrong adapter
When wrappedSendTransaction (and similarly wrappedSignMessage/wrappedSignTransaction) is invoked, it calls this.checkAndRebindContextAdapter() at the very start. If the context's adapter has changed since the method was wrapped, checkAndRebindContextAdapter calls restoreOriginalMethods() which clears this.originalAdapterSendTransaction (line 179), then calls wrapAdapterMethods(newAdapter) which sets this.originalAdapterSendTransaction to the new adapter's bound method.
The wrapped method then continues execution at line 460-461 and uses this.originalAdapterSendTransaction — which now points to the new adapter's method, not the old one that was originally called. The transaction/signature intended for the old adapter gets routed through the new adapter.
Root Cause and Impact
The call chain is:
- User calls
oldAdapter.sendTransaction(tx, conn)which is the wrapped version. wrappedSendTransaction(line 458) callscheckAndRebindContextAdapter().checkAndRebindContextAdapter(line 334) callsrestoreOriginalMethods()→ clearsthis.originalAdapterSendTransactionatSolanaWalletAdapterHandler.ts:179.checkAndRebindContextAdapter(line 340) callswrapAdapterMethods(newAdapter)→ setsthis.originalAdapterSendTransactionto newAdapter'ssendTransactionatSolanaWalletAdapterHandler.ts:432.- Back in
wrappedSendTransaction, line 471 callsthis.originalAdapterSendTransaction(transaction, connection, options)— this is now the new adapter's method.
Impact: A transaction or signature request could be sent through the wrong wallet adapter. This could cause the call to fail with a confusing error, or in the worst case, succeed on a different wallet than intended. The same issue affects wrappedSignMessage (line 498) and wrappedSignTransaction (line 527).
The fix should capture this.originalAdapterSendTransaction in a local variable before calling checkAndRebindContextAdapter(), so that even if the handler rebinds, the in-flight call still uses the correct original method.
| this.checkAndRebindContextAdapter(); | |
| if (!this.originalAdapterSendTransaction) { | |
| throw new Error("sendTransaction not available"); | |
| } | |
| // Capture the original method BEFORE rebinding, so an in-flight call | |
| // is not rerouted to a different adapter if the context changed. | |
| const originalSend = this.originalAdapterSendTransaction; | |
| this.checkAndRebindContextAdapter(); | |
| if (!originalSend) { | |
| throw new Error("sendTransaction not available"); | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
The registerAdapterListener() was incorrectly casting all events to "connect" type, which would register disconnect/error listeners under the wrong event name. Fixed by using 'any' cast to properly handle the overloaded on/off signatures. https://claude.ai/code/session_011PKjHn7x4Pmqwm7tojPz4E
The chainId option was removed from SolanaOptions as it was always derived from the cluster. Updated documentation to reflect this and document the chain ID mapping. https://claude.ai/code/session_011PKjHn7x4Pmqwm7tojPz4E
| } | ||
|
|
||
| if (status) { | ||
| const signatureKey = `${signature}:${status.confirmationStatus}`; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Rename validateMultiChainAddress → validateAddress - Extract validateAddress and validateAndChecksumAddress to utils/address.ts - Rename SolanaWalletAdapter → SolanaAdapter across all files - Narrow public exports: remove internal constants, type guards, address utils - Add signTransaction to docs mock wallet example Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is ON, but it could not run because on-demand usage is turned off. To enable Bugbot Autofix, turn on on-demand usage and set a spend limit in the Cursor dashboard.
| "wagmi": ">=2.0.0", | ||
| "@solana/wallet-adapter-base": ">=0.9.0", | ||
| "@solana/wallet-adapter-react": ">=0.15.0", | ||
| "@solana/web3.js": ">=1.70.0 <2.0.0" |
There was a problem hiding this comment.
Optional peer blocks web3.js v2 projects
Medium Severity
package.json adds @solana/web3.js as an optional peer but pins it to <2.0.0. Even when Solana tracking is unused, apps that already depend on @solana/web3.js@2 can hit peer-resolution failures or strict CI install errors because the version constraint is incompatible.
Additional Locations (1)
| if (validEvmAddress) { | ||
| return toChecksumAddress(validEvmAddress); | ||
| } | ||
|
|
||
| // Check if it's a valid Solana address (Base58, no 0x prefix) | ||
| if (isSolanaAddress(address)) { | ||
| return getValidSolanaAddress(address); | ||
| } | ||
|
|
||
| return null; | ||
| } |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rename resolveNamespace → getNamespace and make it internal to setChainState/clearChainState. Callers can now pass either a namespace string or a chainId number, reducing indirection at call sites. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Eliminates the redundant chainId duplication at call sites like
setChainState(chainId, { address, chainId }). When the first arg
is a number it is now implicitly used as the namespace's chainId.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is ON, but it could not run because on-demand usage is turned off. To enable Bugbot Autofix, turn on on-demand usage and set a spend limit in the Cursor dashboard.
| "wagmi": ">=2.0.0", | ||
| "@solana/wallet-adapter-base": ">=0.9.0", | ||
| "@solana/wallet-adapter-react": ">=0.15.0", | ||
| "@solana/web3.js": ">=1.70.0 <2.0.0" |
There was a problem hiding this comment.
Overly strict Solana peer dependency range
Medium Severity
The new peer range for @solana/web3.js is pinned to <2.0.0, which can create peer conflicts in apps already on v2 even when the SDK only uses structural SolanaConnection typing and no direct @solana/web3.js imports. This can block installs in strict peer-dependency environments.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
The SDK defines its own SolanaConnection interface and never imports from @solana/web3.js directly, so the <2.0.0 cap was unnecessarily blocking users on v2. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
3 participants
Summary
This PR adds comprehensive Solana Wallet Adapter integration to the Formo Analytics SDK, enabling tracking of Solana wallet events (connect, disconnect, signatures, and transactions) alongside existing EVM wallet tracking. This allows dApps supporting multiple chains to track all blockchain interactions in a unified analytics platform.
Key Changes
New Solana Integration Module (
src/solana/)SolanaWalletAdapterHandler.ts: Core orchestrator that hooks into Solana Wallet Adapter events for connection, transaction, and signature trackingtypes.ts: Comprehensive TypeScript interfaces for Solana integration (wallet adapters, contexts, clusters, chain IDs)address.ts: Solana-specific address validation and utilities (Base58 format, blocked addresses, address comparison)Core SDK Updates (
src/FormoAnalytics.ts)setSolanaWallet(),setSolanaConnection(), andsetSolanaCluster()methods for dynamic wallet updatescleanup()methodPackage Configuration (
package.json)@solana/wallet-adapter-base(>=0.9.0)@solana/wallet-adapter-react(>=0.15.0)@solana/web3.js(>=1.70.0)Public API Exports (
src/index.ts)Documentation (
docs/SOLANA_INTEGRATION.md)Implementation Details
Design Philosophy
Key Features
sendTransaction,signMessage, andsignTransactionfor automatic event trackinguseWallet()hook contextssol.wallet.phantom)Chain ID Mapping
Solana clusters are mapped to high numeric IDs (900001-900004) to avoid collision with EVM chain IDs:
Event Lifecycle
Transactions flow through: STARTED → BROADCASTED → CONFIRMED/REVERTED
Signatures flow through: REQUESTED → CONFIRMED/REJECTED
Testing Considerations
https://claude.ai/code/session_011PKjHn7x4Pmqwm7tojPz4E
Note
Medium Risk
Touches core wallet tracking/state management and adds runtime method-wrapping + polling behavior, which could impact event correctness across both EVM and Solana if bugs slip through.
Overview
Adds an opt-in Solana integration to the SDK that tracks Solana wallet
connect/disconnect,signMessage/signTransactionsignatures, andsendTransactionlifecycle (including confirmation polling) via a newSolanaAdapter/SolanaManager.Updates core analytics state handling to isolate EVM vs Solana chain state (preventing disconnects on one namespace from wiping the other) and extends address validation/event enrichment to support strict chain-aware EVM vs Solana address validation.
Exposes the Solana API/types from
src/index.ts, adds optional Solana peer dependencies inpackage.json, adds a comprehensivedocs/SOLANA_INTEGRATION.md, and includes regression tests around per-namespace chain state isolation.Written by Cursor Bugbot for commit 1dbc1f2. This will update automatically on new commits. Configure here.
Greptile Overview
Greptile Summary
This PR successfully adds comprehensive Solana Wallet Adapter integration to enable tracking of Solana wallet events alongside existing EVM functionality. The implementation follows a solid opt-in, additive architecture that preserves backward compatibility.
Key Strengths
_chainState.evmvs_chainState.solana) to prevent cross-contaminationSolanaAdaptercorrectly handlesStandardWalletAdapter._reset()overwrites by detecting and re-wrapping methods after connect/disconnect eventssyncWalletState()/checkAndRebindContextAdapter()mechanism properly handles whenuseWallet().walletchanges in React apps11111111111111111111111111111111) are correctly filtered to prevent spurious eventsSolanaManagerenables deferred wallet setup viasetSolanaWallet()for flexible integration patternschainIdfor strict Solana vs EVM discrimination (addresses previous review concerns about ambiguous 40-char hex/Base58 strings)Architecture
The implementation adds three new Solana modules:
SolanaAdapter: Low-level wallet integration with method wrapping, event listeners, and confirmation pollingSolanaManager: Lifecycle manager that keeps Solana logic out of core SDKIntegration points in core SDK:
FormoAnalytics: Lazy-initializesSolanaManager, exposes publicsolana.*APIvalidateAddress(): Now chain-aware (prefers EVM whenchainIdis absent to avoid misclassification)EventFactory: Uses chain-aware validation for all eventsImplementation Quality
The code demonstrates careful attention to edge cases:
setWallet()to prevent old address/chainId leaking into disconnect eventsgetSignatureStatuses) and legacy (getSignatureStatus) Connection APIsConfidence Score: 4/5
src/solana/SolanaAdapter.ts(method wrapping + polling logic), but it has comprehensive test coverage. Pay extra attention to multi-chain scenarios in production monitoring (simultaneous EVM + Solana wallet operations).Important Files Changed
SolanaManagerwith lazy initialization. Properly isolates EVM and Solana chain state, integrates cleanup lifecycle, and exposes public API for Solana wallet management.SolanaAdapterwith lazy initialization and pending config support. Clean separation of Solana-specific logic from core SDK.Sequence Diagram
sequenceDiagram participant App as dApp participant Formo as FormoAnalytics participant SolMgr as SolanaManager participant Adapter as SolanaAdapter participant Wallet as Solana Wallet participant Conn as Solana Connection App->>Formo: init({ solana: { wallet, connection, cluster } }) Formo->>SolMgr: new SolanaManager(options) SolMgr->>Adapter: new SolanaAdapter(wallet, connection, cluster) Adapter->>Wallet: on("connect", handler) Adapter->>Wallet: on("disconnect", handler) Adapter->>Adapter: wrapAdapterMethods(sendTransaction, signMessage, signTransaction) Adapter->>Adapter: checkInitialConnection() Note over Wallet,App: User connects wallet Wallet->>Adapter: emit("connect", publicKey) Adapter->>Adapter: handleConnect(publicKey) Adapter->>Adapter: Check if blocked address Adapter->>Formo: connect({ chainId, address }) Note over App,Wallet: User sends transaction App->>Wallet: sendTransaction(tx, connection) Wallet->>Adapter: wrappedSendTransaction(tx, connection) Adapter->>Formo: transaction(STARTED) Adapter->>Wallet: originalSendTransaction(tx, connection) Wallet-->>Adapter: signature Adapter->>Formo: transaction(BROADCASTED, signature) Adapter->>Adapter: pollTransactionConfirmation(signature) loop Every 2s, max 30 attempts Adapter->>Conn: getSignatureStatuses([signature]) Conn-->>Adapter: status alt Transaction confirmed Adapter->>Formo: transaction(CONFIRMED, signature) else Transaction failed Adapter->>Formo: transaction(REVERTED, signature) end end Note over App,Wallet: User signs message App->>Wallet: signMessage(message) Wallet->>Adapter: wrappedSignMessage(message) Adapter->>Formo: signature(REQUESTED, message) Adapter->>Wallet: originalSignMessage(message) Wallet-->>Adapter: signature Adapter->>Formo: signature(CONFIRMED, signatureHex) Note over App,Wallet: User switches cluster App->>Formo: solana.setCluster("devnet") Formo->>SolMgr: setCluster("devnet") SolMgr->>Adapter: setCluster("devnet") Adapter->>Adapter: Update chainId (900003) Adapter->>Formo: chain({ chainId: 900003, address }) Note over Wallet,App: User disconnects Wallet->>Adapter: emit("disconnect") Adapter->>Adapter: handleDisconnect() Adapter->>Formo: disconnect({ chainId, address })