Skip to content

Fix NullReferenceException in Web3.BlockHash() and AwaitWsRpcConnection() #261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nikolaysamoil0ff wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix NullReferenceException in Web3.BlockHash() and AwaitWsRpcConnection() #261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
85150e7
Update WalletBase.cs
nikolaysamoil0ff Feb 23, 2026
6ca249d
Update WalletBase.cs
nikolaysamoil0ff Feb 23, 2026
eeb1e05
Update Web3.cs
nikolaysamoil0ff Feb 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Runtime/codebase/WalletBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,10 @@ public Task AwaitWsRpcConnection()
{
var wsConnection = ActiveStreamingRpcClient;
if(wsConnection?.State.Equals(WebSocketState.Open) ?? false) return Task.CompletedTask;

if (_webSocketConnection == null)
throw new InvalidOperationException("WebSocket connection is not initialized.");

return _webSocketConnection.Task;
Comment thread
nikolaysamoil0ff marked this conversation as resolved.
}

Expand Down
6 changes: 4 additions & 2 deletions Runtime/codebase/Web3.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,11 +312,13 @@ public void Logout()
/// <param name="maxSeconds">A given blockhash can only be used by transactions for about 60 to 90 seconds
/// https://docs.solana.com/developing/transaction_confirmation#how-does-transaction-expiration-work</param>
/// <returns></returns>
public static Task<string> BlockHash(
public static Task<string?> BlockHash(
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Feb 23, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for nullable enable in csproj and Directory.Build.props
rg -n "Nullable" --include="*.csproj" --include="*.props" --include="Directory.Build.*" -i

# Check for file-level nullable enable directives in C# files
rg -n "#nullable enable" --type=cs

# Also check the actual BlockHash method and its usages
echo "=== BlockHash method definition ==="
rg -n "BlockHash" --type=cs -A 2 -B 2

# Check if there are any callers of BlockHash
echo "=== BlockHash usages/callers ==="
rg -n "BlockHash\(" --type=cs

Repository: magicblock-labs/Solana.Unity-SDK

Length of output: 21917

🏁 Script executed:

#!/bin/bash

# Find and inspect .csproj files for Nullable settings
echo "=== Finding .csproj files ==="
find . -name "*.csproj" -type f | head -5

# Check the main csproj for Nullable setting
echo -e "\n=== Checking for Nullable in csproj files ==="
find . -name "*.csproj" -exec grep -l "Nullable" {} \;

# Check Web3.cs for file-level nullable directives and full method signature
echo -e "\n=== Web3.cs nullable directives and BlockHash method ==="
head -20 Runtime/codebase/Web3.cs
tail -n +310 Runtime/codebase/Web3.cs | head -20

# Check WalletBase.cs GetBlockHash implementation fully
echo -e "\n=== WalletBase GetBlockHash implementation ==="
sed -n '287,315p' Runtime/codebase/WalletBase.cs

Repository: magicblock-labs/Solana.Unity-SDK

Length of output: 3002

API change is breaking for external SDK consumers, but semantically necessary.

Changing Task<string> to Task<string?> is source-breaking for external callers with #nullable enable. However, this project itself doesn't enable nullable reference types, and the underlying implementation of WalletBase.GetBlockHash already returns null in multiple places despite its non-nullable return type declaration. This change makes the API contract accurately reflect the actual nullability behavior (when Instance is null or when blockhash is null). While external SDK consumers will need to adjust their type handling, this is a necessary correction rather than an unintended breaking change.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Runtime/codebase/Web3.cs` at line 315, The public API method BlockHash was
changed from Task<string> to Task<string?> to reflect that
WalletBase.GetBlockHash actually returns null in some cases (when Instance is
null or blockhash is null), which is source-breaking for nullable-aware
consumers; update the declaration and any related usages to return Task<string?>
and ensure callers handle a possible null result, and audit
WalletBase.GetBlockHash, the BlockHash method, and any call sites that use
Instance or check blockhash to consistently propagate or handle the nullable
string return.

Commitment commitment = Commitment.Confirmed,
bool useCache = true,
int maxSeconds = 0) =>
Instance != null ? Instance.WalletBase.GetBlockHash(commitment, useCache, maxSeconds) : null;
Instance != null
? Instance.WalletBase.GetBlockHash(commitment, useCache, maxSeconds)
: Task.FromResult<string?>(null);
Comment on lines +319 to +321
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Feb 23, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Instance.WalletBase is not null-guarded — NRE still possible after logout or before wallet initialization.

The guard on line 319 only checks Instance != null. When Instance exists but Instance.WalletBase is null (e.g., after Logout() sets WalletBase = null, or before any wallet is set), line 320 dereferences a null reference and throws NullReferenceException. All other static helpers in this class (Rpc, WsRpc, Account) guard against both nulls — this one should too.

🐛 Proposed fix 
-            Instance != null 
-                ? Instance.WalletBase.GetBlockHash(commitment, useCache, maxSeconds) 
-                : Task.FromResult<string?>(null);
+            Instance != null && Instance.WalletBase != null
+                ? Instance.WalletBase.GetBlockHash(commitment, useCache, maxSeconds)
+                : Task.FromResult<string?>(null);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Instance != null
? Instance.WalletBase.GetBlockHash(commitment, useCache, maxSeconds)
: Task.FromResult<string?>(null);
Instance != null && Instance.WalletBase != null
? Instance.WalletBase.GetBlockHash(commitment, useCache, maxSeconds)
: Task.FromResult<string?>(null);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Runtime/codebase/Web3.cs` around lines 319 - 321, The current static helper
checks only Instance != null but still dereferences Instance.WalletBase when
calling Instance.WalletBase.GetBlockHash(commitment, useCache, maxSeconds),
which can cause an NRE if WalletBase is null; update the guard to ensure
WalletBase is non-null (e.g., Instance != null && Instance.WalletBase != null or
using a null-conditional) and return Task.FromResult<string?>(null) when
WalletBase is null so the call to GetBlockHash is only invoked on a valid
WalletBase instance.




Expand Down