From f43be22c7eb503ccf83aee40896683376b99ce20 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 20 Oct 2025 03:27:13 +0530 Subject: [PATCH 1/4] fix: remove incorrect request generation with bad serialization hash We should not be generating the hash using a CTxIn, only a COutPoint --- src/instantsend/instantsend.cpp | 6 +++++- src/instantsend/lock.cpp | 5 +---- src/instantsend/lock.h | 3 +-- src/instantsend/signing.cpp | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/instantsend/instantsend.cpp b/src/instantsend/instantsend.cpp index b3cb1d4b677b..f2d186a27b5f 100644 --- a/src/instantsend/instantsend.cpp +++ b/src/instantsend/instantsend.cpp @@ -44,7 +44,11 @@ Uint256HashSet GetIdsFromLockable(const std::vector& vec) if (vec.empty()) return ret; ret.reserve(vec.size()); for (const auto& in : vec) { - ret.emplace(instantsend::GenInputLockRequestId(in)); + if constexpr (std::is_same_v) { + ret.emplace(instantsend::GenInputLockRequestId(in.prevout)); + } else { + ret.emplace(instantsend::GenInputLockRequestId(in)); + } } return ret; } diff --git a/src/instantsend/lock.cpp b/src/instantsend/lock.cpp index 10ef0afc3c2d..e6d6d0ab50b5 100644 --- a/src/instantsend/lock.cpp +++ b/src/instantsend/lock.cpp @@ -38,11 +38,8 @@ bool InstantSendLock::TriviallyValid() const return inputs_set.size() == inputs.size(); } -template -uint256 GenInputLockRequestId(const T& val) +uint256 GenInputLockRequestId(const COutPoint& val) { return ::SerializeHash(std::make_pair(INPUTLOCK_REQUESTID_PREFIX, val)); } -template uint256 GenInputLockRequestId(const COutPoint& val); -template uint256 GenInputLockRequestId(const CTxIn& val); } // namespace instantsend diff --git a/src/instantsend/lock.h b/src/instantsend/lock.h index 2786ba1ff7cf..2f775c46a763 100644 --- a/src/instantsend/lock.h +++ b/src/instantsend/lock.h @@ -40,8 +40,7 @@ struct InstantSendLock { bool TriviallyValid() const; }; -template -uint256 GenInputLockRequestId(const T& val); +uint256 GenInputLockRequestId(const COutPoint& val); using InstantSendLockPtr = std::shared_ptr; } // namespace instantsend diff --git a/src/instantsend/signing.cpp b/src/instantsend/signing.cpp index 93d2320f7fa3..fa6d40f4623e 100644 --- a/src/instantsend/signing.cpp +++ b/src/instantsend/signing.cpp @@ -295,7 +295,7 @@ bool InstantSendSigner::TrySignInputLocks(const CTransaction& tx, bool fRetroact size_t alreadyVotedCount = 0; for (const auto& in : tx.vin) { - auto id = GenInputLockRequestId(in); + auto id = GenInputLockRequestId(in.prevout); ids.emplace_back(id); uint256 otherTxHash; @@ -344,7 +344,7 @@ void InstantSendSigner::TrySignInstantSendLock(const CTransaction& tx) const auto llmqType = Params().GetConsensus().llmqTypeDIP0024InstantSend; for (const auto& in : tx.vin) { - auto id = GenInputLockRequestId(in); + auto id = GenInputLockRequestId(in.prevout); if (!m_sigman.HasRecoveredSig(llmqType, id, tx.GetHash())) { return; } From 58f671c4c2741e9a80f4335dec1a3027304c842c Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Mon, 20 Oct 2025 06:04:43 +0300 Subject: [PATCH 2/4] refactor: trivial s/val/outpoint/ --- src/instantsend/lock.cpp | 4 ++-- src/instantsend/lock.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/instantsend/lock.cpp b/src/instantsend/lock.cpp index e6d6d0ab50b5..f0f2c1d1508d 100644 --- a/src/instantsend/lock.cpp +++ b/src/instantsend/lock.cpp @@ -38,8 +38,8 @@ bool InstantSendLock::TriviallyValid() const return inputs_set.size() == inputs.size(); } -uint256 GenInputLockRequestId(const COutPoint& val) +uint256 GenInputLockRequestId(const COutPoint& outpoint) { - return ::SerializeHash(std::make_pair(INPUTLOCK_REQUESTID_PREFIX, val)); + return ::SerializeHash(std::make_pair(INPUTLOCK_REQUESTID_PREFIX, outpoint)); } } // namespace instantsend diff --git a/src/instantsend/lock.h b/src/instantsend/lock.h index 2f775c46a763..b0536e8421bc 100644 --- a/src/instantsend/lock.h +++ b/src/instantsend/lock.h @@ -40,7 +40,7 @@ struct InstantSendLock { bool TriviallyValid() const; }; -uint256 GenInputLockRequestId(const COutPoint& val); +uint256 GenInputLockRequestId(const COutPoint& outpoint); using InstantSendLockPtr = std::shared_ptr; } // namespace instantsend From 54563547c397154b61c8ab09001de08fb694a7af Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Mon, 20 Oct 2025 14:30:38 +0300 Subject: [PATCH 3/4] fix: trivial safety belt --- src/instantsend/instantsend.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/instantsend/instantsend.cpp b/src/instantsend/instantsend.cpp index f2d186a27b5f..34537ffb64c8 100644 --- a/src/instantsend/instantsend.cpp +++ b/src/instantsend/instantsend.cpp @@ -44,10 +44,12 @@ Uint256HashSet GetIdsFromLockable(const std::vector& vec) if (vec.empty()) return ret; ret.reserve(vec.size()); for (const auto& in : vec) { - if constexpr (std::is_same_v) { + if constexpr (std::is_same_v) { + ret.emplace(instantsend::GenInputLockRequestId(in)); + } else if constexpr (std::is_same_v) { ret.emplace(instantsend::GenInputLockRequestId(in.prevout)); } else { - ret.emplace(instantsend::GenInputLockRequestId(in)); + assert(false); } } return ret; From a8506ec2be4c5e2137d165b478d6575787c4b08b Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Mon, 20 Oct 2025 14:42:47 +0300 Subject: [PATCH 4/4] test: add tests for `inlock` id generation --- src/test/evo_islock_tests.cpp | 102 ++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/src/test/evo_islock_tests.cpp b/src/test/evo_islock_tests.cpp index eb6767b6216d..7dc76475941d 100644 --- a/src/test/evo_islock_tests.cpp +++ b/src/test/evo_islock_tests.cpp @@ -107,4 +107,106 @@ BOOST_AUTO_TEST_CASE(deserialize_instantlock_from_realdata2) BOOST_CHECK_EQUAL(islock.sig.Get().ToString(), expectedSignatureStr); } +BOOST_AUTO_TEST_CASE(geninputlockrequestid_basic) +{ + // Test that GenInputLockRequestId generates consistent hashes for the same outpoint + const uint256 txHash = uint256S("0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"); + const uint32_t outputIndex = 5; + + COutPoint outpoint1(txHash, outputIndex); + COutPoint outpoint2(txHash, outputIndex); + + // Same outpoints should produce identical request IDs + const uint256 requestId1 = instantsend::GenInputLockRequestId(outpoint1); + const uint256 requestId2 = instantsend::GenInputLockRequestId(outpoint2); + + BOOST_CHECK(requestId1 == requestId2); + BOOST_CHECK(!requestId1.IsNull()); +} + +BOOST_AUTO_TEST_CASE(geninputlockrequestid_different_outpoints) +{ + // Test that different outpoints produce different request IDs + const uint256 txHash1 = uint256S("0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"); + const uint256 txHash2 = uint256S("0xfedcba0987654321fedcba0987654321fedcba0987654321fedcba0987654321"); + + COutPoint outpoint1(txHash1, 0); + COutPoint outpoint2(txHash2, 0); + COutPoint outpoint3(txHash1, 1); // Same hash, different index + + const uint256 requestId1 = instantsend::GenInputLockRequestId(outpoint1); + const uint256 requestId2 = instantsend::GenInputLockRequestId(outpoint2); + const uint256 requestId3 = instantsend::GenInputLockRequestId(outpoint3); + + // All should be different + BOOST_CHECK(requestId1 != requestId2); + BOOST_CHECK(requestId1 != requestId3); + BOOST_CHECK(requestId2 != requestId3); +} + +BOOST_AUTO_TEST_CASE(geninputlockrequestid_only_outpoint_matters) +{ + // Critical test: Verify that only the COutPoint is hashed, not scriptSig or nSequence + // This validates the fix where CTxIn was incorrectly used before + const uint256 txHash = uint256S("0xabcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890"); + const uint32_t outputIndex = 3; + + COutPoint outpoint(txHash, outputIndex); + + // Create two CTxIn objects with the same prevout but different scriptSig and nSequence + CTxIn txin1; + txin1.prevout = outpoint; + txin1.scriptSig = CScript() << OP_1 << OP_2; + txin1.nSequence = 0xFFFFFFFF; + + CTxIn txin2; + txin2.prevout = outpoint; + txin2.scriptSig = CScript() << OP_3 << OP_4 << OP_5; // Different scriptSig + txin2.nSequence = 0x12345678; // Different nSequence + + // The request IDs should be identical because they share the same prevout (COutPoint) + const uint256 requestId1 = instantsend::GenInputLockRequestId(txin1.prevout); + const uint256 requestId2 = instantsend::GenInputLockRequestId(txin2.prevout); + + BOOST_CHECK(requestId1 == requestId2); + + // Also verify against the direct outpoint + const uint256 requestId3 = instantsend::GenInputLockRequestId(outpoint); + BOOST_CHECK(requestId1 == requestId3); +} + +BOOST_AUTO_TEST_CASE(geninputlockrequestid_serialization_format) +{ + // Test that the serialization format is: SerializeHash(pair("inlock", outpoint)) + const uint256 txHash = uint256S("0x0000000000000000000000000000000000000000000000000000000000000001"); + const uint32_t outputIndex = 0; + + COutPoint outpoint(txHash, outputIndex); + + // Calculate the expected hash manually + const uint256 expectedHash = ::SerializeHash(std::make_pair(std::string_view("inlock"), outpoint)); + + // Get the actual hash from the function + const uint256 actualHash = instantsend::GenInputLockRequestId(outpoint); + + BOOST_CHECK(actualHash == expectedHash); +} + +BOOST_AUTO_TEST_CASE(geninputlockrequestid_edge_cases) +{ + // Test edge cases: null hash, max index + COutPoint nullOutpoint(uint256(), 0); + COutPoint maxIndexOutpoint(uint256::ONE, COutPoint::NULL_INDEX); + + const uint256 nullRequestId = instantsend::GenInputLockRequestId(nullOutpoint); + const uint256 maxIndexRequestId = instantsend::GenInputLockRequestId(maxIndexOutpoint); + + // Both should produce valid (non-null) request IDs + BOOST_CHECK(!nullRequestId.IsNull()); + BOOST_CHECK(!maxIndexRequestId.IsNull()); + + // And they should be different from each other + BOOST_CHECK(nullRequestId != maxIndexRequestId); +} + BOOST_AUTO_TEST_SUITE_END()