From b6934fd03f080d437acb1fd2b665503c3d6de785 Mon Sep 17 00:00:00 2001
From: Pieter Wuille <pieter@wuille.net>
Date: Fri, 8 Sep 2023 11:48:09 -0400
Subject: [PATCH] net: merge V2Transport constructors, move key gen

This removes the ability for BIP324Cipher to generate its own key, moving that
responsibility to the caller (mostly, V2Transport). This allows us to write
the random-key V2Transport constructor by delegating to the explicit-key one.
---
 src/bip324.cpp         |  7 -------
 src/bip324.h           |  4 ++--
 src/net.cpp            | 33 +++++++++++++++++++--------------
 src/test/net_tests.cpp |  9 +++++++++
 4 files changed, 30 insertions(+), 23 deletions(-)

diff --git a/src/bip324.cpp b/src/bip324.cpp
index 314e756829f..f579a25193a 100644
--- a/src/bip324.cpp
+++ b/src/bip324.cpp
@@ -22,13 +22,6 @@
 #include <iterator>
 #include <string>
 
-BIP324Cipher::BIP324Cipher() noexcept
-{
-    m_key.MakeNewKey(true);
-    uint256 entropy = GetRandHash();
-    m_our_pubkey = m_key.EllSwiftCreate(MakeByteSpan(entropy));
-}
-
 BIP324Cipher::BIP324Cipher(const CKey& key, Span<const std::byte> ent32) noexcept :
     m_key(key)
 {
diff --git a/src/bip324.h b/src/bip324.h
index 0238c479c08..28e7c411eaa 100644
--- a/src/bip324.h
+++ b/src/bip324.h
@@ -41,8 +41,8 @@ private:
     std::array<std::byte, GARBAGE_TERMINATOR_LEN> m_recv_garbage_terminator;
 
 public:
-    /** Initialize a BIP324 cipher with securely generated random keys. */
-    BIP324Cipher() noexcept;
+    /** No default constructor; keys must be provided to create a BIP324Cipher. */
+    BIP324Cipher() = delete;
 
     /** Initialize a BIP324 cipher with specified key and encoding entropy (testing only). */
     BIP324Cipher(const CKey& key, Span<const std::byte> ent32) noexcept;
diff --git a/src/net.cpp b/src/net.cpp
index 3955005dfa6..98ca7c2bed3 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -979,23 +979,24 @@ public:
 
 const V2MessageMap V2_MESSAGE_MAP;
 
-} // namespace
-
-V2Transport::V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in) noexcept :
-    m_cipher{}, m_initiating{initiating}, m_nodeid{nodeid},
-    m_v1_fallback{nodeid, type_in, version_in}, m_recv_type{type_in}, m_recv_version{version_in},
-    m_recv_state{initiating ? RecvState::KEY : RecvState::KEY_MAYBE_V1},
-    m_send_state{initiating ? SendState::AWAITING_KEY : SendState::MAYBE_V1}
+CKey GenerateRandomKey() noexcept
 {
-    // Construct garbage (including its length) using a FastRandomContext.
-    FastRandomContext rng;
-    size_t garbage_len = rng.randrange(MAX_GARBAGE_LEN + 1);
-    // Initialize the send buffer with ellswift pubkey + garbage.
-    m_send_buffer.resize(EllSwiftPubKey::size() + garbage_len);
-    std::copy(std::begin(m_cipher.GetOurPubKey()), std::end(m_cipher.GetOurPubKey()), MakeWritableByteSpan(m_send_buffer).begin());
-    rng.fillrand(MakeWritableByteSpan(m_send_buffer).subspan(EllSwiftPubKey::size()));
+    CKey key;
+    key.MakeNewKey(/*fCompressed=*/true);
+    return key;
 }
 
+std::vector<uint8_t> GenerateRandomGarbage() noexcept
+{
+    std::vector<uint8_t> ret;
+    FastRandomContext rng;
+    ret.resize(rng.randrange(V2Transport::MAX_GARBAGE_LEN + 1));
+    rng.fillrand(MakeWritableByteSpan(ret));
+    return ret;
+}
+
+} // namespace
+
 V2Transport::V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in, const CKey& key, Span<const std::byte> ent32, Span<const uint8_t> garbage) noexcept :
     m_cipher{key, ent32}, m_initiating{initiating}, m_nodeid{nodeid},
     m_v1_fallback{nodeid, type_in, version_in}, m_recv_type{type_in}, m_recv_version{version_in},
@@ -1009,6 +1010,10 @@ V2Transport::V2Transport(NodeId nodeid, bool initiating, int type_in, int versio
     std::copy(garbage.begin(), garbage.end(), m_send_buffer.begin() + EllSwiftPubKey::size());
 }
 
+V2Transport::V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in) noexcept :
+    V2Transport{nodeid, initiating, type_in, version_in, GenerateRandomKey(),
+                MakeByteSpan(GetRandHash()), GenerateRandomGarbage()} { }
+
 void V2Transport::SetReceiveState(RecvState recv_state) noexcept
 {
     AssertLockHeld(m_recv_mutex);
diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp
index 900e311d225..eac8e8146ae 100644
--- a/src/test/net_tests.cpp
+++ b/src/test/net_tests.cpp
@@ -1008,6 +1008,14 @@ BOOST_AUTO_TEST_CASE(advertise_local_address)
 
 namespace {
 
+CKey GenerateRandomTestKey() noexcept
+{
+    CKey key;
+    uint256 key_data = InsecureRand256();
+    key.Set(key_data.begin(), key_data.end(), true);
+    return key;
+}
+
 /** A class for scenario-based tests of V2Transport
  *
  * Each V2TransportTester encapsulates a V2Transport (the one being tested), and can be told to
@@ -1031,6 +1039,7 @@ public:
     /** Construct a tester object. test_initiator: whether the tested transport is initiator. */
     V2TransportTester(bool test_initiator) :
         m_transport(0, test_initiator, SER_NETWORK, INIT_PROTO_VERSION),
+        m_cipher{GenerateRandomTestKey(), MakeByteSpan(InsecureRand256())},
         m_test_initiator(test_initiator) {}
 
     /** Data type returned by Interact: