Fix UNSAFE_TODO for wallet

brave · Dec 4, 2024 · cf80e4c · cf80e4c
1 parent 25fa26e
commit cf80e4c
Show file tree

Hide file tree

Showing 14 changed files with 262 additions and 299 deletions.
diff --git a/components/brave_wallet/browser/eip1559_transaction.cc b/components/brave_wallet/browser/eip1559_transaction.cc
@@ -9,6 +9,7 @@
 #include <optional>
 #include <utility>
 
+#include "base/containers/extend.h"
 #include "base/containers/to_vector.h"
 #include "base/values.h"
 #include "brave/components/brave_wallet/browser/rlp_encode.h"
@@ -288,8 +289,7 @@ std::vector<uint8_t> Eip1559Transaction::GetMessageToSign(uint256_t chain_id,
   list.Append(base::Value(data_));
   list.Append(base::Value(AccessListToValue(access_list_)));
 
-  const std::string rlp_msg = RLPEncode(base::Value(std::move(list)));
-  result.insert(result.end(), rlp_msg.begin(), rlp_msg.end());
+  base::Extend(result, RLPEncode(list));
   return hash ? base::ToVector(KeccakHash(result)) : result;
 }
 
@@ -363,8 +363,7 @@ std::vector<uint8_t> Eip1559Transaction::Serialize() const {
   std::vector<uint8_t> result;
   result.push_back(type_);
 
-  const std::string rlp_msg = RLPEncode(base::Value(std::move(list)));
-  result.insert(result.end(), rlp_msg.begin(), rlp_msg.end());
+  base::Extend(result, RLPEncode(list));
 
   return result;
 }

diff --git a/components/brave_wallet/browser/eip2930_transaction.cc b/components/brave_wallet/browser/eip2930_transaction.cc
@@ -8,6 +8,7 @@
 #include <optional>
 #include <utility>
 
+#include "base/containers/extend.h"
 #include "base/containers/to_vector.h"
 #include "base/values.h"
 #include "brave/components/brave_wallet/browser/rlp_encode.h"
@@ -178,8 +179,7 @@ std::vector<uint8_t> Eip2930Transaction::GetMessageToSign(uint256_t chain_id,
   list.Append(base::Value(data_));
   list.Append(base::Value(AccessListToValue(access_list_)));
 
-  const std::string rlp_msg = RLPEncode(base::Value(std::move(list)));
-  result.insert(result.end(), rlp_msg.begin(), rlp_msg.end());
+  base::Extend(result, RLPEncode(list));
   return hash ? base::ToVector(KeccakHash(result)) : result;
 }
 
@@ -243,8 +243,7 @@ std::vector<uint8_t> Eip2930Transaction::Serialize() const {
   std::vector<uint8_t> result;
   result.push_back(type_);
 
-  const std::string rlp_msg = RLPEncode(base::Value(std::move(list)));
-  result.insert(result.end(), rlp_msg.begin(), rlp_msg.end());
+  base::Extend(result, RLPEncode(list));
 
   return result;
 }

diff --git a/components/brave_wallet/browser/ens_resolver_task.cc b/components/brave_wallet/browser/ens_resolver_task.cc
@@ -11,8 +11,6 @@
 
 #include "base/compiler_specific.h"
 #include "base/containers/contains.h"
-#include "base/functional/callback_helpers.h"
-#include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/no_destructor.h"
 #include "base/ranges/algorithm.h"
@@ -117,13 +115,13 @@ EnsResolverTaskError::~EnsResolverTaskError() = default;
 std::vector<uint8_t> MakeAddrCall(const std::string& domain) {
   return eth_abi::TupleEncoder()
       .AddFixedBytes(Namehash(domain))
-      .EncodeWithSelector(base::span(kAddrBytes32Selector));
+      .EncodeWithSelector(kAddrBytes32Selector);
 }
 
 std::vector<uint8_t> MakeContentHashCall(const std::string& domain) {
   return eth_abi::TupleEncoder()
       .AddFixedBytes(Namehash(domain))
-      .EncodeWithSelector(base::span(kContentHashBytes32Selector));
+      .EncodeWithSelector(kContentHashBytes32Selector);
 }
 
 OffchainLookupData::OffchainLookupData() = default;
@@ -156,18 +154,22 @@ std::optional<OffchainLookupData> OffchainLookupData::ExtractFromJson(
 
 std::optional<OffchainLookupData> OffchainLookupData::ExtractFromEthAbiPayload(
     eth_abi::Span bytes) {
-  auto [selector, args] =
+  auto selector_and_args =
       eth_abi::ExtractFunctionSelectorAndArgsFromCall(bytes);
+  if (!selector_and_args) {
+    return std::nullopt;
+  }
+  auto [selector, args] = *selector_and_args;
 
   // error OffchainLookup(address sender, string[] urls, bytes callData,
   // bytes4 callbackFunction, bytes extraData)
-  if (!base::ranges::equal(selector, kOffchainLookupSelector)) {
+  if (selector != kOffchainLookupSelector) {
     return std::nullopt;
   }
   auto sender = eth_abi::ExtractAddressFromTuple(args, 0);
   auto urls = eth_abi::ExtractStringArrayFromTuple(args, 1);
   auto call_data = eth_abi::ExtractBytesFromTuple(args, 2);
-  auto callback_function = eth_abi::ExtractFixedBytesFromTuple(args, 4, 3);
+  auto callback_function = eth_abi::ExtractFixedBytesFromTuple<4>(args, 3);
   auto extra_data = eth_abi::ExtractBytesFromTuple(args, 4);
 
   if (!sender.IsValid() || !urls || !call_data || !callback_function ||
@@ -460,7 +462,7 @@ void EnsResolverTask::FetchOffchainData() {
   DCHECK(offchain_lookup_data_);
 
   GURL offchain_url;
-  bool data_substitued = false;
+  bool data_substituted = false;
   bool valid_sender = true;
 
   if (!allow_offchain_.has_value()) {
@@ -470,7 +472,7 @@ void EnsResolverTask::FetchOffchainData() {
     ScheduleWorkOnTask();
     return;
   } else if (!allow_offchain_.value()) {
-    // Offchain lookup explicily disabled.
+    // Offchain lookup explicitly disabled.
     task_error_.emplace(MakeInternalError());
     ScheduleWorkOnTask();
     return;
@@ -495,7 +497,7 @@ void EnsResolverTask::FetchOffchainData() {
   for (auto url_string : offchain_lookup_data_->urls) {
     base::ReplaceSubstringsAfterOffset(&url_string, 0, "{sender}",
                                        offchain_lookup_data_->sender.ToHex());
-    data_substitued = base::Contains(url_string, "{data}");
+    data_substituted = base::Contains(url_string, "{data}");
     base::ReplaceSubstringsAfterOffset(&url_string, 0, "{data}",
                                        ToHex(offchain_lookup_data_->call_data));
     GURL url(url_string);
@@ -513,7 +515,7 @@ void EnsResolverTask::FetchOffchainData() {
   }
 
   std::string payload;
-  if (!data_substitued) {
+  if (!data_substituted) {
     base::Value::Dict payload_dict;
     payload_dict.Set("sender", offchain_lookup_data_->sender.ToHex());
     payload_dict.Set("data", ToHex(offchain_lookup_data_->call_data));

diff --git a/components/brave_wallet/browser/ens_resolver_task.h b/components/brave_wallet/browser/ens_resolver_task.h
@@ -55,7 +55,7 @@ struct OffchainLookupData {
   EthAddress sender;
   std::vector<std::string> urls;
   std::vector<uint8_t> call_data;
-  std::vector<uint8_t> callback_function;
+  eth_abi::Bytes4 callback_function;
   std::vector<uint8_t> extra_data;
 };
 

diff --git a/components/brave_wallet/browser/eth_data_builder_unittest.cc b/components/brave_wallet/browser/eth_data_builder_unittest.cc
@@ -247,14 +247,14 @@ TEST(EthCallDataBuilderTest, GetWalletAddr_ETH) {
                               mojom::kMainnetChainId);
 
     auto [selector, args] =
-        eth_abi::ExtractFunctionSelectorAndArgsFromCall(call);
+        *eth_abi::ExtractFunctionSelectorAndArgsFromCall(call);
     EXPECT_TRUE(base::ranges::equal(selector, kGetManySelector));
 
     auto keys_array = eth_abi::ExtractStringArrayFromTuple(args, 0);
     ASSERT_TRUE(keys_array);
     EXPECT_THAT(*keys_array, ElementsAreArray({"crypto.ETH.address"}));
 
-    auto name_hash = eth_abi::ExtractFixedBytesFromTuple(args, 32, 1);
+    auto name_hash = eth_abi::ExtractFixedBytesFromTuple<32>(args, 1);
     ASSERT_TRUE(name_hash);
     EXPECT_TRUE(base::ranges::equal(*name_hash, Namehash("test.crypto")));
   }
@@ -263,7 +263,7 @@ TEST(EthCallDataBuilderTest, GetWalletAddr_ETH) {
                               mojom::kBnbSmartChainMainnetChainId);
 
     auto [selector, args] =
-        eth_abi::ExtractFunctionSelectorAndArgsFromCall(call);
+        *eth_abi::ExtractFunctionSelectorAndArgsFromCall(call);
     EXPECT_TRUE(base::ranges::equal(selector, kGetManySelector));
 
     auto keys_array = eth_abi::ExtractStringArrayFromTuple(args, 0);
@@ -273,7 +273,7 @@ TEST(EthCallDataBuilderTest, GetWalletAddr_ETH) {
         ElementsAreArray({"crypto.USDT.version.BEP20.address",
                           "crypto.USDT.address", "crypto.ETH.address"}));
 
-    auto name_hash = eth_abi::ExtractFixedBytesFromTuple(args, 32, 1);
+    auto name_hash = eth_abi::ExtractFixedBytesFromTuple<32>(args, 1);
     ASSERT_TRUE(name_hash);
     EXPECT_TRUE(base::ranges::equal(*name_hash, Namehash("test.crypto")));
   }
@@ -283,15 +283,15 @@ TEST(EthCallDataBuilderTest, GetWalletAddr_ETH) {
         GetWalletAddr("test.crypto", mojom::CoinType::ETH, "QWEQWE", "0x12345");
 
     auto [selector, args] =
-        eth_abi::ExtractFunctionSelectorAndArgsFromCall(call);
+        *eth_abi::ExtractFunctionSelectorAndArgsFromCall(call);
     EXPECT_TRUE(base::ranges::equal(selector, kGetManySelector));
 
     auto keys_array = eth_abi::ExtractStringArrayFromTuple(args, 0);
     ASSERT_TRUE(keys_array);
     EXPECT_THAT(*keys_array, ElementsAreArray({"crypto.QWEQWE.address",
                                                "crypto.ETH.address"}));
 
-    auto name_hash = eth_abi::ExtractFixedBytesFromTuple(args, 32, 1);
+    auto name_hash = eth_abi::ExtractFixedBytesFromTuple<32>(args, 1);
     ASSERT_TRUE(name_hash);
     EXPECT_TRUE(base::ranges::equal(*name_hash, Namehash("test.crypto")));
   }
@@ -303,14 +303,14 @@ TEST(EthCallDataBuilderTest, GetWalletAddr_SOL) {
                               mojom::kSolanaMainnet);
 
     auto [selector, args] =
-        eth_abi::ExtractFunctionSelectorAndArgsFromCall(call);
+        *eth_abi::ExtractFunctionSelectorAndArgsFromCall(call);
     EXPECT_TRUE(base::ranges::equal(selector, kGetManySelector));
 
     auto keys_array = eth_abi::ExtractStringArrayFromTuple(args, 0);
     ASSERT_TRUE(keys_array);
     EXPECT_THAT(*keys_array, ElementsAreArray({"crypto.SOL.address"}));
 
-    auto name_hash = eth_abi::ExtractFixedBytesFromTuple(args, 32, 1);
+    auto name_hash = eth_abi::ExtractFixedBytesFromTuple<32>(args, 1);
     ASSERT_TRUE(name_hash);
     EXPECT_TRUE(base::ranges::equal(*name_hash, Namehash("test.crypto")));
   }
@@ -319,14 +319,14 @@ TEST(EthCallDataBuilderTest, GetWalletAddr_SOL) {
                               mojom::kSolanaTestnet);
 
     auto [selector, args] =
-        eth_abi::ExtractFunctionSelectorAndArgsFromCall(call);
+        *eth_abi::ExtractFunctionSelectorAndArgsFromCall(call);
     EXPECT_TRUE(base::ranges::equal(selector, kGetManySelector));
 
     auto keys_array = eth_abi::ExtractStringArrayFromTuple(args, 0);
     ASSERT_TRUE(keys_array);
     EXPECT_THAT(*keys_array, ElementsAreArray({"crypto.SOL.address"}));
 
-    auto name_hash = eth_abi::ExtractFixedBytesFromTuple(args, 32, 1);
+    auto name_hash = eth_abi::ExtractFixedBytesFromTuple<32>(args, 1);
     ASSERT_TRUE(name_hash);
     EXPECT_TRUE(base::ranges::equal(*name_hash, Namehash("test.crypto")));
   }
@@ -336,7 +336,7 @@ TEST(EthCallDataBuilderTest, GetWalletAddr_SOL) {
         GetWalletAddr("test.crypto", mojom::CoinType::SOL, "QWEQWE", "0x12345");
 
     auto [selector, args] =
-        eth_abi::ExtractFunctionSelectorAndArgsFromCall(call);
+        *eth_abi::ExtractFunctionSelectorAndArgsFromCall(call);
     EXPECT_TRUE(base::ranges::equal(selector, kGetManySelector));
 
     auto keys_array = eth_abi::ExtractStringArrayFromTuple(args, 0);
@@ -345,7 +345,7 @@ TEST(EthCallDataBuilderTest, GetWalletAddr_SOL) {
                 ElementsAreArray({"crypto.QWEQWE.version.SOLANA.address",
                                   "crypto.SOL.address"}));
 
-    auto name_hash = eth_abi::ExtractFixedBytesFromTuple(args, 32, 1);
+    auto name_hash = eth_abi::ExtractFixedBytesFromTuple<32>(args, 1);
     ASSERT_TRUE(name_hash);
     EXPECT_TRUE(base::ranges::equal(*name_hash, Namehash("test.crypto")));
   }
@@ -357,14 +357,14 @@ TEST(EthCallDataBuilderTest, GetWalletAddr_FIL) {
                               mojom::kFilecoinMainnet);
 
     auto [selector, args] =
-        eth_abi::ExtractFunctionSelectorAndArgsFromCall(call);
+        *eth_abi::ExtractFunctionSelectorAndArgsFromCall(call);
     EXPECT_TRUE(base::ranges::equal(selector, kGetManySelector));
 
     auto keys_array = eth_abi::ExtractStringArrayFromTuple(args, 0);
     ASSERT_TRUE(keys_array);
     EXPECT_THAT(*keys_array, ElementsAreArray({"crypto.FIL.address"}));
 
-    auto name_hash = eth_abi::ExtractFixedBytesFromTuple(args, 32, 1);
+    auto name_hash = eth_abi::ExtractFixedBytesFromTuple<32>(args, 1);
     ASSERT_TRUE(name_hash);
     EXPECT_TRUE(base::ranges::equal(*name_hash, Namehash("test.crypto")));
   }
@@ -373,14 +373,14 @@ TEST(EthCallDataBuilderTest, GetWalletAddr_FIL) {
                               mojom::kFilecoinTestnet);
 
     auto [selector, args] =
-        eth_abi::ExtractFunctionSelectorAndArgsFromCall(call);
+        *eth_abi::ExtractFunctionSelectorAndArgsFromCall(call);
     EXPECT_TRUE(base::ranges::equal(selector, kGetManySelector));
 
     auto keys_array = eth_abi::ExtractStringArrayFromTuple(args, 0);
     ASSERT_TRUE(keys_array);
     EXPECT_THAT(*keys_array, ElementsAreArray({"crypto.FIL.address"}));
 
-    auto name_hash = eth_abi::ExtractFixedBytesFromTuple(args, 32, 1);
+    auto name_hash = eth_abi::ExtractFixedBytesFromTuple<32>(args, 1);
     ASSERT_TRUE(name_hash);
     EXPECT_TRUE(base::ranges::equal(*name_hash, Namehash("test.crypto")));
   }
@@ -390,14 +390,14 @@ TEST(EthCallDataBuilderTest, GetWalletAddr_FIL) {
         GetWalletAddr("test.crypto", mojom::CoinType::FIL, "QWEQWE", "0x12345");
 
     auto [selector, args] =
-        eth_abi::ExtractFunctionSelectorAndArgsFromCall(call);
+        *eth_abi::ExtractFunctionSelectorAndArgsFromCall(call);
     EXPECT_TRUE(base::ranges::equal(selector, kGetManySelector));
 
     auto keys_array = eth_abi::ExtractStringArrayFromTuple(args, 0);
     ASSERT_TRUE(keys_array);
     EXPECT_THAT(*keys_array, ElementsAreArray({"crypto.FIL.address"}));
 
-    auto name_hash = eth_abi::ExtractFixedBytesFromTuple(args, 32, 1);
+    auto name_hash = eth_abi::ExtractFixedBytesFromTuple<32>(args, 1);
     ASSERT_TRUE(name_hash);
     EXPECT_TRUE(base::ranges::equal(*name_hash, Namehash("test.crypto")));
   }

diff --git a/components/brave_wallet/browser/eth_transaction.cc b/components/brave_wallet/browser/eth_transaction.cc
@@ -188,8 +188,7 @@ std::vector<uint8_t> EthTransaction::GetMessageToSign(uint256_t chain_id,
     list.Append(RLPUint256ToBlob(0));
   }
 
-  const std::string message = RLPEncode(base::Value(std::move(list)));
-  auto result = std::vector<uint8_t>(message.begin(), message.end());
+  auto result = RLPEncode(list);
   return hash ? base::ToVector(KeccakHash(result)) : result;
 }