Skip to content

Commit

Permalink
Merge #6333: backport: merge bitcoin#23496, bitcoin#26272, bitcoin#23443
Browse files Browse the repository at this point in the history
, bitcoin#26381, bitcoin#26396, bitcoin#26448, bitcoin#26359, bitcoin#26686, bitcoin#23670, partial bitcoin#19953 (Erlay support)

cc98f9e merge bitcoin#23670: Build minisketch test in make check, not in make (Kittywhiskers Van Gogh)
606a444 merge bitcoin#26686: Enable erlay setting in process_message(s) targets (Kittywhiskers Van Gogh)
38a16a2 merge bitcoin#26359: Erlay support signaling follow-ups (Kittywhiskers Van Gogh)
b55a6f7 merge bitcoin#26448: fix intermittent failure in p2p_sendtxrcncl.py (Kittywhiskers Van Gogh)
36be978 merge bitcoin#26396: Avoid SetTxRelay for feeler connections (Kittywhiskers Van Gogh)
62dc9cb merge bitcoin#26381: Fix intermittent issue in p2p_sendtxrcncl.py (Kittywhiskers Van Gogh)
6a7868d merge bitcoin#23443: Erlay support signaling (Kittywhiskers Van Gogh)
fdc3c07 partial bitcoin#19953: Implement BIP 340-342 validation (Kittywhiskers Van Gogh)
477157d merge bitcoin#26272: Prevent UB in `minisketch_tests.cpp` (Kittywhiskers Van Gogh)
0cf7401 merge bitcoin#23496: Add minisketch fuzz test (Kittywhiskers Van Gogh)
49ef53c merge bitcoin#23491: Move minisketchwrapper to src/node (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Dependent on #6332
  * Dependent on #6365
  * Erlay requires nodes to be `WTXIDRELAY` (as defined by BIP-339, [source](https://github.com/bitcoin/bips/blob/17c04f9fa1ecae173d6864b65717e13dfc1880af/bip-0339.mediawiki))-capable ([source](https://github.com/bitcoin/bips/blob/17c04f9fa1ecae173d6864b65717e13dfc1880af/bip-0330.mediawiki#sendtxrcncl)) as prior to `WTXIDRELAY` adoption, TXIDs of SegWit transactions didn't include the witness data in the hash, which meant, the witness data was malleable ([source](https://bitcoin.stackexchange.com/a/107394)), which would be a relevant factor when you are building out a reconciliation system where you need TXIDs to authoritatively identify a transaction's contents.

    As Dash _doesn't_ support SegWit, this requirement can be dispensed with. It has instead been replaced with checking if the node is running Dash Core v22 or above to retain the underlying test logic (but this can also be dispensed with as `SENDTXRCNCL`  will simply be ignored by older nodes and major releases are _generally_ mandatory upgrades anyways)

  ## Breaking Changes

  None expected.

  ## Checklist

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  PastaPastaPasta:
    utACK cc98f9e
  UdjinM6:
    utACK cc98f9e

Tree-SHA512: fe985f4df6c96c0a7b984882815a1bce7f23c54370198d099e41a59ac4c46c283a2b8dd95f5c8fc12eb1dc1330c4e5c21626b76d33d83d395326e8eb19d564ce
  • Loading branch information
PastaPastaPasta committed Oct 28, 2024
2 parents 25c3355 + cc98f9e commit 75fd7b5
Show file tree
Hide file tree
Showing 25 changed files with 796 additions and 14 deletions.
7 changes: 5 additions & 2 deletions src/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ noinst_LTLIBRARIES =

bin_PROGRAMS =
noinst_PROGRAMS =
check_PROGRAMS =
TESTS =
BENCHMARKS =

Expand Down Expand Up @@ -258,7 +259,6 @@ BITCOIN_CORE_H = \
memusage.h \
merkleblock.h \
messagesigner.h \
minisketchwrapper.h \
net.h \
net_permissions.h \
net_processing.h \
Expand All @@ -275,8 +275,10 @@ BITCOIN_CORE_H = \
node/context.h \
node/eviction.h \
node/miner.h \
node/minisketchwrapper.h \
node/psbt.h \
node/transaction.h \
node/txreconciliation.h \
node/ui_interface.h \
node/utxo_snapshot.h \
noui.h \
Expand Down Expand Up @@ -494,7 +496,6 @@ libbitcoin_server_a_SOURCES = \
masternode/payments.cpp \
masternode/sync.cpp \
masternode/utils.cpp \
minisketchwrapper.cpp \
net.cpp \
netfulfilledman.cpp \
netgroup.cpp \
Expand All @@ -507,8 +508,10 @@ libbitcoin_server_a_SOURCES = \
node/eviction.cpp \
node/interfaces.cpp \
node/miner.cpp \
node/minisketchwrapper.cpp \
node/psbt.cpp \
node/transaction.cpp \
node/txreconciliation.cpp \
node/ui_interface.cpp \
noui.cpp \
policy/fees.cpp \
Expand Down
2 changes: 1 addition & 1 deletion src/Makefile.minisketch.include
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ if ENABLE_TESTS
if !ENABLE_FUZZ
MINISKETCH_TEST = minisketch/test
TESTS += $(MINISKETCH_TEST)
noinst_PROGRAMS += $(MINISKETCH_TEST)
check_PROGRAMS += $(MINISKETCH_TEST)

minisketch_test_SOURCES = $(MINISKETCH_TEST_SOURCES_INT)
minisketch_test_CPPFLAGS = $(AM_CPPFLAGS) $(LIBMINISKETCH_CPPFLAGS)
Expand Down
3 changes: 3 additions & 0 deletions src/Makefile.test.include
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ FUZZ_SUITE_LD_COMMON = \
$(LIBLEVELDB_SSE42) \
$(LIBMEMENV) \
$(LIBSECP256K1) \
$(MINISKETCH_LIBS) \
$(EVENT_LIBS) \
$(EVENT_PTHREADS_LIBS) \
$(GMP_LIBS) \
Expand Down Expand Up @@ -173,6 +174,7 @@ BITCOIN_TESTS =\
test/torcontrol_tests.cpp \
test/transaction_tests.cpp \
test/txindex_tests.cpp \
test/txreconciliation_tests.cpp \
test/txvalidation_tests.cpp \
test/txvalidationcache_tests.cpp \
test/uint256_tests.cpp \
Expand Down Expand Up @@ -289,6 +291,7 @@ test_fuzz_fuzz_SOURCES = \
test/fuzz/locale.cpp \
test/fuzz/merkleblock.cpp \
test/fuzz/message.cpp \
test/fuzz/minisketch.cpp \
test/fuzz/muhash.cpp \
test/fuzz/multiplication_overflow.cpp \
test/fuzz/net.cpp \
Expand Down
10 changes: 10 additions & 0 deletions src/hash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <crypto/common.h>
#include <crypto/hmac_sha512.h>

#include <string>

inline uint32_t ROTL32(uint32_t x, int8_t r)
{
Expand Down Expand Up @@ -84,3 +85,12 @@ uint256 SHA256Uint256(const uint256& input)
CSHA256().Write(input.begin(), 32).Finalize(result.begin());
return result;
}

CHashWriter TaggedHash(const std::string& tag)
{
CHashWriter writer(SER_GETHASH, 0);
uint256 taghash;
CSHA256().Write((const unsigned char*)tag.data(), tag.size()).Finalize(taghash.begin());
writer << taghash << taghash;
return writer;
}
9 changes: 9 additions & 0 deletions src/hash.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <uint256.h>
#include <version.h>

#include <string>
#include <vector>

typedef uint256 ChainCode;
Expand Down Expand Up @@ -241,4 +242,12 @@ unsigned int MurmurHash3(unsigned int nHashSeed, Span<const unsigned char> vData

void BIP32Hash(const ChainCode &chainCode, unsigned int nChild, unsigned char header, const unsigned char data[32], unsigned char output[64]);

/** Return a CHashWriter primed for tagged hashes (as specified in BIP 340).
*
* The returned object will have SHA256(tag) written to it twice (= 64 bytes).
* A tagged hash can be computed by feeding the message into this object, and
* then calling CHashWriter::GetSHA256().
*/
CHashWriter TaggedHash(const std::string& tag);

#endif // BITCOIN_HASH_H
2 changes: 2 additions & 0 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include <node/blockstorage.h>
#include <node/context.h>
#include <node/ui_interface.h>
#include <node/txreconciliation.h>
#include <policy/feerate.h>
#include <policy/fees.h>
#include <policy/policy.h>
Expand Down Expand Up @@ -578,6 +579,7 @@ void SetupServerArgs(ArgsManager& argsman)
argsman.AddArg("-v2transport", strprintf("Support v2 transport (default: %u)", DEFAULT_V2_TRANSPORT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
argsman.AddArg("-peerblockfilters", strprintf("Serve compact block filters to peers per BIP 157 (default: %u)", DEFAULT_PEERBLOCKFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
argsman.AddArg("-peerbloomfilters", strprintf("Support filtering of blocks and transaction with bloom filters (default: %u)", DEFAULT_PEERBLOOMFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
argsman.AddArg("-txreconciliation", strprintf("Enable transaction reconciliations per BIP 330 (default: %d)", DEFAULT_TXRECONCILIATION_ENABLE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CONNECTION);
argsman.AddArg("-peertimeout=<n>", strprintf("Specify a p2p connection timeout delay in seconds. After connecting to a peer, wait this amount of time before considering disconnection based on inactivity (minimum: 1, default: %d)", DEFAULT_PEER_CONNECT_TIMEOUT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
argsman.AddArg("-permitbaremultisig", strprintf("Relay non-P2SH multisig (default: %u)", DEFAULT_PERMIT_BAREMULTISIG), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
argsman.AddArg("-port=<port>", strprintf("Listen for connections on <port>. Nodes not using the default ports (default: %u, testnet: %u, regtest: %u) are unlikely to get incoming connections. Not relevant for I2P (see doc/i2p.md).", defaultChainParams->GetDefaultPort(), testnetChainParams->GetDefaultPort(), regtestChainParams->GetDefaultPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION);
Expand Down
1 change: 1 addition & 0 deletions src/logging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ const CLogCategoryDesc LogCategories[] =
{BCLog::I2P, "i2p"},
{BCLog::IPC, "ipc"},
{BCLog::LOCK, "lock"},
{BCLog::TXRECONCILIATION, "txreconciliation"},
{BCLog::ALL, "1"},
{BCLog::ALL, "all"},

Expand Down
1 change: 1 addition & 0 deletions src/logging.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ namespace BCLog {
I2P = (1 << 22),
IPC = (1 << 23),
LOCK = (1 << 24),
TXRECONCILIATION = (1 << 27),

//Start Dash
CHAINLOCKS = ((uint64_t)1 << 32),
Expand Down
90 changes: 88 additions & 2 deletions src/net_processing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <netbase.h>
#include <net_types.h>
#include <node/blockstorage.h>
#include <node/txreconciliation.h>
#include <policy/policy.h>
#include <primitives/block.h>
#include <primitives/transaction.h>
Expand Down Expand Up @@ -730,6 +731,7 @@ class PeerManagerImpl final : public PeerManager
BanMan* const m_banman;
ChainstateManager& m_chainman;
CTxMemPool& m_mempool;
std::unique_ptr<TxReconciliationTracker> m_txreconciliation;
const std::unique_ptr<CDeterministicMNManager>& m_dmnman;
const std::unique_ptr<CJContext>& m_cj_ctx;
const std::unique_ptr<LLMQContext>& m_llmq_ctx;
Expand Down Expand Up @@ -1633,6 +1635,7 @@ void PeerManagerImpl::FinalizeNode(const CNode& node) {
mapBlocksInFlight.erase(entry.pindex->GetBlockHash());
}
WITH_LOCK(g_cs_orphans, m_orphanage.EraseForPeer(nodeid));
if (m_txreconciliation) m_txreconciliation->ForgetPeer(nodeid);
m_num_preferred_download_peers -= state->fPreferredDownload;
m_peers_downloading_from -= (state->nBlocksInFlight != 0);
assert(m_peers_downloading_from >= 0);
Expand Down Expand Up @@ -1935,6 +1938,11 @@ PeerManagerImpl::PeerManagerImpl(const CChainParams& chainparams, CConnman& conn
m_mn_activeman(mn_activeman),
m_ignore_incoming_txs(ignore_incoming_txs)
{
// 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>(TXRECONCILIATION_VERSION);
}
}

void PeerManagerImpl::StartScheduledTasks(CScheduler& scheduler)
Expand Down Expand Up @@ -3509,8 +3517,6 @@ void PeerManagerImpl::ProcessMessage(
m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::SENDADDRV2));
}

m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::VERACK));

pfrom.m_has_all_wanted_services = HasAllDesirableServiceFlags(nServices);
peer->m_their_services = nServices;
pfrom.SetAddrLocal(addrMe);
Expand All @@ -3536,6 +3542,22 @@ void PeerManagerImpl::ProcessMessage(
if (fRelay) pfrom.m_relays_txs = true;
}

if (greatest_common_version >= INCREASE_MAX_HEADERS2_VERSION && m_txreconciliation) {
// Per BIP-330, we announce txreconciliation support if:
// - protocol version per the peer's VERSION message supports INCREASE_MAX_HEADERS2_VERSION;
// - transaction relay is supported per the peer's VERSION message (see m_relays_txs);
// - this is not a block-relay-only connection and not a feeler (see m_relays_txs);
// - this is not an addr fetch connection;
// - we are not in -blocksonly mode.
if (pfrom.m_relays_txs && !pfrom.IsAddrFetchConn() && !m_ignore_incoming_txs) {
const uint64_t recon_salt = m_txreconciliation->PreRegisterPeer(pfrom.GetId());
m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::SENDTXRCNCL,
TXRECONCILIATION_VERSION, recon_salt));
}
}

m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::VERACK));

// Potentially mark this peer as a preferred download peer.
{
LOCK(cs_main);
Expand Down Expand Up @@ -3677,6 +3699,15 @@ void PeerManagerImpl::ProcessMessage(
}
}

if (m_txreconciliation) {
if (pfrom.nVersion < INCREASE_MAX_HEADERS2_VERSION || !m_txreconciliation->IsPeerRegistered(pfrom.GetId())) {
// We could have optimistically pre-registered/registered the peer. In that case,
// we should forget about the reconciliation state here if the node version is below
// our minimum supported version.
m_txreconciliation->ForgetPeer(pfrom.GetId());
}
}

pfrom.fSuccessfullyConnected = true;
return;
}
Expand Down Expand Up @@ -3727,6 +3758,61 @@ void PeerManagerImpl::ProcessMessage(
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) {
LogPrint(BCLog::NET, "sendtxrcncl from peer=%d ignored, as our node does not have txreconciliation enabled\n", pfrom.GetId());
return;
}

if (pfrom.fSuccessfullyConnected) {
LogPrint(BCLog::NET, "sendtxrcncl received after verack from peer=%d; disconnecting\n", pfrom.GetId());
pfrom.fDisconnect = true;
return;
}

// Peer must not offer us reconciliations if we specified no tx relay support in VERSION.
if (RejectIncomingTxs(pfrom)) {
LogPrint(BCLog::NET, "sendtxrcncl received from peer=%d to which we indicated no tx relay; disconnecting\n", pfrom.GetId());
pfrom.fDisconnect = true;
return;
}

// Peer must not offer us reconciliations if they specified no tx relay support in VERSION.
// This flag might also be false in other cases, but the RejectIncomingTxs check above
// eliminates them, so that this flag fully represents what we are looking for.
if (!pfrom.m_relays_txs) {
LogPrint(BCLog::NET, "sendtxrcncl received from peer=%d which indicated no tx relay to us; disconnecting\n", pfrom.GetId());
pfrom.fDisconnect = true;
return;
}

uint32_t peer_txreconcl_version;
uint64_t remote_salt;
vRecv >> peer_txreconcl_version >> remote_salt;

const ReconciliationRegisterResult result = m_txreconciliation->RegisterPeer(pfrom.GetId(), pfrom.IsInboundConn(),
peer_txreconcl_version, remote_salt);
switch (result) {
case ReconciliationRegisterResult::NOT_FOUND:
LogPrint(BCLog::NET, "Ignore unexpected txreconciliation signal from peer=%d\n", pfrom.GetId());
break;
case ReconciliationRegisterResult::SUCCESS:
break;
case ReconciliationRegisterResult::ALREADY_REGISTERED:
LogPrint(BCLog::NET, "txreconciliation protocol violation from peer=%d (sendtxrcncl received from already registered peer); disconnecting\n", pfrom.GetId());
pfrom.fDisconnect = true;
return;
case ReconciliationRegisterResult::PROTOCOL_VIOLATION:
LogPrint(BCLog::NET, "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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <minisketchwrapper.h>
#include <node/minisketchwrapper.h>

#include <logging.h>
#include <util/time.h>
Expand Down
6 changes: 3 additions & 3 deletions src/minisketchwrapper.h → src/node/minisketchwrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#ifndef BITCOIN_MINISKETCHWRAPPER_H
#define BITCOIN_MINISKETCHWRAPPER_H
#ifndef BITCOIN_NODE_MINISKETCHWRAPPER_H
#define BITCOIN_NODE_MINISKETCHWRAPPER_H

#include <minisketch.h>
#include <cstddef>
Expand All @@ -14,4 +14,4 @@ Minisketch MakeMinisketch32(size_t capacity);
/** Wrapper around Minisketch::CreateFP. */
Minisketch MakeMinisketch32FP(size_t max_elements, uint32_t fpbits);

#endif // BITCOIN_DBWRAPPER_H
#endif // BITCOIN_NODE_MINISKETCHWRAPPER_H
Loading

0 comments on commit 75fd7b5

Please sign in to comment.