Merge pull request #179 from WalletConnect/fix/wc-sessionevent

Fix `wc_sessionEvent`

Core Modules/WalletConnectSharp.Common/Events/EventHandlerMap.cs

@@ -6,11 +6,11 @@
 /// <typeparam name="TEventArgs">The type of EventHandler's argument to store</typeparam>
 public class EventHandlerMap<TEventArgs> : IDisposable
 {
-    private Dictionary<string, EventHandler<TEventArgs>> mapping = new();
+    private readonly Dictionary<string, EventHandler<TEventArgs>> _mapping = new();
 
     private readonly object _mappingLock = new();
 
-    private EventHandler<TEventArgs> BeforeEventExecuted;
+    private readonly EventHandler<TEventArgs> _beforeEventExecuted;
 
     /// <summary>
     /// Create a new EventHandlerMap with an initial EventHandler to append onto
@@ -23,11 +23,14 @@ public EventHandlerMap(EventHandler<TEventArgs> callbackBeforeExecuted = null)
             callbackBeforeExecuted = CallbackBeforeExecuted;
         }
 
-        this.BeforeEventExecuted = callbackBeforeExecuted;
+        _beforeEventExecuted = callbackBeforeExecuted;
     }
 
-    private void CallbackBeforeExecuted(object sender, TEventArgs e)
+    private static void CallbackBeforeExecuted(object sender, TEventArgs e)
     {
+        // Default event handler used when no specific callback is provided. 
+        // Currently, it doesn't perform any action when an event is triggered.
+        // This is necessary to avoid null reference exceptions when no event handler is provided.
     }
 
     /// <summary>
@@ -41,21 +44,17 @@ public EventHandler<TEventArgs> this[string eventId]
         {
             lock (_mappingLock)
             {
-                mapping.TryAdd(eventId, BeforeEventExecuted);
+                _mapping.TryAdd(eventId, _beforeEventExecuted);
 
-                return mapping[eventId];
+                return _mapping[eventId];
             }
         }
         set
         {
             lock (_mappingLock)
             {
-                if (mapping.ContainsKey(eventId))
-                {
-                    mapping.Remove(eventId);
-                }
-
-                mapping.Add(eventId, value);
+                _mapping.Remove(eventId);
+                _mapping.Add(eventId, value);
             }
         }
     }
@@ -69,7 +68,15 @@ public void ListenOnce(string eventId, EventHandler<TEventArgs> eventHandler)
             eventHandler(src, args);
         };
         this[eventId] += internalHandler;
-    } 
+    }
+
+    public bool TryGetValue(string eventName, out EventHandler<TEventArgs> handler)
+    {
+        lock (_mappingLock)
+        {
+            return _mapping.TryGetValue(eventName, out handler);
+        }
+    }
 
     /// <summary>
     /// Check if a given eventId has any EventHandlers registered yet.
@@ -80,7 +87,7 @@ public bool Contains(string eventId)
     {
         lock (_mappingLock)
         {
-            return mapping.ContainsKey(eventId);
+            return _mapping.ContainsKey(eventId);
         }
     }
 
@@ -92,18 +99,21 @@ public void Clear(string eventId)
     {
         lock (_mappingLock)
         {
-            if (mapping.ContainsKey(eventId))
-            {
-                mapping.Remove(eventId);
-            }
+            _mapping.Remove(eventId);
         }
     }
-
+    
     public void Dispose()
+    {
+        Dispose(true);
+        GC.SuppressFinalize(this);
+    }
+
+    protected virtual void Dispose(bool disposing)
     {
         lock (_mappingLock)
         {
-            mapping.Clear();
+            _mapping.Clear();
         }
     }
 }

Core Modules/WalletConnectSharp.Common/Events/GenericEventHolder.cs

@@ -1,18 +1,27 @@
-namespace WalletConnectSharp.Common.Events;
+using System.Collections.Concurrent;
+
+namespace WalletConnectSharp.Common.Events;
 
 public class GenericEventHolder
 {
-    private Dictionary<Type, object> dynamicTypeMapping = new();
+    private readonly ConcurrentDictionary<Type, object> _dynamicTypeMapping = new();
 
     public EventHandlerMap<T> OfType<T>()
     {
         Type t = typeof(T);
 
-        if (dynamicTypeMapping.TryGetValue(t, out var value))
-            return (EventHandlerMap<T>)value;
+        if (_dynamicTypeMapping.TryGetValue(t, out var value))
+        {
+            if (value is EventHandlerMap<T> eventHandlerMap)
+            {
+                return eventHandlerMap;
+            }
+
+            throw new InvalidCastException($"Stored value cannot be cast to EventHandlerMap<{typeof(T).Name}>");
+        }
 
         var mapping = new EventHandlerMap<T>();
-        dynamicTypeMapping.Add(t, mapping);
+        _dynamicTypeMapping.TryAdd(t, mapping);
 
         return mapping;
     }

Core Modules/WalletConnectSharp.Network/Interfaces/IBaseJsonRpcProvider.cs

@@ -26,11 +26,11 @@ public interface IBaseJsonRpcProvider
         /// <summary>
         /// Send a Json RPC request with a parameter field of type T, and decode a response with the type of TR.
         /// </summary>
-        /// <param name="request">The json rpc request to send</param>
+        /// <param name="requestArgs">The json rpc request to send</param>
         /// <param name="context">The current context</param>
         /// <typeparam name="T">The type of the parameter field in the json rpc request</typeparam>
         /// <typeparam name="TR">The type of the parameter field in the json rpc response</typeparam>
         /// <returns>The decoded response for the request</returns>
-        Task<TR> Request<T, TR>(IRequestArguments<T> request, object context = null);
+        Task<TR> Request<T, TR>(IRequestArguments<T> requestArgs, object context = null);
     }
 }

Core Modules/WalletConnectSharp.Network/JsonRpcProvider.cs

@@ -29,7 +29,7 @@ public class JsonRpcProvider : IJsonRpcProvider, IModule
 
         public event EventHandler<string> RawMessageReceived;
 
-        private GenericEventHolder jsonResponseEventHolder = new();
+        private readonly GenericEventHolder _jsonResponseEventHolder = new();
         protected bool Disposed;
 
         /// <summary>
@@ -200,7 +200,7 @@ public async Task<TR> Request<T, TR>(IRequestArguments<T> requestArgs, object co
 
             TaskCompletionSource<TR> requestTask = new TaskCompletionSource<TR>(TaskCreationOptions.None);
 
-            jsonResponseEventHolder.OfType<string>()[request.Id.ToString()] += (sender, responseJson) =>
+            _jsonResponseEventHolder.OfType<string>()[request.Id.ToString()] += (sender, responseJson) =>
             {
                 if (requestTask.Task.IsCompleted)
                     return;
@@ -217,7 +217,7 @@ public async Task<TR> Request<T, TR>(IRequestArguments<T> requestArgs, object co
                 }
             };
 
-            jsonResponseEventHolder.OfType<WalletConnectException>()[request.Id.ToString()] += (sender, exception) =>
+            _jsonResponseEventHolder.OfType<WalletConnectException>()[request.Id.ToString()] += (sender, exception) =>
             {
                 if (requestTask.Task.IsCompleted)
                     return;
@@ -303,13 +303,13 @@ private void OnPayload(object sender, string json)
                 if (payload.IsError)
                 {
                     var errorPayload = JsonConvert.DeserializeObject<JsonRpcError>(json);
-                    jsonResponseEventHolder.OfType<WalletConnectException>()[payload.Id.ToString()](this,
+                    _jsonResponseEventHolder.OfType<WalletConnectException>()[payload.Id.ToString()](this,
                         errorPayload.Error.ToException());
                 }
                 else
                 {
                     WCLogger.Log($"Triggering event for ID {payload.Id.ToString()}");
-                    jsonResponseEventHolder.OfType<string>()[payload.Id.ToString()](this, json);
+                    _jsonResponseEventHolder.OfType<string>()[payload.Id.ToString()](this, json);
                 }
             }
         }

Tests/WalletConnectSharp.Auth.Tests/SignatureTest.cs

@@ -7,11 +7,12 @@ namespace WalletConnectSharp.Auth.Tests;
 
 public class SignatureTest
 {
-    public string ChainId = "eip155:1";
-    public string ProjectId = TestValues.TestProjectId;
-    public string Address = "0x2faf83c542b68f1b4cdc0e770e8cb9f567b08f71";
+    public const string ChainId = "eip155:1";
+    public const string Address = "0x2faf83c542b68f1b4cdc0e770e8cb9f567b08f71";
 
-    public string ReconstructedMessage = @"localhost wants you to sign in with your Ethereum account:
+    private readonly string _projectId = TestValues.TestProjectId;
+
+    private readonly string _reconstructedMessage = @"localhost wants you to sign in with your Ethereum account:
 0x2faf83c542b68f1b4cdc0e770e8cb9f567b08f71
 
 URI: http://localhost:3000/
@@ -20,27 +21,27 @@ public class SignatureTest
 Nonce: 1665443015700
 Issued At: 2022-10-10T23:03:35.700Z
 Expiration Time: 2022-10-11T23:03:35.700Z".Replace("\r", "");
-    
-    [Fact, Trait("Category", "unit")]
+
+    [Fact] [Trait("Category", "integration")]
     public async Task TestValidEip1271Signature()
     {
         var signature = new Cacao.CacaoSignature.EIP1271CacaoSignature(
             "0xc1505719b2504095116db01baaf276361efd3a73c28cf8cc28dabefa945b8d536011289ac0a3b048600c1e692ff173ca944246cf7ceb319ac2262d27b395c82b1c");
 
         var isValid =
-            await SignatureUtils.VerifySignature(Address, ReconstructedMessage, signature, ChainId, ProjectId);
+            await SignatureUtils.VerifySignature(Address, _reconstructedMessage, signature, ChainId, _projectId);
 
         Assert.True(isValid);
     }
 
-    [Fact, Trait("Category", "unit")]
+    [Fact] [Trait("Category", "integration")]
     public async Task TestBadEip1271Signature()
     {
         var signature = new Cacao.CacaoSignature.EIP1271CacaoSignature(
             "0xdead5719b2504095116db01baaf276361efd3a73c28cf8cc28dabefa945b8d536011289ac0a3b048600c1e692ff173ca944246cf7ceb319ac2262d27b395c82b1c");
 
         var isValid =
-            await SignatureUtils.VerifySignature(Address, ReconstructedMessage, signature, ChainId, ProjectId);
+            await SignatureUtils.VerifySignature(Address, _reconstructedMessage, signature, ChainId, _projectId);
 
         Assert.False(isValid);
     }