-
-
Notifications
You must be signed in to change notification settings - Fork 373
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.
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
chore: Adding better errors for coinbase order errors #826
Merged
Merged
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,10 +29,10 @@ public sealed partial class ExchangeCoinbaseAPI : ExchangeAPI | |
public override string BaseUrl { get; set; } = "https://api.coinbase.com/api/v3/brokerage"; | ||
private readonly string BaseUrlV2 = "https://api.coinbase.com/v2"; // For Wallet Support | ||
public override string BaseUrlWebSocket { get; set; } = "wss://advanced-trade-ws.coinbase.com"; | ||
|
||
private enum PaginationType { None, V2, V3} | ||
private PaginationType pagination = PaginationType.None; | ||
private string cursorNext; | ||
private string cursorNext; | ||
|
||
private Dictionary<string, string> Accounts = null; // Cached Account IDs | ||
|
||
|
@@ -62,7 +62,7 @@ private void ProcessResponse(IAPIRequestMaker maker, RequestMakerState state, ob | |
JToken token = JsonConvert.DeserializeObject<JToken>((string)response); | ||
if (token == null) return; | ||
switch(pagination) | ||
{ | ||
{ | ||
case PaginationType.V2: cursorNext = token["pagination"]?["next_starting_after"]?.ToStringInvariant(); break; | ||
case PaginationType.V3: cursorNext = token[CURSOR]?.ToStringInvariant(); break; | ||
} | ||
|
@@ -77,7 +77,7 @@ private void ProcessResponse(IAPIRequestMaker maker, RequestMakerState state, ob | |
/// <param name="payload"></param> | ||
/// <returns></returns> | ||
protected override bool CanMakeAuthenticatedRequest(IReadOnlyDictionary<string, object> payload) | ||
{ | ||
{ | ||
return (PrivateApiKey != null && PublicApiKey != null); | ||
} | ||
|
||
|
@@ -90,7 +90,7 @@ protected override async Task ProcessRequestAsync(IHttpWebRequest request, Dicti | |
|
||
// V2 wants PathAndQuery, V3 wants LocalPath for the sig (I guess they wanted to shave a nano-second or two - silly) | ||
string path = request.RequestUri.AbsoluteUri.StartsWith(BaseUrlV2) ? request.RequestUri.PathAndQuery : request.RequestUri.LocalPath; | ||
string signature = CryptoUtility.SHA256Sign(timestamp + request.Method.ToUpperInvariant() + path + body, PrivateApiKey.ToUnsecureString()); | ||
string signature = CryptoUtility.SHA256Sign(timestamp + request.Method.ToUpperInvariant() + path + body, PrivateApiKey.ToUnsecureString()); | ||
|
||
request.AddHeader("CB-ACCESS-KEY", PublicApiKey.ToUnsecureString()); | ||
request.AddHeader("CB-ACCESS-SIGN", signature); | ||
|
@@ -141,7 +141,7 @@ protected internal override async Task<IEnumerable<ExchangeMarket>> OnGetMarketS | |
|
||
protected override async Task<IEnumerable<string>> OnGetMarketSymbolsAsync() | ||
{ | ||
return (await GetMarketSymbolsMetadataAsync()).Select(market => market.MarketSymbol); | ||
return (await GetMarketSymbolsMetadataAsync()).Select(market => market.MarketSymbol); | ||
} | ||
|
||
protected override async Task<IReadOnlyDictionary<string, ExchangeCurrency>> OnGetCurrenciesAsync() | ||
|
@@ -176,7 +176,7 @@ protected override async Task<IReadOnlyDictionary<string, ExchangeCurrency>> OnG | |
currencies[currency.Name] = currency; | ||
} | ||
} | ||
return currencies; | ||
return currencies; | ||
} | ||
|
||
protected override async Task<IEnumerable<KeyValuePair<string, ExchangeTicker>>> OnGetTickersAsync() | ||
|
@@ -187,7 +187,7 @@ protected override async Task<IEnumerable<KeyValuePair<string, ExchangeTicker>>> | |
foreach (JToken book in books[PRICEBOOKS]) | ||
{ | ||
var split = book[PRODUCTID].ToString().Split(GlobalMarketSymbolSeparator); | ||
// This endpoint does not provide a last or open for the ExchangeTicker | ||
// This endpoint does not provide a last or open for the ExchangeTicker | ||
tickers.Add(new KeyValuePair<string, ExchangeTicker>(book[PRODUCTID].ToString(), new ExchangeTicker() | ||
{ | ||
MarketSymbol = book[PRODUCTID].ToString(), | ||
|
@@ -224,7 +224,7 @@ protected override async Task<ExchangeTicker> OnGetTickerAsync(string marketSymb | |
QuoteCurrencyVolume = book[ASKS][0][SIZE].ConvertInvariant<decimal>(), | ||
Timestamp = DateTime.UtcNow | ||
} | ||
}; | ||
}; | ||
} | ||
|
||
protected override async Task<ExchangeOrderBook> OnGetOrderBookAsync(string marketSymbol, int maxCount = 50) | ||
|
@@ -267,8 +267,8 @@ protected override async Task<IEnumerable<MarketCandle>> OnGetCandlesAsync(strin | |
if ((RangeEnd - RangeStart).TotalSeconds / periodSeconds > 300) RangeStart = RangeEnd.AddSeconds(-(periodSeconds * 300)); | ||
|
||
List<MarketCandle> candles = new List<MarketCandle>(); | ||
while (true) | ||
{ | ||
while (true) | ||
{ | ||
JToken token = await MakeJsonRequestAsync<JToken>(string.Format("/products/{0}/candles?start={1}&end={2}&granularity={3}", marketSymbol, ((DateTimeOffset)RangeStart).ToUnixTimeSeconds(), ((DateTimeOffset)RangeEnd).ToUnixTimeSeconds(), granularity)); | ||
foreach (JToken candle in token["candles"]) candles.Add(this.ParseCandle(candle, marketSymbol, periodSeconds, "open", "high", "low", "close", "start", TimestampType.UnixSeconds, "volume")); | ||
if (RangeStart > startDate) | ||
|
@@ -278,7 +278,7 @@ protected override async Task<IEnumerable<MarketCandle>> OnGetCandlesAsync(strin | |
RangeEnd = RangeEnd.AddSeconds(-(periodSeconds * 300)); | ||
} | ||
else break; | ||
} | ||
} | ||
return candles.Where(c => c.Timestamp >= startDate).OrderBy(c => c.Timestamp); | ||
} | ||
|
||
|
@@ -301,7 +301,7 @@ protected override async Task<Dictionary<string, decimal>> OnGetFeesAsync() | |
|
||
#region AccountSpecificEndpoints | ||
|
||
// WARNING: Currently V3 doesn't support Coinbase Wallet APIs, so we are reverting to V2 for this call. | ||
// WARNING: Currently V3 doesn't support Coinbase Wallet APIs, so we are reverting to V2 for this call. | ||
protected override async Task<ExchangeDepositDetails> OnGetDepositAddressAsync(string symbol, bool forceRegenerate = false) | ||
{ | ||
if (Accounts == null) await GetAmounts(true); // Populate Accounts Cache | ||
|
@@ -323,13 +323,13 @@ protected override async Task<Dictionary<string, decimal>> OnGetAmountsAvailable | |
return await GetAmounts(true); | ||
} | ||
|
||
// WARNING: Currently V3 doesn't support Coinbase Wallet APIs, so we are reverting to V2 for this call. | ||
// WARNING: Currently V3 doesn't support Coinbase Wallet APIs, so we are reverting to V2 for this call. | ||
protected override async Task<IEnumerable<ExchangeTransaction>> OnGetWithdrawHistoryAsync(string currency) | ||
{ | ||
return await GetTx(true, currency); | ||
} | ||
|
||
// WARNING: Currently V3 doesn't support Coinbase Wallet APIs, so we are reverting to V2 for this call. | ||
// WARNING: Currently V3 doesn't support Coinbase Wallet APIs, so we are reverting to V2 for this call. | ||
protected override async Task<IEnumerable<ExchangeTransaction>> OnGetDepositHistoryAsync(string currency) | ||
{ | ||
return await GetTx(false, currency); | ||
|
@@ -344,7 +344,7 @@ protected override async Task<IEnumerable<ExchangeOrderResult>> OnGetOpenOrderDe | |
string uri = string.IsNullOrEmpty(marketSymbol) ? "/orders/historical/batch?order_status=OPEN" : $"/orders/historical/batch?product_id={marketSymbol}&order_status=OPEN"; // Parameter order is critical | ||
JToken token = await MakeJsonRequestAsync<JToken>(uri); | ||
while(true) | ||
{ | ||
{ | ||
foreach (JToken order in token[ORDERS]) if (order[TYPE].ToStringInvariant().Equals(ADVFILL)) orders.Add(ParseOrder(order)); | ||
if (string.IsNullOrEmpty(cursorNext)) break; | ||
token = await MakeJsonRequestAsync<JToken>(uri + "&cursor=" + cursorNext); | ||
|
@@ -360,7 +360,7 @@ protected override async Task<IEnumerable<ExchangeOrderResult>> OnGetCompletedOr | |
string uri = string.IsNullOrEmpty(marketSymbol) ? "/orders/historical/batch?order_status=FILLED" : $"/orders/historical/batch?product_id={marketSymbol}&order_status=OPEN"; // Parameter order is critical | ||
JToken token = await MakeJsonRequestAsync<JToken>(uri); | ||
while(true) | ||
{ | ||
{ | ||
foreach (JToken order in token[ORDERS]) orders.Add(ParseOrder(order)); | ||
if (string.IsNullOrEmpty(cursorNext)) break; | ||
token = await MakeJsonRequestAsync<JToken>(uri + "&cursor=" + cursorNext); | ||
|
@@ -396,24 +396,25 @@ protected override async Task<ExchangeOrderResult> OnPlaceOrderAsync(ExchangeOrd | |
payload["side"] = order.IsBuy ? BUY : "SELL"; | ||
|
||
Dictionary<string, object> orderConfig = new Dictionary<string, object>(); | ||
var amount = order.ShouldRoundAmount ? order.RoundAmount().ToStringInvariant() : order.Amount.ToStringInvariant(); | ||
switch (order.OrderType) | ||
{ | ||
case OrderType.Limit: | ||
if (order.ExtraParameters.ContainsKey("gtd_timestamp")) | ||
{ | ||
orderConfig.Add("limit_limit_gtd", new Dictionary<string, object>() | ||
{ | ||
{"base_size", order.Amount.ToStringInvariant() }, | ||
{"base_size", amount }, | ||
{"limit_price", order.Price.ToStringInvariant() }, | ||
{"end_time", order.ExtraParameters["gtd_timestamp"] }, | ||
{"end_time", order.ExtraParameters["gtd_timestamp"] }, | ||
{"post_only", order.ExtraParameters.TryGetValueOrDefault( "post_only", false) } | ||
}); | ||
} | ||
else | ||
{ | ||
{ | ||
orderConfig.Add("limit_limit_gtc", new Dictionary<string, object>() | ||
{ | ||
{"base_size", order.Amount.ToStringInvariant() }, | ||
{"base_size", amount }, | ||
{"limit_price", order.Price.ToStringInvariant() }, | ||
{"post_only", order.ExtraParameters.TryGetValueOrDefault( "post_only", "false") } | ||
}); | ||
|
@@ -424,7 +425,7 @@ protected override async Task<ExchangeOrderResult> OnPlaceOrderAsync(ExchangeOrd | |
{ | ||
orderConfig.Add("stop_limit_stop_limit_gtd", new Dictionary<string, object>() | ||
{ | ||
{"base_size", order.Amount.ToStringInvariant() }, | ||
{"base_size", amount }, | ||
{"limit_price", order.Price.ToStringInvariant() }, | ||
{"stop_price", order.StopPrice.ToStringInvariant() }, | ||
{"end_time", order.ExtraParameters["gtd_timestamp"] }, | ||
|
@@ -434,15 +435,15 @@ protected override async Task<ExchangeOrderResult> OnPlaceOrderAsync(ExchangeOrd | |
{ | ||
orderConfig.Add("stop_limit_stop_limit_gtc", new Dictionary<string, object>() | ||
{ | ||
{"base_size", order.Amount.ToStringInvariant() }, | ||
{"base_size", amount }, | ||
{"limit_price", order.Price.ToStringInvariant() }, | ||
{"stop_price", order.StopPrice.ToStringInvariant() }, | ||
}); | ||
} | ||
break; | ||
case OrderType.Market: | ||
if (order.IsBuy) orderConfig.Add("market_market_ioc", new Dictionary<string, object>() { { "quote_size", order.Amount.ToStringInvariant() }}); | ||
else orderConfig.Add("market_market_ioc", new Dictionary<string, object>() { { "base_size", order.Amount.ToStringInvariant() }}); | ||
if (order.IsBuy) orderConfig.Add("market_market_ioc", new Dictionary<string, object>() { { "quote_size", amount }}); | ||
else orderConfig.Add("market_market_ioc", new Dictionary<string, object>() { { "base_size", amount }}); | ||
break; | ||
} | ||
|
||
|
@@ -454,10 +455,22 @@ protected override async Task<ExchangeOrderResult> OnPlaceOrderAsync(ExchangeOrd | |
// The Post doesn't return with any status, just a new OrderId. To get the Order Details we have to reQuery. | ||
return await OnGetOrderDetailsAsync(result[ORDERID].ToStringInvariant()); | ||
} | ||
catch (Exception ex) // All fails come back with an exception. | ||
catch (Exception ex) // All fails come back with an exception. | ||
{ | ||
Logger.Error(ex, "Failed to place coinbase error"); | ||
jjxtra marked this conversation as resolved.
Show resolved
Hide resolved
|
||
var token = JToken.Parse(ex.Message); | ||
return new ExchangeOrderResult(){ Result = ExchangeAPIOrderResult.Rejected, ClientOrderId = order.ClientOrderId, ResultCode = token["error_response"]["error"].ToStringInvariant() }; | ||
return new ExchangeOrderResult(){ | ||
Result = ExchangeAPIOrderResult.Rejected, | ||
IsBuy = payload["side"].ToStringInvariant().Equals(BUY), | ||
MarketSymbol = payload["product_id"].ToStringInvariant(), | ||
ClientOrderId = order.ClientOrderId, | ||
ResultCode = $"{token["error_response"]["error"].ToStringInvariant()} - {token["error_response"]["preview_failure_reason"].ToStringInvariant()}", | ||
AmountFilled = 0, | ||
Amount = order.Amount, | ||
AveragePrice = 0, | ||
Fees = 0, | ||
FeesCurrency = "USDT" | ||
}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. include order details since the error handler uses |
||
} | ||
} | ||
|
||
|
@@ -509,8 +522,8 @@ protected override Task<IWebSocket> OnGetDeltaOrderBookWebSocketAsync(Action<Exc | |
if (askCount >= maxCount && bidCount >=maxCount) break; | ||
} | ||
callback?.Invoke(book); | ||
} | ||
return Task.CompletedTask; | ||
} | ||
return Task.CompletedTask; | ||
}, async (_socket) => | ||
{ | ||
string timestamp = DateTimeOffset.UtcNow.ToUnixTimeSeconds().ToStringInvariant(); | ||
|
@@ -551,7 +564,7 @@ protected override async Task<IWebSocket> OnGetTickersWebSocketAsync(Action<IRea | |
BaseCurrency = split[0], | ||
QuoteCurrency = split[1], | ||
BaseCurrencyVolume = token["volume_24_h"].ConvertInvariant<decimal>(), | ||
Timestamp = timestamp | ||
Timestamp = timestamp | ||
} | ||
} )); | ||
} | ||
|
@@ -630,7 +643,7 @@ private async Task<Dictionary<string, decimal>> GetAmounts(bool AvailableOnly) | |
} | ||
if (string.IsNullOrEmpty(cursorNext)) break; | ||
token = await MakeJsonRequestAsync<JToken>("/accounts?starting_after=" + cursorNext); | ||
} | ||
} | ||
pagination = PaginationType.None; | ||
return amounts; | ||
} | ||
|
@@ -643,12 +656,12 @@ private async Task<Dictionary<string, decimal>> GetAmounts(bool AvailableOnly) | |
/// <returns></returns> | ||
private async Task<List<ExchangeTransaction>> GetTx(bool Withdrawals, string currency) | ||
{ | ||
if (Accounts == null) await GetAmounts(true); | ||
if (Accounts == null) await GetAmounts(true); | ||
pagination = PaginationType.V2; | ||
List<ExchangeTransaction> transfers = new List<ExchangeTransaction>(); | ||
JToken tokens = await MakeJsonRequestAsync<JToken>($"accounts/{Accounts[currency]}/transactions", BaseUrlV2); | ||
while(true) | ||
{ | ||
{ | ||
foreach (JToken token in tokens) | ||
{ | ||
// A "send" to Coinbase is when someone "sent" you coin - or a receive to the rest of the world | ||
|
@@ -658,7 +671,7 @@ private async Task<List<ExchangeTransaction>> GetTx(bool Withdrawals, string cur | |
} | ||
if (string.IsNullOrEmpty(cursorNext)) break; | ||
tokens = await MakeJsonRequestAsync<JToken>($"accounts/{Accounts[currency]}/transactions?starting_after={cursorNext}", BaseUrlV2); | ||
} | ||
} | ||
pagination = PaginationType.None; | ||
return transfers; | ||
} | ||
|
@@ -672,17 +685,17 @@ private ExchangeTransaction ParseTransaction(JToken token) | |
{ | ||
// The Coin Address/TxFee isn't available but can be retrieved using the Network Hash/BlockChainId | ||
return new ExchangeTransaction() | ||
{ | ||
{ | ||
PaymentId = token["id"].ToStringInvariant(), // Not sure how this is used elsewhere but here it is the Coinbase TransactionID | ||
BlockchainTxId = token["network"]["hash"].ToStringInvariant(), | ||
Currency = token[AMOUNT][CURRENCY].ToStringInvariant(), | ||
Amount = token[AMOUNT][AMOUNT].ConvertInvariant<decimal>(), | ||
Timestamp = token["created_at"].ToObject<DateTime>(), | ||
Status = token[STATUS].ToStringInvariant() == "completed" ? TransactionStatus.Complete : TransactionStatus.Unknown, | ||
Notes = token["description"].ToStringInvariant() | ||
// Address | ||
// AddressTag | ||
// TxFee | ||
// Address | ||
// AddressTag | ||
// TxFee | ||
}; | ||
} | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We check the ShouldRound and then
RoundAmount
if neededThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check happens already in
order.RoundAmount
so i moved back to using the method instead of this variable check