Implement kit-plugin-wallet store, types, and tests

[mcintyre94]

Implements the wallet plugin's core store (createWalletStore) with wallet discovery, connection lifecycle, signer creation, and subscribable state management.

Tests are split into store tests (state management, lifecycle) and wallet tests (plugin integration, payer getter).

Also add to list of plugins in root readme and contributing

[changeset-bot]

🦋 Changeset detected

Latest commit: 0a1f210

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@solana/kit-plugin-wallet	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[mcintyre94]

• Add @solana/kit-react core package #205
• Add SubscribeToPayer/Identity, and use for wallet plugin #200
• Implement kit-plugin-wallet store, types, and tests #191 👈 (View in Graphite)
• Scaffold/design for the wallet plugin #173
• Bump @solana/kit peer dependency to ^6.8.0 #183 : 1 other dependent PR (#184 )
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[mcintyre94]

@trevor-cortex

[trevor-cortex]

Summary

This PR implements the wallet plugin's core store (createWalletStore) with wallet discovery via wallet-standard, connection lifecycle management, signer creation, auto-connect persistence, subscribable state for UI frameworks, and signIn (SIWS-as-connect). Tests are thorough and well-structured with a clean mock setup.

Overall this is solid work — the architecture is clean, the SSR guard is well thought out, and the test coverage is excellent. I have a few things to flag, most notably around the signIn error handling and some concurrency edge cases.

Key Things to Watch

1. signIn is missing error handling and status transition — Unlike connect(), signIn() doesn't set status: 'connecting' before the async call, and has no try/catch to call disconnectLocally() on failure. If signIn throws, the store status is never reset.

2. setState notifies listeners even when nothing changed — The listener notification runs unconditionally outside the snapshot-change check. For useSyncExternalStore this is mostly fine (React does its own Object.is check), but other frameworks that re-render on every listener call will get unnecessary updates.

3. No concurrency guard on connect/signIn — Calling connect(walletA) then connect(walletB) before the first resolves can leave leaked event subscriptions from walletA, since setConnected for walletA runs and sets up walletEventsCleanup before walletB overwrites it.

4. Late registration handler fires concurrent async IIFEs — If multiple wallets register in quick succession, multiple attemptSilentReconnect calls can race.

Notes for Subsequent Reviewers

• The persistence format walletName:accountAddress uses lastIndexOf(':') for parsing, which correctly handles wallet names containing colons (base58 addresses don't contain :). The malformed-key case is tested.
• The SSR stub returns a frozen singleton snapshot, which is important for useSyncExternalStore referential stability.
• The walletAsPayer plugin uses Object.defineProperty for a dynamic payer getter since extendClient freezes the result — clever approach.
• Auto-connect tests use fake timers to control the async flow — good pattern.

[trevor-cortex]

Re-review

Great progress — all four issues from my first review have been addressed:

• ✅ signIn now has proper status: 'connecting' transition, try/catch, and disconnectLocally() on error
• ✅ setState only notifies listeners when snapshot-relevant fields actually change
• ✅ connectGeneration counter guards against concurrent connect/signIn races (with thorough tests)
• ✅ Late-registration reconnect handler has .catch() for error recovery

The concurrency test coverage is excellent — stale rejection, concurrent connects, cross-connect/signIn superseding, and auto-reconnect vs explicit connect are all well covered.

Remaining items

I found one state consistency bug and a couple of smaller things:

1. Signer/account mismatch when both accounts and features change in a single event — handleFeaturesChanged reads state.account (the old account) instead of the potentially-updated account from handleAccountsChanged, so the signer can be computed for the wrong account. (See inline comment.)

2. disconnect() finally block can clobber a concurrent connect() — Since disconnect() doesn't participate in the generation counter, its finally { disconnectLocally() } can wipe out a connect() that completed while the disconnect was in flight. Low priority since it's an unusual user interaction pattern, but worth noting.

3. No validation in selectAccount — Any UiWalletAccount is accepted regardless of whether it belongs to the connected wallet. If a foreign account is passed, the persisted key will have a mismatched wallet name + address. Not a crash, but surprising behavior.

None of these are blockers — #1 is the most impactful but only triggers on simultaneous account+feature wallet events (uncommon). Overall the implementation is clean and well-tested.

[trevor-cortex]

Re-review (3rd pass)

All 7 issues from the previous two reviews are now addressed with corresponding test coverage:

• ✅ signIn error handling and status transition (from review 1)
• ✅ setState only notifies on actual changes (from review 1)
• ✅ connectGeneration guards connect/signIn/disconnect concurrency (from review 1)
• ✅ Late-registration reconnect has .catch() recovery (from review 1)
• ✅ Signer/account mismatch fixed — handleFeaturesChanged is now skipped when accounts also changed in the same event, so the signer is always computed for the correct account (from review 2)
• ✅ disconnect() participates in generation counter — captures connectGeneration before the async call and checks it in finally, preventing a stale disconnect from clobbering a concurrent connect (from review 2)
• ✅ selectAccount validates account belongs to wallet — refreshes the wallet and checks the address exists in the account list before accepting (from review 2)

The new tests are thorough — 'disconnect does not clobber concurrent connect', 'uses new account signer when both accounts and features change', 'selectAccount throws for account not in wallet', and 'still reconnects if wallet appears after timeout' all directly verify the fixes.

The wallet plugin architecture is clean: four plugin variants (walletSigner, walletPayer, walletIdentity, walletWithoutSigner) built on a shared createPlugin helper with defineSignerGetter for dynamic payer/identity getters. SSR stub, persistence, auto-reconnect with timeout, and the concurrency model are all solid.

LGTM 🚢

[mcintyre94]

Note to self: make this changeset more of a new plugin announcement, with code examples