test: check for all possible OP_CLTV fail reasons in feature_cltv.py (BIP 65)#19801
Merged
maflcko merged 4 commits intobitcoin:masterfrom Apr 22, 2021
Merged
Conversation
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
Member
|
concept ACK |
This was referenced Aug 25, 2020
theStack
force-pushed
the
20200825-test-check-all-failure-reasons-for-CLTV
branch
from
dfdd34a to
0bf3040
Compare
Contributor
Author
|
Rebased on master. |
theStack
force-pushed
the
20200825-test-check-all-failure-reasons-for-CLTV
branch
from
0bf3040 to
ebfc246
Compare
Contributor
Author
|
Rebased on master again. |
+ reformat python imports + fix PEP8 warnings (all except E501 line too long)
…cltv.py only the "top item on the stack is less than 0" is used in the test right now
theStack
force-pushed
the
20200825-test-check-all-failure-reasons-for-CLTV
branch
from
ebfc246 to
b01cd94
Compare
Contributor
Author
|
Rebased on master. |
Member
|
Concept ACK |
maflcko
approved these changes
Apr 22, 2021
Member
maflcko
left a comment
maflcko
left a comment
There was a problem hiding this comment.
review ACK b01cd94 🐣
Show signature and timestamp
Signature:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
review ACK b01cd9471f435bb36b8ed5211a56baad51111ad2 🐣
-----BEGIN PGP SIGNATURE-----
iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUiGwwv/ehsKGNFfYIyNn3Pb7H8mC7j6SfFijNHeg+rDldGw1lNA3MLjMvPGhs4v
+uuAbS74JCv4T6fEjw8K4ODx6ZU/+fyckT8HL2Qxx9TbqPN9+3Gc5Ww68137+RRW
mOzNy1S4Z6MPu3OtBZxkHSKHXPKF6sSTDyqjrxgslpOFNDceirBIahFTHPXy2geh
gZYLsQYtbY0kDJBLcfCh4vvPW5IpLO2Cr5iRQkVmj31upzvYniKmWtuCwmwrXZG9
mAKP0o9AGnfYWCufH1NpDBp4/JBrRITk+nz0bOgxYO99FiJMalX+YTP6xlkOIGgI
ChrXmz7/qe3FXypz9lA+hL44R7/ApLWhCfc4oK9bbHwSVqbjnTh5V20+EjYSK3ER
5IPjBDjbTN1768iQMRIjq2Aj9zQFrVwCkzlW56zwlECZoFHGEzDOWimHkVWd1vZR
qSvVIya8GmuqwWpEASi9O5yXArtqF+Vzr7s9Ki8TiTbGFOQFFlNFLgTXkOXi4Mrr
DC1R6djf
=TLhB
-----END PGP SIGNATURE-----
Timestamp of file with hash 0aea310db82e5dcfdbcc25d7e801d24635a1a608c8b997bbb98494c524f90747 -
Member
|
Reworked the test some more in #21754 |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Apr 22, 2021
gwillen
pushed a commit
to ElementsProject/elements
that referenced
this pull request
Jun 1, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The functional test for BIP65 /
OP_CHECKLOCKTIMEVERIFY(feature_cltv.py) currently only tests one out of five conditions that lead to failure of the op-code -- by prepending the scriptOP_1NEGATE OP_CHECKLOCKTIMEVERIFY OP_DROPto a tx's first input's scriptSig, the case of "the top item on the stack is less than 0" is checked:bitcoin/test/functional/feature_cltv.py
Lines 26 to 35 in f8462a6
This PR adds the other cases (5 in total) by taking an integer argument to the function
cltv_invalidatethat is called in a loop instead of only once per testing scenario. Here is the full list of failure conditions and how they are tested (note that the scriptSig should still be valid before activation of BIP65, whenOP_CLTVis simply a no-op):➡️ prepending
OP_CHECKLOCKTIMEVERIFYto scriptSig➡️ prepending
OP_1NEGATE OP_CHECKLOCKTIMEVERIFY OP_DROPto scriptSig➡️ prepending
OPNum(1000) OP_CHECKLOCKTIMEVERIFY OP_DROPto scriptSig➡️ setting tx.vin[0].nSequence=0 and tx.nCheckTimeLock=1296688602 (genesis block timestamp)
➡️ prepending
OPNum(1000) OP_CHECKLOCKTIMEVERIFY OP_DROPto scriptSig➡️ setting tx.vin[0].nSequence=0 and tx.nCheckTimeLock=500
➡️ prepending
OPNum(500) OP_CHECKLOCKTIMEVERIFY OP_DROPto scriptSig➡️ setting tx.vin[0].nSequence=0xffffffff and tx.nCheckTimeLock=500
The first commit creates a helper function for the tx modification and also includes some tidying up like turning single-line to multi-line Python imports where necessary and cleaning up some PEP8 warnings. The second commit prepares the invalidation function
cltv_invalidateand the third and the fourth use it and check for the expected reject reason strings ("Operation not valid with the current stack size", "Negative locktime" and "Locktime requirement not satisfied").