Add p2p-fullblocktest.py #6523

pull casey wants to merge 1 commits into bitcoin:master from casey:fullblocktest changing 12 files +702 −53
  1. casey commented at 10:39 PM on August 5, 2015: contributor

    This is a start at implementing FullBlockTestGenerator.java in python. Almost all the work is by @sdaftuar, I just did some final cleanup here and there. The main test is p2p-fullblocktest.py, and should be pretty easy to understand,

    It currently implements only a few of the tests from FullBlockTestGenerator.java, but it's a start.

    create_coinbase() now takes a height argument, so there are some changes to unrelated tests, as well as some new functionality added to the testing framework, to support the new tests.

  2. jgarzik commented at 11:42 AM on August 6, 2015: contributor

    +1

  3. laanwj added the label Tests on Aug 6, 2015
  4. laanwj commented at 12:45 PM on August 6, 2015: member

    Awesome.

  5. gavinandresen commented at 2:47 PM on August 6, 2015: contributor

    Very nice!

    But in the "probably not your fault" category, it crashes on my OSX machine calling the openssl BN_bin2bn function:

    Crashed Thread:        0  Dispatch queue: com.apple.main-thread

Exception Type:        EXC_BAD_ACCESS (SIGSEGV)
Exception Codes:       KERN_INVALID_ADDRESS at 0x00000000296707cc

VM Regions Near 0x296707cc:
--> 
    __TEXT                 0000000107e1d000-0000000107e1f000 [    8K] r-x/rwx SM=COW  /usr/local/Cellar/python/2.7.10_2/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   libcrypto.0.9.8.dylib           0x00007fff93af4446 BN_bin2bn + 118
1   _ctypes.so                      0x00000001087d47c7 ffi_call_unix64 + 79
2   _ctypes.so                      0x00000001087d4fe6 ffi_call + 818
3   _ctypes.so                      0x00000001087d070b _ctypes_callproc + 867
4   _ctypes.so                      0x00000001087cab91 PyCFuncPtr_call + 1100
5   org.python.python               0x0000000107e31ad7 PyObject_Call + 99
6   org.python.python               0x0000000107eade7f PyEval_EvalFrameEx + 11417
7   org.python.python               0x0000000107eb16d1 fast_function + 262
8   org.python.python               0x0000000107eae553 PyEval_EvalFrameEx + 13165
9   org.python.python               0x0000000107eaafb4 PyEval_EvalCodeEx + 1387
10  org.python.python               0x0000000107e4fbf5 function_call + 352
11  org.python.python               0x0000000107e31ad7 PyObject_Call + 99
12  org.python.python               0x0000000107e3c962 instancemethod_call + 174
13  org.python.python               0x0000000107e31ad7 PyObject_Call + 99
14  org.python.python               0x0000000107e78f2d slot_tp_init + 64
15  org.python.python               0x0000000107e746e7 type_call + 182
16  org.python.python               0x0000000107e31ad7 PyObject_Call + 99
17  org.python.python               0x0000000107eade7f PyEval_EvalFrameEx + 11417
18  org.python.python               0x0000000107eaafb4 PyEval_EvalCodeEx + 1387
19  org.python.python               0x0000000107eaaa43 PyEval_EvalCode + 54
20  org.python.python               0x0000000107eca816 run_mod + 53
21  org.python.python               0x0000000107eca8b9 PyRun_FileExFlags + 133
22  org.python.python               0x0000000107eca3f9 PyRun_SimpleFileExFlags + 711
23  org.python.python               0x0000000107edbe09 Py_Main + 3057
24  libdyld.dylib                   0x00007fff950c25c9 start + 1

OS Version:            Mac OS X 10.10.4 (14E46)
Code Type:             X86-64 (Native)

    Probably the same issue: https://github.com/petertodd/python-bitcoinlib/issues/30

  6. in qa/rpc-tests/test_framework/key.py:None in f6035ce863 outdated 
      29 | +        return ctypes.c_void_p (val)
  30 | +
  31 | +ssl.EC_KEY_new_by_curve_name.restype = ctypes.c_void_p
  32 | +ssl.EC_KEY_new_by_curve_name.errcheck = _check_result
  33 | +
  34 | +class CECKey:
    kanzure commented at 3:14 PM on August 6, 2015:

    nit: classes should at least subclass some base class such as object

    casey commented at 8:03 PM on August 7, 2015:

    fixed

  7. in qa/rpc-tests/test_framework/key.py:None in f6035ce863 outdated 
      20 | +
  21 | +# this specifies the curve used with ECDSA.
  22 | +NID_secp256k1 = 714 # from openssl/obj_mac.h
  23 | +
  24 | +# Thx to Sam Devlin for the ctypes magic 64-bit fix.
  25 | +def _check_result (val, func, args):
    kanzure commented at 3:14 PM on August 6, 2015:

    Syntax nit: extra space after the function name, doesn't match style of other functions in the file. Also, docstrings would be nice for every function.

    casey commented at 8:03 PM on August 7, 2015:

    fixed

  8. in qa/rpc-tests/README.md:None in f6035ce863 outdated 
      36 | +
  37 | +Bash-based tests, to be ported to Python:
  38 | +-----------------------------------------
  39 | +- conflictedbalance.sh : More testing of malleable transaction handling
  40 | +
  41 | +
    fanquake commented at 5:18 PM on August 6, 2015:

    Why are you adding the note about conflictedbalance, it was removed in #6428

    casey commented at 8:03 PM on August 7, 2015:

    Whoops, bad merge, fixed.

  9. casey commented at 8:10 PM on August 7, 2015: contributor

    @gavinandresen Unfortunately, I haven't made any progress on fixing this. I can reproduce it on OS X, as well as on my own Arch Linux dev box, but not on Debian.

    On Arch I can get it to work by compiling python from a tarball (instead of using the version from arch's package manager), so I think it's a problem with python/ctypes, and not something weird in the code or libssl.

    It does seem to be the same issue as petertodd/python-bitcoinlib#30

    It might be a good idea to still merge this, assuming it works on travis and for people not on OS X, so we can start building up a good series of block acceptance tests.

  10. TheBlueMatt commented at 7:37 PM on August 9, 2015: member

    While I love the idea of ditching the existing comparison tool, rewriting it as-is seems like a very bad idea. The existing one is very hard to work with in large part because of the way the tests are generated - it's one big monolithic step-by-step. It really needs to separate out into something where individual tests are in individual functions/modules/whatever. Obviously running the tests in one run is probably the only way you can get it to run in reasonable time, but please don't duplicate the existing code.

    On August 7, 2015 10:10:51 PM GMT+02:00, Casey Rodarmor notifications@github.com wrote:

    @gavinandresen Unfortunately, I haven't made any progress on fixing this. I can reproduce it on OS X, as well as on my own Arch Linux dev box, but not on Debian.

    On Arch I can get it to work by compiling python from a tarball (instead of using the version from arch's package manager), so I think it's a problem with python/ctypes, and not something weird in the code or libssl.

    It does seem to be the same issue as petertodd/python-bitcoinlib#30

    It might be a good idea to still merge this, assuming it works on travis and for people not on OS X, so we can start building up a good series of block acceptance tests.

    Reply to this email directly or view it on GitHub: #6523 (comment)

  11. jgarzik commented at 3:10 AM on August 10, 2015: contributor

    A drop-in replacement in a more accessible language and format that behaved equivalently would seem to be a net win.

  12. TheBlueMatt commented at 7:03 AM on August 10, 2015: member

    Sure, but if you're gonna go to the effort to rewrite it, might as well rewrite it better instead of transliterating a very fragile monolithic chunk of code.

    On August 10, 2015 5:10:39 AM GMT+02:00, Jeff Garzik notifications@github.com wrote:

    A drop-in replacement in a more accessible language and format that behaved equivalently would seem to be a net win.

    Reply to this email directly or view it on GitHub: #6523 (comment)

  13. jgarzik commented at 1:38 PM on August 10, 2015: contributor

    "might as well" is in the eye of the do-er... In the absence of someone coming along and doing an even better rewrite, an incremental net-win should not be blocked because it lacks perfection :)

  14. sipa commented at 1:41 PM on August 10, 2015: member

    Agree with @jgarzik. I think that just having a python version available will lower the barrier for extra tests using it.

  15. jonasschnelli commented at 1:45 PM on August 10, 2015: contributor

    +1 utACK. Travis did run this test successfully (https://travis-ci.org/bitcoin/bitcoin/jobs/74625657#L3538).

  16. casey commented at 7:27 PM on August 10, 2015: contributor

    I just pushed changes which should fix the crashes on Arch and OS X.

  17. gavinandresen commented at 7:35 PM on August 10, 2015: contributor

    Tests successful on OSX, ACK from me.

  18. petertodd commented at 10:50 PM on August 18, 2015: contributor

    @casey FWIW I just ported those fixes to python-bitcoinlib: https://github.com/petertodd/python-bitcoinlib/pull/77

  19. laanwj commented at 2:34 PM on August 21, 2015: member

    Needs rebase after #6509 (Fix race condition on test node shutdown)

  20. Add p2p-fullblocktest.py 0ce73985a8
  21. casey commented at 7:43 PM on August 21, 2015: contributor

    Rebased onto master.

  22. fanquake commented at 9:27 AM on August 24, 2015: member

    Retested to verify that the OSX crash has been fixed.

  23. laanwj merged this on Aug 24, 2015
  24. laanwj closed this on Aug 24, 2015
  25. laanwj referenced this in commit 561f8af450 on Aug 24, 2015
  26. luke-jr commented at 5:13 AM on November 15, 2015: member

    In the future, please don't make changes to interfaces that quietly and subtley break things. In this case, create_coinbase's first argument changed meaning while quietly taking the same values and not throwing an error (or maintaining the previous functionality). As a result, I spent time trying to figure out why the CLTV RPC test failed on 0.11.2 when it had nothing to do with CLTV. :(

  27. luke-jr referenced this in commit 1e97a0261e on Nov 18, 2015
  28. zkbot referenced this in commit 025bd44543 on Nov 21, 2020
  29. zkbot referenced this in commit 7a0a268054 on Dec 2, 2020
  30. zkbot referenced this in commit c8896f9907 on Dec 2, 2020
  31. DrahtBot locked this on Sep 8, 2021
Contributors
caseyjgarziklaanwjgavinandresenkanzurefanquakeTheBlueMattsipajonasschnellipetertoddluke-jr
Labels
Tests
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-02 03:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me