diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 43383681f9..02de4f78b8 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -704,6 +704,11 @@ void CInstantSendManager::ProcessMessage(CNode* pfrom, const std::string& strCom if (strCommand == NetMsgType::ISLOCK) { CInstantSendLock islock; vRecv >> islock; + if (islock.inputs.size() > CInstantSendLock::MAX_INPUTS) { + LOCK(cs_main); + Misbehaving(pfrom->id, 100); + return; + } ProcessMessageInstantSendLock(pfrom, islock, connman); } } @@ -728,6 +733,11 @@ void CInstantSendManager::ProcessMessageInstantSendLock(CNode* pfrom, const llmq if (pendingInstantSendLocks.count(hash)) { return; } + if (pendingInstantSendLocks.size() >= MAX_PENDING_INSTANTSEND_LOCKS) { + LogPrint("instantsend", "CInstantSendManager::%s -- pending islock queue full (%d), dropping islock=%s, peer=%d\n", + __func__, pendingInstantSendLocks.size(), hash.ToString(), pfrom->id); + return; + } LogPrint("instantsend", "CInstantSendManager::%s -- txid=%s, islock=%s: received islock, peer=%d\n", __func__, islock.txid.ToString(), hash.ToString(), pfrom->id); @@ -739,7 +749,7 @@ bool CInstantSendManager::PreVerifyInstantSendLock(NodeId nodeId, const llmq::CI { retBan = false; - if (islock.txid.IsNull() || islock.inputs.empty()) { + if (islock.txid.IsNull() || islock.inputs.empty() || islock.inputs.size() > CInstantSendLock::MAX_INPUTS) { retBan = true; return false; } @@ -761,7 +771,18 @@ bool CInstantSendManager::ProcessPendingInstantSendLocks() { LOCK(cs); - pend = std::move(pendingInstantSendLocks); + // Only process 32 locks at a time to avoid duplicate verification of recovered signatures which have been + // verified by CSigningManager in parallel. + const size_t maxCount = 32; + if (pendingInstantSendLocks.size() <= maxCount) { + pend = std::move(pendingInstantSendLocks); + } else { + while (pend.size() < maxCount) { + auto it = pendingInstantSendLocks.begin(); + pend.emplace(it->first, std::move(it->second)); + pendingInstantSendLocks.erase(it); + } + } } if (pend.empty()) { @@ -834,8 +855,10 @@ std::unordered_set CInstantSendManager::ProcessPendingInstantSendLocks( auto id = islock.GetRequestId(); - // no need to verify an ISLOCK if we already have verified the recovered sig that belongs to it - if (quorumSigningManager->HasRecoveredSig(llmqType, id, islock.txid)) { + // no need to verify an ISLOCK if we already have verified the exact recovered sig that belongs to it + CRecoveredSig recoveredSig; + if (quorumSigningManager->GetRecoveredSigForId(llmqType, id, recoveredSig) && + recoveredSig.msgHash == islock.txid && recoveredSig.sig == islock.sig) { continue; } diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index 2cc0722c7b..ea9bbadd16 100644 --- a/src/llmq/quorums_instantsend.h +++ b/src/llmq/quorums_instantsend.h @@ -8,8 +8,9 @@ #include "quorums_signing.h" #include "coins.h" -#include "unordered_lru_cache.h" +#include "consensus/consensus.h" #include "primitives/transaction.h" +#include "unordered_lru_cache.h" #include #include @@ -17,9 +18,14 @@ namespace llmq { +struct CInstantSendManagerTestAccess; + class CInstantSendLock { public: + // A valid transaction must fit in a block, and each input serializes to at least 41 bytes. + static constexpr size_t MAX_INPUTS{MAX_BLOCK_BASE_SIZE / 41}; + std::vector inputs; uint256 txid; CBLSLazySignature sig; @@ -74,7 +80,11 @@ class CInstantSendDb class CInstantSendManager : public CRecoveredSigsListener { + friend struct CInstantSendManagerTestAccess; + private: + static constexpr size_t MAX_PENDING_INSTANTSEND_LOCKS{1024}; + CCriticalSection cs; CInstantSendDb db; diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt index 6408280c81..44ef7b9f2c 100644 --- a/src/test/CMakeLists.txt +++ b/src/test/CMakeLists.txt @@ -94,6 +94,7 @@ add_executable(test_firo ${CMAKE_CURRENT_SOURCE_DIR}/evospork_tests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/evo_deterministicmns_tests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/evo_simplifiedmns_tests.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/quorums_instantsend_tests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/progpow_tests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/bls_tests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/sparkmessage_tests.cpp diff --git a/src/test/quorums_instantsend_tests.cpp b/src/test/quorums_instantsend_tests.cpp new file mode 100644 index 0000000000..2b175b713c --- /dev/null +++ b/src/test/quorums_instantsend_tests.cpp @@ -0,0 +1,190 @@ +// Copyright (c) 2026 The Firo developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "dbwrapper.h" +#include "llmq/quorums_instantsend.h" +#include "net.h" +#include "test/test_bitcoin.h" +#include "validation.h" + +#include + +namespace llmq +{ + +struct CInstantSendManagerTestAccess { + static bool PreVerify(CInstantSendManager& manager, const CInstantSendLock& islock, bool& ban) + { + return manager.PreVerifyInstantSendLock(1, islock, ban); + } + + static void ProcessMessage(CInstantSendManager& manager, CNode& peer, const CInstantSendLock& islock, CConnman& connman) + { + manager.ProcessMessageInstantSendLock(&peer, islock, connman); + } + + static size_t PendingCount(CInstantSendManager& manager) + { + LOCK(manager.cs); + return manager.pendingInstantSendLocks.size(); + } + + static void ProcessPending(CInstantSendManager& manager) + { + manager.ProcessPendingInstantSendLocks(); + } + + static void ProcessPending(CInstantSendManager& manager, NodeId nodeId, const CInstantSendLock& islock) + { + std::unordered_map, StaticSaltedHasher> pending; + pending.emplace(::SerializeHash(islock), std::make_pair(nodeId, islock)); + manager.ProcessPendingInstantSendLocks(0, pending, false); + } +}; + +} // namespace llmq + +namespace +{ + +uint256 HashFromNonce(size_t nonce) +{ + return uint256S(strprintf("%064x", static_cast(nonce))); +} + +llmq::CInstantSendLock MakeInstantSendLock(size_t nonce) +{ + llmq::CInstantSendLock islock; + islock.txid = HashFromNonce(nonce + 1); + islock.inputs.emplace_back(HashFromNonce(1), static_cast(nonce)); + return islock; +} + +struct InstantSendSetup : BasicTestingSetup { + CDBWrapper db; + llmq::CSigningManager signingManager; + llmq::CInstantSendManager manager; + CNode peer; + llmq::CSigningManager* previousSigningManager; + bool previousTxIndex; + + InstantSendSetup() : db(boost::filesystem::temp_directory_path() / boost::filesystem::unique_path(), 1 << 20, true, false), + signingManager(db, true), + manager(db), + peer(1, NODE_NETWORK, 0, INVALID_SOCKET, CAddress(), 0, 0, "", true), + previousSigningManager(llmq::quorumSigningManager), + previousTxIndex(fTxIndex) + { + llmq::quorumSigningManager = &signingManager; + fTxIndex = false; + g_connman = std::make_unique(0x1337, 0x1337); + } + + ~InstantSendSetup() + { + llmq::quorumSigningManager = previousSigningManager; + fTxIndex = previousTxIndex; + } +}; + +} // namespace + +BOOST_FIXTURE_TEST_SUITE(quorums_instantsend_tests, InstantSendSetup) + +BOOST_AUTO_TEST_CASE(input_limit_is_protocol_derived) +{ + llmq::CInstantSendLock islock; + islock.txid = HashFromNonce(1); + islock.inputs.reserve(llmq::CInstantSendLock::MAX_INPUTS + 1); + + BOOST_CHECK_EQUAL(llmq::CInstantSendLock::MAX_INPUTS, MAX_BLOCK_BASE_SIZE / 41); + for (size_t i = 0; i < llmq::CInstantSendLock::MAX_INPUTS - 1; ++i) { + islock.inputs.emplace_back(HashFromNonce(i + 1), 0); + } + + bool ban = false; + BOOST_CHECK(llmq::CInstantSendManagerTestAccess::PreVerify(manager, islock, ban)); + BOOST_CHECK(!ban); + + islock.inputs.emplace_back(HashFromNonce(llmq::CInstantSendLock::MAX_INPUTS), 0); + BOOST_CHECK(llmq::CInstantSendManagerTestAccess::PreVerify(manager, islock, ban)); + BOOST_CHECK(!ban); + + islock.inputs.emplace_back(HashFromNonce(llmq::CInstantSendLock::MAX_INPUTS + 1), 0); + BOOST_CHECK(!llmq::CInstantSendManagerTestAccess::PreVerify(manager, islock, ban)); + BOOST_CHECK(ban); +} + +BOOST_AUTO_TEST_CASE(pending_queue_and_processing_are_bounded) +{ + constexpr size_t maxPending{1024}; + constexpr size_t maxPerPass{32}; + + for (size_t i = 0; i < maxPending - 1; ++i) { + const auto islock = MakeInstantSendLock(i); + llmq::CInstantSendManagerTestAccess::ProcessMessage(manager, peer, islock, *g_connman); + } + BOOST_CHECK_EQUAL(llmq::CInstantSendManagerTestAccess::PendingCount(manager), maxPending - 1); + + auto islock = MakeInstantSendLock(maxPending - 1); + llmq::CInstantSendManagerTestAccess::ProcessMessage(manager, peer, islock, *g_connman); + BOOST_CHECK_EQUAL(llmq::CInstantSendManagerTestAccess::PendingCount(manager), maxPending); + + islock = MakeInstantSendLock(maxPending); + llmq::CInstantSendManagerTestAccess::ProcessMessage(manager, peer, islock, *g_connman); + BOOST_CHECK_EQUAL(llmq::CInstantSendManagerTestAccess::PendingCount(manager), maxPending); + + llmq::CInstantSendManagerTestAccess::ProcessPending(manager); + BOOST_CHECK_EQUAL(llmq::CInstantSendManagerTestAccess::PendingCount(manager), maxPending - maxPerPass); + + for (size_t i = maxPending; i < maxPending + maxPerPass; ++i) { + islock = MakeInstantSendLock(i); + llmq::CInstantSendManagerTestAccess::ProcessMessage(manager, peer, islock, *g_connman); + } + BOOST_CHECK_EQUAL(llmq::CInstantSendManagerTestAccess::PendingCount(manager), maxPending); + + islock = MakeInstantSendLock(maxPending + maxPerPass); + llmq::CInstantSendManagerTestAccess::ProcessMessage(manager, peer, islock, *g_connman); + BOOST_CHECK_EQUAL(llmq::CInstantSendManagerTestAccess::PendingCount(manager), maxPending); +} + +BOOST_AUTO_TEST_CASE(recovered_signature_shortcut_requires_exact_signature) +{ + auto islock = MakeInstantSendLock(1); + CBLSSecretKey recoveredKey; + recoveredKey.MakeNewKey(); + islock.sig.Set(recoveredKey.Sign(HashFromNonce(100))); + + const auto llmqType = Params().GetConsensus().llmqForInstantSend; + llmq::CRecoveredSig recoveredSig; + recoveredSig.llmqType = llmqType; + recoveredSig.quorumHash = HashFromNonce(101); + recoveredSig.id = islock.GetRequestId(); + recoveredSig.msgHash = islock.txid; + recoveredSig.sig = islock.sig; + recoveredSig.UpdateHash(); + + llmq::CRecoveredSigsDb recoveredSigsDb(db); + recoveredSigsDb.WriteRecoveredSig(recoveredSig); + + const auto islockHash = ::SerializeHash(islock); + llmq::CInstantSendManagerTestAccess::ProcessPending(manager, peer.GetId(), islock); + BOOST_CHECK_EQUAL(manager.GetInstantSendLockCount(), 1); + + llmq::CInstantSendLock storedLock; + BOOST_CHECK(db.Read(std::make_tuple(std::string("is_i"), islockHash), storedLock)); + + auto alteredIslock = islock; + CBLSSecretKey alteredKey; + alteredKey.MakeNewKey(); + alteredIslock.sig.Set(alteredKey.Sign(HashFromNonce(102))); + const auto alteredIslockHash = ::SerializeHash(alteredIslock); + BOOST_REQUIRE(islockHash != alteredIslockHash); + + llmq::CInstantSendManagerTestAccess::ProcessPending(manager, peer.GetId(), alteredIslock); + BOOST_CHECK_EQUAL(manager.GetInstantSendLockCount(), 1); + BOOST_CHECK(!db.Read(std::make_tuple(std::string("is_i"), alteredIslockHash), storedLock)); +} + +BOOST_AUTO_TEST_SUITE_END()