Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create flag-less script tests for BCH #31

Open
bitjson opened this issue Aug 22, 2019 · 3 comments
Open

Create flag-less script tests for BCH #31

bitjson opened this issue Aug 22, 2019 · 3 comments

Comments

@bitjson
Copy link
Member

bitjson commented Aug 22, 2019

This might be a good item to start working on when we build support for the BCH_2019_11 VM. It's also a good thing to do while working on full, per-opcode documentation of the VM.

Basically, for each VM version, there will be two sets of test vectors, one for the block-level validation ("consensus" or "miner"), and one for pre-network rebroadcasting validation ("isStandard" or "strict" in this library). For implementations which use flags, they'll run each list of test vectors using the same set of flags for the full list. As much functionality as possible should be tested using only opcodes, and maybe we'll support other state input for a few cases (e.g. locktime and sequenceNumber). As we get the full VM tested by the new vectors, we can remove the current script_tests.json tests and the hack using script_tests_addendum.json.

Bitcoin ABC will likely continue supporting the existing script_tests.json format, but might also be interested in testing against these new vectors. (CC: @markblundeberg)

@markblundeberg
Copy link

markblundeberg commented Aug 22, 2019

Yeah that makes sense on how to do it.

For handling upgrade-dependent tests I'm not sure exactly what to do.

Perhaps this:

  • list 0: legacy/experimental tests to run on specific flag sets (chosen per-test, as in script_tests.json)
  • list 1 (long): tests that must pass on all modern consensus flag sets (pre- and post-upgrade)
  • list 2 (short): tests to run on pre-upgrade consensus flags only
  • list 3 (short): tests to run on post-upgrade consensus flags only
  • list 4,5,6: standardness versions of 1,2,3

As much as possible, tests should be in list 1 since that is the most critical to get right.

When making an upgrade, this could be the process in steps:

  1. Develop unactivated experimental feature on a new script flag, with tests in list 0.
  2. Add activation code for new feature. This means:
    - Move the new feature tests from list 0 to list 3.
    - Move some affected tests out from list 1 to list 2.
  3. After the upgrade has passed, perform cleanup:
    - Move the new feature's tests from list 3 to list 1.
    - Delete tests from list 2 or move them to list 0 for legacy testing.

@bitjson
Copy link
Member Author

bitjson commented Nov 18, 2019

Relevant comment from @joshmg in Bitcoin ABC Telegram group:

Would it at all be possible to add unique-identifiers to each test case? Perhaps just a UUID or something simple. The problem I'm trying to solve is that I have to meticulously disable particular tests within the vectors for various reasons (usually just non-applicability due to codebase differences)--currently I have to do this by line-number, however inserting a test anywhere but the end of the file obviously makes things a mess. Perhaps the UUID could just get added as the last "word" to the comments column to keep things innocuous for you guys. If you guys would be okay with this change then I'll be happy to volunteer to make a PR.

(@joshmg – FYI, you're not alone 😄)

UUIDs for each test are a very good idea, we should definitely make sure to do that. 👍

@markblundeberg
Copy link

@bitjson @joshmg we might not take care of keeping UUIDs sane though ... is it possible that instead you guys can track by test content instead?

Or even, make your own UUID by hashing the test content :)

bitjson added a commit that referenced this issue May 14, 2022
… and compiler

BREAKING CHANGE: requires esm, modifies some crypto interfaces, renames many exports for
consistency, expands the program state available to vms and compilers

fixes #31, re #53, fixes #72
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants