feat(mwa): API parity, auth cache, disconnect/reconnect UX#264
Open
Pratikkale26 wants to merge 4 commits intomagicblock-labs:mainfrom
Open
feat(mwa): API parity, auth cache, disconnect/reconnect UX#264Pratikkale26 wants to merge 4 commits intomagicblock-labs:mainfrom
Pratikkale26 wants to merge 4 commits intomagicblock-labs:mainfrom
Conversation
WalkthroughAdds wallet lifecycle RPCs ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 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 unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Runtime/codebase/SolanaMobileStack/SolanaMobileWalletAdapter.cs (1)
110-122:⚠️ Potential issue | 🔴 CriticalCritical:
StartAndExecuteandExecuteNextActiondo not properly await async actions, creating a fire-and-forget pattern.The issue is in
LocalAssociationScenario.ExecuteNextAction()(line 137):private void ExecuteNextAction(Response<object> response = null) { // ... action.Invoke(_client); // Does not await }When an
asynclambda is passed asAction<IAdapterOperations>, invoking it withoutawaitresults in:
- The async operation runs as
async void(fire-and-forget)- Exceptions inside the lambda are lost
- The caller continues immediately—before the async work completes
- In the code snippet at lines 110-122,
authorizationis assigned inside an unawaitedasync voidlambda, so it remains uninitialized when code afterStartAndExecuteexecutesThis affects all action lambdas: lines 113-120 (_Login), 156-158 (DisconnectWallet), 199-202 (GetCapabilities), 233-248 (_SignAllTransactions), 285-300 (SignMessage), 354-361 (TryReauthorize).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Runtime/codebase/SolanaMobileStack/SolanaMobileWalletAdapter.cs` around lines 110 - 122, LocalAssociationScenario currently takes Action<IAdapterOperations> and calls action.Invoke(_client) in ExecuteNextAction, causing async lambdas to run fire-and-forget; change the action type to Func<IAdapterOperations, Task> (or ValueTask) across the API (e.g., the StartAndExecute parameter and any callers using Action<IAdapterOperations>), update LocalAssociationScenario.ExecuteNextAction to be async Task and await the invoked func (await action.Invoke(_client)), and propagate async/await through StartAndExecute so callers (e.g., the code assigning authorization) properly await completion and exceptions are propagated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@Runtime/codebase/SolanaMobileStack/JsonRpcClient/Responses/WalletCapabilities.cs`:
- Around line 17-41: Remove the [RequiredMember] attribute from
MaxTransactionsPerRequest, MaxMessagesPerRequest, SupportedTransactionVersions,
and SupportsCloneAuthorization so nullable semantics remain consistent with the
XML docs and optional JSON fields; update the XML comments: change
SupportedTransactionVersions to describe supported transaction version
identifiers (e.g., "legacy", "0") instead of "feature set identifiers", and
change SupportsCloneAuthorization to state it indicates whether the wallet
supports clone authorization (not the sign_and_send_transactions endpoint); keep
the properties as nullable (int?, List<string>, bool?) to reflect optional
reporting by wallets.
In `@Runtime/codebase/SolanaMobileStack/MwaAuthCache/EncryptedAuthCache.cs`:
- Around line 17-44: The EncryptedAuthCache class currently silently no-ops
GetAuthToken, SetAuthToken, and ClearAuthToken which drops tokens; change this
to fail fast by throwing a clear exception (e.g., NotImplementedException or
InvalidOperationException) from GetAuthToken, SetAuthToken, and ClearAuthToken
in the EncryptedAuthCache class so callers immediately know the secure backend
is not implemented; optionally add a constructor that accepts an
encryption/secure storage provider interface (e.g., ISecureStorage) and document
that these methods should delegate to that provider once implemented.
In `@Runtime/codebase/SolanaMobileStack/MwaAuthCache/PlayerPrefsAuthCache.cs`:
- Around line 20-46: Validate and sanitize inputs to avoid silent key
collisions: in GetAuthToken, SetAuthToken, and ClearAuthToken check that
walletIdentity is not null/empty and does not contain reserved characters (or
encode it) before calling BuildKey; in SetAuthToken also validate token is not
null/empty (and optionally enforce a max length). Throw
ArgumentNullException/ArgumentException immediately for invalid inputs. Update
BuildKey (and KeyPrefix usage) to produce an unambiguous key by encoding or
escaping walletIdentity (e.g., URI-escape or Base64) to prevent collisions
between different identities that could form the same concatenated key.
In `@Runtime/codebase/SolanaMobileStack/SolanaMobileWalletAdapter.cs`:
- Around line 183-186: ReconnectWallet currently just calls Login, which may
confuse readers; update the method to include a brief inline comment or expand
its XML doc comment above ReconnectWallet explaining that it intentionally
delegates to Login (which performs silent reauthorization) to provide semantic
clarity for callers and preserve a distinct API surface; reference the
ReconnectWallet() method and the Login() method in the comment so future
maintainers understand the intentional wrapper behavior.
- Around line 147-176: Update DisconnectWallet to log the full exception when
deauthorize fails: inside the catch for the
LocalAssociationScenario.StartAndExecute call (where you currently reference
e.Message), include the full exception details (e.ToString()) or use
Debug.LogWarningException so the stack trace is preserved; keep the same
best-effort behavior (do not rethrow), still clear _authToken, call
_authCache.ClearAuthToken(WalletIdentity), invoke OnWalletDisconnected and
base.Logout as before.
- Line 8: Remove the unused using directive "using UnityEngine.Events;" from the
top of the file; the code uses System.Action-based events (e.g., the Action
delegates in SolanaMobileWalletAdapter class) so the UnityEngine.Events
namespace is unnecessary—delete that import to clean up unused references.
- Around line 333-337: The override Logout currently fire-and-forgets
DisconnectWallet(), causing swallowed exceptions and race conditions; change it
to (1) call base.Logout() to synchronously clear Account and Mnemonic, then (2)
synchronously wait for DisconnectWallet() to complete using
DisconnectWallet().ConfigureAwait(false).GetAwaiter().GetResult() (wrap in
try/catch to log or rethrow as appropriate) so exceptions surface and callers
observe completion before Logout returns.
---
Outside diff comments:
In `@Runtime/codebase/SolanaMobileStack/SolanaMobileWalletAdapter.cs`:
- Around line 110-122: LocalAssociationScenario currently takes
Action<IAdapterOperations> and calls action.Invoke(_client) in
ExecuteNextAction, causing async lambdas to run fire-and-forget; change the
action type to Func<IAdapterOperations, Task> (or ValueTask) across the API
(e.g., the StartAndExecute parameter and any callers using
Action<IAdapterOperations>), update LocalAssociationScenario.ExecuteNextAction
to be async Task and await the invoked func (await action.Invoke(_client)), and
propagate async/await through StartAndExecute so callers (e.g., the code
assigning authorization) properly await completion and exceptions are
propagated.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8261121a-e991-41be-8351-cd010039f32d
📒 Files selected for processing (13)
Runtime/codebase/SolanaMobileStack/Interfaces/IAdapterOperations.csRuntime/codebase/SolanaMobileStack/JsonRpcClient/Responses/WalletCapabilities.csRuntime/codebase/SolanaMobileStack/JsonRpcClient/Responses/WalletCapabilities.cs.metaRuntime/codebase/SolanaMobileStack/MobileWalletAdapterClient.csRuntime/codebase/SolanaMobileStack/MwaAuthCache.metaRuntime/codebase/SolanaMobileStack/MwaAuthCache/EncryptedAuthCache.csRuntime/codebase/SolanaMobileStack/MwaAuthCache/EncryptedAuthCache.cs.metaRuntime/codebase/SolanaMobileStack/MwaAuthCache/IMwaAuthCache.csRuntime/codebase/SolanaMobileStack/MwaAuthCache/IMwaAuthCache.cs.metaRuntime/codebase/SolanaMobileStack/MwaAuthCache/PlayerPrefsAuthCache.csRuntime/codebase/SolanaMobileStack/MwaAuthCache/PlayerPrefsAuthCache.cs.metaRuntime/codebase/SolanaMobileStack/SolanaMobileWalletAdapter.csRuntime/codebase/SolanaWalletAdapter.cs
Runtime/codebase/SolanaMobileStack/JsonRpcClient/Responses/WalletCapabilities.cs
Show resolved
Hide resolved
Comment on lines
+20
to
+46
| public Task<string> GetAuthToken(string walletIdentity) | ||
| { | ||
| var key = BuildKey(walletIdentity); | ||
| var token = PlayerPrefs.GetString(key, null); | ||
| return Task.FromResult(string.IsNullOrEmpty(token) ? null : token); | ||
| } | ||
|
|
||
| /// <inheritdoc/> | ||
| public Task SetAuthToken(string walletIdentity, string token) | ||
| { | ||
| var key = BuildKey(walletIdentity); | ||
| PlayerPrefs.SetString(key, token); | ||
| PlayerPrefs.Save(); | ||
| return Task.CompletedTask; | ||
| } | ||
|
|
||
| /// <inheritdoc/> | ||
| public Task ClearAuthToken(string walletIdentity) | ||
| { | ||
| var key = BuildKey(walletIdentity); | ||
| PlayerPrefs.DeleteKey(key); | ||
| PlayerPrefs.Save(); | ||
| return Task.CompletedTask; | ||
| } | ||
|
|
||
| private static string BuildKey(string walletIdentity) => KeyPrefix + walletIdentity; | ||
| } |
There was a problem hiding this comment.
Add input validation for cache keys/tokens to avoid silent key collisions.
walletIdentity and token are currently accepted unchecked. Invalid inputs can produce shared keys or ambiguous cache state.
🛡️ Proposed defensive checks
+using System;
using System.Threading.Tasks;
using UnityEngine;
@@
public Task<string> GetAuthToken(string walletIdentity)
{
var key = BuildKey(walletIdentity);
var token = PlayerPrefs.GetString(key, null);
return Task.FromResult(string.IsNullOrEmpty(token) ? null : token);
}
@@
public Task SetAuthToken(string walletIdentity, string token)
{
+ if (string.IsNullOrEmpty(token))
+ throw new ArgumentException("Auth token must be non-empty.", nameof(token));
+
var key = BuildKey(walletIdentity);
PlayerPrefs.SetString(key, token);
PlayerPrefs.Save();
return Task.CompletedTask;
}
@@
- private static string BuildKey(string walletIdentity) => KeyPrefix + walletIdentity;
+ private static string BuildKey(string walletIdentity)
+ {
+ if (string.IsNullOrWhiteSpace(walletIdentity))
+ throw new ArgumentException("Wallet identity must be non-empty.", nameof(walletIdentity));
+
+ return KeyPrefix + walletIdentity;
+ }
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Runtime/codebase/SolanaMobileStack/MwaAuthCache/PlayerPrefsAuthCache.cs`
around lines 20 - 46, Validate and sanitize inputs to avoid silent key
collisions: in GetAuthToken, SetAuthToken, and ClearAuthToken check that
walletIdentity is not null/empty and does not contain reserved characters (or
encode it) before calling BuildKey; in SetAuthToken also validate token is not
null/empty (and optionally enforce a max length). Throw
ArgumentNullException/ArgumentException immediately for invalid inputs. Update
BuildKey (and KeyPrefix usage) to produce an unambiguous key by encoding or
escaping walletIdentity (e.g., URI-escape or Base64) to prevent collisions
between different identities that could form the same concatenated key.
Runtime/codebase/SolanaMobileStack/SolanaMobileWalletAdapter.cs
Outdated
Show resolved
Hide resolved
Comment on lines
+183
to
+186
| public async Task<Account> ReconnectWallet() | ||
| { | ||
| return await Login(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
ReconnectWallet() is just a wrapper around Login() - consider documenting the distinction.
The method body simply delegates to Login(), which already handles silent reauthorization. The value of this separate method is the semantic clarity for callers. Consider adding an inline comment or expanding the doc comment to clarify this is intentional.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Runtime/codebase/SolanaMobileStack/SolanaMobileWalletAdapter.cs` around lines
183 - 186, ReconnectWallet currently just calls Login, which may confuse
readers; update the method to include a brief inline comment or expand its XML
doc comment above ReconnectWallet explaining that it intentionally delegates to
Login (which performs silent reauthorization) to provide semantic clarity for
callers and preserve a distinct API surface; reference the ReconnectWallet()
method and the Login() method in the comment so future maintainers understand
the intentional wrapper behavior.
Comment on lines
+333
to
+337
| public override void Logout() | ||
| { | ||
| // Fire-and-forget the async disconnect (can't await in override void) | ||
| _ = DisconnectWallet(); | ||
| } |
There was a problem hiding this comment.
Fire-and-forget async call in synchronous Logout() override.
_ = DisconnectWallet() discards the Task, meaning:
- Any exceptions in
DisconnectWalletare silently swallowed - The caller cannot determine when the disconnect operation completes
AccountandMnemonic(cleared inbase.Logout()at line 175) may still be non-null immediately afterLogout()returns, sinceDisconnectWalletruns asynchronously
This can cause race conditions in calling code that expects Logout() to be synchronous.
Proposed mitigation: Add synchronous cache clear as fallback
public override void Logout()
{
- // Fire-and-forget the async disconnect (can't await in override void)
- _ = DisconnectWallet();
+ // Clear local state synchronously for immediate consistency
+ _authToken = null;
+ base.Logout();
+
+ // Fire-and-forget the network deauthorize (best-effort cleanup)
+ _ = Task.Run(async () =>
+ {
+ try
+ {
+ await _authCache.ClearAuthToken(WalletIdentity);
+ // Optionally send deauthorize to wallet
+ }
+ catch (Exception e)
+ {
+ Debug.LogWarning($"[MWA] Async logout cleanup failed: {e.Message}");
+ }
+ });
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Runtime/codebase/SolanaMobileStack/SolanaMobileWalletAdapter.cs` around lines
333 - 337, The override Logout currently fire-and-forgets DisconnectWallet(),
causing swallowed exceptions and race conditions; change it to (1) call
base.Logout() to synchronously clear Account and Mnemonic, then (2)
synchronously wait for DisconnectWallet() to complete using
DisconnectWallet().ConfigureAwait(false).GetAwaiter().GetResult() (wrap in
try/catch to log or rethrow as appropriate) so exceptions surface and callers
observe completion before Logout returns.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@PITCH.md`:
- Around line 121-131: The budget total header "## 💰 Budget — $6,500 USD (in
SKR)" and the table's "**Total** | **$6,500**" are inconsistent with the sum of
line items (3500+800+1200+700+800+500 = $7,500); update either the individual
line item values or change both the header and the Total row to "$7,500" so the
header, table content, and Total row all match (locate the budget section title
"## 💰 Budget — $6,500 USD (in SKR)", the item rows "Core SDK implementation",
"PR review rounds + integration polish", "Full SDK documentation", "Open-source
Example Android App", "Community support + post-merge maintenance", "Buffer /
contingency", and the "**Total**" row to apply the fix).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Runtime/codebase/SolanaMobileStack/LocalAssociationScenario.cs (1)
154-168:⚠️ Potential issue | 🔴 CriticalFix async queue dispatch before this lands.
With
StartAndExecuteAsync(),_actionsis never initialized, so Line 156 dereferencesnullon the first call fromReceivePublicKeyHandler(). This block also falls through afterCloseAssociation(response), which can double-complete_startAssociationTaskCompletionSourceor dequeue from an empty queue on the terminal response. That breaks every new async MWA flow built on this path.Suggested fix
private void ExecuteNextAction(Response<object> response = null) { - if (_actions.Count == 0 || response is { Failed: true }) - CloseAssociation(response); - - if (_useAsyncActions) - { - // Properly await the async action before proceeding - ExecuteNextActionAsync(response); - } - else - { - var action = _actions.Dequeue(); - action.Invoke(_client); - } + var isTerminal = response is { Failed: true }; + var hasPendingActions = _useAsyncActions + ? _asyncActions != null && _asyncActions.Count > 0 + : _actions != null && _actions.Count > 0; + + if (!hasPendingActions || isTerminal) + { + CloseAssociation(response); + return; + } + + if (_useAsyncActions) + { + ExecuteNextActionAsync(response); + return; + } + + var action = _actions.Dequeue(); + action.Invoke(_client); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Runtime/codebase/SolanaMobileStack/LocalAssociationScenario.cs` around lines 154 - 168, ExecuteNextAction currently dereferences a possibly null _actions (StartAndExecuteAsync never initializes it), falls through after CloseAssociation(response) causing double-complete of _startAssociationTaskCompletionSource and dequeues on empty queue; fix by ensuring _actions is initialized in StartAndExecuteAsync (or lazily in ExecuteNextAction) before any use, make ExecuteNextAction return immediately after calling CloseAssociation when response is null/failed or _actions.Count == 0, and when _useAsyncActions is true await the async helper (await ExecuteNextActionAsync(response)) instead of fire-and-forget; also guard completion of _startAssociationTaskCompletionSource in CloseAssociation (check IsCompleted/try-set) to prevent double-complete.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Runtime/codebase/SolanaMobileStack/SolanaMobileWalletAdapter.cs`:
- Around line 95-99: The lifecycle events are raised before the adapter's local
state is updated, causing handlers to see stale session data; move the event
invocations so state changes happen first: for the reauthorization path, assign
_authToken, await _authCache.SetAuthToken(WalletIdentity, _authToken), construct
and set the new Account (using new PublicKey(reauthorizeResult.PublicKey)) and
only then invoke OnWalletReconnected; for logout, call base.Logout() (which
clears Account and Mnemonic) and only after that invoke OnWalletDisconnected so
handlers observe the cleared state.
- Around line 146-170: DisconnectWallet currently only deauthorizes when
_authToken is populated, leaving wallet-side sessions alive if the app restarted
and the token is only in _authCache; update DisconnectWallet so it reads any
cached token from _authCache (e.g., await
_authCache.GetAuthToken(WalletIdentity)) when _authToken is null, capture that
token into a local variable, call LocalAssociationScenario.StartAndExecuteAsync
with client => client.Deauthorize(capturedToken) inside the existing try/catch,
and then clear both _authToken and await
_authCache.ClearAuthToken(WalletIdentity); keep the same best-effort error
handling and ensure you never attempt to deauthorize with a null/empty token.
---
Outside diff comments:
In `@Runtime/codebase/SolanaMobileStack/LocalAssociationScenario.cs`:
- Around line 154-168: ExecuteNextAction currently dereferences a possibly null
_actions (StartAndExecuteAsync never initializes it), falls through after
CloseAssociation(response) causing double-complete of
_startAssociationTaskCompletionSource and dequeues on empty queue; fix by
ensuring _actions is initialized in StartAndExecuteAsync (or lazily in
ExecuteNextAction) before any use, make ExecuteNextAction return immediately
after calling CloseAssociation when response is null/failed or _actions.Count ==
0, and when _useAsyncActions is true await the async helper (await
ExecuteNextActionAsync(response)) instead of fire-and-forget; also guard
completion of _startAssociationTaskCompletionSource in CloseAssociation (check
IsCompleted/try-set) to prevent double-complete.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4dddca93-0175-455b-b975-a80633258db9
📒 Files selected for processing (6)
PITCH.mdRuntime/codebase/SolanaMobileStack/JsonRpcClient/Responses/WalletCapabilities.csRuntime/codebase/SolanaMobileStack/LocalAssociationScenario.csRuntime/codebase/SolanaMobileStack/MwaAuthCache/EncryptedAuthCache.csRuntime/codebase/SolanaMobileStack/MwaAuthCache/PlayerPrefsAuthCache.csRuntime/codebase/SolanaMobileStack/SolanaMobileWalletAdapter.cs
Comment on lines
+95
to
+99
| _authToken = reauthorizeResult.AuthToken; | ||
| await _authCache.SetAuthToken(WalletIdentity, _authToken); | ||
| var cachedPublicKey = new PublicKey(reauthorizeResult.PublicKey); | ||
| OnWalletReconnected?.Invoke(); | ||
| return new Account(string.Empty, cachedPublicKey); |
There was a problem hiding this comment.
Emit lifecycle events after local state changes.
At Line 98, OnWalletReconnected fires before the adapter exposes the new Account; at Line 173, OnWalletDisconnected fires before base.Logout() clears Account and Mnemonic. Handlers that inspect adapter state inside these callbacks will observe the old session.
Also applies to: 172-174
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Runtime/codebase/SolanaMobileStack/SolanaMobileWalletAdapter.cs` around lines
95 - 99, The lifecycle events are raised before the adapter's local state is
updated, causing handlers to see stale session data; move the event invocations
so state changes happen first: for the reauthorization path, assign _authToken,
await _authCache.SetAuthToken(WalletIdentity, _authToken), construct and set the
new Account (using new PublicKey(reauthorizeResult.PublicKey)) and only then
invoke OnWalletReconnected; for logout, call base.Logout() (which clears Account
and Mnemonic) and only after that invoke OnWalletDisconnected so handlers
observe the cleared state.
Comment on lines
+146
to
+170
| public async Task DisconnectWallet() | ||
| { | ||
| if (!string.IsNullOrEmpty(_authToken)) | ||
| { | ||
| try | ||
| { | ||
| var localAssociationScenario = new LocalAssociationScenario(); | ||
| var currentToken = _authToken; // capture before clearing | ||
| await localAssociationScenario.StartAndExecuteAsync( | ||
| new List<Func<IAdapterOperations, Task>> | ||
| { | ||
| async client => await client.Deauthorize(currentToken) | ||
| } | ||
| ); | ||
| } | ||
| catch (Exception e) | ||
| { | ||
| // Best-effort — don't block logout if deauthorize fails (e.g. wallet not installed) | ||
| Debug.LogWarning($"[MWA] Deauthorize failed during disconnect: {e}"); | ||
| } | ||
| } | ||
|
|
||
| // Clear all local auth state | ||
| _authToken = null; | ||
| await _authCache.ClearAuthToken(WalletIdentity); |
There was a problem hiding this comment.
Disconnect should revoke cached sessions after a cold start.
If the app restarts, the only auth token may be in _authCache. DisconnectWallet() only checks _authToken, so an explicit sign-out just clears the local cache and leaves the wallet-side authorization alive.
Suggested fix
public async Task DisconnectWallet()
{
- if (!string.IsNullOrEmpty(_authToken))
+ var tokenToRevoke = _authToken;
+ if (string.IsNullOrEmpty(tokenToRevoke) && _walletOptions.keepConnectionAlive)
+ tokenToRevoke = await _authCache.GetAuthToken(WalletIdentity);
+
+ if (!string.IsNullOrEmpty(tokenToRevoke))
{
try
{
var localAssociationScenario = new LocalAssociationScenario();
- var currentToken = _authToken; // capture before clearing
await localAssociationScenario.StartAndExecuteAsync(
new List<Func<IAdapterOperations, Task>>
{
- async client => await client.Deauthorize(currentToken)
+ async client => await client.Deauthorize(tokenToRevoke)
}
);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Runtime/codebase/SolanaMobileStack/SolanaMobileWalletAdapter.cs` around lines
146 - 170, DisconnectWallet currently only deauthorizes when _authToken is
populated, leaving wallet-side sessions alive if the app restarted and the token
is only in _authCache; update DisconnectWallet so it reads any cached token from
_authCache (e.g., await _authCache.GetAuthToken(WalletIdentity)) when _authToken
is null, capture that token into a local variable, call
LocalAssociationScenario.StartAndExecuteAsync with client =>
client.Deauthorize(capturedToken) inside the existing try/catch, and then clear
both _authToken and await _authCache.ClearAuthToken(WalletIdentity); keep the
same best-effort error handling and ensure you never attempt to deauthorize with
a null/empty token.
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
1 participant
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.
Problem
The Solana.Unity-SDK MWA implementation is missing three features that the React Native MWA SDK has:
deauthorize— No way to explicitly revoke a wallet session token.Logout()just clears local state without telling the wallet app, leaking the token indefinitely.get_capabilities— No way for apps to query a wallet's supported features (max transaction batch sizes, supported versions)._authTokenis only stored in memory. Every app restart forces the user to approve wallet authorization again, even if they connected yesterday. This is the single biggest UX problem for Solana Unity games on Android.Solution
1.
IMwaAuthCache— Extensible Auth Token Cache LayerIntroduced a new
IMwaAuthCacheinterface that decouples token persistence from the adapter. Developers can inject their own encrypted or platform-specific storage backends.PlayerPrefsAuthCache— Default Unity PlayerPrefs implementation (works out of the box)EncryptedAuthCache— Stub template for secure storage backendsnew SolanaWalletAdapter(options, authCache: myCache)2. Silent Reconnect via Cached Token
Login()/Connect()now first checks the cache for a stored token and attempts areauthorizesilently (no user prompt). Falls back to fullauthorizeonly if the token is missing or expired.3.
Deauthorize— Clean Session TerminationDeauthorize(string authToken)toIAdapterOperationsand implemented thedeauthorizeJSON-RPC call inMobileWalletAdapterClientLogout()now callsDeauthorizebefore clearing local state, properly revoking the session in the wallet appDisconnectWallet()public async method wraps this for clean UI-triggered logout4.
GetCapabilities— Wallet Feature DetectionGetCapabilities()toIAdapterOperationsand implementedget_capabilitiesJSON-RPC callWalletCapabilitiesmodel withMaxTransactionsPerRequest,MaxMessagesPerRequest,SupportedTransactionVersions5. New Public API on
SolanaWalletAdapterBefore & After Screenshots
BEFORE — User forced to approve wallet on every app launch:
AFTER — Silent reconnect with cached token:
Other Changes
IAdapterOperations.cs— Added XML doc comments to all existing methods for better IDE supportSolanaMobileWalletAdapter.cs— Auth token is now refreshed in the cache after everySignTransactionandSignMessagecall (wallets may rotate tokens)SolanaWalletAdapter.cs— Constructor now accepts optionalIMwaAuthCache authCacheparameter (backward compatible, defaults toPlayerPrefsAuthCache)Deploy Notes
New files:
Runtime/codebase/SolanaMobileStack/MwaAuthCache/IMwaAuthCache.cs— Cache interfaceRuntime/codebase/SolanaMobileStack/MwaAuthCache/PlayerPrefsAuthCache.cs— Default implRuntime/codebase/SolanaMobileStack/MwaAuthCache/EncryptedAuthCache.cs— Extensible stubRuntime/codebase/SolanaMobileStack/JsonRpcClient/Responses/WalletCapabilities.cs— Response modelNew dependencies: None. All implementations use existing Unity APIs (
PlayerPrefs) and existing SDK infrastructure.Breaking changes: None. All changes are additive. Existing constructor calls continue to work identically.
Summary by CodeRabbit
New Features
Documentation