Fix UNSAFE_TODO for wallet

brave · Oct 31, 2024 · 0f2f592 · 0f2f592
1 parent 795271e
commit 0f2f592
Show file tree

Hide file tree

Showing 45 changed files with 420 additions and 423 deletions.
diff --git a/browser/ui/webui/brave_wallet/android/android_wallet_page_ui.cc b/browser/ui/webui/brave_wallet/android/android_wallet_page_ui.cc
@@ -52,11 +52,9 @@ AndroidWalletPageUI::AndroidWalletPageUI(content::WebUI* web_ui,
 
   // Add required resources.
   if (url.host() == kWalletPageHost) {
-    webui::SetupWebUIDataSource(
-        source,
-        UNSAFE_TODO(base::make_span(kBraveWalletPageGenerated,
-                                    kBraveWalletPageGeneratedSize)),
-        IDR_WALLET_PAGE_HTML);
+    webui::SetupWebUIDataSource(source,
+                                base::make_span(kBraveWalletPageGenerated),
+                                IDR_WALLET_PAGE_HTML);
   } else {
     NOTREACHED_IN_MIGRATION()
         << "Failed to find page resources for:" << url.path();

diff --git a/browser/ui/webui/brave_wallet/ledger/ledger_ui.cc b/browser/ui/webui/brave_wallet/ledger/ledger_ui.cc
@@ -21,8 +21,7 @@ UntrustedLedgerUI::UntrustedLedgerUI(content::WebUI* web_ui)
   auto* untrusted_source = content::WebUIDataSource::CreateAndAdd(
       web_ui->GetWebContents()->GetBrowserContext(), kUntrustedLedgerURL);
   untrusted_source->SetDefaultResource(IDR_BRAVE_WALLET_LEDGER_BRIDGE_HTML);
-  untrusted_source->AddResourcePaths(UNSAFE_TODO(
-      base::make_span(kLedgerBridgeGenerated, kLedgerBridgeGeneratedSize)));
+  untrusted_source->AddResourcePaths(base::make_span(kLedgerBridgeGenerated));
   untrusted_source->AddFrameAncestor(GURL(kBraveUIWalletPageURL));
   untrusted_source->AddFrameAncestor(GURL(kBraveUIWalletPanelURL));
   untrusted_source->OverrideContentSecurityPolicy(

diff --git a/browser/ui/webui/brave_wallet/trezor/trezor_ui.cc b/browser/ui/webui/brave_wallet/trezor/trezor_ui.cc
@@ -25,8 +25,7 @@ UntrustedTrezorUI::UntrustedTrezorUI(content::WebUI* web_ui)
   auto* untrusted_source = content::WebUIDataSource::CreateAndAdd(
       web_ui->GetWebContents()->GetBrowserContext(), kUntrustedTrezorURL);
   untrusted_source->SetDefaultResource(IDR_BRAVE_WALLET_TREZOR_BRIDGE_HTML);
-  untrusted_source->AddResourcePaths(UNSAFE_TODO(
-      base::make_span(kTrezorBridgeGenerated, kTrezorBridgeGeneratedSize)));
+  untrusted_source->AddResourcePaths(base::make_span(kTrezorBridgeGenerated));
   untrusted_source->AddFrameAncestor(GURL(kBraveUIWalletPageURL));
   untrusted_source->AddFrameAncestor(GURL(kBraveUIWalletPanelURL));
   untrusted_source->OverrideContentSecurityPolicy(

diff --git a/browser/ui/webui/brave_wallet/wallet_page_ui.cc b/browser/ui/webui/brave_wallet/wallet_page_ui.cc
@@ -64,10 +64,7 @@ WalletPageUI::WalletPageUI(content::WebUI* web_ui)
   web_ui->AddMessageHandler(std::move(plural_string_handler));
   NavigationBarDataProvider::Initialize(source, profile);
   webui::SetupWebUIDataSource(
-      source,
-      UNSAFE_TODO(base::make_span(kBraveWalletPageGenerated,
-                                  kBraveWalletPageGeneratedSize)),
-      IDR_WALLET_PAGE_HTML);
+      source, base::make_span(kBraveWalletPageGenerated), IDR_WALLET_PAGE_HTML);
   source->AddString("braveWalletLedgerBridgeUrl", kUntrustedLedgerURL);
   source->OverrideContentSecurityPolicy(
       network::mojom::CSPDirectiveName::ImgSrc,

diff --git a/browser/ui/webui/brave_wallet/wallet_panel_ui.cc b/browser/ui/webui/brave_wallet/wallet_panel_ui.cc
@@ -66,11 +66,9 @@ WalletPanelUI::WalletPanelUI(content::WebUI* web_ui)
   plural_string_handler->AddLocalizedString(
       "braveWalletPendingTransactions", IDS_BRAVE_WALLET_PENDING_TRANSACTIONS);
   web_ui->AddMessageHandler(std::move(plural_string_handler));
-  webui::SetupWebUIDataSource(
-      source,
-      UNSAFE_TODO(base::make_span(kBraveWalletPanelGenerated,
-                                  kBraveWalletPanelGeneratedSize)),
-      IDR_WALLET_PANEL_HTML);
+  webui::SetupWebUIDataSource(source,
+                              base::make_span(kBraveWalletPanelGenerated),
+                              IDR_WALLET_PANEL_HTML);
   source->AddString("braveWalletLedgerBridgeUrl", kUntrustedLedgerURL);
   source->OverrideContentSecurityPolicy(
       network::mojom::CSPDirectiveName::FrameSrc,

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/values.h"
 #include "brave/components/brave_wallet/browser/rlp_encode.h"
 #include "brave/components/brave_wallet/common/hash_utils.h"
@@ -270,11 +271,9 @@ std::optional<Eip1559Transaction> Eip1559Transaction::FromValue(
   return tx;
 }
 
-std::vector<uint8_t> Eip1559Transaction::GetMessageToSign(uint256_t chain_id,
-                                                          bool hash) const {
+std::vector<uint8_t> Eip1559Transaction::GetMessageToSign(
+    uint256_t chain_id) const {
   DCHECK(nonce_);
-  std::vector<uint8_t> result;
-  result.push_back(type_);
 
   base::Value::List list;
   list.Append(RLPUint256ToBlob(chain_id_));
@@ -287,9 +286,10 @@ 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());
-  return hash ? KeccakHash(result) : result;
+  std::vector<uint8_t> result;
+  result.push_back(type_);
+  base::Extend(result, RLPEncode(list));
+  return result;
 }
 
 std::string Eip1559Transaction::GetSignedTransaction() const {
@@ -361,10 +361,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/eip1559_transaction.h b/components/brave_wallet/browser/eip1559_transaction.h
@@ -63,10 +63,9 @@ class Eip1559Transaction : public Eip2930Transaction {
     gas_estimation_ = estimation;
   }
 
-  // keccak256(0x02 || rlp([chainId, nonce, maxPriorityFeePerGas, maxFeePerGas,
-  // gasLimit, destination, value, data, access_list]))
-  std::vector<uint8_t> GetMessageToSign(uint256_t chain_id = 0,
-                                        bool hash = true) const override;
+  // 0x02 || rlp([chainId, nonce, maxPriorityFeePerGas, maxFeePerGas,
+  // gasLimit, destination, value, data, access_list])
+  std::vector<uint8_t> GetMessageToSign(uint256_t chain_id) const override;
 
   // 0x02 || rlp([chainId, nonce, maxPriorityFeePerGas, maxFeePerGas, gasLimit,
   // destination, value, data, accessList, signatureYParity, signatureR,

diff --git a/components/brave_wallet/browser/eip1559_transaction_unittest.cc b/components/brave_wallet/browser/eip1559_transaction_unittest.cc
@@ -49,7 +49,7 @@ TEST(Eip1559TransactionUnitTest, GetMessageToSign) {
 
   access_list->push_back(item);
 
-  EXPECT_EQ(base::ToLowerASCII(base::HexEncode(tx.GetMessageToSign())),
+  EXPECT_EQ(base::ToLowerASCII(base::HexEncode(tx.GetHashedMessageToSign(0))),
             "fa81814f7dd57bad435657a05eabdba2815f41e3f15ddd6139027e7db56b0dea");
 }
 
@@ -143,7 +143,7 @@ TEST(Eip1559TransactionUnitTest, GetSignedTransactionAndHash) {
 
     int recid;
     const std::vector<uint8_t> signature =
-        key.SignCompact(tx.GetMessageToSign(), &recid);
+        key.SignCompact(tx.GetHashedMessageToSign(0), &recid);
     tx.ProcessSignature(signature, recid);
     EXPECT_EQ(tx.GetSignedTransaction(), entry.signed_tx);
     EXPECT_EQ(tx.GetTransactionHash(), entry.hash);

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/values.h"
 #include "brave/components/brave_wallet/browser/rlp_encode.h"
 #include "brave/components/brave_wallet/common/eth_address.h"
@@ -161,11 +162,9 @@ Eip2930Transaction::ValueToAccessList(const base::Value::List& value) {
   return access_list;
 }
 
-std::vector<uint8_t> Eip2930Transaction::GetMessageToSign(uint256_t chain_id,
-                                                          bool hash) const {
+std::vector<uint8_t> Eip2930Transaction::GetMessageToSign(
+    uint256_t chain_id) const {
   DCHECK(nonce_);
-  std::vector<uint8_t> result;
-  result.push_back(type_);
 
   base::Value::List list;
   list.Append(RLPUint256ToBlob(chain_id_));
@@ -177,9 +176,10 @@ 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());
-  return hash ? KeccakHash(result) : result;
+  std::vector<uint8_t> result;
+  result.push_back(type_);
+  base::Extend(result, RLPEncode(list));
+  return result;
 }
 
 std::string Eip2930Transaction::GetSignedTransaction() const {
@@ -241,10 +241,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/eip2930_transaction.h b/components/brave_wallet/browser/eip2930_transaction.h
@@ -50,10 +50,9 @@ class Eip2930Transaction : public EthTransaction {
   const AccessList* access_list() const { return &access_list_; }
   AccessList* access_list() { return &access_list_; }
 
-  // keccak256(0x01 || rlp([chainId, nonce, gasPrice, gasLimit, to, value, data,
-  // accessList]))
-  std::vector<uint8_t> GetMessageToSign(uint256_t chain_id = 0,
-                                        bool hash = true) const override;
+  // 0x01 || rlp([chainId, nonce, gasPrice, gasLimit, to, value, data,
+  // accessList])
+  std::vector<uint8_t> GetMessageToSign(uint256_t chain_id) const override;
 
   // 0x01 || rlp([chainId, nonce, gasPrice, gasLimit, to, value, data,
   // accessList, signatureYParity, signatureR, signatureS])

diff --git a/components/brave_wallet/browser/eip2930_transaction_unittest.cc b/components/brave_wallet/browser/eip2930_transaction_unittest.cc
@@ -93,7 +93,7 @@ TEST(Eip2930TransactionUnitTest, GetMessageToSign) {
 
   access_list->push_back(item);
 
-  EXPECT_EQ(base::ToLowerASCII(base::HexEncode(tx.GetMessageToSign())),
+  EXPECT_EQ(base::ToLowerASCII(base::HexEncode(tx.GetHashedMessageToSign(0))),
             "78528e2724aa359c58c13e43a7c467eb721ce8d410c2a12ee62943a3aaefb60b");
 }
 
@@ -126,7 +126,7 @@ TEST(Eip2930TransactionUnitTest, GetSignedTransactionAndHash) {
   key.SetPrivateKey(private_key);
   int recid;
   const std::vector<uint8_t> signature =
-      key.SignCompact(tx.GetMessageToSign(), &recid);
+      key.SignCompact(tx.GetHashedMessageToSign(0), &recid);
 
   ASSERT_FALSE(tx.IsSigned());
   tx.ProcessSignature(signature, recid);

diff --git a/components/brave_wallet/browser/ens_resolver_task.cc b/components/brave_wallet/browser/ens_resolver_task.cc
@@ -167,7 +167,7 @@ std::optional<OffchainLookupData> OffchainLookupData::ExtractFromEthAbiPayload(
   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 ||
@@ -543,14 +543,12 @@ void EnsResolverTask::OnFetchOffchainDone(APIRequestResult api_request_result) {
 
   offchain_lookup_attemps_left_--;
   DCHECK_GE(offchain_lookup_attemps_left_, 0);
-  DCHECK_EQ(offchain_lookup_data_->callback_function.size(), 4u);
-  UNSAFE_TODO(eth_abi::Span4 callback_selector(
-      offchain_lookup_data_->callback_function.begin(), 4u));
-
-  offchain_callback_call_ = eth_abi::TupleEncoder()
-                                .AddBytes(*bytes_result)
-                                .AddBytes(offchain_lookup_data_->extra_data)
-                                .EncodeWithSelector(callback_selector);
+
+  offchain_callback_call_ =
+      eth_abi::TupleEncoder()
+          .AddBytes(*bytes_result)
+          .AddBytes(offchain_lookup_data_->extra_data)
+          .EncodeWithSelector(offchain_lookup_data_->callback_function);
 
   offchain_lookup_data_.reset();
 }

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_allowance_manager.cc b/components/brave_wallet/browser/eth_allowance_manager.cc
@@ -8,6 +8,7 @@
 #include <algorithm>
 #include <utility>
 
+#include "base/containers/span.h"
 #include "base/no_destructor.h"
 #include "brave/components/brave_wallet/browser/blockchain_registry.h"
 #include "brave/components/brave_wallet/browser/json_rpc_service.h"
@@ -164,7 +165,8 @@ void EthAllowanceManager::OnGetCurrentBlock(
     return;
   }
 
-  const auto approval_topic_hash = KeccakHash(kApprovalTopicFunctionSignature);
+  const auto approval_topic_hash = ToHex(KeccakHash(
+      base::byte_span_from_cstring(kApprovalTopicFunctionSignature)));
   for (const auto& account_address : account_addresses) {
     std::string account_address_hex;
     if (!PadHexEncodedParameter(account_address, &account_address_hex)) {

diff --git a/components/brave_wallet/browser/eth_data_builder_unittest.cc b/components/brave_wallet/browser/eth_data_builder_unittest.cc
@@ -253,7 +253,7 @@ TEST(EthCallDataBuilderTest, GetWalletAddr_ETH) {
     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")));
   }
@@ -272,7 +272,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")));
   }
@@ -290,7 +290,7 @@ TEST(EthCallDataBuilderTest, GetWalletAddr_ETH) {
     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")));
   }
@@ -309,7 +309,7 @@ TEST(EthCallDataBuilderTest, GetWalletAddr_SOL) {
     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")));
   }
@@ -325,7 +325,7 @@ TEST(EthCallDataBuilderTest, GetWalletAddr_SOL) {
     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")));
   }
@@ -344,7 +344,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")));
   }
@@ -363,7 +363,7 @@ TEST(EthCallDataBuilderTest, GetWalletAddr_FIL) {
     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")));
   }
@@ -379,7 +379,7 @@ TEST(EthCallDataBuilderTest, GetWalletAddr_FIL) {
     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")));
   }
@@ -396,7 +396,7 @@ TEST(EthCallDataBuilderTest, GetWalletAddr_FIL) {
     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_topics_builder.cc b/components/brave_wallet/browser/eth_topics_builder.cc
@@ -7,6 +7,7 @@
 
 #include <utility>
 
+#include "base/containers/span.h"
 #include "brave/components/brave_wallet/browser/brave_wallet_utils.h"
 #include "brave/components/brave_wallet/common/hash_utils.h"
 #include "brave/components/brave_wallet/common/hex_utils.h"
@@ -17,7 +18,8 @@ bool MakeAssetDiscoveryTopics(
     const std::vector<std::string>& to_account_addresses,
     base::Value::List* topics) {
   // First topic matches full keccak hash of the erc20::Transfer event signature
-  topics->Append(brave_wallet::KeccakHash("Transfer(address,address,uint256)"));
+  topics->Append(ToHex(KeccakHash(
+      base::byte_span_from_cstring("Transfer(address,address,uint256)"))));
 
   // Second topic matches everything (any from_address)
   topics->Append(base::Value());