cleanup

components/brave_wallet/browser/internal/hd_key_ed25519.cc

@@ -35,25 +35,35 @@ bool ValidateKeypair(base::span<const uint8_t, kEd25519KeypairSize> key_pair) {
   return validated_key_pair == key_pair;
 }
 
+// Returns concatenation of (private key, pubic key)
+std::array<uint8_t, kEd25519KeypairSize> MakeKeyPair(
+    base::span<const uint8_t> private_key,
+    base::span<const uint8_t> public_key) {
+  std::array<uint8_t, kEd25519KeypairSize> key_pair = {};
+  base::span(key_pair).first<kEd25519PrivateKeySize>().copy_from(private_key);
+  base::span(key_pair).last<kEd25519PublicKeySize>().copy_from(public_key);
+  return key_pair;
+}
+
 }  // namespace
 
 HDKeyEd25519::HDKeyEd25519() = default;
 HDKeyEd25519::~HDKeyEd25519() = default;
 
 // Child key derivation constructor.
-// https://github.com/bitcoin/bips/blob/master/bip-0032.mediawiki#private-parent-key--private-child-key
+// https://github.com/satoshilabs/slips/blob/master/slip-0010.md#private-parent-key--private-child-key
 HDKeyEd25519::HDKeyEd25519(base::span<const uint8_t> key,
                            base::span<const uint8_t> data) {
   auto hmac = HmacSha512(key, data);
   auto scoped_zero_span = ScopedSecureZeroSpan(hmac);
   auto [il, ir] = base::span(hmac).split_at<32>();
 
-  // `public_key` is not used, we use key pair instead.
-  std::array<uint8_t, ED25519_PUBLIC_KEY_LEN> public_key;
-  ED25519_keypair_from_seed(public_key.data(), key_pair_.AsSpan().data(),
-                            il.data());
-
+  private_key_.AsSpan().copy_from(il);
   chain_code_.AsSpan().copy_from(ir);
+
+  // `key_pair` is not used, we need only public key.
+  std::array<uint8_t, kEd25519KeypairSize> key_pair;
+  ED25519_keypair_from_seed(public_key_.data(), key_pair.data(), il.data());
 }
 
 // static
@@ -64,7 +74,10 @@ std::unique_ptr<HDKeyEd25519> HDKeyEd25519::GenerateFromKeyPair(
   }
 
   auto result = std::make_unique<HDKeyEd25519>();
-  result->key_pair_.AsSpan().copy_from(key_pair);
+  result->private_key_.AsSpan().copy_from(
+      key_pair.first<kEd25519PrivateKeySize>());
+  base::span(result->public_key_)
+      .copy_from(key_pair.last<kEd25519PublicKeySize>());
   return result;
 }
 
@@ -81,6 +94,8 @@ std::unique_ptr<HDKeyEd25519> HDKeyEd25519::DeriveHardenedChild(
 }
 
 std::unique_ptr<HDKeyEd25519> HDKeyEd25519::DeriveChild(uint32_t index) {
+  CHECK_GE(index, kHardenedOffset);
+
   std::array<uint8_t, 37> hmac_payload = {};
 
   auto span_writer = base::SpanWriter(base::span(hmac_payload));
@@ -120,31 +135,35 @@ std::unique_ptr<HDKeyEd25519> HDKeyEd25519::GenerateFromSeedAndPath(
 
 std::optional<std::array<uint8_t, kEd25519SignatureSize>> HDKeyEd25519::Sign(
     base::span<const uint8_t> msg) {
-  std::array<uint8_t, kEd25519SignatureSize> signature = {};
+  // For the sake of performance `ED25519_sign` does not derive public key from
+  // private key, but assumes they are both already available as key_pair.
+  std::array<uint8_t, kEd25519KeypairSize> key_pair =
+      MakeKeyPair(private_key_.AsSpan(), public_key_);
 
+  std::array<uint8_t, kEd25519SignatureSize> signature = {};
   if (!ED25519_sign(signature.data(), msg.data(), msg.size(),
-                    key_pair_.AsSpan().data())) {
+                    key_pair.data())) {
     return std::nullopt;
   }
   return signature;
 }
 
-base::span<const uint8_t, kEd25519SecretKeySize>
+base::span<const uint8_t, kEd25519PrivateKeySize>
 HDKeyEd25519::GetPrivateKeyAsSpan() const {
-  return key_pair_.AsSpan().first<kEd25519SecretKeySize>();
+  return private_key_.AsSpan();
 }
 
 base::span<const uint8_t, kEd25519PublicKeySize>
 HDKeyEd25519::GetPublicKeyAsSpan() const {
-  return key_pair_.AsSpan().last<kEd25519PublicKeySize>();
+  return public_key_;
 }
 
 std::string HDKeyEd25519::GetBase58EncodedPublicKey() const {
   return EncodeBase58(GetPublicKeyAsSpan());
 }
 
 std::string HDKeyEd25519::GetBase58EncodedKeypair() const {
-  return EncodeBase58(key_pair_.AsSpan());
+  return EncodeBase58(MakeKeyPair(private_key_.AsSpan(), public_key_));
 }
 
 }  // namespace brave_wallet

components/brave_wallet/browser/internal/hd_key_ed25519.h

@@ -11,14 +11,15 @@
 #include <string>
 
 #include "base/containers/span.h"
+#include "base/gtest_prod_util.h"
 #include "brave/components/brave_wallet/browser/internal/hd_key_utils.h"
 
 namespace brave_wallet {
 
-inline constexpr size_t kEd25519SecretKeySize = 32;
+inline constexpr size_t kEd25519PrivateKeySize = 32;
 inline constexpr size_t kEd25519PublicKeySize = 32;
 inline constexpr size_t kEd25519KeypairSize =
-    kEd25519SecretKeySize + kEd25519PublicKeySize;
+    kEd25519PrivateKeySize + kEd25519PublicKeySize;
 inline constexpr size_t kEd25519ChainCodeSize = 32;
 inline constexpr size_t kEd25519SignatureSize = 64;
 
@@ -44,20 +45,24 @@ class HDKeyEd25519 {
   std::optional<std::array<uint8_t, kEd25519SignatureSize>> Sign(
       base::span<const uint8_t> msg);
 
-  base::span<const uint8_t, kEd25519SecretKeySize> GetPrivateKeyAsSpan() const
-      LIFETIME_BOUND;
   base::span<const uint8_t, kEd25519PublicKeySize> GetPublicKeyAsSpan() const
       LIFETIME_BOUND;
 
   std::string GetBase58EncodedPublicKey() const;
   std::string GetBase58EncodedKeypair() const;
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(HDKeyEd25519UnitTest, TestVector1);
+  FRIEND_TEST_ALL_PREFIXES(HDKeyEd25519UnitTest, TestVector2);
+
   HDKeyEd25519(base::span<const uint8_t> key, base::span<const uint8_t> data);
 
+  base::span<const uint8_t, kEd25519PrivateKeySize> GetPrivateKeyAsSpan() const
+      LIFETIME_BOUND;
   std::unique_ptr<HDKeyEd25519> DeriveChild(uint32_t index);
 
-  SecureByteArray<kEd25519KeypairSize> key_pair_;
+  SecureByteArray<kEd25519PrivateKeySize> private_key_;
+  std::array<uint8_t, kEd25519PublicKeySize> public_key_ = {};
   SecureByteArray<kEd25519ChainCodeSize> chain_code_;
 };
 

components/brave_wallet/browser/internal/hd_key_utils.h

@@ -25,6 +25,9 @@ inline constexpr uint32_t kHardenedOffset = 0x80000000;
 // expected to end with a single quote per BIP-44 style.
 // https://github.com/bitcoin/bips/blob/master/bip-0032.mediawiki
 // https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki
+// TODO(apaymyshev): We most likely do not need this parsing as at runtime all
+// parsing is done from hardcoded strings. Worth implementing account roots
+// derivations in terms of DeriveHardened/DeriveNormal calls.
 std::optional<std::vector<uint32_t>> ParseFullHDPath(std::string_view path);
 
 class ScopedSecureZeroSpan {