From 5beb052f7d665901c1348d09590f369349b0beed Mon Sep 17 00:00:00 2001 From: Claudio DeSouza Date: Thu, 21 Nov 2024 16:30:36 +0000 Subject: [PATCH] [CodeHealht][wallet] Avoid `NoDestructor` in `eth_data_builder.cc` The use of `NoDestructor` for a map of strings in `ChainIdToVersion` is wasteful of resources, when the actual data can be seen at compile time. This PR converts this static `std::map` into a fixed flat map. This required changing the type of the elements to `string_view`, so this PR goes a bit the extra mile to modernise some of the functions around to use `string_view`. Resolve https://github.com/brave/brave-browser/issues/42432 --- .../brave_wallet/browser/eth_data_builder.cc | 102 +++++++++--------- .../brave_wallet/browser/eth_data_builder.h | 46 ++++---- components/brave_wallet/common/hash_utils.cc | 6 +- components/brave_wallet/common/hash_utils.h | 7 +- components/brave_wallet/common/hex_utils.cc | 28 ++--- components/brave_wallet/common/hex_utils.h | 21 ++-- 6 files changed, 106 insertions(+), 104 deletions(-) diff --git a/components/brave_wallet/browser/eth_data_builder.cc b/components/brave_wallet/browser/eth_data_builder.cc index 944f4be761f0..efdb3add9d2b 100644 --- a/components/brave_wallet/browser/eth_data_builder.cc +++ b/components/brave_wallet/browser/eth_data_builder.cc @@ -10,11 +10,11 @@ #include #include "base/check.h" +#include "base/containers/fixed_flat_map.h" #include "base/logging.h" -#include "base/no_destructor.h" #include "base/notreached.h" +#include "base/strings/strcat.h" #include "base/strings/string_util.h" -#include "base/strings/stringprintf.h" #include "brave/components/brave_wallet/browser/brave_wallet_constants.h" #include "brave/components/brave_wallet/browser/brave_wallet_utils.h" #include "brave/components/brave_wallet/common/brave_wallet.mojom.h" @@ -22,41 +22,43 @@ #include "brave/components/brave_wallet/common/fil_address.h" #include "brave/components/brave_wallet/common/hash_utils.h" #include "brave/components/brave_wallet/common/hex_utils.h" +#include "third_party/abseil-cpp/absl/strings/str_format.h" namespace brave_wallet { namespace { +constexpr auto kIdToVersionMappings = + base::MakeFixedFlatMap({ + {"0x1", "ERC20"}, + {"0x38", "BEP20"}, + {"0x63564c40", "HRC20"}, + {"0x64", "XDAI"}, + {"0x7a", "FUSE"}, + {"0x89", "MATIC"}, + {"0xa", "OP"}, + {"0xa4b1", "AETH"}, + {"0xa86a", "AVAX"}, + {"0xfa", "FANTOM"}, + }); + bool IsValidHostLabelCharacter(char c, bool is_first_char) { return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || (!is_first_char && c == '-') || c == '_'; } -std::optional ChainIdToVersion(const std::string& symbol, - const std::string& chain_id) { - static base::NoDestructor> mapping({ - {"0x1", "ERC20"}, - {"0x38", "BEP20"}, - {"0x63564c40", "HRC20"}, - {"0x64", "XDAI"}, - {"0x7a", "FUSE"}, - {"0x89", "MATIC"}, - {"0xa", "OP"}, - {"0xa4b1", "AETH"}, - {"0xa86a", "AVAX"}, - {"0xfa", "FANTOM"}, - }); - +std::optional ChainIdToVersion(std::string_view symbol, + std::string_view chain_id) { // Special case for crypto.FTM.version.OPERA.address. if (symbol == "FTM" && chain_id == "0xfa") { return "OPERA"; } - auto it = mapping->find(chain_id); - if (it != mapping->end()) { - return it->second; + auto it = kIdToVersionMappings.find(chain_id); + if (it == kIdToVersionMappings.end()) { + return std::nullopt; } - return std::nullopt; + return it->second; } } // namespace @@ -79,7 +81,7 @@ std::optional> Forward(const FilAddress& fil_address) { namespace erc20 { -bool Transfer(const std::string& to_address, +bool Transfer(std::string_view to_address, uint256_t amount, std::string* data) { const std::string function_hash = @@ -97,7 +99,7 @@ bool Transfer(const std::string& to_address, return ConcatHexStrings(hex_strings, data); } -bool BalanceOf(const std::string& address, std::string* data) { +bool BalanceOf(std::string_view address, std::string* data) { const std::string function_hash = GetFunctionHash("balanceOf(address)"); std::string params; if (!brave_wallet::PadHexEncodedParameter(address, ¶ms)) { @@ -106,7 +108,7 @@ bool BalanceOf(const std::string& address, std::string* data) { return brave_wallet::ConcatHexStrings(function_hash, params, data); } -bool Approve(const std::string& spender_address, +bool Approve(std::string_view spender_address, uint256_t amount, std::string* data) { const std::string function_hash = GetFunctionHash("approve(address,uint256)"); @@ -123,8 +125,8 @@ bool Approve(const std::string& spender_address, return ConcatHexStrings(hex_strings, data); } -bool Allowance(const std::string& owner_address, - const std::string& spender_address, +bool Allowance(std::string_view owner_address, + std::string_view spender_address, std::string* data) { const std::string function_hash = GetFunctionHash("allowance(address,address)"); @@ -148,8 +150,8 @@ bool Allowance(const std::string& owner_address, namespace erc721 { bool TransferFromOrSafeTransferFrom(bool is_safe_transfer_from, - const std::string& from, - const std::string& to, + std::string_view from, + std::string_view to, uint256_t token_id, std::string* data) { const std::string function_hash = @@ -203,8 +205,8 @@ bool TokenUri(uint256_t token_id, std::string* data) { namespace erc1155 { -bool SafeTransferFrom(const std::string& from, - const std::string& to, +bool SafeTransferFrom(std::string_view from, + std::string_view to, uint256_t token_id, uint256_t value, std::string* data) { @@ -257,7 +259,7 @@ bool SafeTransferFrom(const std::string& from, return ConcatHexStrings(hex_strings, data); } -bool BalanceOf(const std::string& owner_address, +bool BalanceOf(std::string_view owner_address, uint256_t token_id, std::string* data) { const std::string function_hash = @@ -289,11 +291,12 @@ bool Uri(uint256_t token_id, std::string* data) { namespace erc165 { -bool SupportsInterface(const std::string& interface_id, std::string* data) { +bool SupportsInterface(std::string_view interface_id, std::string* data) { if (!IsValidHexString(interface_id) || interface_id.length() != 10) { return false; } - std::string padded_interface_id = interface_id + std::string(56, '0'); + std::string padded_interface_id = + base::StrCat({interface_id, std::string(56, '0')}); const std::string function_hash = GetFunctionHash("supportsInterface(bytes4)"); @@ -311,7 +314,7 @@ std::vector SupportsInterface(eth_abi::Span4 interface) { namespace unstoppable_domains { std::optional GetMany(const std::vector& keys, - const std::string& domain) { + std::string_view domain) { const std::string function_hash = GetFunctionHash("getMany(string[],uint256)"); @@ -337,8 +340,8 @@ std::optional GetMany(const std::vector& keys, return data; } -std::vector MakeEthLookupKeyList(const std::string& symbol, - const std::string& chain_id) { +std::vector MakeEthLookupKeyList(std::string_view symbol, + std::string_view chain_id) { auto upper_symbol = base::ToUpperASCII(symbol); std::vector lookup_keys; // crypto..version..address @@ -346,15 +349,13 @@ std::vector MakeEthLookupKeyList(const std::string& symbol, if (!(upper_symbol == "ETH" && version == "ERC20")) { // No such key as 'crypto.ETH.version.ERC20.address'. 'crypto.ETH.address' // would be used instead. - lookup_keys.push_back(base::StringPrintf("crypto.%s.version.%s.address", - upper_symbol.c_str(), - version->c_str())); + lookup_keys.push_back(absl::StrFormat("crypto.%s.version.%s.address", + upper_symbol, *version)); } } // crypto..address if (symbol != "ETH") { - lookup_keys.push_back( - base::StringPrintf("crypto.%s.address", upper_symbol.c_str())); + lookup_keys.push_back(absl::StrFormat("crypto.%s.address", upper_symbol)); } // crypto.ETH.address @@ -363,13 +364,12 @@ std::vector MakeEthLookupKeyList(const std::string& symbol, return lookup_keys; } -std::vector MakeSolLookupKeyList(const std::string& symbol) { +std::vector MakeSolLookupKeyList(std::string_view symbol) { std::vector lookup_keys; // crypto..version.SOLANA.address if (symbol != "SOL") { - lookup_keys.push_back( - base::StringPrintf("crypto.%s.version.SOLANA.address", - base::ToUpperASCII(symbol).c_str())); + lookup_keys.push_back(absl::StrFormat("crypto.%s.version.SOLANA.address", + base::ToUpperASCII(symbol))); } // crypto.SOL.address @@ -387,10 +387,10 @@ std::vector MakeFilLookupKeyList() { return lookup_keys; } -std::vector GetWalletAddr(const std::string& domain, +std::vector GetWalletAddr(std::string_view domain, mojom::CoinType coin, - const std::string& symbol, - const std::string& chain_id) { + std::string_view symbol, + std::string_view chain_id) { std::vector key_list; switch (coin) { case mojom::CoinType::ETH: @@ -417,7 +417,7 @@ std::vector GetWalletAddr(const std::string& domain, namespace ens { -std::string Resolver(const std::string& domain) { +std::string Resolver(std::string_view domain) { const std::string function_hash = GetFunctionHash("resolver(bytes32)"); std::string tokenID = ToHex(Namehash(domain)); std::vector hex_strings = {function_hash, tokenID}; @@ -430,7 +430,7 @@ std::string Resolver(const std::string& domain) { // https://docs.ens.domains/ens-improvement-proposals/ensip-10-wildcard-resolution#specification // Similar to chromium's `DNSDomainFromDot` but without length limitation and // support of terminal dot. -std::optional> DnsEncode(const std::string& dotted_name) { +std::optional> DnsEncode(std::string_view dotted_name) { std::vector result; result.resize(dotted_name.size() + 2); result.front() = '.'; // Placeholder for first label length. @@ -460,7 +460,7 @@ std::optional> DnsEncode(const std::string& dotted_name) { namespace balance_scanner { std::optional TokensBalance( - const std::string& owner_address, + std::string_view owner_address, const std::vector& contract_addresses) { const std::string function_hash = GetFunctionHash("tokensBalance(address,address[])"); diff --git a/components/brave_wallet/browser/eth_data_builder.h b/components/brave_wallet/browser/eth_data_builder.h index bf40ed404b07..8c63a926e621 100644 --- a/components/brave_wallet/browser/eth_data_builder.h +++ b/components/brave_wallet/browser/eth_data_builder.h @@ -8,8 +8,10 @@ #include #include +#include #include +#include "base/types/optional_ref.h" #include "base/values.h" #include "brave/components/brave_wallet/common/brave_wallet.mojom.h" #include "brave/components/brave_wallet/common/brave_wallet_types.h" @@ -29,17 +31,15 @@ std::optional> Forward(const FilAddress& fil_address); namespace erc20 { // Allows transferring ERC20 tokens -bool Transfer(const std::string& to_address, - uint256_t amount, - std::string* data); +bool Transfer(std::string_view to_address, uint256_t amount, std::string* data); // Returns the balance of an address -bool BalanceOf(const std::string& address, std::string* data); +bool BalanceOf(std::string_view address, std::string* data); // Approves the use of funds to an address -bool Approve(const std::string& spender_address, +bool Approve(std::string_view spender_address, uint256_t amount, std::string* data); -bool Allowance(const std::string& owner_address, - const std::string& spender_address, +bool Allowance(std::string_view owner_address, + std::string_view spender_address, std::string* data); } // namespace erc20 @@ -48,8 +48,8 @@ namespace erc721 { // Transfer ownership of an NFT. bool TransferFromOrSafeTransferFrom(bool is_safe_transfer_from, - const std::string& from, - const std::string& to, + std::string_view from, + std::string_view to, uint256_t token_id, std::string* data); @@ -64,14 +64,14 @@ bool TokenUri(uint256_t token_id, std::string* data); namespace erc1155 { // Transfer the ownership of token from one address to another address. -bool SafeTransferFrom(const std::string& from_address, - const std::string& to_address, +bool SafeTransferFrom(std::string_view from_address, + std::string_view to_address, uint256_t token_id, uint256_t value, std::string* data); // Return the balance of an address for a token ID. -bool BalanceOf(const std::string& owner_address, +bool BalanceOf(std::string_view owner_address, uint256_t token_id, std::string* data); @@ -85,7 +85,7 @@ namespace erc165 { // supportsInterface(bytes4) inline constexpr uint8_t kSupportsInterfaceBytes4[] = {0x01, 0xff, 0xc9, 0xa7}; -bool SupportsInterface(const std::string& interface_id, std::string* data); +bool SupportsInterface(std::string_view interface_id, std::string* data); std::vector SupportsInterface(eth_abi::Span4 interface); } // namespace erc165 @@ -97,32 +97,32 @@ inline constexpr uint8_t kGetManySelector[] = {0x1b, 0xd8, 0xcc, 0x1a}; // Get mutiple record values mapped with keys of the target domain. std::optional GetMany(const std::vector& keys, - const std::string& domain); + std::string_view domain); -std::vector MakeEthLookupKeyList(const std::string& symbol, - const std::string& chain_id); -std::vector MakeSolLookupKeyList(const std::string& symbol); +std::vector MakeEthLookupKeyList(std::string_view symbol, + std::string_view chain_id); +std::vector MakeSolLookupKeyList(std::string_view symbol); std::vector MakeFilLookupKeyList(); -std::vector GetWalletAddr(const std::string& domain, +std::vector GetWalletAddr(std::string_view domain, mojom::CoinType coin, - const std::string& symbol, - const std::string& chain_id); + std::string_view symbol, + std::string_view chain_id); } // namespace unstoppable_domains namespace ens { -std::string Resolver(const std::string& domain); +std::string Resolver(std::string_view domain); -std::optional> DnsEncode(const std::string& dotted_name); +std::optional> DnsEncode(std::string_view dotted_name); } // namespace ens namespace balance_scanner { std::optional TokensBalance( - const std::string& owner_address, + std::string_view owner_address, const std::vector& contract_addresses); } // namespace balance_scanner diff --git a/components/brave_wallet/common/hash_utils.cc b/components/brave_wallet/common/hash_utils.cc index e5600d7efc0b..5d1e89639dbc 100644 --- a/components/brave_wallet/common/hash_utils.cc +++ b/components/brave_wallet/common/hash_utils.cc @@ -37,18 +37,18 @@ KeccakHashArray KeccakHash(base::span input) { return result; } -std::string GetFunctionHash(const std::string& input) { +std::string GetFunctionHash(std::string_view input) { return ToHex(GetFunctionHashBytes4(input)); } -eth_abi::Bytes4 GetFunctionHashBytes4(const std::string& input) { +eth_abi::Bytes4 GetFunctionHashBytes4(std::string_view input) { eth_abi::Bytes4 bytes_result; base::span(bytes_result) .copy_from(base::span(KeccakHash(base::as_byte_span(input))).first<4>()); return bytes_result; } -eth_abi::Bytes32 Namehash(const std::string& name) { +eth_abi::Bytes32 Namehash(std::string_view name) { eth_abi::Bytes32 hash = {}; auto labels = SplitStringPiece(name, ".", base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY); diff --git a/components/brave_wallet/common/hash_utils.h b/components/brave_wallet/common/hash_utils.h index 2af8e272f15e..f445edca8a3b 100644 --- a/components/brave_wallet/common/hash_utils.h +++ b/components/brave_wallet/common/hash_utils.h @@ -8,6 +8,7 @@ #include #include +#include #include "brave/components/brave_wallet/common/eth_abi_utils.h" #include "crypto/sha2.h" @@ -25,13 +26,13 @@ KeccakHashArray KeccakHash(base::span input); // Returns the hex encoding of the first 4 bytes of the hash. // For example: keccak('balanceOf(address)') -std::string GetFunctionHash(const std::string& input); -eth_abi::Bytes4 GetFunctionHashBytes4(const std::string& input); +std::string GetFunctionHash(std::string_view input); +eth_abi::Bytes4 GetFunctionHashBytes4(std::string_view input); // Implement namehash algorithm based on EIP-137 spec. // Used for converting domain names in the classic format (ex: brave.crypto) to // an ERC-721 token for ENS and Unstoppable Domains. -eth_abi::Bytes32 Namehash(const std::string& name); +eth_abi::Bytes32 Namehash(std::string_view name); // sha256(sha256(input)) SHA256HashArray DoubleSHA256Hash(base::span input); diff --git a/components/brave_wallet/common/hex_utils.cc b/components/brave_wallet/common/hex_utils.cc index ed543bd98d31..f662ffaa2e41 100644 --- a/components/brave_wallet/common/hex_utils.cc +++ b/components/brave_wallet/common/hex_utils.cc @@ -9,13 +9,13 @@ #include #include "base/compiler_specific.h" +#include "base/strings/strcat.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" -#include "base/strings/stringprintf.h" namespace brave_wallet { -std::string ToHex(const std::string& data) { +std::string ToHex(std::string_view data) { if (data.empty()) { return "0x0"; } @@ -36,7 +36,7 @@ std::string HexEncodeLower(base::span bytes) { } // Determines if the passed in hex string is valid -bool IsValidHexString(const std::string& hex_input) { +bool IsValidHexString(std::string_view hex_input) { if (hex_input.length() < 2) { return false; } @@ -53,7 +53,7 @@ bool IsValidHexString(const std::string& hex_input) { // Pads a hex encoded parameter to 32-bytes // i.e. 64 hex characters. -bool PadHexEncodedParameter(const std::string& hex_input, std::string* out) { +bool PadHexEncodedParameter(std::string_view hex_input, std::string* out) { if (!out) { return false; } @@ -64,18 +64,18 @@ bool PadHexEncodedParameter(const std::string& hex_input, std::string* out) { *out = hex_input; return true; } - std::string hex_substr = hex_input.substr(2); + std::string_view hex_substr = hex_input.substr(2); size_t padding_len = 64 - hex_substr.length(); std::string padding(padding_len, '0'); - *out = "0x" + padding + hex_substr; + *out = base::StrCat({"0x", padding, hex_substr}); return true; } // Takes 2 inputs prefixed by 0x and combines them into an output with a single // 0x. For example 0x1 and 0x2 would return 0x12 -bool ConcatHexStrings(const std::string& hex_input1, - const std::string& hex_input2, +bool ConcatHexStrings(std::string_view hex_input1, + std::string_view hex_input2, std::string* out) { if (!out) { return false; @@ -83,7 +83,7 @@ bool ConcatHexStrings(const std::string& hex_input1, if (!IsValidHexString(hex_input1) || !IsValidHexString(hex_input2)) { return false; } - *out = hex_input1 + hex_input2.substr(2); + *out = base::StrCat({hex_input1, hex_input2.substr(2)}); return true; } @@ -111,7 +111,7 @@ bool ConcatHexStrings(const std::vector& hex_inputs, return true; } -bool HexValueToUint256(const std::string& hex_input, uint256_t* out) { +bool HexValueToUint256(std::string_view hex_input, uint256_t* out) { if (!out) { return false; } @@ -131,7 +131,7 @@ bool HexValueToUint256(const std::string& hex_input, uint256_t* out) { return true; } -bool HexValueToInt256(const std::string& hex_input, int256_t* out) { +bool HexValueToInt256(std::string_view hex_input, int256_t* out) { if (!out) { return false; } @@ -162,7 +162,7 @@ std::string Uint256ValueToHex(uint256_t input) { return "0x" + result; } -bool PrefixedHexStringToBytes(const std::string& input, +bool PrefixedHexStringToBytes(std::string_view input, std::vector* bytes) { CHECK(bytes); bytes->clear(); @@ -174,7 +174,7 @@ bool PrefixedHexStringToBytes(const std::string& input, DCHECK_EQ(input, "0x"); return true; } - std::string hex_substr = input.substr(2); + std::string hex_substr = std::string(input.substr(2)); if (hex_substr.length() % 2 == 1) { hex_substr = "0" + hex_substr; } @@ -185,7 +185,7 @@ bool PrefixedHexStringToBytes(const std::string& input, } std::optional> PrefixedHexStringToBytes( - const std::string& input) { + std::string_view input) { std::vector result; if (!PrefixedHexStringToBytes(input, &result)) { return std::nullopt; diff --git a/components/brave_wallet/common/hex_utils.h b/components/brave_wallet/common/hex_utils.h index 831792f8bbc7..1adcdfa07b81 100644 --- a/components/brave_wallet/common/hex_utils.h +++ b/components/brave_wallet/common/hex_utils.h @@ -8,6 +8,7 @@ #include #include +#include #include #include "base/containers/span.h" @@ -17,7 +18,7 @@ namespace brave_wallet { // Equivalent to web3.utils.toHex(string); // TODO(apaymyshev): rename it to To0xHex or something like that. -std::string ToHex(const std::string& data); +std::string ToHex(std::string_view data); std::string ToHex(base::span data); // Returns a hex string representation of a binary buffer. The returned hex @@ -25,28 +26,28 @@ std::string ToHex(base::span data); std::string HexEncodeLower(base::span bytes); // Determines if the passed in hex string is valid -bool IsValidHexString(const std::string& hex_input); +bool IsValidHexString(std::string_view hex_input); // Pads a hex encoded parameter to 32-bytes // i.e. 64 hex characters. // Input must be prefixed with 0x -bool PadHexEncodedParameter(const std::string& hex_input, std::string* out); -std::string PadHexEncodedParameter(const std::string& hex_input); +bool PadHexEncodedParameter(std::string_view hex_input, std::string* out); +std::string PadHexEncodedParameter(std::string_view hex_input); // Takes 2 inputs prefixed by 0x and combines them into an output with a single // 0x. For example 0x1 and 0x2 would return 0x12. // Note thta this doesn't do any special casing like 0x and 0x will make 0x00 // and not 0x. -bool ConcatHexStrings(const std::string& hex_input1, - const std::string& hex_input2, +bool ConcatHexStrings(std::string_view hex_input1, + std::string_view hex_input2, std::string* out); bool ConcatHexStrings(const std::vector& hex_inputs, std::string* out); // Takes a hex string as input and converts it to a uint256_t -bool HexValueToUint256(const std::string& hex_input, uint256_t* out); +bool HexValueToUint256(std::string_view hex_input, uint256_t* out); // Takes a hex string as input and converts it to a int256_t -bool HexValueToInt256(const std::string& hex_input, int256_t* out); +bool HexValueToInt256(std::string_view hex_input, int256_t* out); // Takes a uint256_t and converts it to a hex string std::string Uint256ValueToHex(uint256_t input); @@ -57,10 +58,10 @@ std::string Uint256ValueToHex(uint256_t input); // It also handles values with uneven number of digits unlike // base::HexStringToBytes. // It also clears the output buffer unlike base::HexStringToBytes -bool PrefixedHexStringToBytes(const std::string& input, +bool PrefixedHexStringToBytes(std::string_view input, std::vector* bytes); std::optional> PrefixedHexStringToBytes( - const std::string& input); + std::string_view input); } // namespace brave_wallet