From d8671001de7c3c4ab62c4a10b4759a6cb9fee305 Mon Sep 17 00:00:00 2001 From: Anton Paymyshev Date: Fri, 22 Apr 2022 15:37:42 +0700 Subject: [PATCH] Review fixes --- .../browser/brave_wallet_utils.cc | 13 ++++-- .../brave_wallet/browser/json_rpc_service.cc | 19 ++++++-- .../brave_wallet/browser/json_rpc_service.h | 4 +- .../browser/json_rpc_service_unittest.cc | 43 +++++++++++++++++-- .../unstoppable_domains_multichain_calls.cc | 1 + .../unstoppable_domains_multichain_calls.h | 15 +++++-- .../brave_wallet_ui/common/hooks/send.ts | 4 +- 7 files changed, 84 insertions(+), 15 deletions(-) diff --git a/components/brave_wallet/browser/brave_wallet_utils.cc b/components/brave_wallet/browser/brave_wallet_utils.cc index 2ddbe75e89a0..81aec235b693 100644 --- a/components/brave_wallet/browser/brave_wallet_utils.cc +++ b/components/brave_wallet/browser/brave_wallet_utils.cc @@ -14,6 +14,7 @@ #include "base/environment.h" #include "base/feature_list.h" #include "base/logging.h" +#include "base/notreached.h" #include "base/strings/strcat.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_split.h" @@ -235,12 +236,12 @@ const base::flat_map kFilecoinSubdomains = { {brave_wallet::mojom::kFilecoinMainnet, "mainnet"}, {brave_wallet::mojom::kFilecoinTestnet, "testnet"}}; +// Addesses taken from https://docs.unstoppabledomains.com/developer-toolkit/ +// smart-contracts/uns-smart-contracts/#proxyreader const base::flat_map kUnstoppableDomainsProxyReaderContractAddressMap = { {brave_wallet::mojom::kMainnetChainId, - "0xa6E7cEf2EDDEA66352Fd68E5915b60BDbb7309f5"}, - {brave_wallet::mojom::kRinkebyChainId, - "0x3A2e74CF832cbA3d77E72708d55370119E4323a6"}, + "0xc3C2BAB5e3e52DBF311b2aAcEf2e40344f19494E"}, {brave_wallet::mojom::kPolygonMainnetChainId, "0xA3f32c8cd786dc089Bd1fC175F2707223aeE5d00"}}; @@ -913,7 +914,11 @@ GURL GetUnstoppableDomainsRpcUrl(const std::string& chain_id) { if (chain_id == brave_wallet::mojom::kPolygonMainnetChainId) return GURL("https://polygon-rpc.com"); - return GURL(GetInfuraURLForKnownChainId(chain_id)); + if (chain_id == brave_wallet::mojom::kMainnetChainId) + return GURL(GetInfuraURLForKnownChainId(chain_id)); + + NOTREACHED(); + return GURL(); } std::string GetUnstoppableDomainsProxyReaderContractAddress( diff --git a/components/brave_wallet/browser/json_rpc_service.cc b/components/brave_wallet/browser/json_rpc_service.cc index a514c4589e8d..da8955c66c1d 100644 --- a/components/brave_wallet/browser/json_rpc_service.cc +++ b/components/brave_wallet/browser/json_rpc_service.cc @@ -31,7 +31,6 @@ #include "brave/components/brave_wallet/common/brave_wallet_response_helpers.h" #include "brave/components/brave_wallet/common/eth_address.h" #include "brave/components/brave_wallet/common/eth_request_helper.h" -#include "brave/components/brave_wallet/common/features.h" #include "brave/components/brave_wallet/common/hex_utils.h" #include "brave/components/brave_wallet/common/value_conversion_utils.h" #include "brave/components/brave_wallet/common/web3_provider_constants.h" @@ -52,6 +51,13 @@ namespace { constexpr char kDomainPattern[] = "(?:[A-Za-z0-9][A-Za-z0-9-]*[A-Za-z0-9]\\.)+[A-Za-z]{2,}$"; +// Non empty group of symbols of a-z | 0-9 | hyphen(-). +// Then a dot. +// Then one of fixed suffixes(should match `supportedUDExtensions` array from +// send.ts). +constexpr char kUDPattern[] = + "(?:[a-z0-9-]+)\\.(?:crypto|x|coin|nft|dao|wallet|888|blockchain|bitcoin)"; + net::NetworkTrafficAnnotationTag GetNetworkTrafficAnnotationTag() { return net::DefineNetworkTrafficAnnotation("json_rpc_service", R"( semantics { @@ -1184,7 +1190,7 @@ void JsonRpcService::UnstoppableDomainsGetEthAddr( return; } - if (!IsValidDomain(domain)) { + if (!IsValidUnstoppableDomain(domain)) { std::move(callback).Run( "", mojom::ProviderError::kInvalidParams, l10n_util::GetStringUTF8(IDS_WALLET_INVALID_PARAMETERS)); @@ -1418,9 +1424,16 @@ void JsonRpcService::OnGetIsEip1559( mojom::ProviderError::kSuccess, ""); } +/*static*/ bool JsonRpcService::IsValidDomain(const std::string& domain) { static const base::NoDestructor kDomainRegex(kDomainPattern); - return re2::RE2::FullMatch(domain, kDomainPattern); + return re2::RE2::FullMatch(domain, *kDomainRegex); +} + +/*static*/ +bool JsonRpcService::IsValidUnstoppableDomain(const std::string& domain) { + static const base::NoDestructor kDomainRegex(kUDPattern); + return re2::RE2::FullMatch(domain, *kDomainRegex); } void JsonRpcService::GetERC721OwnerOf(const std::string& contract, diff --git a/components/brave_wallet/browser/json_rpc_service.h b/components/brave_wallet/browser/json_rpc_service.h index 050b01635cee..2b823dcfc56d 100644 --- a/components/brave_wallet/browser/json_rpc_service.h +++ b/components/brave_wallet/browser/json_rpc_service.h @@ -464,8 +464,10 @@ class JsonRpcService : public KeyedService, public mojom::JsonRpcService { bool success); FRIEND_TEST_ALL_PREFIXES(JsonRpcServiceUnitTest, IsValidDomain); + FRIEND_TEST_ALL_PREFIXES(JsonRpcServiceUnitTest, IsValidUnstoppableDomain); FRIEND_TEST_ALL_PREFIXES(JsonRpcServiceUnitTest, Reset); - bool IsValidDomain(const std::string& domain); + static bool IsValidDomain(const std::string& domain); + static bool IsValidUnstoppableDomain(const std::string& domain); void OnGetERC721OwnerOf( GetERC721OwnerOfCallback callback, diff --git a/components/brave_wallet/browser/json_rpc_service_unittest.cc b/components/brave_wallet/browser/json_rpc_service_unittest.cc index 2348546f4ebf..7a3bec4bd1af 100644 --- a/components/brave_wallet/browser/json_rpc_service_unittest.cc +++ b/components/brave_wallet/browser/json_rpc_service_unittest.cc @@ -1934,7 +1934,7 @@ TEST_F(UnstoppableDomainsUnitTest, GetEthAddr_InvalidDomain) { EXPECT_CALL(callback, Run("", mojom::ProviderError::kInvalidParams, l10n_util::GetStringUTF8(IDS_WALLET_INVALID_PARAMETERS))); - json_rpc_service_->UnstoppableDomainsGetEthAddr("brad.x", callback.Get()); + json_rpc_service_->UnstoppableDomainsGetEthAddr("brad.test", callback.Get()); base::RunLoop().RunUntilIdle(); } @@ -2151,14 +2151,51 @@ TEST_F(JsonRpcServiceUnitTest, IsValidDomain) { std::vector valid_domains = {"brave.eth", "test.brave.eth", "brave-test.test-dev.eth"}; for (const auto& domain : valid_domains) - EXPECT_TRUE(json_rpc_service_->IsValidDomain(domain)) + EXPECT_TRUE(JsonRpcService::IsValidDomain(domain)) << domain << " should be valid"; std::vector invalid_domains = { "", ".eth", "-brave.eth", "brave-.eth", "brave.e-th", "b.eth", "brave.e", "-brave.test.eth", "brave-.test.eth"}; for (const auto& domain : invalid_domains) - EXPECT_FALSE(json_rpc_service_->IsValidDomain(domain)) + EXPECT_FALSE(JsonRpcService::IsValidDomain(domain)) + << domain << " should be invalid"; +} + +TEST_F(JsonRpcServiceUnitTest, IsValidUnstoppableDomain) { + // clang-format off + std::vector valid_domains = { + "test.crypto", + "test.x", + "test.coin", + "test.nft", + "test.dao", + "test.wallet", + "test.888", + "test.blockchain", + "test.bitcoin", + "a.crypto", + "1.crypto", + "-.crypto", + }; + std::vector invalid_domains = { + "", + ".", + "crypto.", + "crypto.1", + ".crypto", + "crypto.brave", + "brave.crypto-", + "brave.test.crypto", + "brave.zil", + }; + // clang-format on + for (const auto& domain : valid_domains) + EXPECT_TRUE(JsonRpcService::IsValidUnstoppableDomain(domain)) + << domain << " should be valid"; + + for (const auto& domain : invalid_domains) + EXPECT_FALSE(JsonRpcService::IsValidUnstoppableDomain(domain)) << domain << " should be invalid"; } diff --git a/components/brave_wallet/browser/unstoppable_domains_multichain_calls.cc b/components/brave_wallet/browser/unstoppable_domains_multichain_calls.cc index d28cb3670d0f..23065d43523d 100644 --- a/components/brave_wallet/browser/unstoppable_domains_multichain_calls.cc +++ b/components/brave_wallet/browser/unstoppable_domains_multichain_calls.cc @@ -144,4 +144,5 @@ void UnstoppableDomainsMultichainCalls::SetError( } template class UnstoppableDomainsMultichainCalls; + } // namespace brave_wallet diff --git a/components/brave_wallet/browser/unstoppable_domains_multichain_calls.h b/components/brave_wallet/browser/unstoppable_domains_multichain_calls.h index 8f0e054e4d75..d9cf5cfda7bf 100644 --- a/components/brave_wallet/browser/unstoppable_domains_multichain_calls.h +++ b/components/brave_wallet/browser/unstoppable_domains_multichain_calls.h @@ -6,11 +6,11 @@ #ifndef BRAVE_COMPONENTS_BRAVE_WALLET_BROWSER_UNSTOPPABLE_DOMAINS_MULTICHAIN_CALLS_H_ #define BRAVE_COMPONENTS_BRAVE_WALLET_BROWSER_UNSTOPPABLE_DOMAINS_MULTICHAIN_CALLS_H_ -#include #include #include #include "base/callback.h" +#include "base/containers/flat_map.h" #include "brave/components/brave_wallet/common/brave_wallet.mojom.h" #include "third_party/abseil-cpp/absl/types/optional.h" @@ -30,6 +30,14 @@ class UnstoppableDomainsMultichainCall { UnstoppableDomainsMultichainCall() = default; ~UnstoppableDomainsMultichainCall() = default; + UnstoppableDomainsMultichainCall(const UnstoppableDomainsMultichainCall&) = + delete; + UnstoppableDomainsMultichainCall& operator=( + const UnstoppableDomainsMultichainCall&) = delete; + UnstoppableDomainsMultichainCall(UnstoppableDomainsMultichainCall&&) = + default; + UnstoppableDomainsMultichainCall& operator=( + UnstoppableDomainsMultichainCall&&) = default; bool SetNoResult(const std::string& chain_id); bool SetResult(const std::string& chain_id, ResultType result); @@ -43,7 +51,7 @@ class UnstoppableDomainsMultichainCall { bool MaybeResolveCallbacks(); // chain_id -> response. - std::map responses_; + base::flat_map responses_; std::vector callbacks_; }; @@ -71,7 +79,8 @@ class UnstoppableDomainsMultichainCalls { private: // domain -> call - std::map> calls_; + base::flat_map> + calls_; }; } // namespace brave_wallet diff --git a/components/brave_wallet_ui/common/hooks/send.ts b/components/brave_wallet_ui/common/hooks/send.ts index a3b5f125fd37..7206c2e48382 100644 --- a/components/brave_wallet_ui/common/hooks/send.ts +++ b/components/brave_wallet_ui/common/hooks/send.ts @@ -25,7 +25,9 @@ import { useAssets } from './assets' import { PendingCryptoSendState, SendCryptoActions } from '../reducers/send_crypto_reducer' const supportedENSExtensions = ['.eth'] -const supportedUDExtensions = ['.crypto'] +// Should match `kUDPattern` array from json_rpc_service.cc. +const supportedUDExtensions = [ + '.crypto', '.x', '.coin', '.nft', '.dao', '.wallet', '.888', '.blockchain', '.bitcoin'] const endsWithAny = (extensions: string[], url: string) => { return extensions.some(function (suffix) {