RelayerMessageCallback invokes all session request handlers for each event (and causes json deserialization exceptions)

[aliarbak]

Context (Web3Wallet)

I've two different complex RPC request payloads (for eth_sendTransaction and personal_sign methods):

[RpcMethod("personal_sign"), RpcRequestOptions(Clock.ONE_MINUTE, 99994)]
public class EthPersonalSignRequest : List<string>

[RpcMethod("eth_sendTransaction"), RpcRequestOptions(Clock.ONE_MINUTE, 99997)]
public class EthSendTransactionRequest : List<TransactionInput>

// handlers
client.Engine.SignClient.SessionRequestEvents<EthSendTransactionRequest, EthSendTransactionResponse>().OnRequest += OnEthSendTransactionRequested;
client.Engine.SignClient.SessionRequestEvents<EthPersonalSignRequest, EthPersonalSignResponse>().OnRequest += OnEthPersonalSignRequested;

The Issue

• The wallet client throws a JSON deserialization exception when it receives a personal_sign (or eth_sendTransaction) request.

Reason

• The RelayerMessageCallback function of the TypedMessageHandler invokes all session request handlers for each session request event (regardless of method type).
• When the client receives a personal_sign request (an array of strings), it tries to deserialize it to the payload of the eth_sendTransaction method (an array of objects) and throws an exception.

Actual Root Cause

• When registering session request handlers, the HandleMessageType function of the TypedMessageHandler obtains the method name by calling RpcMethodAttribute.MethodForType<T>
• RpcMethodAttribute.MethodForType<T>, finds the method name from the RpcMethodAttribute of the given type (T). But, for session request events, the type (T) will always be SessionRequest<TR> (TR: the actual RPC request type) and it always returns wc_sessionRequest:

[RpcMethod("wc_sessionRequest")]
public class SessionRequest<T> : IWcMethod

• It registers all session request event handlers (to the messageEventHandlerMap) with the key of wc_sessionRequest.
• When an event is received, the RelayerMessageCallback function retrieves all handlers with the key of wc_sessionRequest and invokes all of them. In the RequestCallback local function (of the HandleMessageType function of the TypedMessageHandler), it tries to decode the message to the target request type, and it throws a JSON deserialization exception when the payload schema does not match:

var payload = await this.Core.Crypto.Decode<JsonRpcRequest<T>>(topic, message, options);

• The WrappedRefOnOnRequest function of the SessionRequestEventHandler checks the method of the event, and it does not execute the RequestCallback function of the TypedEventHandler (not TypedMessageHandler) if it does not match, but it is not enough to prevent this issue.

Proposed Solution

• RpcMethodAttribute.MethodForType<T> should check if the type T is a generic type:
  • If so, it should check whether any of the generic arguments of type T has the RpcMethodAttribute attribute and return the method name of the generic argument.
  • In this way, the event handler of the personal_sign request will be registered as personal_sign instead of wc_sessionRequest (in messageEventHandlerMap)
• The Method property of the JsonRpcPayload class should check whether the param.request.method value exists in the _extraStuff dictionary. If so, it should return the param.request.method instead of the method.
• I created a PR to fix this issue: fix: duplicate wc_sessionRequest handlers issue #187

How to Reproduce the Issue?

You can add this integration test to the SignTests.cs test file of the WalletConnectSharp.Sign.Test project.

 // represents array of strings requests, similar to personal_sign
[RpcMethod("complex_test_method"), RpcRequestOptions(Clock.ONE_MINUTE, 99990)]
public class ComplexTestRequest: List<string>
{
    public ComplexTestRequest()
    {
    }

    public ComplexTestRequest(params string[] args) : base(args)
    {
    }

    public int A
    {
        get
        {
            if (Count != 2 || !int.TryParse(this[0], out var a))
            {
                return 0;
            }

            return a;
        }
    }

    public int B
    {
        get
        {
            if (Count != 2 || !int.TryParse(this[1], out var b))
            {
                return 0;
            }

            return b;
        }
    }
}

// represents array of objects requests, similar to eth_sendTransaction
[RpcMethod("complex_test_method_2"), 
 RpcRequestOptions(Clock.ONE_MINUTE, 99991), 
 RpcResponseOptions(Clock.ONE_MINUTE, 99992)]
public class ComplexTestRequest2 : List<TestRequest2>
{
    public ComplexTestRequest2()
    {
    }

    public ComplexTestRequest2(params TestRequest2[] args): base(args)
    {
    }

    public string X => this.FirstOrDefault()?.x ?? string.Empty;

    public int Y => this.FirstOrDefault()?.y ?? -1;
}


[Fact, Trait("Category", "integration")]
public async Task TestTwoUniqueComplexSessionRequestResponse()
{
    await _cryptoFixture.WaitForClientsReady();

    var testAddress = "0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96045";
    var testMethod = "complex_test_method";
    var testMethod2 = "complex_test_method_2";

    var dappConnectOptions = new ConnectOptions()
    {
        RequiredNamespaces = new RequiredNamespaces()
        {
            {
                "eip155",
                new ProposedNamespace()
                {
                    Methods = new[] {testMethod, testMethod2},
                    Chains = new[] {"eip155:1", "eip155:10"},
                    Events = new[] {"chainChanged", "accountsChanged"}
                }
            }
        }
    };

    var dappClient = ClientA;
    var connectData = await dappClient.Connect(dappConnectOptions);

    var walletClient = ClientB;
    var proposal = await walletClient.Pair(connectData.Uri);

    var approveData = await walletClient.Approve(proposal, testAddress);

    var sessionData = await connectData.Approval;
    await approveData.Acknowledged();

    var rnd = new Random();
    var a = rnd.Next(100);
    var b = rnd.Next(100);
    var x = rnd.NextStrings(AllowedChars, (Math.Min(a, b), Math.Max(a, b)), 1).First();
    var y = x.Length;

    var testData = new ComplexTestRequest(a.ToString(), b.ToString());
    var testData2 = new ComplexTestRequest2(new TestRequest2() {x = x, y = y});

    var pending = new TaskCompletionSource<int>();
    
    // Step 1. Setup event listener for request
    
    // The wallet client will listen for the request with the "test_method" rpc method
    walletClient.Engine.SessionRequestEvents<ComplexTestRequest, TestResponse>()
            .OnRequest += ( requestData) =>
        {
            var request = requestData.Request;
            var data = request.Params;

            requestData.Response = new TestResponse()
            {
                result = data.A * data.B
            };
            
            return Task.CompletedTask;
        };
    
    // The wallet client will listen for the request with the "test_method" rpc method
    walletClient.Engine.SessionRequestEvents<ComplexTestRequest2, bool>()
        .OnRequest += ( requestData) =>
    {
        var request = requestData.Request;
        var data = request.Params;

        requestData.Response = data.X.Length == data.Y;
            
        return Task.CompletedTask;
    };

    // The dapp client will listen for the response
    // Normally, we wouldn't do this and just rely on the return value
    // from the dappClient.Engine.Request function call (the response Result or throws an Exception)
    // We do it here for the sake of testing
    dappClient.Engine.SessionRequestEvents<ComplexTestRequest, TestResponse>()
        .FilterResponses((r) => r.Topic == sessionData.Topic)
        .OnResponse += (responseData) =>
    {
        var response = responseData.Response;
        
        var data = response.Result;

        pending.TrySetResult(data.result);

        return Task.CompletedTask;
    };
    
    // 2. Send the request from the dapp client
    var responseReturned = await dappClient.Engine.Request<ComplexTestRequest, TestResponse>(sessionData.Topic, testData);
    var responseReturned2 = await dappClient.Engine.Request<ComplexTestRequest2, bool>(sessionData.Topic, testData2);
    
    // 3. Wait for the response from the event listener
    var eventResult = await pending.Task.WithTimeout(TimeSpan.FromSeconds(5));
    
    Assert.Equal(eventResult, a * b);
    Assert.Equal(eventResult, testData.A * testData.B);
    Assert.Equal(eventResult, responseReturned.result);
    
    Assert.True(responseReturned2);
}

[skibitsky]

Hey @aliarbak, could you please check if #193 solves it for your?

[aliarbak]

yes, thanks 🙏 @skibitsky