Skip to content

feat: discover Solana wallets through the Wallet Standard (P-2416) - #386

Merged
yosriady merged 14 commits into
mainfrom
worktree-p-2416-solana-connect
Sep 6, 2026
Merged

feat: discover Solana wallets through the Wallet Standard (P-2416)#386
yosriady merged 14 commits into
mainfrom
worktree-p-2416-solana-connect

Conversation

@yosriady

@yosriady yosriady commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes P-2416.

Problem

Solana wallet capture required framework-kit's store. Apps using Solana Kit, wallet-adapter, or another integration with a Wallet Standard wallet could produce no Solana connect events and no error.

Fix

  • Adds a read-only Wallet Standard registry that discovers wallets and observes account changes without wrapping wallet methods or adding a runtime dependency.
  • Captures detect, connect, and disconnect for compatible wallets registered with the page, including wallets used through Solana Kit, wallet-adapter, and framework-kit.
  • Handles restored sessions, late wallet registration, account swaps, multichain wallets, blocked system addresses, and teardown.
  • Uses options.solana.cluster (or formo.solana.setCluster()) because Wallet Standard does not expose the app's active cluster.
  • Keeps framework-kit's store integration for cluster and recorded transaction lifecycle events, with an identity-aware handoff that prevents duplicate wallet events.
  • Preserves historical sol.wallet.* identifiers and legacy string cluster-status types.
  • Enables discovery by default; solana: false is the explicit opt-out.

Compatibility

Existing solana: { store } and solana: { cluster } integrations remain valid. The implementation is dependency-free and does not issue wallet or RPC requests. Signatures and transactions outside a framework-kit store remain explicit SDK calls.

Validation

  • 1,288 tests passing
  • ESLint passing
  • Production build passing
  • Bundle size: 61.2 kB / 61.5 kB
  • Manually verified MetaMask Wallet Standard connect, disconnect, reconnect, Devnet attribution, and transaction events

Companion PRs: docs #153, examples #338.

… are captured with any wallet library

Solana capture was opt-in and needed framework-kit's store, so wallet-adapter, Privy, Dynamic, Reown and custom apps produced no connect events. A new SolanaWalletStandardRegistry runs the Wallet Standard handshake inline, emits detect on registration, and diffs each wallet's accounts into connect and disconnect. It is on by default (solana: false opts out); while a framework-kit store is attached it leaves connect/disconnect to the store so each connection is reported once. Also fixes the SolanaClusterState.status type drift, tracks the option in computeOptionsKey, rewrites docs/SOLANA_INTEGRATION.md, and raises the size budget to 61 KB. Fixes P-2416.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Claude-Session: https://claude.ai/code/session_015USSsGzRcCswNCNv6eTc8X
@linear-code

linear-code Bot commented Sep 3, 2026

Copy link
Copy Markdown

P-2416

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All reported issues were addressed across 15 files

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

Comment thread src/solana/SolanaManager.ts Outdated
Comment thread test/solana/SolanaWalletStandardRegistry.spec.ts Outdated
Comment thread src/solana/storeTypes.ts Outdated
Comment thread test/solana/SolanaManager.spec.ts Outdated
Comment thread docs/SOLANA_INTEGRATION.md Outdated
Comment thread src/FormoAnalyticsProvider.tsx Outdated
Comment thread src/FormoAnalytics.ts
Comment thread src/solana/types.ts Outdated
Comment thread src/solana/types.ts Outdated
@yosriady

yosriady commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Addressed all nine review findings in 73a332b:

  • restore every overwritten global descriptor in both jsdom suites;
  • model framework-kit's complete cluster-status union, including latency and error;
  • qualify the already-authorized-wallet docs for the store path;
  • key React provider reinitialization by Solana store identity;
  • make solana: false disable setStore as well as discovery;
  • hand off late-attached stores without duplicate connects or lost disconnects, with regression tests for both races;
  • preserve case and whitespace in the synthetic wallet identity via URL encoding;
  • document the actual supported-cluster fallback.

Validation: 1,283 tests pass; lint, build, and size-limit pass. The compressed bundle is 61.19 KB under the updated 61.5 KB budget.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All reported issues were addressed across 11 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread src/solana/SolanaManager.ts
Comment thread src/solana/SolanaWalletStandardRegistry.ts Outdated
Comment thread src/solana/SolanaWalletStandardRegistry.ts
@yosriady

yosriady commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Addressed all three follow-up findings in 4f346b4:

  • every store-observed connect now silently synchronizes central wallet/cluster state before the chain-event policy is applied, so disabling chain autocapture cannot leave the registry's guessed cluster active;
  • registry connects are marked as reported only when willTrackEvent confirms that tracking/suppression/chain policy will accept them;
  • store adoption now matches both the case-preserving synthetic rdns and address, so two wallets exposing the same account cannot consume each other's handoff marker.

Added a regression test for each case. Full validation now passes: 1,286 tests, lint, build, and size-limit (61.21 KB / 61.5 KB).

@yosriady

yosriady commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Manual browser validation completed against a locally linked production build and the with-solana example with framework-kit's store deliberately omitted:

  • MetaMask registered through the Wallet Standard and detect was enqueued.
  • Connecting emitted one Solana connect for Devnet (chain_id: 900003).
  • Disconnecting emitted disconnect for the same address and chain.
  • Reconnecting was observed again by standard:events (the identical event was then suppressed by the SDK's existing 30-second queue deduplication window).
  • Explicit transaction tracking remained on Devnet.

The test-only diagnostics and no-store example configuration were removed afterward. The SDK worktree is clean and the full suite still passes: 1,286 tests plus build.

@yosriady

yosriady commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Final compatibility hardening landed in 6182892: the exported cluster state now accepts both framework-kit’s object status union and the pre-1.39 legacy string form, so custom stores/test doubles keep compiling. Added a regression test. Final local validation: 1,287 tests, lint, build, and size-limit (61.21 KB / 61.5 KB).

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All reported issues were addressed across 3 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread src/solana/types.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All reported issues were addressed across 1 file (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread docs/SOLANA_INTEGRATION.md Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All reported issues were addressed across 8 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread src/solana/SolanaManager.ts
yosriady and others added 2 commits September 4, 2026 18:20
`setCluster()` moved only the registry's own snapshot when
`autocapture.chain` was false, because central state is written inside
`FormoAnalytics.chain()`. A devnet switch then left `currentChainId` on
mainnet-beta, so later events carried the stale chain and `excludeChains`
gated against it. The EVM tracker already separates observing a chain
from reporting one; the registry now does the same through
`syncWalletState`, and stays silent while a framework-kit store owns
wallet events and syncs its own state.

Also: name both identities when a late-attached store cannot adopt the
connection the registry reported, so the resulting double connect is
visible in the logs; drop the unused `connectedAccount` getter; stop
claiming `solana: false` in the `setStore` warning, which a cleaned-up
SDK also reaches; restore two unrelated whitespace deletions.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4qyDhFa9MTfvvGiA8SEhM

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1 issue found across 4 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/solana/SolanaWalletStandardRegistry.ts">

<violation number="1" location="src/solana/SolanaWalletStandardRegistry.ts:472">
P2: When multiple Wallet Standard wallets remain connected and chain autocapture is off, this loop makes the last registered wallet the active wallet even if another connected later. Preserve the active wallet when syncing the cluster instead of syncing every tracked wallet.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread src/solana/SolanaWalletStandardRegistry.ts Outdated
The cluster sync wrote central wallet state for every connected wallet,
so two wallets holding an authorized account at once handed the wallet
slot to the last REGISTERED one. The SDK's rule is last CONNECTED wins.
Central state now follows a single wallet: the one the SDK already
treats as active, and otherwise the first connected wallet.

Reported by cubic on #386.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4qyDhFa9MTfvvGiA8SEhM

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All reported issues were addressed across 3 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread test/solana/SolanaWalletStandardRegistry.spec.ts Outdated
Solflare, not Phantom, is the wallet that registers and connects last in
that test. `currentAddress` is what pins the active wallet to Phantom,
which is the point: registration order must not take the slot from it.

Reported by cubic on #386.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4qyDhFa9MTfvvGiA8SEhM
@yosriady
yosriady merged commit a7a9e67 into main Sep 6, 2026
15 checks passed
@yosriady
yosriady deleted the worktree-p-2416-solana-connect branch September 6, 2026 07:44
@yosriady yosriady mentioned this pull request Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant