change default Python block serialization to witness #15664

pull instagibbs wants to merge 1 commits into bitcoin:master from instagibbs:default_wit_block changing 1 files +1 −1
  1. instagibbs commented at 9:14 PM on March 25, 2019: member

    No description provided.

  2. fanquake added the label Tests on Mar 25, 2019
  3. in test/functional/mining_basic.py:251 in c671587ff8 outdated 
     246 | @@ -245,6 +247,15 @@ def chain_tip(b_hash, *, status='headers-only', branchlen=1):
 247 |          node.submitheader(hexdata=CBlockHeader(bad_block_root).serialize().hex())
 248 |          assert_equal(node.submitblock(hexdata=block.serialize().hex()), 'duplicate')  # valid
 249 |  
 250 | +        # Generate a block with witness data and round-trip it using python framework
 251 | +        block_hash = node.generatetoaddress(1, node.getnewaddress())[0]
    stevenroose commented at 2:24 PM on March 26, 2019:

    You're apparently not supposed to use getnewaddress (a wallet call) in mining_basic.py because it is run even in builds where wallet is disabled.

    instagibbs commented at 2:29 PM on March 26, 2019:

    hmmm, maybe we should disable wallet for the nodes?

    instagibbs commented at 2:30 PM on March 26, 2019:

    ah I see node.get_deterministic_priv_key().address

    instagibbs commented at 3:38 PM on March 26, 2019:

    fixed.

  4. instagibbs force-pushed on Mar 26, 2019
  5. jnewbery commented at 4:09 PM on March 26, 2019: member

    Concept ACK moving to segwit being the default for the test framework.

    Your additional code in mining_basic is testing that the test framework can deserialize and then serialize a block (ie block_hex = serialize(deserialize(block_hex))). I don't think tests of the test framework belong in the individual test cases. Those are supposed to be testing the node.

    I'd just drop the changes to mining_basic.py.

  6. instagibbs commented at 4:16 PM on March 26, 2019: member

    @jnewbery do we have a place to self-test python serialization/utility code? I've added this before to other tests. I can drop it here for now.

  7. jnewbery commented at 4:24 PM on March 26, 2019: member

    do we have a place to self-test python serialization/utility code?

    We don't. I don't think it's worth it to be honest. The python code is pretty simple.

    If we did add it, I'd suggest adding it to /test/functional/test_framework/ and using Python's unittest module. But again, I don't think it's worthwhile.

  8. in test/functional/mining_basic.py:258 in f70d0cd815 outdated 
     253 | +        block_hex = node.getblock(block_hash, 0)
 254 | +        block = FromHex(CBlock(), block_hex)
 255 | +        assert_equal(len(block.vtx), 1)
 256 | +        assert_equal(block.vtx[0].wit.vtxinwit[0].scriptWitness.stack, [b'\x00'*32])
 257 | +        assert_equal(ToHex(block), block_hex)
 258 | +        assert(block.serialize(with_witness=False).hex() != block_hex)
    MarcoFalke commented at 2:18 PM on March 27, 2019: 
            assert block.serialize(with_witness=False).hex() != block_hex
  9. MarcoFalke commented at 2:25 PM on March 27, 2019: member

    Could also remove some bloat with a scripted diff:

    sed -i --regexp-extended -e 's/block(_?[2a-z]*)\.serialize\([a-z_]*=?True/block\1.serialize(/g' $(git grep -l serialize ./test)
  10. fanquake deleted a comment on Apr 2, 2019
  11. fanquake deleted a comment on Apr 2, 2019
  12. fanquake deleted a comment on Apr 2, 2019
  13. fanquake deleted a comment on Apr 2, 2019
  14. fanquake deleted a comment on Apr 2, 2019
  15. fanquake deleted a comment on Apr 2, 2019
  16. fanquake deleted a comment on Apr 2, 2019
  17. fanquake deleted a comment on Apr 2, 2019
  18. fanquake deleted a comment on Apr 2, 2019
  19. change default Python block serialization to witness 124ea38e39
  20. instagibbs renamed this:
    change default Python block serialization to witness, test round-trip
    change default Python block serialization to witness
    on Apr 2, 2019
  21. instagibbs force-pushed on Apr 2, 2019
  22. instagibbs commented at 2:19 PM on April 2, 2019: member

    removed round-trip test

  23. stevenroose commented at 1:18 PM on May 8, 2019: contributor

    ACK 124ea38e39320d6f63cdf24979d0c1ff92cd769c

  24. MarcoFalke merged this on May 8, 2019
  25. MarcoFalke closed this on May 8, 2019
  26. MarcoFalke referenced this in commit c459c5f701 on May 8, 2019
  27. MarcoFalke referenced this in commit fa52eb55c9 on May 8, 2019
  28. sidhujag referenced this in commit 1f6765631a on May 8, 2019
  29. MarcoFalke referenced this in commit d9bafca20c on Jun 17, 2019
  30. sidhujag referenced this in commit bf5a4f0d80 on Jun 19, 2019
  31. HashUnlimited referenced this in commit 7be8c30a64 on Aug 30, 2019
  32. Munkybooty referenced this in commit 03b35b48f9 on Oct 25, 2021
  33. Munkybooty referenced this in commit 3271fd7fe4 on Oct 25, 2021
  34. Munkybooty referenced this in commit 196dac9c61 on Oct 29, 2021
  35. Munkybooty referenced this in commit 6e79013ac3 on Oct 30, 2021
  36. Munkybooty referenced this in commit 0864890b3f on Nov 2, 2021
  37. Munkybooty referenced this in commit 1223f60dc9 on Nov 2, 2021
  38. MarcoFalke locked this on Dec 16, 2021
Contributors
instagibbsstevenroosejnewberyMarcoFalke
Labels
Tests
Linked (view graph)
#15982 tests: Make msg_block a witness block
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-04-17 09:14 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me