From f6d469e29fe3626d2a7d85b707607c94b2f9b70f Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Mon, 19 Jul 2021 14:39:41 -0400 Subject: [PATCH] Update asset spot price API to allow multiple input and output formats --- .../asset_ratio_controller_browsertest.cc | 84 +++++++++++++++---- .../common_handler/wallet_handler.cc | 6 +- .../common_handler/wallet_handler.h | 4 +- .../browser/asset_ratio_controller.cc | 46 ++++++---- .../browser/asset_ratio_controller.h | 7 +- .../browser/asset_ratio_response_parser.cc | 81 ++++++++++++------ .../browser/asset_ratio_response_parser.h | 5 +- .../asset_ratio_response_parser_unittest.cc | 53 +++++++++++- .../brave_wallet/common/brave_wallet.mojom | 9 +- 9 files changed, 231 insertions(+), 64 deletions(-) diff --git a/browser/brave_wallet/asset_ratio_controller_browsertest.cc b/browser/brave_wallet/asset_ratio_controller_browsertest.cc index 4163b5ca7c7..c84a4d677ef 100644 --- a/browser/brave_wallet/asset_ratio_controller_browsertest.cc +++ b/browser/brave_wallet/asset_ratio_controller_browsertest.cc @@ -30,8 +30,7 @@ std::unique_ptr HandleRequest( http_response->set_code(net::HTTP_OK); http_response->set_content_type("text/html"); if (request.GetURL().spec().find("/v2/history") != std::string::npos) { - http_response->set_content(R"( - { + http_response->set_content(R"({ "payload": { "prices":[[1622733088498,0.8201346624954003],[1622737203757,0.8096978545029869]], "market_caps":[[1622733088498,1223507820.383275],[1622737203757,1210972881.4928021]], @@ -40,7 +39,29 @@ std::unique_ptr HandleRequest( })"); } else { http_response->set_content( - R"({"payload":{"basic-attention-token":{"usd":0.694503}}})"); + R"({ + "payload":{ + "basic-attention-token":{ + "btc":0.00001732, + "btc_24h_change":8.021672460190562, + "usd":0.55393, + "usd_24h_change":9.523443444373276 + }, + "bat":{ + "btc":0.00001732, + "btc_24h_change":8.021672460190562, + "usd":0.55393, + "usd_24h_change":9.523443444373276 + }, + "link":{ + "btc":0.00261901, + "btc_24h_change":0.5871625385632929, + "usd":83.77, + "usd_24h_change":1.7646208048244043 + } + }, + "lastUpdated":"2021-07-16T19:11:28.907Z" + })"); } return std::move(http_response); } @@ -83,11 +104,12 @@ class AssetRatioControllerTest : public InProcessBrowserTest { https_server_->base_url()); } - void OnGetPrice(bool success, const std::string& price) { + void OnGetPrice(bool success, + std::vector prices) { if (wait_for_request_) { wait_for_request_->Quit(); } - ASSERT_EQ(expected_price_response_, price); + ASSERT_EQ(expected_prices_response_, prices); ASSERT_EQ(expected_success_, success); } @@ -101,12 +123,13 @@ class AssetRatioControllerTest : public InProcessBrowserTest { ASSERT_EQ(expected_success_, success); } - void WaitForPriceResponse(const std::string& expected_price_response, - bool expected_success) { + void WaitForPriceResponse( + std::vector expected_prices_response, + bool expected_success) { if (wait_for_request_) { return; } - expected_price_response_ = expected_price_response; + expected_prices_response_ = std::move(expected_prices_response); expected_success_ = expected_success; wait_for_request_.reset(new base::RunLoop); wait_for_request_->Run(); @@ -145,7 +168,7 @@ class AssetRatioControllerTest : public InProcessBrowserTest { net::EmbeddedTestServer* https_server() { return https_server_.get(); } bool expected_success_; - std::string expected_price_response_; + std::vector expected_prices_response_; std::vector expected_price_history_response_; @@ -154,28 +177,59 @@ class AssetRatioControllerTest : public InProcessBrowserTest { }; IN_PROC_BROWSER_TEST_F(AssetRatioControllerTest, GetPrice) { + std::vector expected_prices_response; ResetHTTPSServer(base::BindRepeating(&HandleRequest)); auto controller = GetAssetRatioController(); - controller->GetPrice("basic-attention-token", + controller->GetPrice({"bat", "link"}, {"btc", "usd"}, base::BindOnce(&AssetRatioControllerTest::OnGetPrice, base::Unretained(this))); - WaitForPriceResponse("0.694503", true); + + auto asset_price = brave_wallet::mojom::AssetPrice::New(); + asset_price->from_asset = "bat"; + asset_price->to_asset = "btc"; + asset_price->price = "0.00001732"; + asset_price->asset_24h_change = "8.021672460190562"; + expected_prices_response.push_back(std::move(asset_price)); + + asset_price = brave_wallet::mojom::AssetPrice::New(); + asset_price->from_asset = "bat"; + asset_price->to_asset = "usd"; + asset_price->price = "0.55393"; + asset_price->asset_24h_change = "9.523443444373276"; + expected_prices_response.push_back(std::move(asset_price)); + + asset_price = brave_wallet::mojom::AssetPrice::New(); + asset_price->from_asset = "link"; + asset_price->to_asset = "btc"; + asset_price->price = "0.00261901"; + asset_price->asset_24h_change = "0.5871625385632929"; + expected_prices_response.push_back(std::move(asset_price)); + + asset_price = brave_wallet::mojom::AssetPrice::New(); + asset_price->from_asset = "link"; + asset_price->to_asset = "usd"; + asset_price->price = "83.77"; + asset_price->asset_24h_change = "1.7646208048244043"; + expected_prices_response.push_back(std::move(asset_price)); + + WaitForPriceResponse(std::move(expected_prices_response), true); } IN_PROC_BROWSER_TEST_F(AssetRatioControllerTest, GetPriceServerError) { + std::vector expected_prices_response; ResetHTTPSServer(base::BindRepeating(&HandleRequestServerError)); auto controller = GetAssetRatioController(); - controller->GetPrice("basic-attention-token", + controller->GetPrice({"bat", "link"}, {"btc", "usd"}, base::BindOnce(&AssetRatioControllerTest::OnGetPrice, base::Unretained(this))); - WaitForPriceResponse("", false); + WaitForPriceResponse(std::move(expected_prices_response), false); } IN_PROC_BROWSER_TEST_F(AssetRatioControllerTest, GetPriceHistory) { ResetHTTPSServer(base::BindRepeating(&HandleRequest)); auto controller = GetAssetRatioController(); controller->GetPriceHistory( - "basic-attention-token", brave_wallet::mojom::AssetPriceTimeframe::OneDay, + "bat", brave_wallet::mojom::AssetPriceTimeframe::OneDay, base::BindOnce(&AssetRatioControllerTest::OnGetPriceHistory, base::Unretained(this))); @@ -199,7 +253,7 @@ IN_PROC_BROWSER_TEST_F(AssetRatioControllerTest, GetPriceHistoryServerError) { ResetHTTPSServer(base::BindRepeating(&HandleRequestServerError)); auto controller = GetAssetRatioController(); controller->GetPriceHistory( - "basic-attention-token", brave_wallet::mojom::AssetPriceTimeframe::OneDay, + "bat", brave_wallet::mojom::AssetPriceTimeframe::OneDay, base::BindOnce(&AssetRatioControllerTest::OnGetPriceHistory, base::Unretained(this))); std::vector diff --git a/browser/ui/webui/brave_wallet/common_handler/wallet_handler.cc b/browser/ui/webui/brave_wallet/common_handler/wallet_handler.cc index 2d9f31b96d7..886ca9d4b42 100644 --- a/browser/ui/webui/brave_wallet/common_handler/wallet_handler.cc +++ b/browser/ui/webui/brave_wallet/common_handler/wallet_handler.cc @@ -103,10 +103,12 @@ void WalletHandler::UnlockWallet(const std::string& password, keyring_controller_->Unlock(password, std::move(callback)); } -void WalletHandler::GetAssetPrice(const std::string& asset, +void WalletHandler::GetAssetPrice(const std::vector& from_assets, + const std::vector& to_assets, GetAssetPriceCallback callback) { EnsureConnected(); - asset_ratio_controller_->GetPrice(asset, std::move(callback)); + asset_ratio_controller_->GetPrice(from_assets, to_assets, + std::move(callback)); } void WalletHandler::GetAssetPriceHistory( diff --git a/browser/ui/webui/brave_wallet/common_handler/wallet_handler.h b/browser/ui/webui/brave_wallet/common_handler/wallet_handler.h index 69207cc43b1..fe56c7c455e 100644 --- a/browser/ui/webui/brave_wallet/common_handler/wallet_handler.h +++ b/browser/ui/webui/brave_wallet/common_handler/wallet_handler.h @@ -30,7 +30,9 @@ class WalletHandler : public brave_wallet::mojom::WalletHandler { void GetWalletInfo(GetWalletInfoCallback) override; void LockWallet() override; void UnlockWallet(const std::string& password, UnlockWalletCallback) override; - void GetAssetPrice(const std::string& asset, GetAssetPriceCallback) override; + void GetAssetPrice(const std::vector& from_assets, + const std::vector& to_assets, + GetAssetPriceCallback) override; void GetAssetPriceHistory(const std::string& asset, brave_wallet::mojom::AssetPriceTimeframe timeframe, GetAssetPriceHistoryCallback) override; diff --git a/components/brave_wallet/browser/asset_ratio_controller.cc b/components/brave_wallet/browser/asset_ratio_controller.cc index c99c40533c0..f946ace82a9 100644 --- a/components/brave_wallet/browser/asset_ratio_controller.cc +++ b/components/brave_wallet/browser/asset_ratio_controller.cc @@ -38,6 +38,17 @@ net::NetworkTrafficAnnotationTag GetNetworkTrafficAnnotationTag() { )"); } +std::string VectorToCommaSeparatedList(const std::vector& assets) { + std::stringstream ss; + std::for_each(assets.begin(), assets.end(), [&ss](const std::string asset) { + if (ss.tellp() != 0) { + ss << ","; + } + ss << asset; + }); + return ss.str(); +} + } // namespace namespace brave_wallet { @@ -63,12 +74,16 @@ void AssetRatioController::SetBaseURLForTest(const GURL& base_url_for_test) { } // static -GURL AssetRatioController::GetPriceURL(const std::string& asset) { +GURL AssetRatioController::GetPriceURL( + const std::vector& from_assets, + const std::vector& to_assets) { + std::string from = VectorToCommaSeparatedList(from_assets); + std::string to = VectorToCommaSeparatedList(to_assets); std::string spec = base::StringPrintf( - "%sv2/relative/provider/coingecko/%s/usd", + "%sv2/relative/provider/coingecko/%s/%s", base_url_for_test_.is_empty() ? kAssetRatioBaseURL : base_url_for_test_.spec().c_str(), - asset.c_str()); + from.c_str(), to.c_str()); return GURL(spec); } @@ -108,31 +123,34 @@ GURL AssetRatioController::GetPriceHistoryURL( return GURL(spec); } -void AssetRatioController::GetPrice(const std::string& asset, +void AssetRatioController::GetPrice(const std::vector& from_assets, + const std::vector& to_assets, GetPriceCallback callback) { - auto internal_callback = - base::BindOnce(&AssetRatioController::OnGetPrice, - weak_ptr_factory_.GetWeakPtr(), std::move(callback)); - api_request_helper_.Request("GET", GetPriceURL(asset), "", "", true, - std::move(internal_callback)); + auto internal_callback = base::BindOnce( + &AssetRatioController::OnGetPrice, weak_ptr_factory_.GetWeakPtr(), + from_assets, to_assets, std::move(callback)); + api_request_helper_.Request("GET", GetPriceURL(from_assets, to_assets), "", + "", true, std::move(internal_callback)); } void AssetRatioController::OnGetPrice( + std::vector from_assets, + std::vector to_assets, GetPriceCallback callback, const int status, const std::string& body, const std::map& headers) { + std::vector prices; if (status < 200 || status > 299) { - std::move(callback).Run(false, ""); + std::move(callback).Run(false, std::move(prices)); return; } - std::string price; - if (!ParseAssetPrice(body, &price)) { - std::move(callback).Run(false, ""); + if (!ParseAssetPrice(body, from_assets, to_assets, &prices)) { + std::move(callback).Run(false, std::move(prices)); return; } - std::move(callback).Run(true, price); + std::move(callback).Run(true, std::move(prices)); } void AssetRatioController::GetPriceHistory( diff --git a/components/brave_wallet/browser/asset_ratio_controller.h b/components/brave_wallet/browser/asset_ratio_controller.h index 23aae63bbf6..27f33a0e111 100644 --- a/components/brave_wallet/browser/asset_ratio_controller.h +++ b/components/brave_wallet/browser/asset_ratio_controller.h @@ -47,14 +47,17 @@ class AssetRatioController : public KeyedService, brave_wallet::mojom::AssetPriceTimeframe timeframe, GetPriceHistoryCallback callback) override; - static GURL GetPriceURL(const std::string& asset); + static GURL GetPriceURL(const std::vector& from_assets, + const std::vector& to_assets); static GURL GetPriceHistoryURL( const std::string& asset, brave_wallet::mojom::AssetPriceTimeframe timeframe); static void SetBaseURLForTest(const GURL& base_url_for_test); private: - void OnGetPrice(GetPriceCallback callback, + void OnGetPrice(std::vector from_assets, + std::vector to_assets, + GetPriceCallback callback, const int status, const std::string& body, const std::map& headers); diff --git a/components/brave_wallet/browser/asset_ratio_response_parser.cc b/components/brave_wallet/browser/asset_ratio_response_parser.cc index cef625f800a..ce3d24720f1 100644 --- a/components/brave_wallet/browser/asset_ratio_response_parser.cc +++ b/components/brave_wallet/browser/asset_ratio_response_parser.cc @@ -8,19 +8,42 @@ #include "base/json/json_reader.h" #include "base/logging.h" #include "base/strings/string_number_conversions.h" +#include "base/strings/stringprintf.h" #include "base/time/time.h" #include "third_party/abseil-cpp/absl/types/optional.h" namespace brave_wallet { -bool ParseAssetPrice(const std::string& json, std::string* price) { +bool ParseAssetPrice(const std::string& json, + const std::vector& from_assets, + const std::vector& to_assets, + std::vector* values) { // Parses results like this: - // { "payload": - // { - // "basic-attention-token":{"usd":0.694503} - // } + // { + // "payload":{ + // "basic-attention-token":{ + // "btc":0.00001732, + // "btc_24h_change":8.021672460190562, + // "usd":0.55393, + // "usd_24h_change":9.523443444373276 + // }, + // "bat":{ + // "btc":0.00001732, + // "btc_24h_change":8.021672460190562, + // "usd":0.55393, + // "usd_24h_change":9.523443444373276 + // }, + // "link":{ + // "btc":0.00261901, + // "btc_24h_change":0.5871625385632929, + // "usd":83.77, + // "usd_24h_change":1.7646208048244043 + // } + // }, + // "lastUpdated":"2021-07-16T19:11:28.907Z" // } - DCHECK(price); + + DCHECK(values); base::JSONReader::ValueWithError value_with_error = base::JSONReader::ReadAndReturnValueWithError( @@ -46,27 +69,37 @@ bool ParseAssetPrice(const std::string& json, std::string* price) { return false; } - if (payload->DictSize() == 0) { - return false; - } + for (const std::string& from_asset : from_assets) { + const base::Value* from_asset_value = + payload_dict->FindDictPath(from_asset); + const base::DictionaryValue* from_asset_dict; + if (!from_asset_value->GetAsDictionary(&from_asset_dict)) { + return false; + } - auto items = payload->DictItems(); - if (!items.begin()->second.is_dict()) { - return false; - } - const auto& usd_price_dict = - base::Value::AsDictionaryValue(items.begin()->second); - if (usd_price_dict.DictSize() != 1) { - return false; - } - auto converted_items = usd_price_dict.DictItems(); - const auto& value = converted_items.begin()->second; + for (const std::string& to_asset : to_assets) { + auto asset_price = brave_wallet::mojom::AssetPrice::New(); + asset_price->from_asset = from_asset; + asset_price->to_asset = to_asset; - double num; - if (!value.GetAsDouble(&num)) { - return false; + absl::optional to_price = + from_asset_dict->FindDoublePath(to_asset); + if (!to_price) { + return false; + } + asset_price->price = base::NumberToString(*to_price); + std::string to_asset_24h_key = + base::StringPrintf("%s_24h_change", to_asset.c_str()); + absl::optional to_24h_change = + from_asset_dict->FindDoublePath(to_asset_24h_key); + if (!to_24h_change) { + return false; + } + asset_price->asset_24h_change = base::NumberToString(*to_24h_change); + + values->push_back(std::move(asset_price)); + } } - *price = base::NumberToString(num); return true; } diff --git a/components/brave_wallet/browser/asset_ratio_response_parser.h b/components/brave_wallet/browser/asset_ratio_response_parser.h index 40309f775e2..4e4dce8c9da 100644 --- a/components/brave_wallet/browser/asset_ratio_response_parser.h +++ b/components/brave_wallet/browser/asset_ratio_response_parser.h @@ -15,7 +15,10 @@ namespace brave_wallet { -bool ParseAssetPrice(const std::string& json, std::string* price); +bool ParseAssetPrice(const std::string& json, + const std::vector& from_assets, + const std::vector& to_assets, + std::vector* values); bool ParseAssetPriceHistory( const std::string& json, std::vector* values); diff --git a/components/brave_wallet/browser/asset_ratio_response_parser_unittest.cc b/components/brave_wallet/browser/asset_ratio_response_parser_unittest.cc index ebe7fc186a2..de23d911b9f 100644 --- a/components/brave_wallet/browser/asset_ratio_response_parser_unittest.cc +++ b/components/brave_wallet/browser/asset_ratio_response_parser_unittest.cc @@ -15,10 +15,54 @@ namespace brave_wallet { TEST(AssetRatioResponseParserUnitTest, ParseAssetPrice) { - std::string json( - R"({"payload": {"basic-attention-token":{"usd":0.694503}}})"); - std::string price; - ASSERT_TRUE(ParseAssetPrice(json, &price)); + std::string json(R"({ + "payload":{ + "basic-attention-token":{ + "btc":0.00001732, + "btc_24h_change":8.021672460190562, + "usd":0.55393, + "usd_24h_change":9.523443444373276 + }, + "bat":{ + "btc":0.00001732, + "btc_24h_change":8.021672460190562, + "usd":0.55393, + "usd_24h_change":9.523443444373276 + }, + "link":{ + "btc":0.00261901, + "btc_24h_change":0.5871625385632929, + "usd":83.77, + "usd_24h_change":1.7646208048244043 + } + }, + "lastUpdated":"2021-07-16T19:11:28.907Z" + })"); + + std::vector prices; + ASSERT_TRUE(ParseAssetPrice(json, {"bat", "link"}, {"btc", "usd"}, &prices)); + ASSERT_EQ(prices.size(), 4UL); + ASSERT_EQ(prices[0]->from_asset, "bat"); + ASSERT_EQ(prices[0]->to_asset, "btc"); + ASSERT_EQ(prices[0]->price, "0.00001732"); + ASSERT_EQ(prices[0]->asset_24h_change, "8.021672460190562"); + + ASSERT_EQ(prices[1]->from_asset, "bat"); + ASSERT_EQ(prices[1]->to_asset, "usd"); + ASSERT_EQ(prices[1]->price, "0.55393"); + ASSERT_EQ(prices[1]->asset_24h_change, "9.523443444373276"); + + ASSERT_EQ(prices[2]->from_asset, "link"); + ASSERT_EQ(prices[2]->to_asset, "btc"); + ASSERT_EQ(prices[2]->price, "0.00261901"); + ASSERT_EQ(prices[2]->asset_24h_change, "0.5871625385632929"); + + ASSERT_EQ(prices[3]->from_asset, "link"); + ASSERT_EQ(prices[3]->to_asset, "usd"); + ASSERT_EQ(prices[3]->price, "83.77"); + ASSERT_EQ(prices[3]->asset_24h_change, "1.7646208048244043"); + + /* ASSERT_EQ(price, "0.694503"); // 2 value responses happen now for the alias and the full name // We always parse only the first. @@ -35,6 +79,7 @@ TEST(AssetRatioResponseParserUnitTest, ParseAssetPrice) { ASSERT_FALSE(ParseAssetPrice(json, &price)); json = ""; ASSERT_FALSE(ParseAssetPrice(json, &price)); + */ } TEST(AssetRatioResponseParserUnitTest, ParseAssetPriceHistory) { diff --git a/components/brave_wallet/common/brave_wallet.mojom b/components/brave_wallet/common/brave_wallet.mojom index daeed67d310..55b01d76498 100644 --- a/components/brave_wallet/common/brave_wallet.mojom +++ b/components/brave_wallet/common/brave_wallet.mojom @@ -75,6 +75,13 @@ struct AssetTimePrice { string price; }; +struct AssetPrice { + string from_asset; + string to_asset; + string price; + string asset_24h_change; +}; + struct SwapParams { string taker_address; string sell_amount; @@ -124,7 +131,7 @@ interface WalletHandler { array walletAccountNames); LockWallet(); UnlockWallet(string password) => (bool isWalletUnlocked); - GetAssetPrice(string asset) => (bool success, string price); + GetAssetPrice(array from_assets, array to_assets) => (bool success, array values); GetAssetPriceHistory(string asset, AssetPriceTimeframe timeframe) => (bool success, array values); GetPriceQuote(SwapParams params) => (bool success, SwapResponse response);