Skip to content

Commit

Permalink
review(nuo-xu): reuse ZeroExQuote and drop ZeroExQuoteInfo struct
Browse files Browse the repository at this point in the history
  • Loading branch information
onyb committed Oct 30, 2024
1 parent 8b20e11 commit 1bbee57
Show file tree
Hide file tree
Showing 7 changed files with 114 additions and 85 deletions.
17 changes: 8 additions & 9 deletions components/brave_wallet/browser/swap_response_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,8 @@ mojom::ZeroExQuotePtr ParseQuote(

} // namespace

mojom::ZeroExQuoteInfoPtr ParseQuoteResponse(const base::Value& json_value,
const std::string& chain_id) {
mojom::ZeroExQuotePtr ParseQuoteResponse(const base::Value& json_value,
const std::string& chain_id) {
// {
// "blockNumber": "20114692",
// "buyAmount": "100037537",
Expand Down Expand Up @@ -217,21 +217,20 @@ mojom::ZeroExQuoteInfoPtr ParseQuoteResponse(const base::Value& json_value,
return nullptr;
}

auto swap_response = mojom::ZeroExQuoteInfo::New();
swap_response->allowance_target =
GetZeroExAllowanceHolderAddress(chain_id).value_or("");
auto swap_response = mojom::ZeroExQuote::New();

if (!swap_response_value->liquidity_available) {
swap_response->liquidity_available = false;
return swap_response;
}

if (auto quote = ParseQuote(swap_response_value.value())) {
swap_response->quote = std::move(quote);
} else {
swap_response = ParseQuote(swap_response_value.value());
if (!swap_response) {
return nullptr;
}

swap_response->liquidity_available = swap_response_value->liquidity_available;
swap_response->allowance_target =
GetZeroExAllowanceHolderAddress(chain_id).value_or("");

return swap_response;
}
Expand Down
4 changes: 2 additions & 2 deletions components/brave_wallet/browser/swap_response_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ namespace brave_wallet {

namespace zeroex {

mojom::ZeroExQuoteInfoPtr ParseQuoteResponse(const base::Value& json_value,
const std::string& chain_id);
mojom::ZeroExQuotePtr ParseQuoteResponse(const base::Value& json_value,
const std::string& chain_id);
mojom::ZeroExTransactionPtr ParseTransactionResponse(
const base::Value& json_value);
mojom::ZeroExErrorPtr ParseErrorResponse(const base::Value& json_value);
Expand Down
69 changes: 31 additions & 38 deletions components/brave_wallet/browser/swap_response_parser_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -153,40 +153,37 @@ TEST(SwapResponseParserUnitTest, ParseZeroExQuoteResponse) {
"zid": "0x111111111111111111111111"
}
)");
auto quote_info =
auto quote =
zeroex::ParseQuoteResponse(ParseJson(json), mojom::kMainnetChainId);
ASSERT_TRUE(quote_info);
ASSERT_TRUE(quote_info->quote);
ASSERT_TRUE(quote);

EXPECT_EQ(quote_info->quote->buy_amount, "100032748");
EXPECT_EQ(quote_info->quote->buy_token,
"0xdac17f958d2ee523a2206206994597c13d831ec7");
EXPECT_EQ(quote->buy_amount, "100032748");
EXPECT_EQ(quote->buy_token, "0xdac17f958d2ee523a2206206994597c13d831ec7");

ASSERT_TRUE(quote_info->quote->fees->zero_ex_fee);
EXPECT_EQ(quote_info->quote->fees->zero_ex_fee->amount, "0");
EXPECT_EQ(quote_info->quote->fees->zero_ex_fee->token, "0xdeadbeef");
EXPECT_EQ(quote_info->quote->fees->zero_ex_fee->type, "volume");
ASSERT_TRUE(quote->fees->zero_ex_fee);
EXPECT_EQ(quote->fees->zero_ex_fee->amount, "0");
EXPECT_EQ(quote->fees->zero_ex_fee->token, "0xdeadbeef");
EXPECT_EQ(quote->fees->zero_ex_fee->type, "volume");

EXPECT_EQ(quote_info->quote->gas, "288095");
EXPECT_EQ(quote_info->quote->gas_price, "7062490000");
EXPECT_EQ(quote_info->quote->liquidity_available, true);
EXPECT_EQ(quote_info->quote->min_buy_amount, "99032421");
EXPECT_EQ(quote->gas, "288095");
EXPECT_EQ(quote->gas_price, "7062490000");
EXPECT_EQ(quote->liquidity_available, true);
EXPECT_EQ(quote->min_buy_amount, "99032421");

ASSERT_EQ(quote_info->quote->route->fills.size(), 1UL);
EXPECT_EQ(quote_info->quote->route->fills.at(0)->from,
ASSERT_EQ(quote->route->fills.size(), 1UL);
EXPECT_EQ(quote->route->fills.at(0)->from,
"0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48");
EXPECT_EQ(quote_info->quote->route->fills.at(0)->to,
EXPECT_EQ(quote->route->fills.at(0)->to,
"0xdac17f958d2ee523a2206206994597c13d831ec7");
EXPECT_EQ(quote_info->quote->route->fills.at(0)->source, "SolidlyV3");
EXPECT_EQ(quote_info->quote->route->fills.at(0)->proportion_bps, "10000");
EXPECT_EQ(quote->route->fills.at(0)->source, "SolidlyV3");
EXPECT_EQ(quote->route->fills.at(0)->proportion_bps, "10000");

EXPECT_EQ(quote_info->quote->sell_amount, "100000000");
EXPECT_EQ(quote_info->quote->sell_token,
"0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48");
EXPECT_EQ(quote_info->quote->total_network_fee, "2034668056550000");
EXPECT_EQ(quote->sell_amount, "100000000");
EXPECT_EQ(quote->sell_token, "0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48");
EXPECT_EQ(quote->total_network_fee, "2034668056550000");

EXPECT_EQ(quote_info->liquidity_available, true);
EXPECT_EQ(quote_info->allowance_target,
EXPECT_EQ(quote->liquidity_available, true);
EXPECT_EQ(quote->allowance_target,
"0x0000000000001fF3684f28c67538d4D072C22734");

// Case 2: null zeroExFee
Expand Down Expand Up @@ -253,12 +250,11 @@ TEST(SwapResponseParserUnitTest, ParseZeroExQuoteResponse) {
"zid": "0x111111111111111111111111"
}
)";
quote_info =
zeroex::ParseQuoteResponse(ParseJson(json), mojom::kMainnetChainId);
ASSERT_TRUE(quote_info);
EXPECT_FALSE(quote_info->quote->fees->zero_ex_fee);
EXPECT_EQ(quote_info->liquidity_available, true);
EXPECT_EQ(quote_info->allowance_target,
quote = zeroex::ParseQuoteResponse(ParseJson(json), mojom::kMainnetChainId);
ASSERT_TRUE(quote);
EXPECT_FALSE(quote->fees->zero_ex_fee);
EXPECT_EQ(quote->liquidity_available, true);
EXPECT_EQ(quote->allowance_target,
"0x0000000000001fF3684f28c67538d4D072C22734");

// Case 3: malformed fees field
Expand Down Expand Up @@ -330,13 +326,10 @@ TEST(SwapResponseParserUnitTest, ParseZeroExQuoteResponse) {
"liquidityAvailable": false,
}
)";
quote_info =
zeroex::ParseQuoteResponse(ParseJson(json), mojom::kMainnetChainId);
ASSERT_TRUE(quote_info);
EXPECT_FALSE(quote_info->liquidity_available);
EXPECT_EQ(quote_info->allowance_target,
"0x0000000000001fF3684f28c67538d4D072C22734");
EXPECT_FALSE(quote_info->quote);
quote = zeroex::ParseQuoteResponse(ParseJson(json), mojom::kMainnetChainId);
ASSERT_TRUE(quote);
EXPECT_FALSE(quote->liquidity_available);
EXPECT_EQ(quote->buy_token, "");

// Case 5: other invalid cases
json = R"({"totalNetworkFee": "2034668056550000"})";
Expand Down
12 changes: 12 additions & 0 deletions components/brave_wallet/browser/swap_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,18 @@ void SwapService::OnGetZeroExQuote(const std::string& chain_id,

if (auto swap_response = zeroex::ParseQuoteResponse(
api_request_result.value_body(), chain_id)) {
if (!swap_response->liquidity_available) {
std::move(callback).Run(
nullptr, nullptr,
mojom::SwapErrorUnion::NewZeroExError(mojom::ZeroExError::New(
"INSUFFICIENT_LIQUIDITY",
l10n_util::GetStringUTF8(
IDS_BRAVE_WALLET_SWAP_INSUFFICIENT_LIQUIDITY),
true)),
"");
return;
}

std::move(callback).Run(
mojom::SwapQuoteUnion::NewZeroExQuote(std::move(swap_response)),
std::move(swap_fee), nullptr, "");
Expand Down
70 changes: 49 additions & 21 deletions components/brave_wallet/browser/swap_service_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -556,37 +556,36 @@ TEST_F(SwapServiceUnitTest, GetZeroExQuote) {
}
)");

auto zero_ex_quote = mojom::ZeroExQuote::New();
zero_ex_quote->buy_amount = "100032748";
zero_ex_quote->buy_token = "0xdac17f958d2ee523a2206206994597c13d831ec7";
zero_ex_quote->sell_amount = "100000000";
zero_ex_quote->sell_token = "0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48";
auto expected_zero_ex_quote = mojom::ZeroExQuote::New();
expected_zero_ex_quote->buy_amount = "100032748";
expected_zero_ex_quote->buy_token =
"0xdac17f958d2ee523a2206206994597c13d831ec7";
expected_zero_ex_quote->sell_amount = "100000000";
expected_zero_ex_quote->sell_token =
"0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48";

auto zero_ex_fee = mojom::ZeroExFee::New();
zero_ex_fee->type = "volume";
zero_ex_fee->token = "0xdeadbeef";
zero_ex_fee->amount = "0";
zero_ex_quote->fees = mojom::ZeroExFees::New();
zero_ex_quote->fees->zero_ex_fee = std::move(zero_ex_fee);
expected_zero_ex_quote->fees = mojom::ZeroExFees::New();
expected_zero_ex_quote->fees->zero_ex_fee = std::move(zero_ex_fee);

zero_ex_quote->gas = "288095";
zero_ex_quote->gas_price = "7062490000";
zero_ex_quote->liquidity_available = true;
zero_ex_quote->min_buy_amount = "99032421";
zero_ex_quote->total_network_fee = "2034668056550000";
expected_zero_ex_quote->gas = "288095";
expected_zero_ex_quote->gas_price = "7062490000";
expected_zero_ex_quote->liquidity_available = true;
expected_zero_ex_quote->min_buy_amount = "99032421";
expected_zero_ex_quote->total_network_fee = "2034668056550000";

auto fill = mojom::ZeroExRouteFill::New();
fill->from = "0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48";
fill->to = "0xdac17f958d2ee523a2206206994597c13d831ec7";
fill->source = "SolidlyV3";
fill->proportion_bps = "10000";
zero_ex_quote->route = mojom::ZeroExRoute::New();
zero_ex_quote->route->fills.push_back(fill.Clone());
expected_zero_ex_quote->route = mojom::ZeroExRoute::New();
expected_zero_ex_quote->route->fills.push_back(fill.Clone());

auto expected_zero_ex_quote_info = mojom::ZeroExQuoteInfo::New();
expected_zero_ex_quote_info->quote = std::move(zero_ex_quote);
expected_zero_ex_quote_info->liquidity_available = true;
expected_zero_ex_quote_info->allowance_target =
expected_zero_ex_quote->allowance_target =
"0x0000000000001fF3684f28c67538d4D072C22734";

auto expected_swap_fees = mojom::SwapFees::New();
Expand All @@ -598,7 +597,7 @@ TEST_F(SwapServiceUnitTest, GetZeroExQuote) {

base::MockCallback<mojom::SwapService::GetQuoteCallback> callback;
EXPECT_CALL(callback, Run(EqualsMojo(mojom::SwapQuoteUnion::NewZeroExQuote(
expected_zero_ex_quote_info.Clone())),
expected_zero_ex_quote.Clone())),
EqualsMojo(expected_swap_fees.Clone()),
EqualsMojo(mojom::SwapErrorUnionPtr()), ""));

Expand Down Expand Up @@ -676,9 +675,9 @@ TEST_F(SwapServiceUnitTest, GetZeroExQuote) {
}
)");

expected_zero_ex_quote_info->quote->fees->zero_ex_fee = nullptr;
expected_zero_ex_quote->fees->zero_ex_fee = nullptr;
EXPECT_CALL(callback, Run(EqualsMojo(mojom::SwapQuoteUnion::NewZeroExQuote(
std::move(expected_zero_ex_quote_info))),
std::move(expected_zero_ex_quote))),
EqualsMojo(expected_swap_fees.Clone()),
EqualsMojo(mojom::SwapErrorUnionPtr()), ""));

Expand All @@ -693,6 +692,7 @@ TEST_F(SwapServiceUnitTest, GetZeroExQuote) {
}

TEST_F(SwapServiceUnitTest, GetZeroExQuoteError) {
// Case 1: validation error
std::string error = R"(
{
"name": "INPUT_INVALID",
Expand All @@ -714,6 +714,34 @@ TEST_F(SwapServiceUnitTest, GetZeroExQuoteError) {
mojom::SwapProvider::kZeroEx),
callback.Get());
task_environment_.RunUntilIdle();
testing::Mock::VerifyAndClearExpectations(&callback);

// Case 2: insufficient liquidity
SetInterceptor(R"(
{
"liquidityAvailable": false,
"zid": "0x111111111111111111111111"
}
)");
auto error_response = mojom::ZeroExError::New();
error_response->name = "INSUFFICIENT_LIQUIDITY";
error_response->message =
l10n_util::GetStringUTF8(IDS_BRAVE_WALLET_SWAP_INSUFFICIENT_LIQUIDITY);
error_response->is_insufficient_liquidity = true;

EXPECT_CALL(callback, Run(EqualsMojo(mojom::SwapQuoteUnionPtr()),
EqualsMojo(mojom::SwapFeesPtr()),
EqualsMojo(mojom::SwapErrorUnion::NewZeroExError(
std::move(error_response))),
""));

swap_service_->GetQuote(
GetCannedSwapQuoteParams(
mojom::CoinType::ETH, mojom::kPolygonMainnetChainId, "DAI",
mojom::CoinType::ETH, mojom::kPolygonMainnetChainId, "ETH",
mojom::SwapProvider::kZeroEx),
callback.Get());
task_environment_.RunUntilIdle();
}

TEST_F(SwapServiceUnitTest, GetZeroExQuoteUnexpectedReturn) {
Expand Down
9 changes: 3 additions & 6 deletions components/brave_wallet/common/brave_wallet.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ union SwapTransactionParamsUnion {

union SwapQuoteUnion {
JupiterQuote jupiter_quote;
ZeroExQuoteInfo zero_ex_quote;
ZeroExQuote zero_ex_quote;
LiFiQuote lifi_quote;
SquidQuote squid_quote;
};
Expand Down Expand Up @@ -501,18 +501,15 @@ struct ZeroExQuote {
string sell_amount;
string sell_token;
string total_network_fee;
};

struct ZeroExQuoteInfo {
// quote is undefined if liquidity_available is false
ZeroExQuote? quote;
// Custom field
string allowance_target;
bool liquidity_available;
};

struct ZeroExError {
string name;
string message;
bool is_insufficient_liquidity;
};

union SwapErrorUnion {
Expand Down
18 changes: 9 additions & 9 deletions components/brave_wallet_ui/page/screens/swap/hooks/useSwap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -308,9 +308,9 @@ export const useSwap = () => {
})
}

if (quoteUnion.zeroExQuote?.quote) {
if (quoteUnion.zeroExQuote) {
return getZeroExQuoteOptions({
quote: quoteUnion.zeroExQuote.quote,
quote: quoteUnion.zeroExQuote,
fromNetwork,
fromToken,
toToken,
Expand Down Expand Up @@ -565,18 +565,18 @@ export const useSwap = () => {
}
}

if (quoteResponse.response.zeroExQuote?.quote) {
if (quoteResponse.response.zeroExQuote) {
if (params.editingFromOrToAmount === 'from') {
setToAmount(
getZeroExToAmount({
quote: quoteResponse.response.zeroExQuote.quote,
quote: quoteResponse.response.zeroExQuote,
toToken: params.toToken
}).format(6)
)
} else {
setFromAmount(
getZeroExFromAmount({
quote: quoteResponse.response.zeroExQuote.quote,
quote: quoteResponse.response.zeroExQuote,
fromToken: params.fromToken
}).format(6)
)
Expand Down Expand Up @@ -1007,11 +1007,11 @@ export const useSwap = () => {
}

// 0x specific validations
if (quoteUnion?.zeroExQuote?.liquidityAvailable === false) {
return 'insufficientLiquidity'
}

if (quoteErrorUnion?.zeroExError) {
if (quoteErrorUnion.zeroExError.isInsufficientLiquidity) {
return 'insufficientLiquidity'
}

return 'unknownError'
}

Expand Down

0 comments on commit 1bbee57

Please sign in to comment.