chore(wallet): refactor swap service interface (#21406)

android/java/apk_for_test.flags

@@ -10,7 +10,7 @@
 
 # Do not obfuscate the above classes as we check on fields in tests
 -keep class org.chromium.brave_wallet.mojom.BlockchainToken { *; }
--keep class org.chromium.brave_wallet.mojom.SwapParams { *; }
+-keep class org.chromium.brave_wallet.mojom.SwapQuoteParams { *; }
 -keep class org.chromium.brave_wallet.mojom.AccountInfo { *; }
 -keep class org.chromium.brave_wallet.mojom.TxData { *; }
 -keep class org.chromium.brave_wallet.mojom.GasEstimation1559 { *; }

android/javatests/org/chromium/chrome/browser/brave_wallet/BraveWalletUtilsTest.java

@@ -16,12 +16,14 @@
 import org.junit.runner.RunWith;
 
 import org.chromium.base.test.util.Batch;
+import org.chromium.brave_wallet.mojom.AccountId;
 import org.chromium.brave_wallet.mojom.BlockchainToken;
 import org.chromium.brave_wallet.mojom.BraveWalletConstants;
 import org.chromium.brave_wallet.mojom.CoinType;
 import org.chromium.brave_wallet.mojom.GasEstimation1559;
 import org.chromium.brave_wallet.mojom.NetworkInfo;
-import org.chromium.brave_wallet.mojom.SwapParams;
+import org.chromium.brave_wallet.mojom.RoutePriority;
+import org.chromium.brave_wallet.mojom.SwapQuoteParams;
 import org.chromium.brave_wallet.mojom.TxData;
 import org.chromium.brave_wallet.mojom.TxData1559;
 import org.chromium.chrome.browser.crypto_wallet.util.Utils;
@@ -266,30 +268,34 @@ public void validateBlockchainTokenTest() {
 
     @Test
     @SmallTest
-    public void validateSwapParamsTest() {
-        SwapParams testStruct = new SwapParams();
+    public void validateSwapQuoteParamsTest() {
+        SwapQuoteParams testStruct = new SwapQuoteParams();
         java.lang.reflect.Field[] fields = testStruct.getClass().getDeclaredFields();
         for (java.lang.reflect.Field f : fields) {
             try {
                 java.lang.Class t = f.getType();
                 java.lang.Object v = f.get(testStruct);
                 if (!t.isPrimitive()) {
                     String varName = f.getName();
-                    if (varName.equals("takerAddress")
-                            || varName.equals("sellAmount")
-                            || varName.equals("buyAmount")
-                            || varName.equals("buyToken")
-                            || varName.equals("sellToken")
-                            || varName.equals("gasPrice")
-                            || varName.equals("chainId")) {
+                    if (varName.equals("fromAccountId")
+                            || varName.equals("fromChainId")
+                            || varName.equals("fromToken")
+                            || varName.equals("fromAmount")
+                            || varName.equals("toAccountId")
+                            || varName.equals("toChainId")
+                            || varName.equals("toToken")
+                            || varName.equals("toAmount")
+                            || varName.equals("slippagePercentage")
+                            || varName.equals("routePriority")) {
                         continue;
                     }
                     if (v == null) {
                         String message =
                                 "Check that "
                                         + varName
                                         + " is initialized everywhere\n"
-                                        + "in Java files, where SwapParams object is created. It\n"
+                                        + "in Java files, where SwapQuoteParams object is created."
+                                        + " It\n"
                                         + "could be safely added to the above if to skip that var"
                                         + " on checks\n"
                                         + "after that.";
@@ -301,22 +307,27 @@ public void validateSwapParamsTest() {
                 // interested in public members of a mojom structure
             }
         }
-        testStruct.takerAddress = "";
-        testStruct.sellAmount = "";
-        testStruct.buyAmount = "";
-        testStruct.buyToken = "";
-        testStruct.sellToken = "";
-        testStruct.gasPrice = "";
-        testStruct.chainId = "";
+        testStruct.fromAccountId = new AccountId();
+        testStruct.fromChainId = "";
+        testStruct.fromToken = "";
+        testStruct.fromAmount = "";
+        testStruct.toAccountId = new AccountId();
+        testStruct.toChainId = "";
+        testStruct.toToken = "";
+        testStruct.toAmount = "";
+        testStruct.slippagePercentage = "";
+        testStruct.routePriority = RoutePriority.RECOMMENDED;
+
         try {
             java.nio.ByteBuffer byteBuffer = testStruct.serialize();
-            SwapParams testStructDeserialized = SwapParams.deserialize(byteBuffer);
+            SwapQuoteParams testStructDeserialized = SwapQuoteParams.deserialize(byteBuffer);
         } catch (Exception exc) {
-            String message = "Check that a variable with a type in the exception below is\n"
-                    + "initialized everywhere in Java files, where SwapParams object is\n"
-                    + "created('git grep \"new SwapParams\"' inside src/brave).\n"
-                    + "Initialisation of it could be safely added to the test to pass it,\n"
-                    + "but only after all places where it's created are fixed.\n";
+            String message =
+                    "Check that a variable with a type in the exception below is\n"
+                        + "initialized everywhere in Java files, where SwapQuoteParams object is\n"
+                        + "created('git grep \"new SwapQuoteParams\"' inside src/brave).\n"
+                        + "Initialisation of it could be safely added to the test to pass it,\n"
+                        + "but only after all places where it's created are fixed.\n";
             fail(message + "\n" + getStackTrace(exc));
         }
     }

components/brave_wallet/browser/swap_request_helper.cc

@@ -19,17 +19,16 @@
 namespace brave_wallet {
 
 std::optional<std::string> EncodeJupiterTransactionParams(
-    mojom::JupiterSwapParamsPtr params,
+    const mojom::JupiterTransactionParams& params,
     bool has_fee) {
-  DCHECK(params);
   base::Value::Dict tx_params;
 
   // The code below does the following two things:
   //   - compute the ATA address that should be used to receive fees
   //   - verify if output_mint is a valid address
   std::optional<std::string> associated_token_account =
       SolanaKeyring::GetAssociatedTokenAccount(
-          params->output_mint, brave_wallet::kSolanaFeeRecipient);
+          params.output_mint, brave_wallet::kSolanaFeeRecipient);
   if (!associated_token_account) {
     return std::nullopt;
   }
@@ -43,20 +42,20 @@ std::optional<std::string> EncodeJupiterTransactionParams(
     tx_params.Set("feeAccount", *associated_token_account);
   }
 
-  tx_params.Set("userPublicKey", params->user_public_key);
+  tx_params.Set("userPublicKey", params.user_public_key);
 
   base::Value::Dict route;
-  route.Set("inAmount", base::NumberToString(params->route->in_amount));
-  route.Set("outAmount", base::NumberToString(params->route->out_amount));
-  route.Set("amount", base::NumberToString(params->route->amount));
+  route.Set("inAmount", base::NumberToString(params.route->in_amount));
+  route.Set("outAmount", base::NumberToString(params.route->out_amount));
+  route.Set("amount", base::NumberToString(params.route->amount));
   route.Set("otherAmountThreshold",
-            base::NumberToString(params->route->other_amount_threshold));
-  route.Set("swapMode", params->route->swap_mode);
-  route.Set("priceImpactPct", params->route->price_impact_pct);
-  route.Set("slippageBps", base::NumberToString(params->route->slippage_bps));
+            base::NumberToString(params.route->other_amount_threshold));
+  route.Set("swapMode", params.route->swap_mode);
+  route.Set("priceImpactPct", params.route->price_impact_pct);
+  route.Set("slippageBps", base::NumberToString(params.route->slippage_bps));
 
   base::Value::List market_infos_value;
-  for (const auto& market_info : params->route->market_infos) {
+  for (const auto& market_info : params.route->market_infos) {
     base::Value::Dict market_info_value;
     market_info_value.Set("id", market_info->id);
     market_info_value.Set("label", market_info->label);
@@ -104,7 +103,7 @@ std::optional<std::string> EncodeJupiterTransactionParams(
   result = std::string(json::convert_string_value_to_uint64(
       "/route/slippageBps", result, false));
 
-  for (int i = 0; i < static_cast<int>(params->route->market_infos.size());
+  for (int i = 0; i < static_cast<int>(params.route->market_infos.size());
        i++) {
     result = std::string(json::convert_string_value_to_uint64(
         base::StringPrintf("/route/marketInfos/%d/inAmount", i), result,

components/brave_wallet/browser/swap_request_helper.h

@@ -14,7 +14,7 @@
 namespace brave_wallet {
 
 std::optional<std::string> EncodeJupiterTransactionParams(
-    mojom::JupiterSwapParamsPtr params,
+    const mojom::JupiterTransactionParams& params,
     bool has_fee);
 
 }  // namespace brave_wallet

components/brave_wallet/browser/swap_request_helper_unittest.cc

@@ -64,14 +64,15 @@ const char* GetJupiterQuoteTemplate() {
 TEST(SwapRequestHelperUnitTest, EncodeJupiterTransactionParams) {
   auto* json_template = GetJupiterQuoteTemplate();
   std::string json = base::StringPrintf(json_template, "10000", "30");
-  mojom::JupiterQuotePtr swap_quote = ParseJupiterQuote(ParseJson(json));
+  mojom::JupiterQuotePtr swap_quote =
+      ParseJupiterQuoteResponse(ParseJson(json));
   ASSERT_TRUE(swap_quote);
 
-  mojom::JupiterSwapParams params;
+  mojom::JupiterTransactionParams params;
   params.route = swap_quote->routes.at(0).Clone();
   params.user_public_key = "mockPubKey";
   params.output_mint = "EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v";  // USDC
-  auto encoded_params = EncodeJupiterTransactionParams(params.Clone(), true);
+  auto encoded_params = EncodeJupiterTransactionParams(params, true);
 
   std::string expected_params(R"(
     {
@@ -117,7 +118,7 @@ TEST(SwapRequestHelperUnitTest, EncodeJupiterTransactionParams) {
 
   // OK: Jupiter transaction params WITHOUT feeAccount
   params.output_mint = "SHDWyBxihqiCj6YekG2GUr7wqKLeLAMK1gHZck9pL6y";  // SHDW
-  encoded_params = EncodeJupiterTransactionParams(params.Clone(), false);
+  encoded_params = EncodeJupiterTransactionParams(params, false);
   expected_params = R"(
     {
       "route": {
@@ -157,15 +158,11 @@ TEST(SwapRequestHelperUnitTest, EncodeJupiterTransactionParams) {
   ASSERT_NE(encoded_params, std::nullopt);
   ASSERT_EQ(*encoded_params, GetJSON(expected_params_value));
 
-  // KO: empty params
-  EXPECT_DCHECK_DEATH(EncodeJupiterTransactionParams(nullptr, true));
-  EXPECT_DCHECK_DEATH(EncodeJupiterTransactionParams(nullptr, false));
-
   // KO: invalid output mint
   params.output_mint = "invalid output mint";
-  encoded_params = EncodeJupiterTransactionParams(params.Clone(), true);
+  encoded_params = EncodeJupiterTransactionParams(params, true);
   ASSERT_EQ(encoded_params, std::nullopt);
-  encoded_params = EncodeJupiterTransactionParams(params.Clone(), false);
+  encoded_params = EncodeJupiterTransactionParams(params, false);
   ASSERT_EQ(encoded_params, std::nullopt);
 }
 }  // namespace brave_wallet

components/brave_wallet/browser/swap_response_parser.cc

@@ -74,8 +74,8 @@ mojom::ZeroExFeePtr ParseZeroExFee(const base::Value& value) {
 
 }  // namespace
 
-mojom::SwapResponsePtr ParseSwapResponse(const base::Value& json_value,
-                                         bool expect_transaction_data) {
+mojom::ZeroExQuotePtr ParseZeroExQuoteResponse(const base::Value& json_value,
+                                               bool expect_transaction_data) {
   // {
   //   "price":"1916.27547998814058355",
   //   "guaranteedPrice":"1935.438234788021989386",
@@ -126,7 +126,7 @@ mojom::SwapResponsePtr ParseSwapResponse(const base::Value& json_value,
     return nullptr;
   }
 
-  auto swap_response = mojom::SwapResponse::New();
+  auto swap_response = mojom::ZeroExQuote::New();
   swap_response->price = swap_response_value->price;
 
   if (expect_transaction_data) {
@@ -180,8 +180,7 @@ mojom::SwapResponsePtr ParseSwapResponse(const base::Value& json_value,
   return swap_response;
 }
 
-mojom::SwapErrorResponsePtr ParseSwapErrorResponse(
-    const base::Value& json_value) {
+mojom::ZeroExErrorPtr ParseZeroExErrorResponse(const base::Value& json_value) {
   // https://github.com/0xProject/0x-monorepo/blob/development/packages/json-schemas/schemas/relayer_api_error_response_schema.json
   //
   // {
@@ -211,13 +210,13 @@ mojom::SwapErrorResponsePtr ParseSwapErrorResponse(
     return nullptr;
   }
 
-  auto result = mojom::SwapErrorResponse::New();
+  auto result = mojom::ZeroExError::New();
   result->code = swap_error_response_value->code;
   result->reason = swap_error_response_value->reason;
 
   if (swap_error_response_value->validation_errors) {
     for (auto& error_item : *swap_error_response_value->validation_errors) {
-      result->validation_errors.emplace_back(mojom::SwapErrorResponseItem::New(
+      result->validation_errors.emplace_back(mojom::ZeroExErrorItem::New(
           error_item.field, error_item.code, error_item.reason));
     }
   }
@@ -233,7 +232,8 @@ mojom::SwapErrorResponsePtr ParseSwapErrorResponse(
   return result;
 }
 
-mojom::JupiterQuotePtr ParseJupiterQuote(const base::Value& json_value) {
+mojom::JupiterQuotePtr ParseJupiterQuoteResponse(
+    const base::Value& json_value) {
   //    {
   //      "data": [
   //        {
@@ -369,27 +369,25 @@ mojom::JupiterQuotePtr ParseJupiterQuote(const base::Value& json_value) {
   return swap_quote;
 }
 
-mojom::JupiterSwapTransactionsPtr ParseJupiterSwapTransactions(
+std::optional<std::string> ParseJupiterTransactionResponse(
     const base::Value& json_value) {
   auto value = swap_responses::JupiterSwapTransactions::FromValue(json_value);
   if (!value) {
-    return nullptr;
+    return std::nullopt;
   }
 
-  auto result = mojom::JupiterSwapTransactions::New();
-  result->swap_transaction = value->swap_transaction;
-  return result;
+  return value->swap_transaction;
 }
 
-mojom::JupiterErrorResponsePtr ParseJupiterErrorResponse(
+mojom::JupiterErrorPtr ParseJupiterErrorResponse(
     const base::Value& json_value) {
   auto jupiter_error_response_value =
       swap_responses::JupiterErrorResponse::FromValue(json_value);
   if (!jupiter_error_response_value) {
     return nullptr;
   }
 
-  auto result = mojom::JupiterErrorResponse::New();
+  auto result = mojom::JupiterError::New();
   result->status_code = jupiter_error_response_value->status_code;
   result->error = jupiter_error_response_value->error;
   result->message = jupiter_error_response_value->message;

components/brave_wallet/browser/swap_response_parser.h

@@ -13,16 +13,14 @@
 
 namespace brave_wallet {
 
-mojom::SwapResponsePtr ParseSwapResponse(const base::Value& json_value,
-                                         bool expect_transaction_data);
-mojom::SwapErrorResponsePtr ParseSwapErrorResponse(
-    const base::Value& json_value);
+mojom::ZeroExQuotePtr ParseZeroExQuoteResponse(const base::Value& json_value,
+                                               bool expect_transaction_data);
+mojom::ZeroExErrorPtr ParseZeroExErrorResponse(const base::Value& json_value);
 
-mojom::JupiterQuotePtr ParseJupiterQuote(const base::Value& json_value);
-mojom::JupiterSwapTransactionsPtr ParseJupiterSwapTransactions(
-    const base::Value& json_value);
-mojom::JupiterErrorResponsePtr ParseJupiterErrorResponse(
+mojom::JupiterQuotePtr ParseJupiterQuoteResponse(const base::Value& json_value);
+std::optional<std::string> ParseJupiterTransactionResponse(
     const base::Value& json_value);
+mojom::JupiterErrorPtr ParseJupiterErrorResponse(const base::Value& json_value);
 std::optional<std::string> ConvertAllNumbersToString(const std::string& json);
 }  // namespace brave_wallet