Adding the json script tests from ABC#53
Open
Danconnolly wants to merge 108 commits into
Open
Conversation
…FAIL, SIGHASH_FORKID, REPLAY_PROTECTION
…ificationException
Danconnolly
force-pushed
the
jsonscripttests
branch
from
c84b36d to
199daf0
Compare
Collaborator
Author
|
rebased off cash-0.14 branch |
The new NUM2BIN tests in script_tests.json were expecting an IMPOSSIBLE_ENCODING error message. Added new exception EncodingMessage and adjusted NUM2BIN interpreter to produce this exception. Exceptions were previously being generated but were the wrong type.
…bitcoinj-cash into jsonscripttests
…ERIFY. Based on Bitcoin-abc implementation.
… one scenrio is still not covered
… are now using the "ScriptError" where all the possible causes for the Script failure can be stored and organized. This way, the Script failure always throws an ScriptException (easier to managed). This is how it's been implementrd in bitcoinJ (reference).
…e there is an specific error that needs to be captured and processed separately.
These scenarios have been fixed (Expected results from script_tests.json): SIG_DER, ILLEGAL_FORKID, SIG_HASHTYPE, SIG_PUSHONLY, CLEANSTACK, PUBKEYTYPE.
…e there is an specific error that needs to be captured and processed separately.
These scenarios have been fixed (Expected results from script_tests.json): SIG_DER, ILLEGAL_FORKID, SIG_HASHTYPE, SIG_PUSHONLY, CLEANSTACK, PUBKEYTYPE.
…has no ScriptChunks in it.
…tories (like bitcoinj or bitcopinj-cash), since the structure of the tests might be a bit different in those cases.
…n IllegalOperation
This reverts commit f5c8eca.
Danconnolly
force-pushed
the
jsonscripttests
branch
from
405d3af to
b55d0f6
Compare
…bitcoinj-cash into jsonscripttests # Conflicts: # core/src/main/java/org/bitcoinj/script/ScriptError.java # core/src/test/java/org/bitcoinj/script/ScriptHelpers.java
…is changed and new data is generated (and compatible with the new Verifications implemented in the Script Engine).
…legacy tests. Until all the legacy tests are refactored, we change slightly the ALL_VERIFY_FLAGS Collection to remove those Flags that are not compatible(SIGHASH_FORKID and PUBKEYTYPE).
After looking into the code, I think the workaround (removing some Flags from ALL_VERIFY_FLAGS) is not a workaround, but a valid and needed change, in order to keep the existing functionality. Still, I consider that a deep research into the code is needed, in order to refactor all the uses of these Flags. Since these Flags affect the way the script is executed, the different invocations to the script engine should be aware of what flags are being used 8as a parameter, for example). At this moment, these Flags are hardcoded, and that shouldn,t be the case. If we want to harcode the Verification Flags used when validating the script, then we shoul Still, I consider that a deep research into the code is needed, in order to refactor all the uses of these Flags. Since these Flags affect the way the script is executed, the different invocations to the script engine should be aware of what flags are being used 8as a parameter, for example). At this moment, these Flags are hardcoded in some places, and that shouldn't be the case (unles there is a clear consensus about that). The current commit runs OK and the legacy tests also run OK. But we should make it clear that ALL_VERIFY_FLAGS doen NOT measn ALL of them, just the ones that allow a "safe" script execution.
…ode). After looking into the code, I think the workaround (removing some Flags from ALL_VERIFY_FLAGS) is not a workaround, but a valid and needed change, in order to keep the existing functionality. Still, I consider that a deep research into the code is needed, in order to refactor all the uses of these Flags. Since these Flags affect the way the script is executed, the different invocations to the script engine should be aware of what flags are being used (as a parameter, for example). At this moment, these Flags are hardcoded, and that should not be the case (unless there is a clear consensus about that). The current commit runs OK and the legacy tests also run OK. But we should make it clear that ALL_VERIFY_FLAGS doen NOT measn ALL of them, just the ones that allow a "safe" script execution.
November 15, 2018 Hard fork. Code from SV branch used as a reference.
November 15, 2018 Hard fork. Code from SV branch used as a reference. the test classes have also been refactored, to make them more flexible for future OpCodes test coverage.
A new class "StackItem" has been added to add more flexibility for testing, and the test classes have been refactorized.
It makes it easier to perform tests on OP_CODEs. It includes more "casting" mehods, to allow to use it with different data types or fortmats /binary, hex, numbers, etc).
Includes LSHIFT and RSHIFT functionality and tests, and some refactoring has ¡been made in the testing classes, to make them more modular and flexible.
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
This pull request incorporates the standard script unit tests from the script_tests.json file. These unit tests are used by the major node implementations. Care has been taken to avoid modifying the script_tests.json file (with the exception of attribution), so that it can be re-imported in the future as new tests are added.
The tests have shown several deficiencies in the BitcoinJ Script implementation. I have corrected enough to get the basic tests working but many are still not working. Tests which cover features that have not been implemented in bitcoinj-cash yet are ignored. See the TODO notes in ScriptDataDrivenTest.java.
These broken tests need to be addressed at some point. I have run out of time to devote on this right now and am posting it here as a "work in progress".