feat: add GetCapabilities MWA 2.0 method#277
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughA new nullable Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
🚥 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: 2
🤖 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/CapabilitiesResult.cs`:
- Around line 18-24: CapabilitiesResult's collection properties
SupportedTransactionVersions and Features can be deserialized as null and cause
NREs; update the property declarations in the CapabilitiesResult class to
initialize both SupportedTransactionVersions and Features to empty List<string>
instances (e.g., new List<string>()) so consumers can safely iterate without
null checks and keep the existing JsonProperty/RequiredMember attributes intact.
In `@Runtime/codebase/SolanaMobileStack/SolanaMobileWalletAdapter.cs`:
- Around line 191-208: The method captures a CapabilitiesResult into the local
variable capabilities via LocalAssociationScenario.StartAndExecute/
GetCapabilities but may return null even when result.WasSuccessful; after the
call to StartAndExecute and the success check, add a null guard for the
capabilities variable (from this method's local “capabilities”) and handle it
consistently—either throw a descriptive Exception or log an error with
Debug.LogError and throw (e.g., "Capabilities payload was null after successful
association")—so callers never receive a null CapabilitiesResult.
🪄 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: 4e9512e0-7855-4ee6-a1a5-218cc1c3b5a7
📒 Files selected for processing (5)
Runtime/codebase/SolanaMobileStack/Interfaces/IAdapterOperations.csRuntime/codebase/SolanaMobileStack/JsonRpcClient/Responses/CapabilitiesResult.csRuntime/codebase/SolanaMobileStack/JsonRpcClient/Responses/CapabilitiesResult.cs.metaRuntime/codebase/SolanaMobileStack/MobileWalletAdapterClient.csRuntime/codebase/SolanaMobileStack/SolanaMobileWalletAdapter.cs
|
@mstevens843 Hey, thanks for looking into this. I just merged another PR that implements this too however. Can you double check if anything from this PR is still necessary? |
mstevens843
force-pushed
the
feature/get-capabilities
branch
from
28f0757 to
387f9d1
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Runtime/codebase/SolanaMobileStack/JsonRpcClient/Responses/CapabilitiesResult.cs (1)
20-22:⚠️ Potential issue | 🟡 MinorConsider defaulting
Featuresto an empty array.
NullValueHandling.Ignoreonly affects serialization — on deserialization, if"features"is absent the property remainsnull, so downstream iteration can NRE. Initialize toArray.Empty<string>()(same applies toSupportedTransactionVersionson Line 19).Proposed fix
- [JsonProperty("supported_transaction_versions")] - public string[] SupportedTransactionVersions { get; set; } - - [JsonProperty("features", NullValueHandling = NullValueHandling.Ignore)] - public string[] Features { get; set; } + [JsonProperty("supported_transaction_versions")] + public string[] SupportedTransactionVersions { get; set; } = System.Array.Empty<string>(); + + [JsonProperty("features", NullValueHandling = NullValueHandling.Ignore)] + public string[] Features { get; set; } = System.Array.Empty<string>();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Runtime/codebase/SolanaMobileStack/JsonRpcClient/Responses/CapabilitiesResult.cs` around lines 20 - 22, The Features property (and SupportedTransactionVersions) can remain null after deserialization when the JSON field is absent, causing potential NREs; initialize both properties to empty arrays by assigning Array.Empty<string>() as their default values on their auto-properties (e.g., public string[] Features { get; set; } = Array.Empty<string>(); and same for SupportedTransactionVersions) so consumers can safely enumerate them even when the JSON omits the fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@Runtime/codebase/SolanaMobileStack/JsonRpcClient/Responses/CapabilitiesResult.cs`:
- Around line 20-22: The Features property (and SupportedTransactionVersions)
can remain null after deserialization when the JSON field is absent, causing
potential NREs; initialize both properties to empty arrays by assigning
Array.Empty<string>() as their default values on their auto-properties (e.g.,
public string[] Features { get; set; } = Array.Empty<string>(); and same for
SupportedTransactionVersions) so consumers can safely enumerate them even when
the JSON omits the fields.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2b70d5b1-747e-4dca-8e6b-a03f238ebfcd
📒 Files selected for processing (1)
Runtime/codebase/SolanaMobileStack/JsonRpcClient/Responses/CapabilitiesResult.cs
|
Hey @Kuldotha , thanks for the heads up on #269. Checked it out and you're right, most of what I had is already covered. I rebased this PR down to just one thing that's still missing though: the features array on CapabilitiesResult. So this PR is now a 3-line add to the file from #269, nothing else. If you'd rather just fold it into a future patch yourself, no worries, happy to close it. Otherwise it should be a pretty quick review. |
The MWA 2.0 spec defines get_capabilities as a non-privileged method that queries wallet capabilities. This method did not exist in the SDK. Added GetCapabilities() to the MWA client layer: - IAdapterOperations.cs: added GetCapabilities() interface method - MobileWalletAdapterClient.cs: sends get_capabilities JSON-RPC request - SolanaMobileWalletAdapter.cs: async wrapper via LocalAssociationScenario - CapabilitiesResult.cs: response model (MaxTransactionsPerRequest, MaxMessagesPerRequest, SupportedTransactionVersions, Features) Tested on Solana Seeker with Phantom — returns real values: maxTransactions=10, maxMessages=1, supportedVersions=[legacy, 0], features=[supports_sign_and_send_transactions] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
mstevens843
force-pushed
the
feature/get-capabilities
branch
from
387f9d1 to
2737e6d
Compare
|
Quick update on this one. Rebased onto current main so it sits cleanly on top of #269 now. After the rebase the diff is exactly the 3-line add I described in the previous comment: just the Features property on CapabilitiesResult, in the same string[] style as the rest of the class. Both CodeRabbit comments from the original review went stale through the rebase (the null guard is already in #269's GetCapabilities, and the collection-init suggestion was for the List shape from before the rebase, not the string[] we ended up with). |
2 participants
The MWA 2.0 spec defines get_capabilities as a non-privileged method that queries wallet capabilities. This method did not exist in the SDK.
Added GetCapabilities() to the MWA client layer:
Tested on Solana Seeker with Phantom — returns real values: maxTransactions=10, maxMessages=1, supportedVersions=[legacy, 0], features=[supports_sign_and_send_transactions]
Problem
The MWA 2.0 spec defines get_capabilities as a non-privileged method for querying wallet limits and supported features. Without it, developers have no way to check how many transactions or messages a wallet can handle per request. Sending more than the wallet supports fails the entire request with
ERROR_TOO_MANY_PAYLOADS and there is no way to prevent it ahead of time.
Solution
Added GetCapabilities() across the MWA client layer following the same pattern as existing methods (authorize, signTransactions, signMessages):
Before & After Screenshots
BEFORE:
No method exists to query wallet capabilities. Developers must guess batch sizes or hardcode limits.
AFTER:
Tested on Solana Seeker with Phantom wallet:
Other changes (e.g. bug fixes, small refactors)
None. Self-contained addition.
Deploy Notes
No new dependencies, scripts, or configuration changes. Pure C# addition to the existing MWA client layer.
New scripts:
New dependencies:
Summary by CodeRabbit