Merge bitcoin/bitcoin#36163: test: Add coverage for unsatisfiable locktime combination in PSBT `ComputeTimeLock()`
What the developer wrote
Authored by merge-script
Merge bitcoin/bitcoin#36163: test: Add coverage for unsatisfiable locktime combination in PSBT `ComputeTimeLock()`
69a640e05ec072c8bdce8d52dfd982df274e8262 test: Add coverage for unsatisfiable locktime combination in PSBT ComputeTimeLock (nebula-21)
Pull request description:
This PR adds a test case to `psbt2_timelock_test` covering an unsatisfiable locktime combination in `PartiallySignedTransaction::ComputeTimeLock()`.
When different PSBT v2 inputs specify their own timelock requirement, `ComputeTimeLock()` needs to reconcile all of those into a single locktime for the whole transaction. To reconcile this locktime, all the inputs locktimes need to be height or time-based, but not a mix of them.
The existing test already covers this failure when the input #0 is height-based and a later input is time-based, returning `std::nullopt`.
This PR adds the other case when the input #0 is time-based and a later input is height-based, returning `std::nullopt`.
I've basically swapped the PSBT inputs from the already existing case to cover this one.
ACKs for top commit:
sedited:
ACK 69a640e05ec072c8bdce8d52dfd982df274e8262
Tree-SHA512: e7a7556df3bd278686a2d53a11b228f6f8c0e8dda79f050bad83dea89824de2e3766518fa3450b3722be86927fb4e6b9061dcd2cdb45010080b37a4fa2baffe0
What changed, and why it matters
This commit only adds a new test case to Bitcoin Core's test suite. It checks that a function called ComputeTimeLock() correctly returns 'no valid locktime' when a transaction's inputs mix a time-based lock with a block-height-based lock in a specific order. There is no change to production code, no bug fix, and no security patch.
No action required; this is a benign test-only commit.
Security signals we found
No production code changes
Pure test-coverage addition
No functional bug fix or hardening
Evidence from the diff
The commit adds one line to src/test/psbt_tests.cpp in the psbt2_timelock_test BOOST_AUTO_TEST_CASE. It calls CheckTimeLock() with a base64 PSBT and std::nullopt, mirroring an existing test but swapping which input is time-based and which is height-based. The goal is to ensure PartiallySignedTransaction::ComputeTimeLock() rejects mixed time/height locktime requirements regardless of which input appears first. No source code outside the test file is modified.
Changed components
src/test/psbt_tests.cppInspect captured patch +1 / −0
### src/test/psbt_tests.cpp
@@ -60,6 +60,7 @@ BOOST_AUTO_TEST_CASE(psbt2_timelock_test)
CheckTimeLock("cHNidP8BAgQCAAAAAQMEAAAAAAEEAQIBBQEBAfsEAgAAAAABDiAPdY2/vU2nwWyKMwnDyB4RAPVh6mRttbAXUsSF4b3enwEPBAEAAAABEQSLjcRiARIEECcAAAABDiA6Gzs8g31kiep6Mdjmx91QPAAb7z4GlY51dICNaMp4pQEPBAAAAAABEQSMjcRiAAEDCE+TNXcAAAAAAQQWABQLE1LKzQPPaqG388jWOIZxs0peEQA=", 1657048460);
CheckTimeLock("cHNidP8BAgQCAAAAAQMEAAAAAAEEAQIBBQEBAfsEAgAAAAABDiAPdY2/vU2nwWyKMwnDyB4RAPVh6mRttbAXUsSF4b3enwEPBAEAAAAAAQ4gOhs7PIN9ZInqejHY5sfdUDwAG+8+BpWOdXSAjWjKeKUBDwQAAAAAAREEjI3EYgABAwhPkzV3AAAAAAEEFgAUCxNSys0Dz2qht/PI1jiGcbNKXhEA", 1657048460);
CheckTimeLock("cHNidP8BAgQCAAAAAQMEAAAAAAEEAQIBBQEBAfsEAgAAAAABDiAPdY2/vU2nwWyKMwnDyB4RAPVh6mRttbAXUsSF4b3enwEPBAEAAAABEgQQJwAAAAEOIDobOzyDfWSJ6nox2ObH3VA8ABvvPgaVjnV0gI1oynilAQ8EAAAAAAERBIyNxGIAAQMIT5M1dwAAAAABBBYAFAsTUsrNA89qobfzyNY4hnGzSl4RAA==", std::nullopt);
+ CheckTimeLock("cHNidP8BAgQCAAAAAQMEAAAAAAEEAQIBBQEBAfsEAgAAAAABDiA6Gzs8g31kiep6Mdjmx91QPAAb7z4GlY51dICNaMp4pQEPBAAAAAABEQSMjcRiAAEOIA91jb+9TafBbIozCcPIHhEA9WHqZG21sBdSxIXhvd6fAQ8EAQAAAAESBBAnAAAAAQMIT5M1dwAAAAABBBYAFAsTUsrNA89qobfzyNY4hnGzSl4RAA==", std::nullopt);
}
BOOST_AUTO_TEST_CASE(psbt2_addinput)Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.