HD Key cleanup (#27118)

components/brave_wallet/browser/BUILD.gn

@@ -129,6 +129,8 @@ static_library("browser") {
     "fil_tx_meta.h",
     "fil_tx_state_manager.cc",
     "fil_tx_state_manager.h",
+    "json_keystore_parser.cc",
+    "json_keystore_parser.h",
     "json_rpc_requests_helper.cc",
     "json_rpc_requests_helper.h",
     "json_rpc_response_parser.cc",
@@ -271,6 +273,7 @@ static_library("browser") {
     "//brave/components/brave_component_updater/browser",
     "//brave/components/brave_stats/browser",
     "//brave/components/brave_wallet/api",
+    "//brave/components/brave_wallet/browser/internal:hd_key_common",
     "//brave/components/brave_wallet/common",
     "//brave/components/brave_wallet/common:buildflags",
     "//brave/components/brave_wallet/common:common_constants",
@@ -424,9 +427,9 @@ source_set("hd_keyring") {
 
   deps = [
     ":transaction",
-    "internal:hd_key",
     "//base",
     "//brave/components/brave_wallet/browser:utils",
+    "//brave/components/brave_wallet/browser/internal:hd_key",
     "//brave/components/brave_wallet/common",
     "//brave/components/brave_wallet/common:buildflags",
     "//brave/components/brave_wallet/common:common_constants",

components/brave_wallet/browser/bitcoin/bitcoin_discover_account_task.cc

@@ -7,6 +7,7 @@
 
 #include <stdint.h>
 
+#include <array>
 #include <optional>
 #include <utility>
 
@@ -15,10 +16,12 @@
 #include "base/functional/bind.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/types/cxx23_to_underlying.h"
 #include "base/types/expected.h"
 #include "brave/components/brave_wallet/browser/bitcoin/bitcoin_import_keyring.h"
 #include "brave/components/brave_wallet/browser/bitcoin/bitcoin_task_utils.h"
 #include "brave/components/brave_wallet/browser/bitcoin/bitcoin_wallet_service.h"
+#include "brave/components/brave_wallet/browser/internal/hd_key_common.h"
 #include "brave/components/brave_wallet/browser/keyring_service.h"
 #include "brave/components/brave_wallet/common/bitcoin_utils.h"
 #include "brave/components/brave_wallet/common/common_utils.h"
@@ -238,10 +241,12 @@ DiscoverExtendedKeyAccountTask::DiscoverExtendedKeyAccountTask(
     return;
   }
 
-  if (testnet_ && parsed_key->version != ExtendedKeyVersion::kTpub) {
+  if (testnet_ &&
+      parsed_key->version != base::to_underlying(ExtendedKeyVersion::kTpub)) {
     return;
   }
-  if (!testnet_ && parsed_key->version != ExtendedKeyVersion::kXpub) {
+  if (!testnet_ &&
+      parsed_key->version != base::to_underlying(ExtendedKeyVersion::kXpub)) {
     return;
   }
   account_key_ = std::move(parsed_key->hdkey);
@@ -254,15 +259,9 @@ mojom::BitcoinAddressPtr DiscoverExtendedKeyAccountTask::GetAddressById(
     return nullptr;
   }
 
-  auto key = account_key_->DeriveNormalChild(key_id->change);
-  if (!key) {
-    return nullptr;
-  }
-
-  key = key->DeriveNormalChild(key_id->index);
-  if (!key) {
-    return nullptr;
-  }
+  auto key = account_key_->DeriveChildFromPath(
+      std::array{DerivationIndex::Normal(key_id->change),
+                 DerivationIndex::Normal(key_id->index)});
 
   return mojom::BitcoinAddress::New(
       PubkeyToSegwitAddress(key->GetPublicKeyBytes(), testnet_),

components/brave_wallet/browser/bitcoin/bitcoin_hardware_keyring.cc

@@ -11,6 +11,7 @@
 
 #include "base/check.h"
 #include "base/notimplemented.h"
+#include "base/types/cxx23_to_underlying.h"
 #include "brave/components/brave_wallet/browser/internal/hd_key.h"
 #include "brave/components/brave_wallet/common/bitcoin_utils.h"
 
@@ -31,10 +32,12 @@ bool BitcoinHardwareKeyring::AddAccount(uint32_t account,
     return false;
   }
 
-  if (testnet_ && parsed_key->version != ExtendedKeyVersion::kTpub) {
+  if (testnet_ &&
+      parsed_key->version != base::to_underlying(ExtendedKeyVersion::kTpub)) {
     return false;
   }
-  if (!testnet_ && parsed_key->version != ExtendedKeyVersion::kXpub) {
+  if (!testnet_ &&
+      parsed_key->version != base::to_underlying(ExtendedKeyVersion::kXpub)) {
     return false;
   }
 
@@ -95,12 +98,9 @@ std::unique_ptr<HDKey> BitcoinHardwareKeyring::DeriveKey(
 
   DCHECK(key_id.change == 0 || key_id.change == 1);
 
-  auto key = account_key->DeriveNormalChild(key_id.change);
-  if (!key) {
-    return nullptr;
-  }
-
-  return key->DeriveNormalChild(key_id.index);
+  return account_key->DeriveChildFromPath(
+      std::array{DerivationIndex::Normal(key_id.change),
+                 DerivationIndex::Normal(key_id.index)});
 }
 
 }  // namespace brave_wallet

components/brave_wallet/browser/bitcoin/bitcoin_hd_keyring.cc

@@ -12,16 +12,37 @@
 #include "base/check.h"
 #include "base/containers/span.h"
 #include "base/notimplemented.h"
+#include "brave/components/brave_wallet/browser/internal/hd_key_common.h"
 #include "brave/components/brave_wallet/common/bitcoin_utils.h"
 
 namespace brave_wallet {
 
+namespace {
+
+std::unique_ptr<HDKey> ConstructAccountsRootKey(base::span<const uint8_t> seed,
+                                                bool testnet) {
+  auto result = HDKey::GenerateFromSeed(seed);
+  if (!result) {
+    return nullptr;
+  }
+
+  if (testnet) {
+    // Testnet: m/84'/1'
+    return result->DeriveChildFromPath({DerivationIndex::Hardened(84),  //
+                                        DerivationIndex::Hardened(1)});
+  } else {
+    // Mainnet: m/84'/0'
+    return result->DeriveChildFromPath({DerivationIndex::Hardened(84),  //
+                                        DerivationIndex::Hardened(0)});
+  }
+}
+
+}  // namespace
+
 BitcoinHDKeyring::BitcoinHDKeyring(base::span<const uint8_t> seed, bool testnet)
-    : Secp256k1HDKeyring(
-          seed,
-          GetRootPath(testnet ? mojom::KeyringId::kBitcoin84Testnet
-                              : mojom::KeyringId::kBitcoin84)),
-      testnet_(testnet) {}
+    : testnet_(testnet) {
+  accounts_root_ = ConstructAccountsRootKey(seed, testnet);
+}
 
 mojom::BitcoinAddressPtr BitcoinHDKeyring::GetAddress(
     uint32_t account,
@@ -94,7 +115,7 @@ std::string BitcoinHDKeyring::GetAddressInternal(const HDKey& hd_key) const {
 std::unique_ptr<HDKey> BitcoinHDKeyring::DeriveAccount(uint32_t index) const {
   // Mainnet - m/84'/0'/{index}'
   // Testnet - m/84'/1'/{index}'
-  return root_->DeriveHardenedChild(index);
+  return accounts_root_->DeriveChild(DerivationIndex::Hardened(index));
 }
 
 std::unique_ptr<HDKey> BitcoinHDKeyring::DeriveKey(
@@ -109,14 +130,11 @@ std::unique_ptr<HDKey> BitcoinHDKeyring::DeriveKey(
   // TODO(apaymyshev): think if |key_id.change| should be a boolean.
   DCHECK(key_id.change == 0 || key_id.change == 1);
 
-  auto key = account_key->DeriveNormalChild(key_id.change);
-  if (!key) {
-    return nullptr;
-  }
-
   // Mainnet - m/84'/0'/{account}'/{key_id.change}/{key_id.index}
   // Testnet - m/84'/1'/{account}'/{key_id.change}/{key_id.index}
-  return key->DeriveNormalChild(key_id.index);
+  return account_key->DeriveChildFromPath(
+      std::array{DerivationIndex::Normal(key_id.change),
+                 DerivationIndex::Normal(key_id.index)});
 }
 
 }  // namespace brave_wallet

components/brave_wallet/browser/bitcoin/bitcoin_import_keyring.cc

@@ -10,6 +10,7 @@
 #include <utility>
 
 #include "base/check.h"
+#include "base/types/cxx23_to_underlying.h"
 #include "brave/components/brave_wallet/browser/internal/hd_key.h"
 #include "brave/components/brave_wallet/common/bitcoin_utils.h"
 
@@ -29,10 +30,12 @@ bool BitcoinImportKeyring::AddAccount(uint32_t account,
     return false;
   }
 
-  if (testnet_ && parsed_key->version != ExtendedKeyVersion::kVprv) {
+  if (testnet_ &&
+      parsed_key->version != base::to_underlying(ExtendedKeyVersion::kVprv)) {
     return false;
   }
-  if (!testnet_ && parsed_key->version != ExtendedKeyVersion::kZprv) {
+  if (!testnet_ &&
+      parsed_key->version != base::to_underlying(ExtendedKeyVersion::kZprv)) {
     return false;
   }
 
@@ -98,14 +101,11 @@ std::unique_ptr<HDKey> BitcoinImportKeyring::DeriveKey(
 
   DCHECK(key_id.change == 0 || key_id.change == 1);
 
-  auto key = account_key->DeriveNormalChild(key_id.change);
-  if (!key) {
-    return nullptr;
-  }
-
   // Mainnet - m/84'/0'/{account}'/{key_id.change}/{key_id.index}
   // Testnet - m/84'/1'/{account}'/{key_id.change}/{key_id.index}
-  return key->DeriveNormalChild(key_id.index);
+  return account_key->DeriveChildFromPath(
+      std::array{DerivationIndex::Normal(key_id.change),
+                 DerivationIndex::Normal(key_id.index)});
 }
 
 }  // namespace brave_wallet

components/brave_wallet/browser/eip1559_transaction_unittest.cc

@@ -125,10 +125,10 @@ TEST(Eip1559TransactionUnitTest, GetSignedTransactionAndHash) {
        "0x863c02549182b91f1764714b93d7e882f010539c0907adaf4de761f7b06a713c"}};
   for (const auto& entry : cases) {
     SCOPED_TRACE(entry.signed_tx);
-    std::vector<uint8_t> private_key;
-    EXPECT_TRUE(base::HexStringToBytes(
+    std::array<uint8_t, 32> private_key;
+    EXPECT_TRUE(base::HexStringToSpan(
         "8f2a55949038a9610f50fb23b5883af3b4ecb3c3bb792cbcefbd1542c692be63",
-        &private_key));
+        private_key));
 
     HDKey key;
     key.SetPrivateKey(private_key);

components/brave_wallet/browser/eip2930_transaction_unittest.cc

@@ -117,10 +117,10 @@ TEST(Eip2930TransactionUnitTest, GetSignedTransactionAndHash) {
 
   access_list->push_back(item);
 
-  std::vector<uint8_t> private_key;
-  EXPECT_TRUE(base::HexStringToBytes(
+  std::array<uint8_t, 32> private_key;
+  EXPECT_TRUE(base::HexStringToSpan(
       "fad9c8855b740a0b7ed4c221dbad0f33a83a49cad6b3fe8d5817ac83d38b6a19",
-      &private_key));
+      private_key));
 
   HDKey key;
   key.SetPrivateKey(private_key);

components/brave_wallet/browser/eth_transaction_unittest.cc

@@ -113,10 +113,10 @@ TEST(EthTransactionUnitTest, GetMessageToSign) {
 }
 
 TEST(EthTransactionUnitTest, GetSignedTransactionAndHash) {
-  std::vector<uint8_t> private_key;
-  EXPECT_TRUE(base::HexStringToBytes(
+  std::array<uint8_t, 32> private_key;
+  EXPECT_TRUE(base::HexStringToSpan(
       "4646464646464646464646464646464646464646464646464646464646464646",
-      &private_key));
+      private_key));
 
   HDKey key;
   key.SetPrivateKey(private_key);

components/brave_wallet/browser/ethereum_keyring.cc

@@ -14,6 +14,7 @@
 #include "base/strings/strcat.h"
 #include "base/strings/string_number_conversions.h"
 #include "brave/components/brave_wallet/browser/eth_transaction.h"
+#include "brave/components/brave_wallet/browser/internal/hd_key_common.h"
 #include "brave/components/brave_wallet/common/eth_address.h"
 #include "brave/components/brave_wallet/common/hash_utils.h"
 
@@ -30,10 +31,25 @@ std::vector<uint8_t> GetMessageHash(base::span<const uint8_t> message) {
   return base::ToVector(KeccakHash(hash_input));
 }
 
+std::unique_ptr<HDKey> ConstructAccountsRootKey(
+    base::span<const uint8_t> seed) {
+  auto result = HDKey::GenerateFromSeed(seed);
+  if (!result) {
+    return nullptr;
+  }
+
+  // m/44'/60'/0'/0
+  return result->DeriveChildFromPath({DerivationIndex::Hardened(44),  //
+                                      DerivationIndex::Hardened(60),
+                                      DerivationIndex::Hardened(0),
+                                      DerivationIndex::Normal(0)});
+}
+
 }  // namespace
 
-EthereumKeyring::EthereumKeyring(base::span<const uint8_t> seed)
-    : Secp256k1HDKeyring(seed, GetRootPath(mojom::KeyringId::kDefault)) {}
+EthereumKeyring::EthereumKeyring(base::span<const uint8_t> seed) {
+  accounts_root_ = ConstructAccountsRootKey(seed);
+}
 
 // static
 std::optional<std::string> EthereumKeyring::RecoverAddress(
@@ -179,7 +195,7 @@ std::string EthereumKeyring::EncodePrivateKeyForExport(
 
 std::unique_ptr<HDKey> EthereumKeyring::DeriveAccount(uint32_t index) const {
   // m/44'/60'/0'/0/{index}
-  return root_->DeriveNormalChild(index);
+  return accounts_root_->DeriveChild(DerivationIndex::Normal(index));
 }
 
 }  // namespace brave_wallet

components/brave_wallet/browser/ethereum_keyring_unittest.cc

@@ -41,11 +41,13 @@ TEST(EthereumKeyringUnitTest, ConstructRootHDKey) {
       &seed));
   EthereumKeyring keyring(seed);
   EXPECT_EQ(
-      keyring.root_.get()->GetPrivateExtendedKey(ExtendedKeyVersion::kXprv),
+      keyring.accounts_root_.get()->GetPrivateExtendedKey(
+          ExtendedKeyVersion::kXprv),
       "xprvA1YGbmYkUq9KMyPwADQehauc1vG7TSbNLc1dwYbvU7VzyAr7TPhj9VoJJoP2CV5kDmXX"
       "SZvbJ79ieLnD7Pt4rhbuaQjVr2JE3vcDBAvDoUg");
   EXPECT_EQ(
-      keyring.root_.get()->GetPublicExtendedKey(ExtendedKeyVersion::kXpub),
+      keyring.accounts_root_.get()->GetPublicExtendedKey(
+          ExtendedKeyVersion::kXpub),
       "xpub6EXd1H5eKChcaTUQGEwf4irLZx6bruKDhpwEjw1Y2T2yqyBFzw1yhJ7nA5EeBKozqYKB"
       "8jHxmhe7bEqyBEdPNWyPgCm2aZfs9tbLVYujvL3");
 }
@@ -115,10 +117,10 @@ TEST(EthereumKeyringUnitTest, SignTransaction) {
 }
 
 TEST(EthereumKeyringUnitTest, SignMessage) {
-  std::vector<uint8_t> private_key;
-  EXPECT_TRUE(base::HexStringToBytes(
+  std::array<uint8_t, 32> private_key;
+  EXPECT_TRUE(base::HexStringToSpan(
       "6969696969696969696969696969696969696969696969696969696969696969",
-      &private_key));
+      private_key));
 
   std::unique_ptr<HDKey> key = std::make_unique<HDKey>();
   key->SetPrivateKey(private_key);