Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/instantsend/instantsend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,13 @@ Uint256HashSet GetIdsFromLockable(const std::vector<T>& 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<T, COutPoint>) {
ret.emplace(instantsend::GenInputLockRequestId(in));
} else if constexpr (std::is_same_v<T, CTxIn>) {
ret.emplace(instantsend::GenInputLockRequestId(in.prevout));
} else {
assert(false);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should be static_assert here, because type is known on compilation time here

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the assert necessary? requires already constrains types, more restrictive than open ended template.

}
}
return ret;
}
Expand Down
7 changes: 2 additions & 5 deletions src/instantsend/lock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,8 @@ bool InstantSendLock::TriviallyValid() const
return inputs_set.size() == inputs.size();
}

template <typename T>
uint256 GenInputLockRequestId(const T& val)
uint256 GenInputLockRequestId(const COutPoint& outpoint)
{
return ::SerializeHash(std::make_pair(INPUTLOCK_REQUESTID_PREFIX, val));
return ::SerializeHash(std::make_pair(INPUTLOCK_REQUESTID_PREFIX, outpoint));
Comment thread
UdjinM6 marked this conversation as resolved.
}
template uint256 GenInputLockRequestId(const COutPoint& val);
template uint256 GenInputLockRequestId(const CTxIn& val);
} // namespace instantsend
3 changes: 1 addition & 2 deletions src/instantsend/lock.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ struct InstantSendLock {
bool TriviallyValid() const;
};

template <typename T>
uint256 GenInputLockRequestId(const T& val);
uint256 GenInputLockRequestId(const COutPoint& outpoint);

using InstantSendLockPtr = std::shared_ptr<InstantSendLock>;
} // namespace instantsend
Expand Down
4 changes: 2 additions & 2 deletions src/instantsend/signing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
102 changes: 102 additions & 0 deletions src/test/evo_islock_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Comment thread
UdjinM6 marked this conversation as resolved.
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()
Loading