From 5c665ad54ca40e473b236ddacf54aa9da563320e Mon Sep 17 00:00:00 2001 From: Jkorf Date: Fri, 10 Dec 2021 16:35:42 +0100 Subject: [PATCH] Refactoring and comments --- .../ConverterTests.cs | 4 +- .../TestImplementations/TestBaseClient.cs | 14 ++-- .../TestImplementations/TestRestClient.cs | 49 +++++++++-- .../Authentication/AuthenticationProvider.cs | 81 +++++++++++++------ CryptoExchange.Net/Clients/BaseRestClient.cs | 58 +++++++------ CryptoExchange.Net/Clients/RestApiClient.cs | 34 ++++---- CryptoExchange.Net/CryptoExchange.Net.csproj | 4 +- CryptoExchange.Net/ExtensionMethods.cs | 21 ++++- CryptoExchange.Net/Objects/TimeSyncModel.cs | 21 ----- CryptoExchange.Net/Objects/TimeSyncState.cs | 80 ++++++++++++++++++ 10 files changed, 261 insertions(+), 105 deletions(-) delete mode 100644 CryptoExchange.Net/Objects/TimeSyncModel.cs create mode 100644 CryptoExchange.Net/Objects/TimeSyncState.cs diff --git a/CryptoExchange.Net.UnitTests/ConverterTests.cs b/CryptoExchange.Net.UnitTests/ConverterTests.cs index 1d4705f..a4f43f8 100644 --- a/CryptoExchange.Net.UnitTests/ConverterTests.cs +++ b/CryptoExchange.Net.UnitTests/ConverterTests.cs @@ -140,10 +140,10 @@ namespace CryptoExchange.Net.UnitTests [TestCase("four", TestEnum.Four)] [TestCase("Four1", null)] [TestCase(null, null)] - public void TestEnumConverterNullableDeserializeTests(string? value, TestEnum? expected) + public void TestEnumConverterNullableDeserializeTests(string value, TestEnum? expected) { var val = value == null ? "null" : $"\"{value}\""; - var output = JsonConvert.DeserializeObject($"{{ \"Value\": {val} }}"); + var output = JsonConvert.DeserializeObject($"{{ \"Value\": {val} }}"); Assert.AreEqual(output.Value, expected); } } diff --git a/CryptoExchange.Net.UnitTests/TestImplementations/TestBaseClient.cs b/CryptoExchange.Net.UnitTests/TestImplementations/TestBaseClient.cs index adb591e..dc580f1 100644 --- a/CryptoExchange.Net.UnitTests/TestImplementations/TestBaseClient.cs +++ b/CryptoExchange.Net.UnitTests/TestImplementations/TestBaseClient.cs @@ -1,4 +1,5 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Net.Http; using CryptoExchange.Net.Authentication; using CryptoExchange.Net.Objects; @@ -33,14 +34,11 @@ namespace CryptoExchange.Net.UnitTests { } - public override Dictionary AddAuthenticationToHeaders(string uri, HttpMethod method, Dictionary parameters, bool signed, HttpMethodParameterPosition postParameters, ArrayParametersSerialization arraySerialization) + public override void AuthenticateRequest(RestApiClient apiClient, Uri uri, HttpMethod method, Dictionary providedParameters, bool auth, ArrayParametersSerialization arraySerialization, HttpMethodParameterPosition parameterPosition, out SortedDictionary uriParameters, out SortedDictionary bodyParameters, out Dictionary headers) { - return base.AddAuthenticationToHeaders(uri, method, parameters, signed, postParameters, arraySerialization); - } - - public override Dictionary AddAuthenticationToParameters(string uri, HttpMethod method, Dictionary parameters, bool signed, HttpMethodParameterPosition postParameters, ArrayParametersSerialization arraySerialization) - { - return base.AddAuthenticationToParameters(uri, method, parameters, signed, postParameters, arraySerialization); + bodyParameters = new SortedDictionary(); + uriParameters = new SortedDictionary(); + headers = new Dictionary(); } public override string Sign(string toSign) diff --git a/CryptoExchange.Net.UnitTests/TestImplementations/TestRestClient.cs b/CryptoExchange.Net.UnitTests/TestImplementations/TestRestClient.cs index 535702a..e425830 100644 --- a/CryptoExchange.Net.UnitTests/TestImplementations/TestRestClient.cs +++ b/CryptoExchange.Net.UnitTests/TestImplementations/TestRestClient.cs @@ -56,10 +56,10 @@ namespace CryptoExchange.Net.UnitTests.TestImplementations request.Setup(c => c.GetHeaders()).Returns(() => headers); var factory = Mock.Get(RequestFactory); - factory.Setup(c => c.Create(It.IsAny(), It.IsAny(), It.IsAny())) - .Callback((method, uri, id) => + factory.Setup(c => c.Create(It.IsAny(), It.IsAny(), It.IsAny())) + .Callback((method, uri, id) => { - request.Setup(a => a.Uri).Returns(new Uri(uri)); + request.Setup(a => a.Uri).Returns(uri); request.Setup(a => a.Method).Returns(method); }) .Returns(request.Object); @@ -76,7 +76,7 @@ namespace CryptoExchange.Net.UnitTests.TestImplementations request.Setup(c => c.GetResponseAsync(It.IsAny())).Throws(we); var factory = Mock.Get(RequestFactory); - factory.Setup(c => c.Create(It.IsAny(), It.IsAny(), It.IsAny())) + factory.Setup(c => c.Create(It.IsAny(), It.IsAny(), It.IsAny())) .Returns(request.Object); } @@ -99,8 +99,8 @@ namespace CryptoExchange.Net.UnitTests.TestImplementations request.Setup(c => c.GetHeaders()).Returns(headers); var factory = Mock.Get(RequestFactory); - factory.Setup(c => c.Create(It.IsAny(), It.IsAny(), It.IsAny())) - .Callback((method, uri, id) => request.Setup(a => a.Uri).Returns(new Uri(uri))) + factory.Setup(c => c.Create(It.IsAny(), It.IsAny(), It.IsAny())) + .Callback((method, uri, id) => request.Setup(a => a.Uri).Returns(uri)) .Returns(request.Object); } @@ -122,8 +122,23 @@ namespace CryptoExchange.Net.UnitTests.TestImplementations } + public override TimeSpan GetTimeOffset() + { + throw new NotImplementedException(); + } + protected override AuthenticationProvider CreateAuthenticationProvider(ApiCredentials credentials) => new TestAuthProvider(credentials); + + protected override Task> GetServerTimestampAsync() + { + throw new NotImplementedException(); + } + + protected override TimeSyncInfo GetTimeSyncInfo() + { + throw new NotImplementedException(); + } } public class TestRestApi2Client : RestApiClient @@ -133,8 +148,23 @@ namespace CryptoExchange.Net.UnitTests.TestImplementations } + public override TimeSpan GetTimeOffset() + { + throw new NotImplementedException(); + } + protected override AuthenticationProvider CreateAuthenticationProvider(ApiCredentials credentials) => new TestAuthProvider(credentials); + + protected override Task> GetServerTimestampAsync() + { + throw new NotImplementedException(); + } + + protected override TimeSyncInfo GetTimeSyncInfo() + { + throw new NotImplementedException(); + } } public class TestAuthProvider : AuthenticationProvider @@ -142,6 +172,13 @@ namespace CryptoExchange.Net.UnitTests.TestImplementations public TestAuthProvider(ApiCredentials credentials) : base(credentials) { } + + public override void AuthenticateRequest(RestApiClient apiClient, Uri uri, HttpMethod method, Dictionary providedParameters, bool auth, ArrayParametersSerialization arraySerialization, HttpMethodParameterPosition parameterPosition, out SortedDictionary uriParameters, out SortedDictionary bodyParameters, out Dictionary headers) + { + uriParameters = parameterPosition == HttpMethodParameterPosition.InUri ? new SortedDictionary(providedParameters) : new SortedDictionary(); + bodyParameters = parameterPosition == HttpMethodParameterPosition.InBody ? new SortedDictionary(providedParameters) : new SortedDictionary(); + headers = new Dictionary(); + } } public class ParseErrorTestRestClient: TestRestClient diff --git a/CryptoExchange.Net/Authentication/AuthenticationProvider.cs b/CryptoExchange.Net/Authentication/AuthenticationProvider.cs index eaf4d4a..2942d9e 100644 --- a/CryptoExchange.Net/Authentication/AuthenticationProvider.cs +++ b/CryptoExchange.Net/Authentication/AuthenticationProvider.cs @@ -1,6 +1,8 @@ -using CryptoExchange.Net.Objects; +using CryptoExchange.Net.Converters; +using CryptoExchange.Net.Objects; using System; using System.Collections.Generic; +using System.Globalization; using System.Net.Http; using System.Security.Cryptography; using System.Text; @@ -35,43 +37,41 @@ namespace CryptoExchange.Net.Authentication } /// - /// Authenticate a request where the parameters need to be in the Uri + /// Authenticate a request. Output parameters should include the providedParameters input /// /// The Api client sending the request /// The uri for the request /// The method of the request - /// The request parameters - /// The request headers + /// The request parameters /// If the requests should be authenticated /// Array serialization type - /// - public abstract void AuthenticateUriRequest( + /// The position where the providedParameters should go + /// Parameters that need to be in the Uri of the request. Should include the provided parameters if they should go in the uri + /// Parameters that need to be in the body of the request. Should include the provided parameters if they should go in the body + /// The headers that should be send with the request + public abstract void AuthenticateRequest( RestApiClient apiClient, Uri uri, HttpMethod method, - SortedDictionary parameters, - Dictionary headers, + Dictionary providedParameters, bool auth, - ArrayParametersSerialization arraySerialization); + ArrayParametersSerialization arraySerialization, + HttpMethodParameterPosition parameterPosition, + out SortedDictionary uriParameters, + out SortedDictionary bodyParameters, + out Dictionary headers + ); /// - /// Authenticate a request where the parameters need to be in the request body + /// SHA256 sign the data and return the bytes /// - /// The Api client sending the request - /// The uri for the request - /// The method of the request - /// The request parameters - /// The request headers - /// If the requests should be authenticated - /// Array serialization type - public abstract void AuthenticateBodyRequest( - RestApiClient apiClient, - Uri uri, - HttpMethod method, - SortedDictionary parameters, - Dictionary headers, - bool auth, - ArrayParametersSerialization arraySerialization); + /// + /// + protected static byte[] SignSHA256Bytes(string data) + { + using var encryptor = SHA256.Create(); + return encryptor.ComputeHash(Encoding.UTF8.GetBytes(data)); + } /// /// SHA256 sign the data and return the hash @@ -158,9 +158,18 @@ namespace CryptoExchange.Net.Authentication /// String type /// protected string SignHMACSHA512(string data, SignOutputType? outputType = null) + => SignHMACSHA512(Encoding.UTF8.GetBytes(data), outputType); + + /// + /// HMACSHA512 sign the data and return the hash + /// + /// Data to sign + /// String type + /// + protected string SignHMACSHA512(byte[] data, SignOutputType? outputType = null) { using var encryptor = new HMACSHA512(_sBytes); - var resultBytes = encryptor.ComputeHash(Encoding.UTF8.GetBytes(data)); + var resultBytes = encryptor.ComputeHash(data); return outputType == SignOutputType.Base64 ? BytesToBase64String(resultBytes) : BytesToHexString(resultBytes); } @@ -206,5 +215,25 @@ namespace CryptoExchange.Net.Authentication { return Convert.ToBase64String(buff); } + + /// + /// Get current timestamp including the time sync offset from the api client + /// + /// + /// + protected static DateTime GetTimestamp(RestApiClient apiClient) + { + return DateTime.UtcNow.Add(apiClient?.GetTimeOffset() ?? TimeSpan.Zero)!; + } + + /// + /// Get millisecond timestamp as a string including the time sync offset from the api client + /// + /// + /// + protected static string GetMillisecondTimestamp(RestApiClient apiClient) + { + return DateTimeConverter.ConvertToMilliseconds(GetTimestamp(apiClient)).Value.ToString(CultureInfo.InvariantCulture); + } } } diff --git a/CryptoExchange.Net/Clients/BaseRestClient.cs b/CryptoExchange.Net/Clients/BaseRestClient.cs index f7c6efd..1d21c3a 100644 --- a/CryptoExchange.Net/Clients/BaseRestClient.cs +++ b/CryptoExchange.Net/Clients/BaseRestClient.cs @@ -119,10 +119,13 @@ namespace CryptoExchange.Net { var requestId = NextId(); - var syncTimeResult = await apiClient.SyncTimeAsync().ConfigureAwait(false); - if (!syncTimeResult) - return syncTimeResult.As(default); - + if (signed) + { + var syncTimeResult = await apiClient.SyncTimeAsync().ConfigureAwait(false); + if (!syncTimeResult) + return syncTimeResult.As(default); + } + log.Write(LogLevel.Debug, $"[{requestId}] Creating request for " + uri); if (signed && apiClient.AuthenticationProvider == null) { @@ -285,33 +288,40 @@ namespace CryptoExchange.Net int requestId, Dictionary? additionalHeaders) { - SortedDictionary sortedParameters = new SortedDictionary(GetParameterComparer()); - if (parameters != null) - sortedParameters = new SortedDictionary(parameters, GetParameterComparer()); - + parameters ??= new Dictionary(); if (parameterPosition == HttpMethodParameterPosition.InUri) { - foreach (var parameter in sortedParameters) + foreach (var parameter in parameters) uri = uri.AddQueryParmeter(parameter.Key, parameter.Value.ToString()); } - var length = sortedParameters.Count; var headers = new Dictionary(); + var uriParameters = parameterPosition == HttpMethodParameterPosition.InUri ? new SortedDictionary(parameters) : new SortedDictionary(); + var bodyParameters = parameterPosition == HttpMethodParameterPosition.InBody ? new SortedDictionary(parameters) : new SortedDictionary(); if (apiClient.AuthenticationProvider != null) + apiClient.AuthenticationProvider.AuthenticateRequest( + apiClient, + uri, + method, + parameters, + signed, + arraySerialization, + parameterPosition, + out uriParameters, + out bodyParameters, + out headers); + + // Sanity check + foreach(var param in parameters) { - if(parameterPosition == HttpMethodParameterPosition.InUri) - apiClient.AuthenticationProvider.AuthenticateUriRequest(apiClient, uri, method, sortedParameters, headers, signed, arraySerialization); - else - apiClient.AuthenticationProvider.AuthenticateBodyRequest(apiClient, uri, method, sortedParameters, headers, signed, arraySerialization); - } - - if (parameterPosition == HttpMethodParameterPosition.InUri) - { - // Add the auth parameters to the uri, start with a new URI to be able to sort the parameters including the auth parameters - if (sortedParameters.Count != length) - uri = uri.SetParameters(sortedParameters); + if (!uriParameters.ContainsKey(param.Key) && !bodyParameters.ContainsKey(param.Key)) + throw new Exception($"Missing parameter {param.Key} after authentication processing. AuthenticationProvider implementation " + + $"should return provided parameters in either the uri or body parameters output"); } + // Add the auth parameters to the uri, start with a new URI to be able to sort the parameters including the auth parameters + uri = uri.SetParameters(uriParameters); + var request = RequestFactory.Create(method, uri, requestId); request.Accept = Constants.JsonContentHeader; @@ -335,8 +345,8 @@ namespace CryptoExchange.Net if (parameterPosition == HttpMethodParameterPosition.InBody) { var contentType = requestBodyFormat == RequestBodyFormat.Json ? Constants.JsonContentHeader : Constants.FormContentHeader; - if (sortedParameters?.Any() == true) - WriteParamBody(request, sortedParameters, contentType); + if (bodyParameters.Any()) + WriteParamBody(request, bodyParameters, contentType); else request.SetContent(requestBodyEmptyContent, contentType); } @@ -386,8 +396,6 @@ namespace CryptoExchange.Net //return request; } - protected virtual IComparer GetParameterComparer() => null; - /// /// Writes the parameters of the request to the request object body /// diff --git a/CryptoExchange.Net/Clients/RestApiClient.cs b/CryptoExchange.Net/Clients/RestApiClient.cs index 1247425..00ef774 100644 --- a/CryptoExchange.Net/Clients/RestApiClient.cs +++ b/CryptoExchange.Net/Clients/RestApiClient.cs @@ -14,8 +14,16 @@ namespace CryptoExchange.Net /// public abstract class RestApiClient: BaseApiClient { - protected abstract TimeSyncModel GetTimeSyncParameters(); - protected abstract void UpdateTimeOffset(TimeSpan offset); + /// + /// Get time sync info for an API client + /// + /// + protected abstract TimeSyncInfo GetTimeSyncInfo(); + + /// + /// Get time offset for an API client + /// + /// public abstract TimeSpan GetTimeOffset(); /// @@ -33,8 +41,6 @@ namespace CryptoExchange.Net /// internal IEnumerable RateLimiters { get; } - private Log _log; - /// /// ctor /// @@ -58,12 +64,12 @@ namespace CryptoExchange.Net internal async Task> SyncTimeAsync() { - var timeSyncParams = GetTimeSyncParameters(); - if (await timeSyncParams.Semaphore.WaitAsync(0).ConfigureAwait(false)) + var timeSyncParams = GetTimeSyncInfo(); + if (await timeSyncParams.TimeSyncState.Semaphore.WaitAsync(0).ConfigureAwait(false)) { - if (!timeSyncParams.SyncTime || (DateTime.UtcNow - timeSyncParams.LastSyncTime < TimeSpan.FromHours(1))) + if (!timeSyncParams.SyncTime || (DateTime.UtcNow - timeSyncParams.TimeSyncState.LastSyncTime < TimeSpan.FromHours(1))) { - timeSyncParams.Semaphore.Release(); + timeSyncParams.TimeSyncState.Semaphore.Release(); return new WebCallResult(null, null, true, null); } @@ -71,7 +77,7 @@ namespace CryptoExchange.Net var result = await GetServerTimestampAsync().ConfigureAwait(false); if (!result) { - timeSyncParams.Semaphore.Release(); + timeSyncParams.TimeSyncState.Semaphore.Release(); return result.As(false); } @@ -82,7 +88,7 @@ namespace CryptoExchange.Net result = await GetServerTimestampAsync().ConfigureAwait(false); if (!result) { - timeSyncParams.Semaphore.Release(); + timeSyncParams.TimeSyncState.Semaphore.Release(); return result.As(false); } } @@ -92,13 +98,13 @@ namespace CryptoExchange.Net if (offset.TotalMilliseconds >= 0 && offset.TotalMilliseconds < 500) { // Small offset, probably mainly due to ping. Don't adjust time - UpdateTimeOffset(offset); - timeSyncParams.Semaphore.Release(); + timeSyncParams.UpdateTimeOffset(offset); + timeSyncParams.TimeSyncState.Semaphore.Release(); } else { - UpdateTimeOffset(offset); - timeSyncParams.Semaphore.Release(); + timeSyncParams.UpdateTimeOffset(offset); + timeSyncParams.TimeSyncState.Semaphore.Release(); } } diff --git a/CryptoExchange.Net/CryptoExchange.Net.csproj b/CryptoExchange.Net/CryptoExchange.Net.csproj index a587352..b5fca32 100644 --- a/CryptoExchange.Net/CryptoExchange.Net.csproj +++ b/CryptoExchange.Net/CryptoExchange.Net.csproj @@ -1,4 +1,4 @@ - + netstandard2.0;netstandard2.1 @@ -41,7 +41,7 @@ all runtime; build; native; contentfiles; analyzers; buildtransitive - + all runtime; build; native; contentfiles; analyzers; buildtransitive diff --git a/CryptoExchange.Net/ExtensionMethods.cs b/CryptoExchange.Net/ExtensionMethods.cs index 1cf4ab0..de4a859 100644 --- a/CryptoExchange.Net/ExtensionMethods.cs +++ b/CryptoExchange.Net/ExtensionMethods.cs @@ -151,7 +151,7 @@ namespace CryptoExchange.Net public static string ToFormData(this SortedDictionary parameters) { var formData = HttpUtility.ParseQueryString(string.Empty); - foreach (var kvp in parameters.OrderBy(p => p.Key)) + foreach (var kvp in parameters) { if (kvp.Value.GetType().IsArray) { @@ -433,6 +433,25 @@ namespace CryptoExchange.Net return uriBuilder.Uri; } + /// + /// Create a new uri with the provided parameters as query + /// + /// + /// + /// + public static Uri SetParameters(this Uri baseUri, IOrderedEnumerable> parameters) + { + var uriBuilder = new UriBuilder(); + uriBuilder.Scheme = baseUri.Scheme; + uriBuilder.Host = baseUri.Host; + uriBuilder.Path = baseUri.AbsolutePath; + var httpValueCollection = HttpUtility.ParseQueryString(string.Empty); + foreach (var parameter in parameters) + httpValueCollection.Add(parameter.Key, parameter.Value.ToString()); + uriBuilder.Query = httpValueCollection.ToString(); + return uriBuilder.Uri; + } + /// /// Add parameter to URI diff --git a/CryptoExchange.Net/Objects/TimeSyncModel.cs b/CryptoExchange.Net/Objects/TimeSyncModel.cs deleted file mode 100644 index 6bc1c64..0000000 --- a/CryptoExchange.Net/Objects/TimeSyncModel.cs +++ /dev/null @@ -1,21 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Text; -using System.Threading; - -namespace CryptoExchange.Net.Objects -{ - public class TimeSyncModel - { - public bool SyncTime { get; set; } - public SemaphoreSlim Semaphore { get; set; } - public DateTime LastSyncTime { get; set; } - - public TimeSyncModel(bool syncTime, SemaphoreSlim semaphore, DateTime lastSyncTime) - { - SyncTime = syncTime; - Semaphore = semaphore; - LastSyncTime = lastSyncTime; - } - } -} diff --git a/CryptoExchange.Net/Objects/TimeSyncState.cs b/CryptoExchange.Net/Objects/TimeSyncState.cs new file mode 100644 index 0000000..81f453b --- /dev/null +++ b/CryptoExchange.Net/Objects/TimeSyncState.cs @@ -0,0 +1,80 @@ +using System; +using System.Threading; +using CryptoExchange.Net.Logging; +using Microsoft.Extensions.Logging; + +namespace CryptoExchange.Net.Objects +{ + /// + /// The time synchronization state of an API client + /// + public class TimeSyncState + { + /// + /// Semaphore to use for checking the time syncing. Should be shared instance among the API client + /// + public SemaphoreSlim Semaphore { get; } + /// + /// Last sync time for the API client + /// + public DateTime LastSyncTime { get; set; } + /// + /// Time offset for the API client + /// + public TimeSpan TimeOffset { get; set; } + + /// + /// ctor + /// + public TimeSyncState() + { + Semaphore = new SemaphoreSlim(1, 1); + } + } + + /// + /// Time synchronization info + /// + public class TimeSyncInfo + { + /// + /// Logger + /// + public Log Log { get; } + /// + /// Should synchronize time + /// + public bool SyncTime { get; } + /// + /// Time sync state for the API client + /// + public TimeSyncState TimeSyncState { get; } + + /// + /// ctor + /// + /// + /// + /// + public TimeSyncInfo(Log log, bool syncTime, TimeSyncState syncState) + { + Log = log; + SyncTime = syncTime; + TimeSyncState = syncState; + } + + /// + /// Set the time offset + /// + /// + public void UpdateTimeOffset(TimeSpan offset) + { + TimeSyncState.LastSyncTime = DateTime.UtcNow; + if (offset.TotalMilliseconds > 0 && offset.TotalMilliseconds < 500) + return; + + Log.Write(LogLevel.Information, $"Time offset set to {Math.Round(offset.TotalMilliseconds)}ms"); + TimeSyncState.TimeOffset = offset; + } + } +}