p2p: Finish negotiating reconciliation support

Once we received a reconciliation announcement support
message from a peer and it doesn't violate our protocol,
we store the negotiated parameters which will be used
for future reconciliations.

Author: naumenkogs

src/net_processing.cpp

@@ -1782,7 +1782,7 @@ PeerManagerImpl::PeerManagerImpl(CConnman& connman, AddrMan& addrman,
     // While Erlay support is incomplete, it must be enabled explicitly via -txreconciliation.
     // This argument can go away after Erlay support is complete.
     if (gArgs.GetBoolArg("-txreconciliation", DEFAULT_TXRECONCILIATION_ENABLE)) {
-        m_txreconciliation = std::make_unique<TxReconciliationTracker>();
+        m_txreconciliation = std::make_unique<TxReconciliationTracker>(TXRECONCILIATION_VERSION);
     }
 }
 
@@ -3477,6 +3477,58 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
         return;
     }
 
+    // Received from a peer demonstrating readiness to announce transactions via reconciliations.
+    // This feature negotiation must happen between VERSION and VERACK to avoid relay problems
+    // from switching announcement protocols after the connection is up.
+    if (msg_type == NetMsgType::SENDTXRCNCL) {
+        if (!m_txreconciliation) {
+            LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "sendtxrcncl from peer=%d ignored, as our node does not have txreconciliation enabled\n", pfrom.GetId());
+            return;
+        }
+
+        if (pfrom.fSuccessfullyConnected) {
+            // Disconnect peers that send a SENDTXRCNCL message after VERACK.
+            LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "sendtxrcncl received after verack from peer=%d; disconnecting\n", pfrom.GetId());
+            pfrom.fDisconnect = true;
+            return;
+        }
+
+        if (!peer->GetTxRelay()) {
+            // Disconnect peers that send a SENDTXRCNCL message even though we indicated we don't
+            // support transaction relay.
+            LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "sendtxrcncl received from peer=%d to which we indicated no tx relay; disconnecting\n", pfrom.GetId());
+            pfrom.fDisconnect = true;
+            return;
+        }
+
+        bool is_peer_initiator, is_peer_responder;
+        uint32_t peer_txreconcl_version;
+        uint64_t remote_salt;
+        vRecv >> is_peer_initiator >> is_peer_responder >> peer_txreconcl_version >> remote_salt;
+
+        if (m_txreconciliation->IsPeerRegistered(pfrom.GetId())) {
+            // A peer is already registered, meaning we already received SENDTXRCNCL from them.
+            LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "txreconciliation protocol violation from peer=%d (sendtxrcncl received from already registered peer); disconnecting\n", pfrom.GetId());
+            pfrom.fDisconnect = true;
+            return;
+        }
+
+        const ReconciliationRegisterResult result = m_txreconciliation->RegisterPeer(pfrom.GetId(), pfrom.IsInboundConn(),
+                                                                                is_peer_initiator, is_peer_responder,
+                                                                                peer_txreconcl_version,
+                                                                                remote_salt);
+
+        // If it's a protocol violation, disconnect.
+        // If the peer was not found (but something unexpected happened) or it was registered,
+        // nothing to be done.
+        if (result == ReconciliationRegisterResult::PROTOCOL_VIOLATION) {
+            LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "txreconciliation protocol violation from peer=%d; disconnecting\n", pfrom.GetId());
+            pfrom.fDisconnect = true;
+            return;
+        }
+        return;
+    }
+
     if (!pfrom.fSuccessfullyConnected) {
         LogPrint(BCLog::NET, "Unsupported message \"%s\" prior to verack from peer=%d\n", SanitizeString(msg_type), pfrom.GetId());
         return;

src/node/txreconciliation.cpp

@@ -13,11 +13,46 @@
 
 namespace {
 
+/** Static salt component used to compute short txids for sketch construction, see BIP-330. */
+const std::string RECON_STATIC_SALT = "Tx Relay Salting";
+const HashWriter RECON_SALT_HASHER = TaggedHash(RECON_STATIC_SALT);
+
+/**
+ * Salt (specified by BIP-330) constructed from contributions from both peers. It is used
+ * to compute transaction short IDs, which are then used to construct a sketch representing a set
+ * of transactions we want to announce to the peer.
+ */
+uint256 ComputeSalt(uint64_t salt1, uint64_t salt2)
+{
+    // According to BIP-330, salts should be combined in ascending order.
+    return (HashWriter(RECON_SALT_HASHER) << std::min(salt1, salt2) << std::max(salt1, salt2)).GetSHA256();
+}
+
 /**
  * Keeps track of txreconciliation-related per-peer state.
  */
 class TxReconciliationState
 {
+public:
+    /**
+     * TODO: This field is public to ignore -Wunused-private-field. Make private once used in
+     * the following commits.
+     *
+     * Reconciliation protocol assumes using one role consistently: either a reconciliation
+     * initiator (requesting sketches), or responder (sending sketches). This defines our role.
+     *
+     */
+    bool m_we_initiate;
+
+    /**
+     * TODO: These fields are public to ignore -Wunused-private-field. Make private once used in
+     * the following commits.
+     *
+     * These values are used to salt short IDs, which is necessary for transaction reconciliations.
+     */
+    uint64_t m_k0, m_k1;
+
+    TxReconciliationState(bool we_initiate, uint64_t k0, uint64_t k1) : m_we_initiate(we_initiate), m_k0(k0), m_k1(k1) {}
 };
 
 } // namespace
@@ -28,6 +63,9 @@ class TxReconciliationTracker::Impl
 private:
     mutable Mutex m_txreconciliation_mutex;
 
+    // Local protocol version
+    uint32_t m_recon_version;
+
     /**
      * Keeps track of txreconciliation states of eligible peers.
      * For pre-registered peers, the locally generated salt is stored.
@@ -37,7 +75,7 @@ class TxReconciliationTracker::Impl
     std::unordered_map<NodeId, std::variant<uint64_t, TxReconciliationState>> m_states GUARDED_BY(m_txreconciliation_mutex);
 
 public:
-    explicit Impl() {}
+    explicit Impl(uint32_t recon_version) : m_recon_version(recon_version) {}
 
     uint64_t PreRegisterPeer(NodeId peer_id) EXCLUSIVE_LOCKS_REQUIRED(!m_txreconciliation_mutex)
     {
@@ -55,6 +93,50 @@ class TxReconciliationTracker::Impl
         return local_salt;
     }
 
+    ReconciliationRegisterResult RegisterPeer(NodeId peer_id, bool is_peer_inbound, bool is_peer_recon_initiator,
+                                     bool is_peer_recon_responder, uint32_t peer_recon_version,
+                                     uint64_t remote_salt) EXCLUSIVE_LOCKS_REQUIRED(!m_txreconciliation_mutex)
+    {
+        AssertLockNotHeld(m_txreconciliation_mutex);
+        LOCK(m_txreconciliation_mutex);
+        auto recon_state = m_states.find(peer_id);
+
+        // A peer should be in the pre-registered state to proceed here.
+        if (recon_state == m_states.end()) return NOT_FOUND;
+        uint64_t* local_salt = std::get_if<uint64_t>(&recon_state->second);
+        // A peer is already registered. This should be checked by the caller.
+        Assume(local_salt);
+
+        // If the peer supports the version which is lower than ours, we downgrade to the version
+        // it supports. For now, this only guarantees that nodes with future reconciliation
+        // versions have the choice of reconciling with this current version. However, they also
+        // have the choice to refuse supporting reconciliations if the common version is not
+        // satisfactory (e.g. too low).
+        const uint32_t recon_version{std::min(peer_recon_version, m_recon_version)};
+        // v1 is the lowest version, so suggesting something below must be a protocol violation.
+        if (recon_version < 1) return PROTOCOL_VIOLATION;
+
+        // Must match SENDTXRCNCL logic.
+        const bool they_initiate = is_peer_recon_initiator && is_peer_inbound;
+        const bool we_initiate = !is_peer_inbound && is_peer_recon_responder;
+
+        // If we ever announce support for both requesting and responding, this will need
+        // tie-breaking. For now, this is mutually exclusive because both are based on the
+        // inbound flag.
+        assert(!(they_initiate && we_initiate));
+
+        // The peer set both flags to false, we treat it as a protocol violation.
+        if (!(they_initiate || we_initiate)) return PROTOCOL_VIOLATION;
+
+        LogPrintLevel(BCLog::TXRECONCILIATION, BCLog::Level::Debug, "Register peer=%d with the following params: " /* Continued */
+                                                                    "we_initiate=%i, they_initiate=%i.\n",
+                      peer_id, we_initiate, they_initiate);
+
+        const uint256 full_salt{ComputeSalt(*local_salt, remote_salt)};
+        recon_state->second = TxReconciliationState(we_initiate, full_salt.GetUint64(0), full_salt.GetUint64(1));
+        return SUCCESS;
+    }
+
     void ForgetPeer(NodeId peer_id) EXCLUSIVE_LOCKS_REQUIRED(!m_txreconciliation_mutex)
     {
         AssertLockNotHeld(m_txreconciliation_mutex);
@@ -74,7 +156,7 @@ class TxReconciliationTracker::Impl
     }
 };
 
-TxReconciliationTracker::TxReconciliationTracker() : m_impl{std::make_unique<TxReconciliationTracker::Impl>()} {}
+TxReconciliationTracker::TxReconciliationTracker(uint32_t recon_version) : m_impl{std::make_unique<TxReconciliationTracker::Impl>(recon_version)} {}
 
 TxReconciliationTracker::~TxReconciliationTracker() = default;
 
@@ -83,6 +165,14 @@ uint64_t TxReconciliationTracker::PreRegisterPeer(NodeId peer_id)
     return m_impl->PreRegisterPeer(peer_id);
 }
 
+ReconciliationRegisterResult TxReconciliationTracker::RegisterPeer(NodeId peer_id, bool is_peer_inbound,
+                                                          bool is_peer_recon_initiator, bool is_peer_recon_responder,
+                                                          uint32_t peer_recon_version, uint64_t remote_salt)
+{
+    return m_impl->RegisterPeer(peer_id, is_peer_inbound, is_peer_recon_initiator, is_peer_recon_responder,
+                                peer_recon_version, remote_salt);
+}
+
 void TxReconciliationTracker::ForgetPeer(NodeId peer_id)
 {
     m_impl->ForgetPeer(peer_id);

src/node/txreconciliation.h

@@ -16,6 +16,12 @@ static constexpr bool DEFAULT_TXRECONCILIATION_ENABLE{false};
 /** Supported transaction reconciliation protocol version */
 static constexpr uint32_t TXRECONCILIATION_VERSION{1};
 
+enum ReconciliationRegisterResult {
+    NOT_FOUND = 0,
+    SUCCESS = 1,
+    PROTOCOL_VIOLATION = 2,
+};
+
 /**
  * Transaction reconciliation is a way for nodes to efficiently announce transactions.
  * This object keeps track of all txreconciliation-related communications with the peers.
@@ -50,7 +56,7 @@ class TxReconciliationTracker
     const std::unique_ptr<Impl> m_impl;
 
 public:
-    explicit TxReconciliationTracker();
+    explicit TxReconciliationTracker(uint32_t recon_version);
     ~TxReconciliationTracker();
 
     /**
@@ -62,6 +68,13 @@ class TxReconciliationTracker
      */
     uint64_t PreRegisterPeer(NodeId peer_id);
 
+    /**
+     * Step 0. Once the peer agreed to reconcile txs with us, generate the state required to track
+     * ongoing reconciliations. Must be called only after pre-registering the peer and only once.
+     */
+    ReconciliationRegisterResult RegisterPeer(NodeId peer_id, bool is_peer_inbound, bool is_peer_recon_initiator,
+                                     bool is_peer_recon_responder, uint32_t peer_recon_version, uint64_t remote_salt);
+
     /**
      * Attempts to forget txreconciliation-related state of the peer (if we previously stored any).
      * After this, we won't be able to reconcile transactions with the peer.