Feature/3535 multiple jsonrpc urls #3566

640774n6 · 2021-10-31T08:05:41Z

Resolves #3535

Changes:

Added AdditionalRPCUrls option to IJsonRPCConfig
Added new class JsonRpcUrl to represent the data parsed from the config format
Added new interface IJsonRpcUrlCollection and implementation for managing all of the parsed JsonRpcUrls
Refactored JsonRpcContext to include optional JsonRpcUrl
Refactored JsonRpcRunner to use urls defined in JsonRpcUrlCollection
Refactored JsonRpc Startup to match connection ports to JsonRpcUrls which are passed along through to the service/processor via JsonRpcContext
Added port (int) parameter to the CreateClient() method of IWebSocketsModule. Needed to match the connection port to a JsonRpcUrl
Refactored IRpcModuleProvider/RpcModuleProvider Check() method to use JsonRpcContext instead of RpcEndpoint. If context.Url is not null: use JsonRpcUrl's enables modules list for check; Else: fallback on default EnabledModules check logic.
Updated unit tests to work with refactors

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation Update
Code style update (formatting, renaming)
Refactoring (no functional changes, no api changes)
Build related changes
Other (please describe):

Testing

Requires testing

Yes
No

In case you checked yes, did you write tests??

Yes
No

Comments about testing , should you have some (optional)

Adding testing to ensure RpcModuleProvider.Check() uses enabled modules scoped to a url when provided
Added tests for JsonRpcUrl
Added tests for JsonRpcUrlCollection

Further comments (optional)

First PR to this project (hopefully first of many) so please enlighten me if I missed a step in the process. I tried to follow the existing coding styles in use.

Config file format should be backwards compatible and continue to work without presence of AdditionalRPCUrls config option.

I'm sure there will need to be some iteration to get this right, but figured it was a good starting point to get the conversation going.

Thanks!

dceleda

I like the implementation.
I've added a few minor comments.

src/Nethermind/Nethermind.JsonRpc/JsonRpcUrl.cs

dceleda · 2021-11-02T13:05:24Z

src/Nethermind/Nethermind.JsonRpc/JsonRpcUrl.cs

+
+        public bool Equals(JsonRpcUrl other)
+        {
+            if (ReferenceEquals(null, other))


[minor]
if(other == null) {return false;}
?

dceleda · 2021-11-02T13:19:31Z

src/Nethermind/Nethermind.JsonRpc/JsonRpcUrlCollection.cs

+                        if (url.RpcEndpoint == RpcEndpoint.None)
+                        {
+                            if (_logger.IsWarn)
+                                _logger.Warn($"Additional JSON RPC URL '{url}' has web socket endpoint type and web sockets are not enabled; skipping...");


I think Info would be better in this case.

dceleda · 2021-11-02T13:35:04Z

src/Nethermind/Nethermind.JsonRpc/Modules/RpcModuleProvider.cs

+
+            if (context.Url != null) return context.Url.EnabledModules.Contains(result.ModuleType, StringComparer.InvariantCultureIgnoreCase) ? ModuleResolution.Enabled : ModuleResolution.Disabled;
+
+            return _enabledModules.Contains(result.ModuleType) == true ? ModuleResolution.Enabled : ModuleResolution.Disabled;


[minor] Why adding "== true" ?

I'm actually not sure how that got in there. I usually don't do == true unless it's a nullable bool. It also wasn't in that line to begin with. Anyways, I will fix it.

dceleda · 2021-11-02T13:39:32Z

src/Nethermind/Nethermind.JsonRpc/Modules/RpcModuleProvider.cs

-            return _enabledModules.Contains(result.ModuleType) ? ModuleResolution.Enabled : ModuleResolution.Disabled;
+            if ((result.Availability & context.RpcEndpoint) == RpcEndpoint.None) return ModuleResolution.EndpointDisabled;
+
+            if (context.Url != null) return context.Url.EnabledModules.Contains(result.ModuleType, StringComparer.InvariantCultureIgnoreCase) ? ModuleResolution.Enabled : ModuleResolution.Disabled;


I think we should return ModuleResolution.EndpointDisabled for this case if it's not defined for the URL.

dceleda · 2021-11-02T14:26:45Z

src/Nethermind/Nethermind.JsonRpc/WebSockets/JsonRpcWebSocketsModule.cs

        {
+            JsonRpcUrl jsonRpcUrl = _jsonRpcUrlCollection.FirstOrDefault(x => x.Port == port && x.RpcEndpoint.HasFlag(RpcEndpoint.WebSocket));


I'm not sure if I like how we configure these additional URLs and then actually we use only ports.
Maybe the config should contain only ports instead of scheme+host+port ?
@MarekM25 ?

I'm not sure if I like how we configure these additional URLs and then actually we use only ports. Maybe the config should contain only ports instead of scheme+host+port ? @MarekM25 ?

No, it shouldn't contain only ports. We want to be able to set up the engine module for localhost only. However, other RPC modules could be public. That is the goal of this issue.

Right, since the urls are pass in to UseUrls() in the JsonRpcRunner.cs WebHostBuilder.

This is guaranteed not to be null?

also potentially slow please use dictionary

640774n6 · 2021-11-02T15:08:04Z

@dceleda Pushed code review suggestions

dceleda · 2021-11-02T15:21:52Z

src/Nethermind/Nethermind.JsonRpc/Modules/RpcModuleProvider.cs

@@ -97,9 +97,9 @@ public ModuleResolution Check(string methodName, JsonRpcContext context)

            if ((result.Availability & context.RpcEndpoint) == RpcEndpoint.None) return ModuleResolution.EndpointDisabled;

-            if (context.Url != null) return context.Url.EnabledModules.Contains(result.ModuleType, StringComparer.InvariantCultureIgnoreCase) ? ModuleResolution.Enabled : ModuleResolution.Disabled;
+            if (context.Url != null) return context.Url.EnabledModules.Contains(result.ModuleType, StringComparer.InvariantCultureIgnoreCase) ? ModuleResolution.Enabled : ModuleResolution.EndpointDisabled;


I'm sorry I wasn't specific enough. I meant more smth like this:

if (context.Url != null && !context.Url.EnabledModules.Contains(result.ModuleType, StringComparer.InvariantCultureIgnoreCase)) { return ModuleResolution.EndpointDisabled; }

The last line returns if a module is enabled/disabled at all. The preceding lines indicate if the module is enabled/disabled at the context level.

Shouldn't it determine enabled/disabled solely off of the context enabled modules if context.Url is not null?

If I'm reading that logic right, I think it would end up hitting the last line to determine if it is enabled again using _enabledModules which comes from JsonRpcConfig.EnabledModules which is only there as a fallback for when Url is null (not http/websocket).

Good question. We have this logic in JsonRpcService

return result switch { ModuleResolution.Unknown => ((int?) ErrorCodes.MethodNotFound, $"Method {methodName} is not supported"), ModuleResolution.Disabled => (ErrorCodes.InvalidRequest, $"{methodName} found but the containing module is disabled, consider adding module to JsonRpcConfig.EnabledModule"), ModuleResolution.EndpointDisabled => (ErrorCodes.InvalidRequest, $"{methodName} found but disabled for {context.RpcEndpoint}"), _ => (null, null) };

Which indicates that the Disabled switch is linked with JsonRpcConfig.EnabledModules .
I can imagine that we have a module enabled in this config (EnabledModules) but it's not enabled for the URL - then we want to return EndpointDisabled .
The question is how the system should behave if the module is NOT listed in EnabledModules but it's listed in the URL config. What is the priority? Should we still switch it ON for the URL or it should be disabled in general ?
@MarekM25 , @LukaszRozmej ?

If it helps, the implementation was made with the requirement of keeping this backwards compatible, so JsonRpcConfig.Host/Port/WebSocketPort/EnabledModules is used to create the first url in the url collection. This way the config will still work as it did before these changes if a user does not want to use the AdditonalRpcUrls functionality.

Confirmed with @MarekM25 - both configurations (the old and the new one) are independent. So, it's good as it is - we can ignore my last suggestion :)

OK cool. So just to confirm, the latest commit (changing it to return EndpointDisabled instead of Disabled), is correct?

I'm just thinking that following the same logic, EndpointDisabled would imply that the specific endpoint is disabled (net.peerCount, but not net.listening for example), when in fact it is the entire module (net) for that url, which is what Disabled represents. If that is the case, the error message in JsonRpcService might need to be reworded.

LukaszRozmej · 2021-11-05T10:17:54Z

Hi @640774n6!
First let me thank you for fantastic contribution! Its great and the task wasn't simple.

Still I managed to find some things to potentially change which I left in code comments.
Maybe we also can add a RPC test that invokes something on non-default url, to prove it works correctly?

Other way we will definitely use this code.

src/Nethermind/Nethermind.JsonRpc/JsonRpcConfig.cs

LukaszRozmej · 2021-11-05T08:57:30Z

src/Nethermind/Nethermind.JsonRpc/JsonRpcService.cs

+                ModuleResolution.Disabled => (ErrorCodes.InvalidRequest, $"{methodName} found but the containing module is disabled, consider adding module to JsonRpcConfig.EnabledModule"),
+                ModuleResolution.EndpointDisabled => (ErrorCodes.InvalidRequest, $"{methodName} found but disabled for {context.RpcEndpoint}"),


I think we need to enhance messages

I agree. Anyone have any suggestions? If not, I can try and come up with something.

LukaszRozmej · 2021-11-05T09:01:54Z

src/Nethermind/Nethermind.JsonRpc/JsonRpcUrl.cs

+        private const string HttpEndpointValue = "http";
+        private const string WebSocketEndpointValue = "ws";


We could use Uri.UriSchemeXXX instead of duplicating values, but they are standard so it doesn't matter. I wonder If we should support https and wss here to just for ease of use.

I agree, that would be cleaner, but Uri.UriSchemeWs and Uri.UriScheme.Wss do not appear to be available until net6.0, but NethermindEth is currently targeting net5.0.

dotnet/runtime#35180

I was checking dissasembly and didn't catch they are internal 🤦‍♂️

LukaszRozmej · 2021-11-05T09:03:12Z

src/Nethermind/Nethermind.JsonRpc/JsonRpcUrl.cs

+
+            string url = parts[0];
+            if (!Uri.TryCreate(url, UriKind.Absolute, out Uri? uri) ||
+                uri.Scheme != "http" ||


Support http, https, ws, wss values based on Uri.UriSchemeXXX

I can add wss, and https but I don't think we can use Uri.UriSchemeXXX for all of them for the same reason in my comment on the previous MR review suggestion.

I believe the actual scheme of the urls we pass into UseUrls() on the web host builder of JsonRpcRunner.cs needs to be http or https. I don't think ws or wss would work.

The format in AdditionalRpcUrls can allow the same url to be enabled for http/https or ws/wss like so: "https://127.0.0.1:1234|https,ws|eth,net,admin".

The only thing the web socket code does with it is check the port of the incoming request and makes sure a websocket RpcEndpoint flag is set for a url with that port. It doesn't care about the scheme of the url.

LukaszRozmej · 2021-11-05T09:05:54Z

src/Nethermind/Nethermind.JsonRpc/JsonRpcUrl.cs

+            if (!Uri.TryCreate(url, UriKind.Absolute, out Uri? uri) ||
+                uri.Scheme != "http" ||
+                uri.Segments.Count() > 1 ||
+                uri.Port <= 0)


Doesn't Uri class handle this in parsing?

uri.Scheme != "http": can be changed to allow https, but as mentioned in the comment above, wss and ws urls won't work with web host builder UseUrls().

uri.Segments.Count() > 1: ensures we don't get Uris with path components like: "http://127.0.0.1:80/something/else".

uri.Port <= 0: I just tested and Uri.TryCreate does parse correctly when port is 0 which should be invalid. So: "http://127.0.0.1:0" is successfully parsed. Negative numbers are not allowed by the parser, so I can just do a zero check to ensure it is not zero.

Can't we allow subpaths?

Web builder's UseUrls() doesn't allow subpaths. You would have to lookup the url based on port and path in app.Use(). Port would no longer be unique as you could have multiple urls with with the same port but different subpaths which might be kind of weird, but could be done if this is desired.

src/Nethermind/Nethermind.Runner/JsonRpc/JsonRpcIpcRunner.cs

LukaszRozmej · 2021-11-05T10:10:22Z

src/Nethermind/Nethermind.Runner/JsonRpc/Startup.cs

-                                   && ctx.Connection.LocalPort == jsonRpcConfig.WebSocketsPort,
+                app.UseWhen(ctx =>
+                    ctx.WebSockets.IsWebSocketRequest &&
+                    jsonRpcUrlCollection.Any(x => x.Port == ctx.Connection.LocalPort && x.RpcEndpoint.HasFlag(RpcEndpoint.WebSocket)),


This is potentially slow, please create a Dictionary of websocket supported jsonRcpUrl's

I agree. I will refactor JsonRpcUrlCollection to be backed by a Dictionary<int, JsonRpcUrl>() with port being the key.

LukaszRozmej · 2021-11-05T10:10:57Z

src/Nethermind/Nethermind.Runner/JsonRpc/Startup.cs

@@ -144,13 +147,14 @@ long SerializeTimeoutException(IJsonRpcService service, Stream resultStream)
                    await ctx.Response.WriteAsync("Nethermind JSON RPC");
                }

-                if (ctx.Connection.LocalPort == jsonRpcConfig.Port && ctx.Request.Method == "POST")
+                JsonRpcUrl jsonRpcUrl = jsonRpcUrlCollection.FirstOrDefault(x => x.Port == ctx.Connection.LocalPort && x.RpcEndpoint.HasFlag(RpcEndpoint.Http));


This is potentially slow, please create a Dictionary of http supported jsonRcpUrl's

LukaszRozmej · 2021-11-05T10:12:04Z

src/Nethermind/Nethermind.Runner/JsonRpc/Startup.cs

@@ -144,13 +147,14 @@ long SerializeTimeoutException(IJsonRpcService service, Stream resultStream)
                    await ctx.Response.WriteAsync("Nethermind JSON RPC");
                }

-                if (ctx.Connection.LocalPort == jsonRpcConfig.Port && ctx.Request.Method == "POST")
+                JsonRpcUrl jsonRpcUrl = jsonRpcUrlCollection.FirstOrDefault(x => x.Port == ctx.Connection.LocalPort && x.RpcEndpoint.HasFlag(RpcEndpoint.Http));
+                if (ctx.Request.Method == "POST" && jsonRpcUrl != null)


minor: reverse order of checks (or with dictionary use TryGetValue)

LukaszRozmej · 2021-11-05T10:13:09Z

src/Nethermind/Nethermind.JsonRpc/WebSockets/JsonRpcWebSocketsModule.cs

        {
+            JsonRpcUrl jsonRpcUrl = _jsonRpcUrlCollection.FirstOrDefault(x => x.Port == port && x.RpcEndpoint.HasFlag(RpcEndpoint.WebSocket));


also potentially slow please use dictionary

640774n6 · 2021-11-07T07:35:56Z

I took a pass at resolving the code review comments. I also attempted to reword the ModuleResolution.Disabled message, but will definitely not be offended if someone has something better 😄

I wouldn't mind adding an RPC test to ensure the new functionality works, but I am not sure where I would do that. This sounds like an integration test and I couldn't locate where this should be added.

Thanks!

LukaszRozmej · 2021-11-08T12:42:42Z

src/Nethermind/Nethermind.JsonRpc/JsonRpcService.cs

+                ModuleResolution.Disabled => (ErrorCodes.InvalidRequest, $"{methodName} found but the containing module is disabled for the url, consider adding module for additional url in JsonRpcConfig.AdditionalRpcUrls or to JsonRpcConfig.EnabledModules"),
+                ModuleResolution.EndpointDisabled => (ErrorCodes.InvalidRequest, $"{methodName} found but disabled for {context.RpcEndpoint}"),


Cool, one thing still to add into the message now would be the url (or maybe just port?) for which it is disabled, so user can see that maybe he is querying wrong one.

Yeah for sure. I will do that.

LukaszRozmej · 2021-11-08T12:44:48Z

src/Nethermind/Nethermind.JsonRpc/JsonRpcUrl.cs

+        private const string HttpEndpointValue = "http";
+        private const string WebSocketEndpointValue = "ws";


I was checking dissasembly and didn't catch they are internal 🤦‍♂️

LukaszRozmej · 2021-11-08T12:57:39Z

src/Nethermind/Nethermind.JsonRpc/JsonRpcUrlCollection.cs

+
+                    Add(url.Port, url);
+                }
+                catch (Exception)


Shouldn't we handle only FormatException here?

It needs to catch any exception that JsonRpcUrl.Parse() can throw to allow it to recover, log the warning that the url parse failed, then skip it, and continue parsing the next url. Maybe the warning should be reworded from "not formatted correctly" to "could not be parsed"

JsonRpcUrl.Parse() could also catch other exceptions and throw FormatExceptions to keep it consistent

Refactored to make JsonRpcUrl.Parse() thrown exceptions consistent by using FormatException; Also passing on relevant and helpful format exception messages in the log.

640774n6 · 2021-11-08T18:19:36Z

Tested passing additional urls using the command line args. Looks like using a comma as part of the url format doesn't work because the config already does a Split(',') as AdditionalRPCUrls is a collection. I don't think a semi-colon is used by the config, so I vote we use a semi-colon as a delimiter for the RpcEndpoint part and the EnabledModules part.

For example instead of "http://127.0.0.1:8888|http,ws|net,web3,admin" it would be "http://127.0.0.1:8888|http;ws|net;web3;admin".

…at to support command line args; Hardened JsonRpcUrl parsing; Updated tests

640774n6 · 2021-11-08T20:02:18Z

I believe this is ready for another review pass. Eventually we'll get it right 😄

JsonRpcUrl format now using semi-colon instead of comma for RpcEndpoints/EnabledModules as comma is already used by config.
JsonRpcUrl now throws FormatExceptions for any format related problems.
JsonRpcUrlCollection logs FormatException messages if a url throws FormatException providing more context to the user.
ModuleResolution.Disabled now includes url string in reworded log message.
Updated tests.

LukaszRozmej

Looks good

LukaszRozmej · 2021-11-09T08:30:23Z

src/Nethermind/Nethermind.JsonRpc/JsonRpcService.cs

-                ModuleResolution.Disabled => (ErrorCodes.InvalidRequest, $"{methodName} found but the containing module is disabled, consider adding module to JsonRpcConfig.EnabledModules"),
-                ModuleResolution.EndpointDisabled => (ErrorCodes.InvalidRequest, $"{methodName} found but disabled for {rpcEndpoint}"),
+                ModuleResolution.Disabled => (ErrorCodes.InvalidRequest, $"{methodName} found but the containing module is disabled for the url '{context.Url?.ToString() ?? string.Empty}', consider adding module in JsonRpcConfig.AdditionalRpcUrls for additional url, or to JsonRpcConfig.EnabledModules for default url"),
+                ModuleResolution.EndpointDisabled => (ErrorCodes.InvalidRequest, $"{methodName} found but disabled for {context.RpcEndpoint}"),


We should add this url details to EndpointDisabled too

Url added to EndpointDisabled message in latest commit

LukaszRozmej · 2021-11-09T08:31:20Z

src/Nethermind/Nethermind.JsonRpc/JsonRpcUrlCollection.cs

+                        _logger.Info($"Additional JSON RPC URL packed value '{additionalRpcUrl}' format error: {fe.Message}; skipping...");
+                    continue;
+                }
+                catch (Exception e)


Still, do we want to catch any exception here? Is it really needed?

My original thinking was to allow it to recover from any exception on a per url basis so that if something unexpected occurs for one url, it doesn't stop it from parsing the rest of the urls. I guess we should only recover & continue from expected exceptions (FormatException). I can clean that up.

Cleaned up catch-all in latest commit

…ointDisabled message

LukaszRozmej · 2021-11-09T11:13:55Z

@640774n6 Cool! Great code! We will do a bit of internal testing before merging as JSON RPC is crucial functionality.

The only thing that potentially could make this better is adding some more tests around this 'routing' of modules and endpoints.

640774n6 added 4 commits October 31, 2021 01:27

Added additional rpc urls to config and began refactoring

37d44a9

Finished initial refactor; Need to fix unit tests

de6db85

Fixed unit tests and fixed config default value (thanks unit tests)

883cb63

Cleanup

b2dbad3

640774n6 force-pushed the feature/3535-multiple-jsonrpc-urls branch from a5d4dcb to b2dbad3 Compare October 31, 2021 08:34

640774n6 added 2 commits November 1, 2021 12:42

Hardened JsonRpcUrlCollection and added tests

fd982a9

Cleanup pass

2671b87

dceleda requested review from LukaszRozmej and dceleda November 2, 2021 10:59

dceleda reviewed Nov 2, 2021

View reviewed changes

Cleanup after code review

daf26d9

dceleda reviewed Nov 2, 2021

View reviewed changes

LukaszRozmej reviewed Nov 5, 2021

View reviewed changes

Refactored JsonRpcUrlCollection to be a Dictionary; Misc CR cleanup

4ea1e91

640774n6 requested a review from LukaszRozmej November 7, 2021 07:31

640774n6 added 2 commits November 7, 2021 00:50

Fixed broken test after refactor

d771e91

Testing and bug fixing

ebf46a4

LukaszRozmej reviewed Nov 8, 2021

View reviewed changes

LukaszRozmej approved these changes Nov 8, 2021

View reviewed changes

Additional url format exception consistency pass; Fix JsonRpcUrl form…

f798513

…at to support command line args; Hardened JsonRpcUrl parsing; Updated tests

LukaszRozmej reviewed Nov 9, 2021

View reviewed changes

Cleaned up exception handling in JsonRpcCollection; Added url to Endp…

57a1795

…ointDisabled message

kjazgar approved these changes Nov 9, 2021

View reviewed changes

LukaszRozmej merged commit 6281f3f into NethermindEth:master Nov 9, 2021


		if (context.Url != null) return context.Url.EnabledModules.Contains(result.ModuleType, StringComparer.InvariantCultureIgnoreCase) ? ModuleResolution.Enabled : ModuleResolution.Disabled;

		return _enabledModules.Contains(result.ModuleType) == true ? ModuleResolution.Enabled : ModuleResolution.Disabled;

		{
		JsonRpcUrl jsonRpcUrl = _jsonRpcUrlCollection.FirstOrDefault(x => x.Port == port && x.RpcEndpoint.HasFlag(RpcEndpoint.WebSocket));

		ModuleResolution.Disabled => (ErrorCodes.InvalidRequest, $"{methodName} found but the containing module is disabled, consider adding module to JsonRpcConfig.EnabledModule"),
		ModuleResolution.EndpointDisabled => (ErrorCodes.InvalidRequest, $"{methodName} found but disabled for {context.RpcEndpoint}"),

		private const string HttpEndpointValue = "http";
		private const string WebSocketEndpointValue = "ws";

Feature/3535 multiple jsonrpc urls #3566

Feature/3535 multiple jsonrpc urls #3566

Conversation

640774n6 commented Oct 31, 2021 • edited

Changes:

Types of changes

Testing

Further comments (optional)

dceleda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

640774n6 Nov 2, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

640774n6 commented Nov 2, 2021

Choose a reason for hiding this comment

640774n6 Nov 2, 2021 • edited

Choose a reason for hiding this comment

dceleda Nov 2, 2021 • edited

Choose a reason for hiding this comment

640774n6 Nov 2, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

640774n6 Nov 3, 2021 • edited

Choose a reason for hiding this comment

640774n6 Nov 3, 2021 • edited

Choose a reason for hiding this comment

LukaszRozmej commented Nov 5, 2021

Choose a reason for hiding this comment

640774n6 Nov 6, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

640774n6 Nov 6, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

640774n6 Nov 6, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

640774n6 commented Nov 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

640774n6 commented Nov 8, 2021 • edited

640774n6 commented Nov 8, 2021 • edited

LukaszRozmej left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LukaszRozmej commented Nov 9, 2021

640774n6 commented Oct 31, 2021 •

edited

640774n6 Nov 2, 2021 •

edited

640774n6 Nov 2, 2021 •

edited

dceleda Nov 2, 2021 •

edited

640774n6 Nov 2, 2021 •

edited

640774n6 Nov 3, 2021 •

edited

640774n6 Nov 3, 2021 •

edited

640774n6 Nov 6, 2021 •

edited

640774n6 Nov 6, 2021 •

edited

640774n6 Nov 6, 2021 •

edited

640774n6 commented Nov 8, 2021 •

edited

640774n6 commented Nov 8, 2021 •

edited

LukaszRozmej left a comment •

edited